Re: [RFC PATCH] hw/acpi: do not let OSPM set pcie native hotplug when acpi hotplug is enabled

[Ani Sinha]

On Tue, 6 Sep 2022, Igor Mammedov wrote:

> On Mon, 5 Sep 2022 22:25:25 +0530 (IST)
> Ani Sinha <ani@anisinha.ca> wrote:
>
> > On Mon, 5 Sep 2022, Ani Sinha wrote:
> >
> > >
> > >
> >
> > > > >
> > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > > index 0355bd3dda..3dc9379f27 100644
> > > > > --- a/hw/i386/acpi-build.c
> > > > > +++ b/hw/i386/acpi-build.c
> > > > > @@ -1348,10 +1348,12 @@ static Aml *build_q35_osc_method(bool 
> > > > > enable_native_pcie_hotplug)
> > > > >  {
> > > > >      Aml *if_ctx;
> > > > >      Aml *if_ctx2;
> > > > > +    Aml *if_ctx3;
> > > > >      Aml *else_ctx;
> > > > >      Aml *method;
> > > > >      Aml *a_cwd1 = aml_name("CDW1");
> > > > >      Aml *a_ctrl = aml_local(0);
> > > > > +    Aml *a_pcie_nhp_ctl = aml_local(1);
> > > > >
> > > > >      method = aml_method("_OSC", 4, AML_NOTSERIALIZED);
> > > > >      aml_append(method, aml_create_dword_field(aml_arg(3), 
> > > > > aml_int(0), "CDW1"));
> > > > > @@ -1366,11 +1368,26 @@ static Aml *build_q35_osc_method(bool 
> > > > > enable_native_pcie_hotplug)
> > > > >      /*
> > > > >       * Always allow native PME, AER (no dependencies)
> > > > >       * Allow SHPC (PCI bridges can have SHPC controller)
> > > > > -     * Disable PCIe Native Hot-plug if ACPI PCI Hot-plug is enabled.
> > > > >       */
> > > > > -    aml_append(if_ctx, aml_and(a_ctrl,
> > > > > -        aml_int(0x1E | (enable_native_pcie_hotplug ? 0x1 : 0x0)), 
> > > > > a_ctrl));
> > > > > +    aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl));
> > > >
> > > > that makes us not actually mask any capabilities since you forgot to 
> > > > mask
> > > > bit 1 later under if_ctx3 context.
> > > >
> > > > So OSPM will see a permanent failure (_OSC failure bit in CWD1)
> > > > and will have no idea that PCI Hotplug is not supported since we return 
> > > > CWD3
> > > > with this bit still set whoever much it tries to negotiate.
> > >
> > > The failure is only returned when the OS requests/probes native hotplug
> > > capability in CWD1.
> >
> > I meant CWD3.
> For OSPM to know which features are denied by platform, the later must mask
> them in returned bitmask, how otherwise you would see above behavior.

In that case, mst's idea does not work, sadly.

> (or alternatively OSPM might ignore _OSC results and resort to 
> workarounds/probing
> as result enabling native hotplug in which case you would see ATS
> detected).
>
> To verify which hotplug is used, you can just trace acpi_pci_* in QEMU and 
> observe
> if it's used for unplug or not.
>
>