[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 16 May 2023 11:13:17 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ML7NNYnIEVvp9wboHB3nARQtK27CnZkjSArpdibskKQ=; b=iMqWK+y2j5XPfavqr/marj7uOuMNl4NH0jzFIf4L/jVVWUjJEvs7UzlK+7JYWSBtfn/O/rm2sZA5AV1Cy2iPBjmVrzk/4CrIoO9RbwNZWa6ZHAWdQpWhGOHbUOLOFjZL0UgnUyFeew/kx+tbc6u2xHJkbENtAvxCNAsci9jixaOFgRnfeZq1oyPPAri2awiXxffPErHVgLJN4bm5ZZ68yjg7MJFszdRkkj3FTeJ79zAY6r9iOer8L4ty25XmrOMZrdEamkUG41OKaB3r1lBKOKGoqnhMIC0t9ng6X2xhSFIt6ozL9WH72lgvt1noCCm2a2uBLQSOrWbvrJxH7Yw7Jg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cpsVJDsbC++Tf8PSbRlecHOhvLkqmLLEiuQDHgxHlCttYartm7RFSH97hA1aQB/xNWCAQ9JCu6Kb5Vg6LQykqoVXMatQX6TTFdXRf9hE8tm5fo8W5pydX5tVGcNkR3NXRg9m7MMnK/0+JN6Mj2aO4LxT0wrB1RN1UU6FXzlIf+m7RY97pk43ZGHpAhBTQ2SpDwSXyjRH2u+U+QYAQV0LRbnJRisD3sZiqeg76u8OEWh4YzBbgLtHGG+TEkrTOg6iQNkIDie9wOWWJGr6v6UdNir/2AGSMcOGhL4oK3gRBXM7sZgnLs0ySKlBlrZSlZvYf0K8VMRPpWpKTlNnioD0KA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: andrew.cooper3@xxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, Xenia.Ragiadakou@xxxxxxx, Stefano Stabellini <stefano.stabellini@xxxxxxx>
  • Delivery-date: Tue, 16 May 2023 09:13:26 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 16.05.2023 10:24, Roger Pau Monné wrote:
> On Tue, May 16, 2023 at 10:10:07AM +0200, Roger Pau Monné wrote:
>> On Mon, May 15, 2023 at 05:16:27PM -0700, Stefano Stabellini wrote:
>>> 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 )
>>
>> To be slightly more consistent, could you use:
>>
>> /*
>>  * 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)
>>     sdt_paddr = rsdp->xsdt_physical_address;
>> else
>>     sdt_paddr = rsdp->rsdt_physical_address;
>>
>> It was an oversight of mine to not check for the RSDP revision, as
>> RSDP < 2 will never have an XSDT.  Also add:
>>
>> Fixes: 1d74282c455f ('x86: setup PVHv2 Dom0 ACPI tables')
> 
> Just realized this will require some more work so that the guest
> (dom0) provided RSDP is at least revision 2.  You will need to adjust
> the field and recalculate the checksum if needed.

We could also mandate ACPI version >= 2 for PVH Dom0.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.