qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/3] pci: acpi: Windows 'PCI Label Id' bug workaround


From: Ani Sinha
Subject: Re: [PATCH 2/3] pci: acpi: Windows 'PCI Label Id' bug workaround
Date: Fri, 17 Jan 2025 12:35:47 +0530

On Wed, Jan 15, 2025 at 6:23 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> Current versions of Windows call _DSM(func=7) regardless
> of whether it is supported or not. It leads to NICs having bogus
> 'PCI Label Id = 0', where none should be set at all.
>
> Also presence of 'PCI Label Id' triggers another Windows bug
> on localized versions that leads to hangs. The later bug is fixed
> in latest updates for 'Windows Server' but not in consumer
> versions of Windows (and there is no plans to fix it
> as far as I'm aware).
>
> Given it's easy, implement Microsoft suggested workaround
> (return invalid Package) so that affected Windows versions
> could boot on QEMU.
> This would effectvely remove bogus 'PCI Label Id's on NICs,
> but MS teem confirmed that flipping 'PCI Label Id' should not
> change 'Network Connection' ennumeration, so it should be safe
> for QEMU to change _DSM without any compat code.
>
> Smoke tested with WinXP and WS2022
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/774
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/acpi-build.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 733b8f0851..1311a0d4f3 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -654,6 +654,7 @@ static Aml *aml_pci_pdsm(void)
>      Aml *acpi_index = aml_local(2);
>      Aml *zero = aml_int(0);
>      Aml *one = aml_int(1);
> +    Aml *not_supp = aml_int(0xFFFFFFFF);
>      Aml *func = aml_arg(2);
>      Aml *params = aml_arg(4);
>      Aml *bnum = aml_derefof(aml_index(params, aml_int(0)));
> @@ -678,7 +679,7 @@ static Aml *aml_pci_pdsm(void)
>           */
>          ifctx1 = aml_if(aml_lnot(
>                       aml_or(aml_equal(acpi_index, zero),
> -                            aml_equal(acpi_index, aml_int(0xFFFFFFFF)), NULL)
> +                            aml_equal(acpi_index, not_supp), NULL)
>                   ));
>          {
>              /* have supported functions */
> @@ -704,18 +705,30 @@ static Aml *aml_pci_pdsm(void)
>      {
>         Aml *pkg = aml_package(2);
>
> -       aml_append(pkg, zero);
> -       /*
> -        * optional, if not impl. should return null string
> -        */
> -       aml_append(pkg, aml_string("%s", ""));
> -       aml_append(ifctx, aml_store(pkg, ret));
> -
>         aml_append(ifctx, aml_store(aml_call2("AIDX", bnum, sunum), 
> acpi_index));
> +       aml_append(ifctx, aml_store(pkg, ret));
>         /*
> -        * update acpi-index to actual value
> +        * Windows calls func=7 without checking if it's available,
> +        * as workaround Microsoft has suggested to return invalid for func7
> +        * Package, so return 2 elements package but only initialize elements
> +        * when acpi_index is supported and leave them uninitialized, which
> +        * leads elements to being Uninitialized ObjectType and should trip
> +        * Windows into discarding result as an unexpected and prevent setting
> +        * bogus 'PCI Label' on the device.

This comment is very confusing!

>          */
> -       aml_append(ifctx, aml_store(acpi_index, aml_index(ret, zero)));
> +       ifctx1 = aml_if(aml_lnot(aml_lor(
> +                    aml_equal(acpi_index, zero), aml_equal(acpi_index, 
> not_supp)
> +                )));

So this conditional checks if the acpi index is supported (because its
aml_lnot()).

> +       {
> +           aml_append(ifctx1, aml_store(acpi_index, aml_index(ret, zero)));
> +           /*
> +            * optional, if not impl. should return null string
> +            */

I know this comes from the existing code but I am still confused. Why
is this appending "return null string" logic to "if acpi index is
supprted" conditional?

> +           aml_append(ifctx1, aml_store(aml_string("%s", ""),
> +                                        aml_index(ret, one)));
> +       }
> +       aml_append(ifctx, ifctx1);
> +
>         aml_append(ifctx, aml_return(ret));
>      }
>
> --
> 2.43.0
>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]