[PULL 0/3] virtio,acpi: bugfixes

[PULL 0/3] virtio,acpi: bugfixes

[Michael S. Tsirkin]

The following changes since commit 5c1c3e4f02e458cf280c677c817ae4fd1ed9bf10:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200803' into staging (2020-08-03 20:34:26 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 5957b49b423fe456896e10f7e4a6c69be07f9407:

  virtio-mem: Correct format specifier mismatch for RISC-V (2020-08-04 09:13:34 -0400)

----------------------------------------------------------------
virtio,acpi: bugfixes

A couple of last minute bugfixes.

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

----------------------------------------------------------------
Bruce Rogers (1):
      virtio-mem: Correct format specifier mismatch for RISC-V

Michael S. Tsirkin (2):
      i386/acpi: fix inconsistent QEMU/OVMF device paths
      arm/acpi: fix an out of spec _UID for PCI root

 hw/arm/virt-acpi-build.c | 2 +-
 hw/i386/acpi-build.c     | 4 ++--
 hw/virtio/virtio-mem.c   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

[PULL 1/3] i386/acpi: fix inconsistent QEMU/OVMF device paths

[Michael S. Tsirkin]

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.

Cc: qemu-stable@nongnu.org
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@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

[PULL 2/3] arm/acpi: fix an out of spec _UID for PCI root

[Michael S. Tsirkin]

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.

Cc: qemu-stable@nongnu.org
Reported-by: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 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

[PULL 3/3] virtio-mem: Correct format specifier mismatch for RISC-V

[Michael S. Tsirkin]

From: Bruce Rogers <brogers@suse.com>

This likely affects other, less popular host architectures as well.
Less common host architectures under linux get QEMU_VMALLOC_ALIGN (from
which VIRTIO_MEM_MIN_BLOCK_SIZE is derived) define to a variable of
type uintptr, which isn't compatible with the format specifier used to
print a user message. Since this particular usage of the underlying data
seems unique to this file, the simple fix is to just cast
QEMU_VMALLOC_ALIGN to uint32_t, which corresponds to the format specifier
used.

Signed-off-by: Bruce Rogers <brogers@suse.com>
Message-Id: <20200730130519.168475-1-brogers@suse.com>
Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
---
 hw/virtio/virtio-mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index c12e9f79b0..7740fc613f 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -36,7 +36,7 @@
  * Use QEMU_VMALLOC_ALIGN, so no THP will have to be split when unplugging
  * memory (e.g., 2MB on x86_64).
  */
-#define VIRTIO_MEM_MIN_BLOCK_SIZE QEMU_VMALLOC_ALIGN
+#define VIRTIO_MEM_MIN_BLOCK_SIZE ((uint32_t)QEMU_VMALLOC_ALIGN)
 /*
  * Size the usable region bigger than the requested size if possible. Esp.
  * Linux guests will only add (aligned) memory blocks in case they fully
-- 
MST

Re: [PULL 0/3] virtio,acpi: bugfixes

[Peter Maydell]

On Tue, 4 Aug 2020 at 15:17, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> The following changes since commit 5c1c3e4f02e458cf280c677c817ae4fd1ed9bf10:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200803' into staging (2020-08-03 20:34:26 +0100)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 5957b49b423fe456896e10f7e4a6c69be07f9407:
>
>   virtio-mem: Correct format specifier mismatch for RISC-V (2020-08-04 09:13:34 -0400)
>
> ----------------------------------------------------------------
> virtio,acpi: bugfixes
>
> A couple of last minute bugfixes.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ----------------------------------------------------------------

(as noted on irc, fails 'make check' due to DSDT mismatch).

thanks
-- PMM

Re: [PULL 0/3] virtio,acpi: bugfixes

[Peter Maydell]

On Tue, 4 Aug 2020 at 15:17, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> The following changes since commit 5c1c3e4f02e458cf280c677c817ae4fd1ed9bf10:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200803' into staging (2020-08-03 20:34:26 +0100)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 5957b49b423fe456896e10f7e4a6c69be07f9407:
>
>   virtio-mem: Correct format specifier mismatch for RISC-V (2020-08-04 09:13:34 -0400)
>
> ----------------------------------------------------------------
> virtio,acpi: bugfixes
>
> A couple of last minute bugfixes.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ----------------------------------------------------------------
> Bruce Rogers (1):
>       virtio-mem: Correct format specifier mismatch for RISC-V
>
> Michael S. Tsirkin (2):
>       i386/acpi: fix inconsistent QEMU/OVMF device paths
>       arm/acpi: fix an out of spec _UID for PCI root


I applied your updated pull with just the virtio-mem fix.

thanks
-- PMM

Re: [PULL 0/3] virtio,acpi: bugfixes

[Michael S. Tsirkin]

On Tue, Aug 04, 2020 at 10:17:01AM -0400, Michael S. Tsirkin wrote:
> The following changes since commit 5c1c3e4f02e458cf280c677c817ae4fd1ed9bf10:
> 
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200803' into staging (2020-08-03 20:34:26 +0100)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> 
> for you to fetch changes up to 5957b49b423fe456896e10f7e4a6c69be07f9407:
> 
>   virtio-mem: Correct format specifier mismatch for RISC-V (2020-08-04 09:13:34 -0400)


I dropped patches 1-2 and pushed a new tag.
Pls apply, sorry about the noise!

> ----------------------------------------------------------------
> virtio,acpi: bugfixes
> 
> A couple of last minute bugfixes.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ----------------------------------------------------------------
> Bruce Rogers (1):
>       virtio-mem: Correct format specifier mismatch for RISC-V
> 
> Michael S. Tsirkin (2):
>       i386/acpi: fix inconsistent QEMU/OVMF device paths
>       arm/acpi: fix an out of spec _UID for PCI root
> 
>  hw/arm/virt-acpi-build.c | 2 +-
>  hw/i386/acpi-build.c     | 4 ++--
>  hw/virtio/virtio-mem.c   | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 

Re: [PULL 0/3] virtio,acpi: bugfixes

[Michael S. Tsirkin]

On Tue, Aug 04, 2020 at 06:20:53PM +0100, Peter Maydell wrote:
> On Tue, 4 Aug 2020 at 15:17, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > The following changes since commit 5c1c3e4f02e458cf280c677c817ae4fd1ed9bf10:
> >
> >   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200803' into staging (2020-08-03 20:34:26 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >
> > for you to fetch changes up to 5957b49b423fe456896e10f7e4a6c69be07f9407:
> >
> >   virtio-mem: Correct format specifier mismatch for RISC-V (2020-08-04 09:13:34 -0400)
> >
> > ----------------------------------------------------------------
> > virtio,acpi: bugfixes
> >
> > A couple of last minute bugfixes.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > ----------------------------------------------------------------
> > Bruce Rogers (1):
> >       virtio-mem: Correct format specifier mismatch for RISC-V
> >
> > Michael S. Tsirkin (2):
> >       i386/acpi: fix inconsistent QEMU/OVMF device paths
> >       arm/acpi: fix an out of spec _UID for PCI root
> 
> 
> I applied your updated pull with just the virtio-mem fix.
> 
> thanks
> -- PMM

Thanks!
OK I think this is a good place to stop. No need to postpone
the rc for the rest of them - not regressions, fixes can wait
until the next release.

-- 
MST