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

[Michael S. Tsirkin]

On Mon, Sep 05, 2022 at 12:55:31PM +0530, Ani Sinha wrote:
> Possible fix for https://bugzilla.redhat.com/show_bug.cgi?id=2089545
> 
> Change in AML:
> 
> @@ -47,33 +47,39 @@
>      Scope (_SB)
>      {
>          Device (PCI0)
>          {
>              Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: 
> Hardware ID
>              Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: 
> Compatible ID
>              Name (_ADR, Zero)  // _ADR: Address
>              Name (_UID, Zero)  // _UID: Unique ID
>              Method (_OSC, 4, NotSerialized)  // _OSC: Operating System 
> Capabilities
>              {
>                  CreateDWordField (Arg3, Zero, CDW1)
>                  If ((Arg0 == ToUUID ("33db4d5b-1ff7-401c-9657-7441c03dd766") 
> /* PCI Host Bridge Device */))
>                  {
>                      CreateDWordField (Arg3, 0x04, CDW2)
>                      CreateDWordField (Arg3, 0x08, CDW3)
>                      Local0 = CDW3 /* \_SB_.PCI0._OSC.CDW3 */
> -                    Local0 &= 0x1E
> +                    Local0 &= 0x1F
> +                    Local1 = (CDW3 & One)
> +                    If ((One == Local1))
> +                    {
> +                        CDW1 |= 0x12
> +                    }
> +
>                      If ((Arg1 != One))
>                      {
>                          CDW1 |= 0x08
>                      }
> 
>                      If ((CDW3 != Local0))
>                      {
>                          CDW1 |= 0x10
>                      }
> 
>                      CDW3 = Local0
>                  }
>                  Else
>                  {
>                      CDW1 |= 0x04
>                  }
> **
> 
> Signed-off-by: Ani Sinha <ani@anisinha.ca>
> ---
>  hw/i386/acpi-build.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> 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));
>  
> +    /*
> +     * if ACPI PCI Hot-plug is enabled, do not let OSPM set OSC PCIE
> +     * Native hotplug ctrl bit.
> +     */
> +    if (!enable_native_pcie_hotplug) {
> +        /* check if the ACPI native hotplug bit is set by the OS in DWORD3 */
> +        aml_append(if_ctx, aml_and(aml_name("CDW3"),
> +                                   aml_int(0x01), a_pcie_nhp_ctl));
> +        if_ctx3 = aml_if(aml_equal(aml_int(1), a_pcie_nhp_ctl));
> +        /*
> +         * ACPI spec 5.1, section 6.2.11
> +         * bit 1 in first DWORD - _OSC failure
> +         * bit 4 in first DWORD - capabilities masked
> +         */
> +        aml_append(if_ctx3, aml_or(a_cwd1, aml_int(0x12), a_cwd1));


0x12 ->

 ( 0x1 << 4 ) /* _OSC failure */ | ( 0x1 << 1) /* capabilities masked */


> +        aml_append(if_ctx, if_ctx3);
> +    }
>      if_ctx2 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(1))));
>      /* Unknown revision */
>      aml_append(if_ctx2, aml_or(a_cwd1, aml_int(0x08), a_cwd1));

Hmm wait a sec


    if_ctx2 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), a_ctrl)));
    /* Capabilities bits were masked */
    aml_append(if_ctx2, aml_or(a_cwd1, aml_int(0x10), a_cwd1));
    aml_append(if_ctx, if_ctx2);


this one seems subtly different ...


> -- 
> 2.34.1