By the way, any ideas on how to avoid 'bus overstealing' would
be greatly appreciated.
Static BIOS variable isn't applicable since its value isn't saved across
reboots.
2017-07-23 19:43 GMT+03:00 Alexander Bezzubikov <zuban...@gmail.com>:
> 2017-07-23 5:49 GMT+03:00 Michael S. Tsirkin <m...@redhat.com>:
>
>> On Sun, Jul 23, 2017 at 01:11:50AM +0300, Aleksandr Bezzubikov wrote:
>> > In case of Red Hat PCI bridges reserve additional buses, which number
>> is provided
>> > in a vendor-specific capability.
>> >
>> > Signed-off-by: Aleksandr Bezzubikov <zuban...@gmail.com>
>> > ---
>> > src/fw/pciinit.c | 14 ++++++++++++--
>> > 1 file changed, 12 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
>> > index 864954f..f05a8b9 100644
>> > --- a/src/fw/pciinit.c
>> > +++ b/src/fw/pciinit.c
>> > @@ -15,6 +15,7 @@
>> > #include "hw/pcidevice.h" // pci_probe_devices
>> > #include "hw/pci_ids.h" // PCI_VENDOR_ID_INTEL
>> > #include "hw/pci_regs.h" // PCI_COMMAND
>> > +#include "hw/pci_cap.h" // qemu_pci_cap
>> > #include "list.h" // struct hlist_node
>> > #include "malloc.h" // free
>> > #include "output.h" // dprintf
>> > @@ -578,9 +579,18 @@ pci_bios_init_bus_rec(int bus, u8 *pci_bus)
>> > pci_bios_init_bus_rec(secbus, pci_bus);
>> >
>> > if (subbus != *pci_bus) {
>> > + u8 res_bus = 0;
>> > + if (pci_config_readw(bdf, PCI_VENDOR_ID) ==
>> PCI_VENDOR_ID_REDHAT) {
>>
>> Check device ID as well.
>
> Not sure what ID/IDs we want to see here exactly. Now it can be
> only pcie-root-port (0xC then), but
>
>
>>
>> > + u8 cap = pci_find_capability(bdf, PCI_CAP_ID_VNDR, 0);
>>
>> There could be multiple vendor capabilities. You want to scan them all.
>>
>>
>> > + if (cap) {
>> > + res_bus = pci_config_readb(bdf,
>> > + cap + offsetof(struct
>> redhat_pci_bridge_cap,
>> > + bus_res));
>>
>>
>> You might want to add sanity checks e.g. overflow, and capability
>> length.
>>
>> Also, if all you use is offsetof, don't bother with a struct, just
>> add some defines.
>>
> OK, will move this to defines.
>
>
>>
>> > + }
>> > + }
>> > dprintf(1, "PCI: subordinate bus = 0x%x -> 0x%x\n",
>> > - subbus, *pci_bus);
>> > - subbus = *pci_bus;
>> > + subbus, *pci_bus + res_bus);
>> > + subbus = *pci_bus + res_bus;
>>
>> So you take all present devices and add reserved ones - is that it?
>> If so it looks like this will steal extra buses each time you
>> add a child bus and reboot.
>>
>
> You're right, such problem persists. Will think on what can be done here.
>
>
>>
>>
>> > } else {
>> > dprintf(1, "PCI: subordinate bus = 0x%x\n", subbus);
>> > }
>> > --
>> > 2.7.4
>>
>
>
>
> --
> Alexander Bezzubikov
>
--
Alexander Bezzubikov
