Re: OVMF and PCI0 UID

Aml *dev = aml_device("%s", "PCI0");

aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));

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("_ADR", aml_int(0)));

aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));

aml_append(dev, aml_name_decl("_STR", aml_unicode("PCIe 0 Device")));

aml_append(dev, aml_name_decl("_CCA", aml_int(1)));

https://github.com/qemu/qemu/blob/2c1fb4d/hw/arm/virt-acpi-build.c#L168-L175

It makes UID on ARM builds a string, which is certainly not expected. We do not have ARM test setups, but I hope this can be useful too.

Best wishes,

Vitaly

21 июля 2020 г., в 09:58, Michael S. Tsirkin <mst@redhat.com> написал(а):

On Mon, Jul 20, 2020 at 11:25:58PM +0200, Laszlo Ersek wrote:
Hi Vitaly,

adding Igor, Michael, Marcel, and qemu-devel.

On 07/20/20 11:06, vit9696 wrote:
Hello,

I discovered an issue with inconsistent QEMU/OVMF device paths, and
while I am unsure whether directing this e-mail is appropriate to you,
I believe that you likely have the contacts you could forward this
e-mail to.

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 (or OVMF) 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. To me this looks like a bug here:
https://github.com/qemu/qemu/blob/8f06f22/hw/i386/acpi-build.c#L1511-L1515
Which does not correspond to the primary PCI identifier here:
https://github.com/qemu/qemu/blob/5a79d10/hw/pci/pci.c#L160-L162

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

Would you be able to forward this to the right people or perhaps keep
an eye on the issue itself?

I think you are right.

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.

Does the following patch work for you? (I can see you proposed the same
in
<https://github.com/acidanthera/bugtracker/issues/1050#issuecomment-660734139>)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bcbbbb2a35..7a5a8b3521b0 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1496,9 +1496,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
        sb_scope = aml_scope("_SB");
        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);

        build_hpet_aml(dsdt);
@@ -1511,9 +1511,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
        dev = aml_device("PCI0");
        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);

If it does, I suggest submitting the above patch to qemu-devel, and/or
filing a bug for upstream QEMU at <https://bugs.launchpad.net/qemu/>.

Or even just reporting whether the above helps you, we can
take it from there.

(Note: I didn't even compile the above change.)

Thanks
Laszlo

From:	vit9696
Subject:	Re: OVMF and PCI0 UID
Date:	Tue, 21 Jul 2020 09:24:22 +0000