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

[Jan Beulich <jbeulich@xxxxxxxx>]

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. ACPI specifically says "Platforms
provide the RSDT to enable compatibility with ACPI 1.0 operating
systems." IOW any halfway modern system (including qemu, that is) ought
to supply an XSDT anyway.

Jan