QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths
@ 2020-07-30 15:58 Michael S. Tsirkin
  2020-07-30 15:58 ` [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root Michael S. Tsirkin
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2020-07-30 15:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: vit9696, Eduardo Habkost, Paolo Bonzini, Igor Mammedov,
	Laszlo Ersek, Richard Henderson

macOS uses ACPI UIDs to build the DevicePath for NVRAM boot options,
while OVMF firmware gets them via an internal channel through QEMU.
Due to a bug in QEMU ACPI currently UEFI firmware and ACPI have
different values, and this makes the underlying operating system
unable to report its boot option.

The particular node in question is the primary PciRoot (PCI0 in ACPI),
which for some reason gets assigned 1 in ACPI UID and 0 in the
DevicePath. This is due to the _UID assigned to it by build_dsdt in
hw/i386/acpi-build.c Which does not correspond to the primary PCI
identifier given by pcibus_num in hw/pci/pci.c

Reference with the device paths, OVMF startup logs, and ACPI table
dumps (SysReport):
https://github.com/acidanthera/bugtracker/issues/1050

In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
the paragraph,

    Root PCI bridges will use the plug and play ID of PNP0A03, This will
    be stored in the ACPI Device Path _HID field, or in the Expanded
    ACPI Device Path _CID field to match the ACPI name space. The _UID
    in the ACPI Device Path structure must match the _UID in the ACPI
    name space.

(See especially the last sentence.)

Considering *extra* root bridges / root buses (with bus number > 0),
QEMU's ACPI generator actually does the right thing; since QEMU commit
c96d9286a6d7 ("i386/acpi-build: more traditional _UID and _HID for PXB
root buses", 2015-06-11).

However, the _UID values for root bridge zero (on both i440fx and q35)
have always been "wrong" (from UEFI perspective), going back in QEMU to
commit 74523b850189 ("i386: add ACPI table files from seabios",
2013-10-14).

Even in SeaBIOS, these _UID values have always been 1; see commit
a4d357638c57 ("Port rombios32 code from bochs-bios.", 2008-03-08) for
i440fx, and commit ecbe3fd61511 ("seabios: q35: add dsdt", 2012-12-01)
for q35.

Suggested-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: vit9696 <vit9696@protonmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bcbbbb2a..7a5a8b3521 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1497,7 +1497,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         dev = aml_device("PCI0");
         aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
         aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
         aml_append(sb_scope, dev);
         aml_append(dsdt, sb_scope);
 
@@ -1512,7 +1512,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
         aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
         aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
         aml_append(dev, build_q35_osc_method());
         aml_append(sb_scope, dev);
         aml_append(dsdt, sb_scope);
-- 
MST



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root
  2020-07-30 15:58 [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths Michael S. Tsirkin
@ 2020-07-30 15:58 ` Michael S. Tsirkin
  2020-07-30 16:12   ` Philippe Mathieu-Daudé
                     ` (3 more replies)
  2020-07-30 16:11 ` [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 4 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2020-07-30 15:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, vit9696, Eduardo Habkost, Shannon Zhao, qemu-arm,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek, Richard Henderson

On ARM/virt machine type QEMU currently reports an incorrect _UID in
ACPI.

The particular node in question is the primary PciRoot (PCI0 in ACPI),
which gets assigned PCI0 in ACPI UID and 0 in the
DevicePath. This is due to the _UID assigned to it by build_dsdt in
hw/arm/virt-acpi-build.c Which does not correspond to the primary PCI
identifier given by pcibus_num in hw/pci/pci.c

In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
the paragraph,

    Root PCI bridges will use the plug and play ID of PNP0A03, This will
    be stored in the ACPI Device Path _HID field, or in the Expanded
    ACPI Device Path _CID field to match the ACPI name space. The _UID
    in the ACPI Device Path structure must match the _UID in the ACPI
    name space.

(See especially the last sentence.)

A similar bug has been reported on i386, on that architecture it has
been reported to confuse at least macOS which uses ACPI UIDs to build
the DevicePath for NVRAM boot options, while OVMF firmware gets them via
an internal channel through QEMU.  When UEFI firmware and ACPI have
different values, this makes the underlying operating system unable to
report its boot option.

Reported-by: vit9696 <vit9696@protonmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Peter can you either ack or merge this one pls?

 hw/arm/virt-acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 91f0df7b13..0a482ff6f7 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -170,7 +170,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
     aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
     aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
     aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
-    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
     aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
     aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
 
-- 
MST



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths
  2020-07-30 15:58 [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths Michael S. Tsirkin
  2020-07-30 15:58 ` [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root Michael S. Tsirkin
@ 2020-07-30 16:11 ` Philippe Mathieu-Daudé
  2020-07-30 19:35   ` Michael S. Tsirkin
  2020-07-30 19:34 ` Laszlo Ersek
  2020-07-31  9:30 ` Igor Mammedov
  3 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-30 16:11 UTC (permalink / raw)
  To: Michael S. Tsirkin, qemu-devel
  Cc: vit9696, Eduardo Habkost, Igor Mammedov, Paolo Bonzini,
	Laszlo Ersek, Richard Henderson

On 7/30/20 5:58 PM, Michael S. Tsirkin wrote:
> macOS uses ACPI UIDs to build the DevicePath for NVRAM boot options,
> while OVMF firmware gets them via an internal channel through QEMU.
> Due to a bug in QEMU ACPI currently UEFI firmware and ACPI have
> different values, and this makes the underlying operating system
> unable to report its boot option.
> 
> The particular node in question is the primary PciRoot (PCI0 in ACPI),
> which for some reason gets assigned 1 in ACPI UID and 0 in the
> DevicePath. This is due to the _UID assigned to it by build_dsdt in
> hw/i386/acpi-build.c Which does not correspond to the primary PCI
> identifier given by pcibus_num in hw/pci/pci.c
> 
> Reference with the device paths, OVMF startup logs, and ACPI table
> dumps (SysReport):
> https://github.com/acidanthera/bugtracker/issues/1050
> 
> In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
> the paragraph,
> 
>     Root PCI bridges will use the plug and play ID of PNP0A03, This will
>     be stored in the ACPI Device Path _HID field, or in the Expanded
>     ACPI Device Path _CID field to match the ACPI name space. The _UID
>     in the ACPI Device Path structure must match the _UID in the ACPI
>     name space.
> 
> (See especially the last sentence.)
> 
> Considering *extra* root bridges / root buses (with bus number > 0),
> QEMU's ACPI generator actually does the right thing; since QEMU commit
> c96d9286a6d7 ("i386/acpi-build: more traditional _UID and _HID for PXB
> root buses", 2015-06-11).
> 
> However, the _UID values for root bridge zero (on both i440fx and q35)
> have always been "wrong" (from UEFI perspective), going back in QEMU to
> commit 74523b850189 ("i386: add ACPI table files from seabios",
> 2013-10-14).
> 
> Even in SeaBIOS, these _UID values have always been 1; see commit
> a4d357638c57 ("Port rombios32 code from bochs-bios.", 2008-03-08) for
> i440fx, and commit ecbe3fd61511 ("seabios: q35: add dsdt", 2012-12-01)
> for q35.
> 
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: vit9696 <vit9696@protonmail.com>

Vitaly uses his full name on EDK2 mailing list, so I don't think he'll
have a problem to use it in QEMU too:
Tested-by: Vitaly Cheptsov <vit9696@protonmail.com>

From:
https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line
"Please use your real name to sign a patch (not an alias or acronym)."

> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/i386/acpi-build.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bcbbbb2a..7a5a8b3521 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1497,7 +1497,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          dev = aml_device("PCI0");
>          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
>          aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> -        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>          aml_append(sb_scope, dev);
>          aml_append(dsdt, sb_scope);
>  
> @@ -1512,7 +1512,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
>          aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
>          aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> -        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>          aml_append(dev, build_q35_osc_method());
>          aml_append(sb_scope, dev);
>          aml_append(dsdt, sb_scope);
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root
  2020-07-30 15:58 ` [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root Michael S. Tsirkin
@ 2020-07-30 16:12   ` Philippe Mathieu-Daudé
  2020-07-30 19:35   ` Laszlo Ersek
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-30 16:12 UTC (permalink / raw)
  To: Michael S. Tsirkin, qemu-devel
  Cc: Peter Maydell, vit9696, Shannon Zhao, qemu-arm, Igor Mammedov,
	Paolo Bonzini, Richard Henderson, Laszlo Ersek, Eduardo Habkost

On 7/30/20 5:58 PM, Michael S. Tsirkin wrote:
> On ARM/virt machine type QEMU currently reports an incorrect _UID in
> ACPI.
> 
> The particular node in question is the primary PciRoot (PCI0 in ACPI),
> which gets assigned PCI0 in ACPI UID and 0 in the
> DevicePath. This is due to the _UID assigned to it by build_dsdt in
> hw/arm/virt-acpi-build.c Which does not correspond to the primary PCI
> identifier given by pcibus_num in hw/pci/pci.c
> 
> In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
> the paragraph,
> 
>     Root PCI bridges will use the plug and play ID of PNP0A03, This will
>     be stored in the ACPI Device Path _HID field, or in the Expanded
>     ACPI Device Path _CID field to match the ACPI name space. The _UID
>     in the ACPI Device Path structure must match the _UID in the ACPI
>     name space.
> 
> (See especially the last sentence.)
> 
> A similar bug has been reported on i386, on that architecture it has
> been reported to confuse at least macOS which uses ACPI UIDs to build
> the DevicePath for NVRAM boot options, while OVMF firmware gets them via
> an internal channel through QEMU.  When UEFI firmware and ACPI have
> different values, this makes the underlying operating system unable to
> report its boot option.
> 
> Reported-by: vit9696 <vit9696@protonmail.com>

Ditto:
Reported-by: Vitaly Cheptsov <vit9696@protonmail.com>

> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Peter can you either ack or merge this one pls?
> 
>  hw/arm/virt-acpi-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 91f0df7b13..0a482ff6f7 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -170,7 +170,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>      aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
>      aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
>      aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
> -    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>      aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
>      aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>  
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths
  2020-07-30 15:58 [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths Michael S. Tsirkin
  2020-07-30 15:58 ` [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root Michael S. Tsirkin
  2020-07-30 16:11 ` [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths Philippe Mathieu-Daudé
@ 2020-07-30 19:34 ` Laszlo Ersek
  2020-07-31  9:30 ` Igor Mammedov
  3 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2020-07-30 19:34 UTC (permalink / raw)
  To: Michael S. Tsirkin, qemu-devel
  Cc: Eduardo Habkost, Paolo Bonzini, Igor Mammedov, vit9696,
	Richard Henderson

On 07/30/20 17:58, Michael S. Tsirkin wrote:
> macOS uses ACPI UIDs to build the DevicePath for NVRAM boot options,
> while OVMF firmware gets them via an internal channel through QEMU.
> Due to a bug in QEMU ACPI currently UEFI firmware and ACPI have
> different values, and this makes the underlying operating system
> unable to report its boot option.
> 
> The particular node in question is the primary PciRoot (PCI0 in ACPI),
> which for some reason gets assigned 1 in ACPI UID and 0 in the
> DevicePath. This is due to the _UID assigned to it by build_dsdt in
> hw/i386/acpi-build.c Which does not correspond to the primary PCI
> identifier given by pcibus_num in hw/pci/pci.c
> 
> Reference with the device paths, OVMF startup logs, and ACPI table
> dumps (SysReport):
> https://github.com/acidanthera/bugtracker/issues/1050
> 
> In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
> the paragraph,
> 
>     Root PCI bridges will use the plug and play ID of PNP0A03, This will
>     be stored in the ACPI Device Path _HID field, or in the Expanded
>     ACPI Device Path _CID field to match the ACPI name space. The _UID
>     in the ACPI Device Path structure must match the _UID in the ACPI
>     name space.
> 
> (See especially the last sentence.)
> 
> Considering *extra* root bridges / root buses (with bus number > 0),
> QEMU's ACPI generator actually does the right thing; since QEMU commit
> c96d9286a6d7 ("i386/acpi-build: more traditional _UID and _HID for PXB
> root buses", 2015-06-11).
> 
> However, the _UID values for root bridge zero (on both i440fx and q35)
> have always been "wrong" (from UEFI perspective), going back in QEMU to
> commit 74523b850189 ("i386: add ACPI table files from seabios",
> 2013-10-14).
> 
> Even in SeaBIOS, these _UID values have always been 1; see commit
> a4d357638c57 ("Port rombios32 code from bochs-bios.", 2008-03-08) for
> i440fx, and commit ecbe3fd61511 ("seabios: q35: add dsdt", 2012-12-01)
> for q35.
> 
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: vit9696 <vit9696@protonmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/i386/acpi-build.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bcbbbb2a..7a5a8b3521 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1497,7 +1497,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          dev = aml_device("PCI0");
>          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
>          aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> -        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>          aml_append(sb_scope, dev);
>          aml_append(dsdt, sb_scope);
>  
> @@ -1512,7 +1512,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
>          aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
>          aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> -        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>          aml_append(dev, build_q35_osc_method());
>          aml_append(sb_scope, dev);
>          aml_append(dsdt, sb_scope);
> 

with Phil's feedback included:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thank you very much for writing up the commit message on this patch,
Michael!

Laszlo



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root
  2020-07-30 15:58 ` [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root Michael S. Tsirkin
  2020-07-30 16:12   ` Philippe Mathieu-Daudé
@ 2020-07-30 19:35   ` Laszlo Ersek
  2020-07-30 20:33   ` Peter Maydell
  2020-07-31  9:31   ` Igor Mammedov
  3 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2020-07-30 19:35 UTC (permalink / raw)
  To: Michael S. Tsirkin, qemu-devel
  Cc: Peter Maydell, Eduardo Habkost, Shannon Zhao, qemu-arm,
	Paolo Bonzini, Igor Mammedov, vit9696, Richard Henderson

On 07/30/20 17:58, Michael S. Tsirkin wrote:
> On ARM/virt machine type QEMU currently reports an incorrect _UID in
> ACPI.
> 
> The particular node in question is the primary PciRoot (PCI0 in ACPI),
> which gets assigned PCI0 in ACPI UID and 0 in the
> DevicePath. This is due to the _UID assigned to it by build_dsdt in
> hw/arm/virt-acpi-build.c Which does not correspond to the primary PCI
> identifier given by pcibus_num in hw/pci/pci.c
> 
> In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
> the paragraph,
> 
>     Root PCI bridges will use the plug and play ID of PNP0A03, This will
>     be stored in the ACPI Device Path _HID field, or in the Expanded
>     ACPI Device Path _CID field to match the ACPI name space. The _UID
>     in the ACPI Device Path structure must match the _UID in the ACPI
>     name space.
> 
> (See especially the last sentence.)
> 
> A similar bug has been reported on i386, on that architecture it has
> been reported to confuse at least macOS which uses ACPI UIDs to build
> the DevicePath for NVRAM boot options, while OVMF firmware gets them via
> an internal channel through QEMU.  When UEFI firmware and ACPI have
> different values, this makes the underlying operating system unable to
> report its boot option.
> 
> Reported-by: vit9696 <vit9696@protonmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Peter can you either ack or merge this one pls?
> 
>  hw/arm/virt-acpi-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 91f0df7b13..0a482ff6f7 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -170,7 +170,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>      aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
>      aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
>      aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
> -    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>      aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
>      aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>  
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(Vitaly's full name could be included in the Reported-by here as well,
arguably.)

Thanks!
Laszlo



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths
  2020-07-30 16:11 ` [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths Philippe Mathieu-Daudé
@ 2020-07-30 19:35   ` Michael S. Tsirkin
  2020-07-31  2:55     ` vit9696 via
  0 siblings, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2020-07-30 19:35 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Eduardo Habkost, qemu-devel, Igor Mammedov, Paolo Bonzini,
	Richard Henderson, Laszlo Ersek, vit9696

On Thu, Jul 30, 2020 at 06:11:17PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/30/20 5:58 PM, Michael S. Tsirkin wrote:
> > macOS uses ACPI UIDs to build the DevicePath for NVRAM boot options,
> > while OVMF firmware gets them via an internal channel through QEMU.
> > Due to a bug in QEMU ACPI currently UEFI firmware and ACPI have
> > different values, and this makes the underlying operating system
> > unable to report its boot option.
> > 
> > The particular node in question is the primary PciRoot (PCI0 in ACPI),
> > which for some reason gets assigned 1 in ACPI UID and 0 in the
> > DevicePath. This is due to the _UID assigned to it by build_dsdt in
> > hw/i386/acpi-build.c Which does not correspond to the primary PCI
> > identifier given by pcibus_num in hw/pci/pci.c
> > 
> > Reference with the device paths, OVMF startup logs, and ACPI table
> > dumps (SysReport):
> > https://github.com/acidanthera/bugtracker/issues/1050
> > 
> > In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
> > the paragraph,
> > 
> >     Root PCI bridges will use the plug and play ID of PNP0A03, This will
> >     be stored in the ACPI Device Path _HID field, or in the Expanded
> >     ACPI Device Path _CID field to match the ACPI name space. The _UID
> >     in the ACPI Device Path structure must match the _UID in the ACPI
> >     name space.
> > 
> > (See especially the last sentence.)
> > 
> > Considering *extra* root bridges / root buses (with bus number > 0),
> > QEMU's ACPI generator actually does the right thing; since QEMU commit
> > c96d9286a6d7 ("i386/acpi-build: more traditional _UID and _HID for PXB
> > root buses", 2015-06-11).
> > 
> > However, the _UID values for root bridge zero (on both i440fx and q35)
> > have always been "wrong" (from UEFI perspective), going back in QEMU to
> > commit 74523b850189 ("i386: add ACPI table files from seabios",
> > 2013-10-14).
> > 
> > Even in SeaBIOS, these _UID values have always been 1; see commit
> > a4d357638c57 ("Port rombios32 code from bochs-bios.", 2008-03-08) for
> > i440fx, and commit ecbe3fd61511 ("seabios: q35: add dsdt", 2012-12-01)
> > for q35.
> > 
> > Suggested-by: Laszlo Ersek <lersek@redhat.com>
> > Tested-by: vit9696 <vit9696@protonmail.com>
> 
> Vitaly uses his full name on EDK2 mailing list, so I don't think he'll
> have a problem to use it in QEMU too:
> Tested-by: Vitaly Cheptsov <vit9696@protonmail.com>
> 
> From:
> https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line
> "Please use your real name to sign a patch (not an alias or acronym)."

Right. Tested-by is different though, I don't think we have
a problem with anonymous testing.

Anyway, updated.

> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  hw/i386/acpi-build.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index b7bcbbbb2a..7a5a8b3521 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1497,7 +1497,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >          dev = aml_device("PCI0");
> >          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
> >          aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> > -        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> > +        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> >          aml_append(sb_scope, dev);
> >          aml_append(dsdt, sb_scope);
> >  
> > @@ -1512,7 +1512,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
> >          aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> >          aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> > -        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> > +        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> >          aml_append(dev, build_q35_osc_method());
> >          aml_append(sb_scope, dev);
> >          aml_append(dsdt, sb_scope);
> > 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root
  2020-07-30 15:58 ` [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root Michael S. Tsirkin
  2020-07-30 16:12   ` Philippe Mathieu-Daudé
  2020-07-30 19:35   ` Laszlo Ersek
@ 2020-07-30 20:33   ` Peter Maydell
  2020-07-31  9:31   ` Igor Mammedov
  3 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2020-07-30 20:33 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: vit9696, QEMU Developers, Eduardo Habkost, Shannon Zhao,
	qemu-arm, Paolo Bonzini, Igor Mammedov, Laszlo Ersek,
	Richard Henderson

On Thu, 30 Jul 2020 at 16:58, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On ARM/virt machine type QEMU currently reports an incorrect _UID in
> ACPI.
>
> The particular node in question is the primary PciRoot (PCI0 in ACPI),
> which gets assigned PCI0 in ACPI UID and 0 in the
> DevicePath. This is due to the _UID assigned to it by build_dsdt in
> hw/arm/virt-acpi-build.c Which does not correspond to the primary PCI
> identifier given by pcibus_num in hw/pci/pci.c
>
> In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
> the paragraph,
>
>     Root PCI bridges will use the plug and play ID of PNP0A03, This will
>     be stored in the ACPI Device Path _HID field, or in the Expanded
>     ACPI Device Path _CID field to match the ACPI name space. The _UID
>     in the ACPI Device Path structure must match the _UID in the ACPI
>     name space.
>
> (See especially the last sentence.)
>
> A similar bug has been reported on i386, on that architecture it has
> been reported to confuse at least macOS which uses ACPI UIDs to build
> the DevicePath for NVRAM boot options, while OVMF firmware gets them via
> an internal channel through QEMU.  When UEFI firmware and ACPI have
> different values, this makes the underlying operating system unable to
> report its boot option.
>
> Reported-by: vit9696 <vit9696@protonmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> Peter can you either ack or merge this one pls?

Since you have the x86 one to do anyway, I'll let you take
this one.
Acked-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths
  2020-07-30 19:35   ` Michael S. Tsirkin
@ 2020-07-31  2:55     ` vit9696 via
  0 siblings, 0 replies; 11+ messages in thread
From: vit9696 via @ 2020-07-31  2:55 UTC (permalink / raw)
  To: Michael S. Tsirkin, Philippe Mathieu-Daudé
  Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Igor Mammedov,
	Laszlo Ersek, Richard Henderson

[-- Attachment #1.1: Type: text/html, Size: 5320 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 489 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths
  2020-07-30 15:58 [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths Michael S. Tsirkin
                   ` (2 preceding siblings ...)
  2020-07-30 19:34 ` Laszlo Ersek
@ 2020-07-31  9:30 ` Igor Mammedov
  3 siblings, 0 replies; 11+ messages in thread
From: Igor Mammedov @ 2020-07-31  9:30 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eduardo Habkost, qemu-devel, Paolo Bonzini, vit9696,
	Laszlo Ersek, Richard Henderson

On Thu, 30 Jul 2020 11:58:38 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> macOS uses ACPI UIDs to build the DevicePath for NVRAM boot options,
> while OVMF firmware gets them via an internal channel through QEMU.
> Due to a bug in QEMU ACPI currently UEFI firmware and ACPI have
> different values, and this makes the underlying operating system
> unable to report its boot option.
> 
> The particular node in question is the primary PciRoot (PCI0 in ACPI),
> which for some reason gets assigned 1 in ACPI UID and 0 in the
> DevicePath. This is due to the _UID assigned to it by build_dsdt in
> hw/i386/acpi-build.c Which does not correspond to the primary PCI
> identifier given by pcibus_num in hw/pci/pci.c
> 
> Reference with the device paths, OVMF startup logs, and ACPI table
> dumps (SysReport):
> https://github.com/acidanthera/bugtracker/issues/1050
> 
> In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
> the paragraph,
> 
>     Root PCI bridges will use the plug and play ID of PNP0A03, This will
>     be stored in the ACPI Device Path _HID field, or in the Expanded
>     ACPI Device Path _CID field to match the ACPI name space. The _UID
>     in the ACPI Device Path structure must match the _UID in the ACPI
>     name space.
> 
> (See especially the last sentence.)
> 
> Considering *extra* root bridges / root buses (with bus number > 0),
> QEMU's ACPI generator actually does the right thing; since QEMU commit
> c96d9286a6d7 ("i386/acpi-build: more traditional _UID and _HID for PXB
> root buses", 2015-06-11).
> 
> However, the _UID values for root bridge zero (on both i440fx and q35)
> have always been "wrong" (from UEFI perspective), going back in QEMU to
> commit 74523b850189 ("i386: add ACPI table files from seabios",
> 2013-10-14).
> 
> Even in SeaBIOS, these _UID values have always been 1; see commit
> a4d357638c57 ("Port rombios32 code from bochs-bios.", 2008-03-08) for
> i440fx, and commit ecbe3fd61511 ("seabios: q35: add dsdt", 2012-12-01)
> for q35.
> 
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: vit9696 <vit9696@protonmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/acpi-build.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bcbbbb2a..7a5a8b3521 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1497,7 +1497,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          dev = aml_device("PCI0");
>          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
>          aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> -        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>          aml_append(sb_scope, dev);
>          aml_append(dsdt, sb_scope);
>  
> @@ -1512,7 +1512,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
>          aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
>          aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> -        aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>          aml_append(dev, build_q35_osc_method());
>          aml_append(sb_scope, dev);
>          aml_append(dsdt, sb_scope);



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root
  2020-07-30 15:58 ` [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root Michael S. Tsirkin
                     ` (2 preceding siblings ...)
  2020-07-30 20:33   ` Peter Maydell
@ 2020-07-31  9:31   ` Igor Mammedov
  3 siblings, 0 replies; 11+ messages in thread
From: Igor Mammedov @ 2020-07-31  9:31 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Peter Maydell, vit9696, qemu-devel, Eduardo Habkost,
	Shannon Zhao, qemu-arm, Paolo Bonzini, Laszlo Ersek,
	Richard Henderson

On Thu, 30 Jul 2020 11:58:41 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On ARM/virt machine type QEMU currently reports an incorrect _UID in
> ACPI.
> 
> The particular node in question is the primary PciRoot (PCI0 in ACPI),
> which gets assigned PCI0 in ACPI UID and 0 in the
> DevicePath. This is due to the _UID assigned to it by build_dsdt in
> hw/arm/virt-acpi-build.c Which does not correspond to the primary PCI
> identifier given by pcibus_num in hw/pci/pci.c
> 
> In UEFI v2.8, section "10.4.2 Rules with ACPI _HID and _UID" ends with
> the paragraph,
> 
>     Root PCI bridges will use the plug and play ID of PNP0A03, This will
>     be stored in the ACPI Device Path _HID field, or in the Expanded
>     ACPI Device Path _CID field to match the ACPI name space. The _UID
>     in the ACPI Device Path structure must match the _UID in the ACPI
>     name space.
> 
> (See especially the last sentence.)
> 
> A similar bug has been reported on i386, on that architecture it has
> been reported to confuse at least macOS which uses ACPI UIDs to build
> the DevicePath for NVRAM boot options, while OVMF firmware gets them via
> an internal channel through QEMU.  When UEFI firmware and ACPI have
> different values, this makes the underlying operating system unable to
> report its boot option.
> 
> Reported-by: vit9696 <vit9696@protonmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> 
> Peter can you either ack or merge this one pls?
> 
>  hw/arm/virt-acpi-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 91f0df7b13..0a482ff6f7 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -170,7 +170,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>      aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
>      aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
>      aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
> -    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>      aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));
>      aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>  



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 15:58 [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths Michael S. Tsirkin
2020-07-30 15:58 ` [PATCH 2/2] arm/acpi: fix an out of spec _UID for PCI root Michael S. Tsirkin
2020-07-30 16:12   ` Philippe Mathieu-Daudé
2020-07-30 19:35   ` Laszlo Ersek
2020-07-30 20:33   ` Peter Maydell
2020-07-31  9:31   ` Igor Mammedov
2020-07-30 16:11 ` [PATCH 1/2] i386/acpi: fix inconsistent QEMU/OVMF device paths Philippe Mathieu-Daudé
2020-07-30 19:35   ` Michael S. Tsirkin
2020-07-31  2:55     ` vit9696 via
2020-07-30 19:34 ` Laszlo Ersek
2020-07-31  9:30 ` Igor Mammedov

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git