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

[Laszlo Ersek <lersek@redhat.com>]

Hi Igor,

On 12/04/18 18:09, Laszlo Ersek wrote:
> 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__
>>
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

I'm dropping this patch series from my tracked / tagged email backlog;
it's overflowing. Please ping me once you have the QEMU-side patches
ready, and then I can push this with Ard's R-b. (I'm happy to wait for
Phil's test results too, if he prefers that, of course.)

Thanks
Laszlo