Re: [PATCH v2 06/25] hw/pci-host/gpex-acpi: Propagate hotplug type info from virt machine downto gpex

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 11 Jun 2025 11:00:02 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> Hi Igor,
> 
> On 5/27/25 2:33 PM, Igor Mammedov wrote:
> > On Tue, 27 May 2025 09:40:08 +0200
> > Eric Auger <eric.auger@redhat.com> wrote:
> >  
> >> Propagate the type of pci hotplug mode downto the gpex
> >> acpi code. In case machine acpi_pcihp is unset we configure
> >> pci native hotplug on pci0. For expander bridges we keep
> >> legacy pci native hotplug, as done on x86 q35.  
> > this is not needed if done as suggested in 2/25  
> So since the property would be applied to the GED device we will still
> need a patch that retrieves the GED setting and propagate it to the GPEX
> device.

I don't see a need for propagating it to GPEX device itself for ACPI sake.

For ACPI code, what we do in q35 case is fetching
pm->pcihp_bridge_en from acpi_dev(whatever it is) and then
passing it down trough call-chain

  build_dsdt->build_q35_osc_method

the only hack that touches GPEX device model, is the need to
wire up root bus to GED, when GED is being created.
So that GED would be able to swap hotplug handler on
an acpi specific one when configured to do so.


> Thanks
> 
> Eric
> >  
> >> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >> Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
[...]
> >> @@ -279,7 +280,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
> >>      }
> >>      aml_append(dev, aml_name_decl("_CRS", rbuf));
> >>  
> >> -    acpi_dsdt_add_host_bridge_methods(dev, true);
> >> +    acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);

also, having sufficiently forgotten this series context.
It might be better to abandon idea of consolidating osc and dsm methods
in acpi_dsdt_add_host_bridge_methods().

Just looking at above is confusing and forces one to re-rear previous
commits to figure out what acpi_dsdt_add_host_bridge_methods() is doing.

having here separate sequence of
  add_osc()
  add_dsm()
would be much more readable.
Lets drop acpi_dsdt_add_host_bridge_methods() helper.

> >>  
> >>      Aml *dev_res0 = aml_device("%s", "RES0");
> >>      aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));  
> >  
>