[OpenBIOS] Adding new device in the middle of device-tree

[Mark Cave-Ayland]

On 15/07/17 21:27, BALATON Zoltan wrote:

> On Thu, 13 Jul 2017, Mark Cave-Ayland wrote:
>> On 28/05/17 11:38, BALATON Zoltan wrote:
>>> I'm trying to do this so I don't need the patch to OpenBIOS:
>>>
>>> https://mail.coreboot.org/pipermail/openbios/2017-January/009867.html
>>>
>>> Unfortunately the code relying on this (which I can't change) seems to
>>> have some assumptions about order of pci busses in the device tree and
>>> only works if added before the exising entry (i.e. /pci at f0000000 has to
>>> come before /pci at f2000000).
>>
>> FWIW I've just been testing OpenBIOS git master ready for upstreaming to
>> QEMU and noticed the the recent changes to the PCI host bridge
>> properties have fixed PCI bus enumeration on FreeBSD PPC under -M mac99
>> (it still panics, but hey at least it finally sees a proper set of
>> devices).
>>
>> If you get a moment, can you try again with git master and report back
>> whether things have improved for you or not?
> 
> It works as good as before: that is I still need the above patch but
> your changes haven't seem to do any harm nor fix anything for what I
> have tested.
> 
> I'm still not sure how to add another PCI bus to MAC99 because the
> assumption of a single PCI bus seems to be in the OpenBIOS pci driver.
> I've noticed some possible redundancy though in the pci arch definition:
> 
> [ARCH_MAC99] = {
>     .name = "MAC99",
>     .vendor_id = PCI_VENDOR_ID_APPLE,
>     .device_id = PCI_DEVICE_ID_APPLE_UNI_N_PCI,
>     .cfg_addr = 0xf2800000,
>     .cfg_data = 0xf2c00000,
>     .cfg_base = 0xf2000000,
>     .cfg_len = 0x02000000,
>     .host_pci_base = 0x0,
>     .pci_mem_base = 0x80000000,
>     .mem_len = 0x10000000,
>     .io_base = 0xf2000000,
>     .io_len = 0x00800000,
>     .host_ranges = {
>         { .type = IO_SPACE, .parentaddr = 0, .childaddr = 0xf2000000,
> .len = 0x00800000 },
>         { .type = MEMORY_SPACE_32, .parentaddr = 0x80000000, .childaddr
> = 0x80000000, .len = 0x10000000 },
>         { .type = 0, .parentaddr = 0, .childaddr = 0, .len = 0 }
>      },
>     .irqs = { 0x1b, 0x1c, 0x1d, 0x1e }
> },
> 
> Aren't pci_mem_base/mem_len and io_base/io_len the same as the IO and
> MEMORY ranges in host_ranges (and the cfg_* values offsets into the IO
> space)? Do we need these separately or could it be simplified?

They are for PPC, but unfortunately not for SPARC64. That's why they
were split out as separate values in the last set of patches because the
existing logic couldn't handle all cases of multiple address spaces with
different offsets :(

> To add another PCI bus I think this arch structure needs to be extended
> to be able to define ids and ranges for multiple buses but I'm not sure
> how to do that without breaking other archs. Do you have any idea how to
> go about this? (Also does any other arch need this? If not is it worth
> doing it when the other PCI bus on QEMU is empty with no devices and we
> only need it to be present in the device tree? So could we just go with
> the above hack until handling multiple PCI buses are implemented
> eventually?)

As a starting point, why not make the above struct an array of host
bridges, e.g.


 [ARCH_MAC99] =
  [
   {
     .name = "MAC99",
     .vendor_id = PCI_VENDOR_ID_APPLE,
     .device_id = PCI_DEVICE_ID_APPLE_UNI_N_PCI,
     .cfg_addr = 0xf2800000,
     .cfg_data = 0xf2c00000,
     .cfg_base = 0xf2000000,
     .cfg_len = 0x02000000,
     .host_pci_base = 0x0,
     .pci_mem_base = 0x80000000,
     .mem_len = 0x10000000,
     .io_base = 0xf2000000,
     .io_len = 0x00800000,
     .host_ranges = {
         { .type = IO_SPACE, .parentaddr = 0, .childaddr = 0xf2000000,
           .len = 0x00800000 },
         { .type = MEMORY_SPACE_32, .parentaddr = 0x80000000,
           .childaddr = 0x80000000, .len = 0x10000000 },
         { .type = 0, .parentaddr = 0, .childaddr = 0, .len = 0 }
      },
     .irqs = { 0x1b, 0x1c, 0x1d, 0x1e }
   }, {
      ....
   }
  ]
 },


Then remove the "break" from ob_pci_init() and see how you get on.
You'll need to figure out what to do with the configuration space,
however with the last set of fix-ups I really don't think there is much
to do.

Also a good tip: get it working on PPC first and then make the changes
to the other architectures and test. I don't mind helping out with this
part if you get stuck.


ATB,

Mark.