Re: [v2 1/1] hw/i386/acpi-build: add OSHP method support for SHPC driver load

[Gao,Shiyuan]

> > > > the PCI bridge will fail when we use SHPC Native type:
> > > >
> > > >   [3.336059] shpchp 0000:00:03.0: Requesting control of SHPC hotplug 
> > > >via OSHP (\_SB_.PCI0.S28_)
> > > >   [3.337408] shpchp 0000:00:03.0: Requesting control of SHPC hotplug 
> > > >via OSHP (\_SB_.PCI0)
> > > >   [3.338710] shpchp 0000:00:03.0: Cannot get control of SHPC hotplug
> > > >
> > > > Add OSHP method support for transfer control to the operating system,
> > > > after this SHPC driver will be loaded success and the hotplug device to
> > > > the PCI bridge will success when we use SHPC Native type.
> > > >
> > > >   [1.703975] shpchp 0000:00:03.0: Requesting control of SHPC hotplug 
> > > >via OSHP (\_SB_.PCI0.S18_)
> > > >   [1.704934] shpchp 0000:00:03.0: Requesting control of SHPC hotplug 
> > > >via OSHP (\_SB_.PCI0)
> > > >   [1.705855] shpchp 0000:00:03.0: Gained control of SHPC hotplug 
> > > >(\_SB_.PCI0)
> > > >   [1.707054] shpchp 0000:00:03.0: HPC vendor_id 1b36 device_id 1 ss_vid 
> > > >0 ss_did 0
> > >
> > > please describe in commit message reproducer
> > > (aka QEMU CLI and guest OS and if necessary other details)
> >
> > qemu-system-x86_64 -machine pc-q35-9.0
> >     ...
> >     -global PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off
>
> please use full QEMU CLI and follow up steps to trigger the issue.
>
> From above it's not obvious what and where you are trying to hotplug

Nothing needs to be done when you start a i440fx VM, this issue will be 
triggered.
PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off is used to verify shpc driver 
load sucess.

>
> > guest OS: centos7/ubuntu22.04
> >
> > I will add it in the next version.
> >
> > > > +/*
> > > > + * PCI Firmware Specification 3.0
> > > > + * 4.8. The OSHP Control Method
> > > > + */
> > > > +static Aml *build_oshp_method(void)
> > > > +{
> > > > +    Aml *method;
> > > > +
> > > > +    /*
> > > > +     * We don't use ACPI to control the SHPC, so just return
> > > > +     * success is enough.
> > > > +     */
> > > > +    method = aml_method("OSHP", 0, AML_NOTSERIALIZED);
> > > > +    aml_append(method, aml_return(aml_int(0x0)));
> > > > +    return method;
> > > > +}
> > > > +
> > > >  static void
> > > >  build_dsdt(GArray *table_data, BIOSLinker *linker,
> > > >             AcpiPmInfo *pm, AcpiMiscInfo *misc,
> > > > @@ -1452,6 +1469,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > > >          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
> > > >          aml_append(dev, aml_name_decl("_UID", 
> > > >aml_int(pcmc->pci_root_uid)));
> > > >          aml_append(dev, aml_pci_edsm());
> > > > +        aml_append(dev, build_oshp_method());
> > >
> > > it's global and what will happen if we have ACPI PCI hotplug enabled
> > > and guest calls this NOP method?
> >
> > ths OS get the control of SHPC hotplug and SHPC driver load fail later.
> >
> > [    6.170345] shpchp 0000:00:03.0: Requesting control of SHPC hotplug via 
> > OSHP (\_SB_.PCI0.S18_)
> > [    6.171962] shpchp 0000:00:03.0: Requesting control of SHPC hotplug via 
> > OSHP (\_SB_.PCI0)
> > [    6.173556] shpchp 0000:00:03.0: Gained control of SHPC hotplug 
> > (\_SB_.PCI0)
> > [    6.175144] shpchp 0000:00:03.0: HPC vendor_id 1b36 device_id 1 ss_vid 0 
> > ss_did 0
> > [    6.196153] shpchp 0000:00:03.0: irq 24 for MSI/MSI-X
> > [    6.197211] shpchp 0000:00:03.0: pci_hp_register failed with error -16
> > [    6.198272] shpchp 0000:00:03.0: Slot initialization failed
> >
> > this looks more suitable.
> >
> > +    if (!pm->pcihp_bridge_en) {
> > +        aml_append(dev, build_i440fx_oshp_method());
> > +    }
>
> we also have
>  PIIX4_PM.acpi-root-pci-hotplug (default true)
> though it seems that ACPI hotplug takes precedence of SHPC if both are 
> enabled.
> So I'd take it and OSHP approach seems simpler than adding _OSC to do the 
> same.

yes, I tried to add an OSC method, but the OS and the firmware failed to 
negotiate in the guest, dmesg as follow.
Through analyzing the code, I found that this process relies on 
pci_ext_cfg_avail in negotiate_os_control. On i440fx,
pci_ext_cfg_avail is false.

[    0.631156] acpi PNP0A03:00: _OSC: OS supports [ASPM ClockPM Segments MSI 
EDR HPX-Type3]
[    0.632184] acpi PNP0A03:00: _OSC: not requesting OS control; OS requires 
[ExtendedConfig ASPM ClockPM MSI]
[    0.633160] acpi PNP0A03:00: fail to add MMCONFIG information, can't access 
extended PCI configuration space under this bridge.

Therefore, I chose the OSHP method.
>
> >
> > >
> > > >          aml_append(sb_scope, dev);
> > > >          aml_append(dsdt, sb_scope);
> > > >
> > > > @@ -1586,6 +1604,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> > > >                  aml_append(dev, build_q35_osc_method(true));
> > > >              } else {
> > > >                  aml_append(dev, aml_name_decl("_HID", 
> > > >aml_eisaid("PNP0A03")));
> > > > +                aml_append(dev, build_oshp_method());
> > > >              }
> > > >
> > > >              if (numa_node != NUMA_NODE_UNASSIGNED) {
> >
> > Hot plug/unplug a device using SHPC will take more time than ACPI PCI 
> > hotplug, because
> > after pressing the button, it can be cancelled within 5 seconds in SHPC 
> > driver.
>
> for SHPC on PXB see,
> commit d10dda2d60 hw/pci-bridge: disable SHPC in PXB
>
> it seems that enabling SHPC on PXB in QEMU is not enough,
> UEFI needs to support that as well
> (CCing Gerd to check whether it is possible at all)
>
> > If I want to use ACPI PCI hotplug in the pxb bridge, what else need to be 
> > done?
>
> does it have to be hotplug directly into pxb or
> would be it be sufficient to have hotplug support
> on pci-bridge attached to a pxb?

It's sufficient to hotplug support on pci-bridge attached to a pxb.

>
> I particularly do not like spreading ACPI hotplug
> to any host bridges, as it's quite complicated
> code.
>