[v2] xen/x86/pvh: handle ACPI RSDT table in PVH Dom0 build

Commit Message

Stefano Stabellini May 25, 2023, 11:18 p.m. UTC

From: Stefano Stabellini <[e-mail]>

Xen always generates a XSDT table even if the firmware provided a RSDT
table. Copy the RSDT header from the firmware table when the XSDT table
is missing.

Fixes: 1d74282c455f ('x86: setup PVHv2 Dom0 ACPI tables')
Suggested-by: Roger Pau Monné <[e-mail]>
Signed-off-by: Stefano Stabellini <[e-mail]>
---

Note that this patch is sufficient to get Xen and Dom0 PVH to boot on
QEMU. It turns out that dom0-iommu=none is not needed because QEMU can
actually emulate an AMD IOMMU. I just needed to use the right command
line arguments. Without dom0-iommu=none, the error fixed by the second
patch in v1 doesn't manifest, so I dropped patch 2/2.

FYI this is the QEMU command line to use:

qemu-system-x86_64 \
    -machine q35 \
    -device amd-iommu \
    -m 2G -smp 2 \
    -monitor none -serial stdio \
    -nographic \
    -device virtio-net-pci,netdev=n0 \
    -netdev user,id=n0,tftp=binaries,bootfile=/pxelinux.0

This is pxelinux.0:

kernel xen console=com1 dom0=pvh dom0_mem=1G noreboot
module bzImage console=hvc0
module xen-rootfs.cpio.gz
boot

I'll work on adding a gitlab-ci test for this next.

---
 xen/arch/x86/hvm/dom0_build.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Jan Beulich May 26, 2023, 12:43 p.m. UTC | #1

On 26.05.2023 01:18, Stefano Stabellini wrote:
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -966,7 +966,16 @@ static int __init pvh_setup_acpi_xsdt(struct domain *d, paddr_t madt_addr,
>          rc = -EINVAL;
>          goto out;
>      }
> -    xsdt_paddr = rsdp->xsdt_physical_address;
> +    /*
> +     * Note the header is the same for both RSDT and XSDT, so it's fine to
> +     * copy the native RSDT header to the Xen crafted XSDT if no native
> +     * XSDT is available.
> +     */
> +    if ( rsdp->revision > 1 && rsdp->xsdt_physical_address )
> +        xsdt_paddr = rsdp->xsdt_physical_address;
> +    else
> +        xsdt_paddr = rsdp->rsdt_physical_address;
> +
>      acpi_os_unmap_memory(rsdp, sizeof(*rsdp));
>      table = acpi_os_map_memory(xsdt_paddr, sizeof(*table));
>      if ( !table )

To continue context some:

    {
        printk("Unable to map XSDT\n");
        rc = -EINVAL;
        goto out;
    }
    xsdt->header = *table;
    acpi_os_unmap_memory(table, sizeof(*table));

You're now pointing the guest's XSDT addr at a table that's really an
RSDT. After copying, I think you want to adjust the table signature
('R' -> 'X'). Luckily a revision of 1 is valid for both tables.

Jan

Patch

diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index fd2cbf68bc..e1043e40d2 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -966,7 +966,16 @@  static int __init pvh_setup_acpi_xsdt(struct domain *d, paddr_t madt_addr,
         rc = -EINVAL;
         goto out;
     }
-    xsdt_paddr = rsdp->xsdt_physical_address;
+    /*
+     * Note the header is the same for both RSDT and XSDT, so it's fine to
+     * copy the native RSDT header to the Xen crafted XSDT if no native
+     * XSDT is available.
+     */
+    if ( rsdp->revision > 1 && rsdp->xsdt_physical_address )
+        xsdt_paddr = rsdp->xsdt_physical_address;
+    else
+        xsdt_paddr = rsdp->rsdt_physical_address;
+
     acpi_os_unmap_memory(rsdp, sizeof(*rsdp));
     table = acpi_os_map_memory(xsdt_paddr, sizeof(*table));
     if ( !table )