Re: [PATCH 1/2] xen/x86/pvh: use preset XSDT header for XSDT generation

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 15 May 2023, Jan Beulich wrote:
> On 15.05.2023 10:48, Roger Pau Monné wrote:
> > On Fri, May 12, 2023 at 06:17:19PM -0700, Stefano Stabellini wrote:
> >> From: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> >>
> >> Xen always generates a XSDT table even if the firmware provided a RSDT
> >> table. Instead of copying the XSDT header from the firmware table (that
> >> might be missing), generate the XSDT header from a preset.
> >>
> >> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> >> ---
> >>  xen/arch/x86/hvm/dom0_build.c | 32 +++++++++-----------------------
> >>  1 file changed, 9 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> >> index 307edc6a8c..5fde769863 100644
> >> --- a/xen/arch/x86/hvm/dom0_build.c
> >> +++ b/xen/arch/x86/hvm/dom0_build.c
> >> @@ -963,13 +963,18 @@ static int __init pvh_setup_acpi_xsdt(struct domain 
> >> *d, paddr_t madt_addr,
> >>                                        paddr_t *addr)
> >>  {
> >>      struct acpi_table_xsdt *xsdt;
> >> -    struct acpi_table_header *table;
> >> -    struct acpi_table_rsdp *rsdp;
> >>      const struct acpi_table_desc *tables = 
> >> acpi_gbl_root_table_list.tables;
> >>      unsigned long size = sizeof(*xsdt);
> >>      unsigned int i, j, num_tables = 0;
> >> -    paddr_t xsdt_paddr;
> >>      int rc;
> >> +    struct acpi_table_header header = {
> >> +        .signature    = "XSDT",
> >> +        .length       = sizeof(struct acpi_table_header),
> >> +        .revision     = 0x1,
> >> +        .oem_id       = "Xen",
> >> +        .oem_table_id = "HVM",
> > 
> > I think this is wrong, as according to the spec the OEM Table ID must
> > match the OEM Table ID in the FADT.
> > 
> > We likely want to copy the OEM ID and OEM Table ID from the RSDP, and
> > possibly also the other OEM related fields.
> > 
> > Alternatively we might want to copy and use the RSDT on systems that
> > lack an XSDT, or even just copy the header from the RSDT into Xen's
> > crafted XSDT, since the format of the RSDP and the XSDT headers are
> > exactly the same (the difference is in the size of the description
> > headers that come after).
> 
> I guess I'd prefer that last variant.

I tried this approach (together with the second patch for necessity) and
it worked.

diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index fd2cbf68bc..11d6d1bc23 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -967,6 +967,10 @@ static int __init pvh_setup_acpi_xsdt(struct domain *d, 
paddr_t madt_addr,
         goto out;
     }
     xsdt_paddr = rsdp->xsdt_physical_address;
+    if ( !xsdt_paddr )
+    {
+        xsdt_paddr = rsdp->rsdt_physical_address;
+    }
     acpi_os_unmap_memory(rsdp, sizeof(*rsdp));
     table = acpi_os_map_memory(xsdt_paddr, sizeof(*table));
     if ( !table )