Re: [PATCH v5 2/7] hw/i386/acpi-build: Add ACPI PCI hot-plug methods to Q35

[Michael S. Tsirkin]

On Thu, Jul 01, 2021 at 02:36:30PM +1000, David Gibson wrote:
> On Thu, Jun 17, 2021 at 09:07:34PM +0200, Julia Suvorova wrote:
> > Implement notifications and gpe to support q35 ACPI PCI hot-plug.
> > Use 0xcc4 - 0xcd7 range for 'acpi-pci-hotplug' io ports.
> > 
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> 
> I don't know ACPI or x86 particular well, so I could well have missed
> something, but..
> 
> [snip]
> > @@ -392,6 +392,9 @@ static void build_append_pci_bus_devices(Aml 
> > *parent_scope, PCIBus *bus,
> >  
> >          if (!pdev) {
> >              if (bsel) { /* add hotplug slots for non present devices */
> > +                if (pci_bus_is_express(bus) && slot > 0) {
> > +                    break;
> > +                }
> >                  dev = aml_device("S%.02X", PCI_DEVFN(slot, 0));
> >                  aml_append(dev, aml_name_decl("_SUN", aml_int(slot)));
> >                  aml_append(dev, aml_name_decl("_ADR", aml_int(slot << 
> > 16)));
> > @@ -516,7 +519,7 @@ static void build_append_pci_bus_devices(Aml 
> > *parent_scope, PCIBus *bus,
> >              QLIST_FOREACH(sec, &bus->child, sibling) {
> >                  int32_t devfn = sec->parent_dev->devfn;
> >  
> > -                if (pci_bus_is_root(sec) || pci_bus_is_express(sec)) {
> > +                if (pci_bus_is_root(sec)) {
> >                      continue;
> >                  }
> 
> .. what will this logic do if we encounter a PCIe-switch.  AIUI, it
> should be possible to hotplug 1 slot under each downstream port, but
> we can't hotplug anything directly under the upstream port.  AFAICT
> both the upstream and downstream ports will show up as 'is_bridge'
> though.
> 
> So, IIUC we want to traverse a PCIe upstream switch port, but not
> generate hotplug slots until we encounter the downstream ports below
> it.

Julia could you comment on this please?
I already applied the patches so patch on top if necessary please ...

> -- 
> David Gibson                  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au        | minimalist, thank you.  NOT _the_ 
> _other_
>                               | _way_ _around_!
> http://www.ozlabs.org/~dgibson