Re: [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID

[Laszlo Ersek <lersek@redhat.com>]

On 12/03/18 18:09, Igor Mammedov wrote:
> On Sun, 25 Nov 2018 11:01:49 +0100
> Laszlo Ersek <lersek@redhat.com> wrote:
> 
>> QEMU's test suite includes a set of cases called "BIOS tables test". Among
>> other things, it locates the RSD PTR ACPI table in guest RAM, and then
>> (chasing pointers to other ACPI tables) performs various sanity checks on
>> the QEMU-generated and firmware-installed tables.
>>
>> Currently this set of test cases doesn't work with UEFI guests, because
>> the ACPI spec's definition for locating the RSD PTR in UEFI guests is a
>> lot harder to implement from the hypervisor side -- just with raw guest
>> memory access -- than it is when scraping the memory of BIOS guests.
>>
>> Introduce a signature GUID, and a small, MB-aligned structure, identified
>> by the GUID. The hypervisor should loop over all MB-aligned pages in guest
>> RAM until one matches the signature GUID at offset 0, at which point the
>> hypervisor can fetch the RSDP address field(s) from the structure.
>>
>> QEMU's test logic currently spins on a pre-determined guest address, until
>> that address assumes a magic value. The method described in this patch is
>> conceptually the same ("busy loop until match is found"), except there is
>> no hard-coded address. This plays a lot more nicely with UEFI guest
>> firmware (we'll be able to use the normal page allocation UEFI service).
>> Given the size of EFI_GUID (16 bytes -- 128 bits), mismatches should be
>> astronomically unlikely. In addition, given the typical guest RAM size for
>> such tests (128 MB), there are 128 locations to check in one iteration of
>> the "outer" loop, which shouldn't introduce an intolerable delay after the
>> guest stores the RSDP address(es), and then the GUID.
>>
>> The GUID that the hypervisor should search for is
>>
>>   AB87A6B1-2034-BDA0-71BD-375007757785
>>
>> Expressed as a byte array:
>>
>>   {
>>     0xb1, 0xa6, 0x87, 0xab,
>>     0x34, 0x20,
>>     0xa0, 0xbd,
>>     0x71, 0xbd, 0x37, 0x50, 0x07, 0x75, 0x77, 0x85
>>   }
>>
>> Note that in the patch, we define "gAcpiTestSupportGuid" with all bits
>> inverted. This is a simple method to prevent the UEFI binaries that
>> incorporate "gAcpiTestSupportGuid" from matching the actual GUID in guest
>> RAM.
>>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Drew Jones <drjones@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Julien Grall <julien.grall@linaro.org>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  OvmfPkg/OvmfPkg.dec                    |  1 +
>>  OvmfPkg/Include/Guid/AcpiTestSupport.h | 67 ++++++++++++++++++++
>>  2 files changed, 68 insertions(+)
>>
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
>> index 7666297cf8f1..e8c7d9423f43 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -73,11 +73,12 @@ [LibraryClasses]
>>  [Guids]
>>    gUefiOvmfPkgTokenSpaceGuid          = {0x93bb96af, 0xb9f2, 0x4eb8, {0x94, 0x62, 0xe0, 0xba, 0x74, 0x56, 0x42, 0x36}}
>>    gEfiXenInfoGuid                     = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
>>    gOvmfPlatformConfigGuid             = {0x7235c51c, 0x0c80, 0x4cab, {0x87, 0xac, 0x3b, 0x08, 0x4a, 0x63, 0x04, 0xb1}}
>>    gVirtioMmioTransportGuid            = {0x837dca9e, 0xe874, 0x4d82, {0xb2, 0x9a, 0x23, 0xfe, 0x0e, 0x23, 0xd1, 0xe2}}
>>    gQemuRamfbGuid                      = {0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
>>    gXenBusRootDeviceGuid               = {0xa732241f, 0x383d, 0x4d9c, {0x8a, 0xe1, 0x8e, 0x09, 0x83, 0x75, 0x89, 0xd7}}
>>    gRootBridgesConnectedEventGroupGuid = {0x24a2d66f, 0xeedd, 0x4086, {0x90, 0x42, 0xf2, 0x6e, 0x47, 0x97, 0xee, 0x69}}
>> +  gAcpiTestSupportGuid                = {0x5478594e, 0xdfcb, 0x425f, {0x8e, 0x42, 0xc8, 0xaf, 0xf8, 0x8a, 0x88, 0x7a}}
>>  
>>  [Protocols]
>>    gVirtioDeviceProtocolGuid           = {0xfa920010, 0x6785, 0x4941, {0xb6, 0xec, 0x49, 0x8c, 0x57, 0x9f, 0x16, 0x0a}}
>> diff --git a/OvmfPkg/Include/Guid/AcpiTestSupport.h b/OvmfPkg/Include/Guid/AcpiTestSupport.h
>> new file mode 100644
>> index 000000000000..8526f2bfb391
>> --- /dev/null
>> +++ b/OvmfPkg/Include/Guid/AcpiTestSupport.h
>> @@ -0,0 +1,67 @@
>> +/** @file
>> +  Expose the address(es) of the ACPI RSD PTR table(s) in a MB-aligned structure
>> +  to the hypervisor.
>> +
>> +  The hypervisor locates the MB-aligned structure based on the signature GUID
>> +  that is at offset 0 in the structure. Once the RSD PTR address(es) are
>> +  retrieved, the hypervisor may perform various ACPI checks.
>> +
>> +  This feature is a development aid, for supporting ACPI table unit tests in
>> +  hypervisors. Do not enable in production builds.
>> +
>> +  Copyright (C) 2018, Red Hat, Inc.
>> +
>> +  This program and the accompanying materials are licensed and made available
>> +  under the terms and conditions of the BSD License that accompanies this
>> +  distribution. The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php.
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
>> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +**/
>> +
>> +#ifndef __ACPI_TEST_SUPPORT_H__
>> +#define __ACPI_TEST_SUPPORT_H__
>> +
>> +#include <Uefi/UefiBaseType.h>
>> +
>> +#define ACPI_TEST_SUPPORT_GUID                         \
>> +  {                                                    \
>> +    0x5478594e,                                        \
>> +    0xdfcb,                                            \
>> +    0x425f,                                            \
>> +    { 0x8e, 0x42, 0xc8, 0xaf, 0xf8, 0x8a, 0x88, 0x7a } \
>> +  }
>> +
>> +extern EFI_GUID gAcpiTestSupportGuid;
>> +
>> +//
>> +// The following structure must be allocated in Boot Services Data type memory,
>> +// aligned at a 1MB boundary.
>> +//
>> +#pragma pack (1)
>> +typedef struct {
>> +  //
>> +  // The signature GUID is written to the MB-aligned structure from
>> +  // gAcpiTestSupportGuid, but with all bits inverted. That's the actual GUID
>> +  // value that the hypervisor should look for at each MB boundary, looping
>> +  // over all guest RAM pages with that alignment, until a match is found. The
>> +  // bit-flipping occurs in order not to store the actual GUID in any UEFI
>> +  // executable, which might confuse guest memory analysis. Note that EFI_GUID
>> +  // has little endian representation.
>> +  //
>> +  EFI_GUID             InverseSignatureGuid;
>> +  //
>> +  // The Rsdp10 and Rsdp20 fields may be read when the signature GUID matches.
>> +  // Rsdp10 is the guest-physical address of the ACPI 1.0 specification RSD PTR
>> +  // table, in 8-byte little endian representation. Rsdp20 is the same, for the
>> +  // ACPI 2.0 or later specification RSD PTR table. Each of these fields may be
>> +  // zero (independently of the other) if the UEFI System Table does not
>> +  // provide the corresponding UEFI Configuration Table.
>> +  //
>> +  EFI_PHYSICAL_ADDRESS Rsdp10;
>> +  EFI_PHYSICAL_ADDRESS Rsdp20;
> Is it possible to have both different tables at the same time?

Yes, it is. The UEFI and ACPI specs define both "configuration tables",
referenced by the UEFI system table, with separate GUIDs. It is possible
to populate both table-trees in parallel, and indeed that's what happens
in OVMF. (The core driver that offers EFI_ACPI_TABLE_PROTOCOL populates
both table-trees in parallel dependent on firmware platform
configuration.) In the ArmVirtQemu firmware platform, you'll only see
Rsdp20 as non-NULL though.

I exposed both fields (both table-trees) in the structure for the
following reasons:
- I didn't want to add any selection logic;
- the structure should be suitable for testing both OVMF and ArmVirtQemu
(and the fields are populated differently between them);
- the fields match exactly what the OS sees.

Thanks,
Laszlo

> If not just rsdp address should be enough.
> 
> 
>> +} ACPI_TEST_SUPPORT;
>> +#pragma pack ()
>> +
>> +#endif // __ACPI_TEST_SUPPORT_H__
>