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

[Igor Mammedov <imammedo@redhat.com>]

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?
If not just rsdp address should be enough.


> +} ACPI_TEST_SUPPORT;
> +#pragma pack ()
> +
> +#endif // __ACPI_TEST_SUPPORT_H__