public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support
@ 2018-11-25 10:01 Laszlo Ersek
  2018-11-25 10:01 ` [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID Laszlo Ersek
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-25 10:01 UTC (permalink / raw)
  To: edk2-devel
  Cc: Anthony Perard, Ard Biesheuvel, Drew Jones, Igor Mammedov,
	Jordan Justen, Julien Grall, Philippe Mathieu-Daudé

Repo:   https://github.com/lersek/edk2.git
Branch: acpi_test_support

The feature is described in the first patch. Build OvmfPkg and
ArmVirtPkg platforms with "--pcd PcdAcpiTestSupport=TRUE" to enable it.

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>

Thanks,
Laszlo

Laszlo Ersek (4):
  OvmfPkg: introduce ACPI Test Support data structure and GUID
  OvmfPkg/AcpiPlatformDxe: list missing lib classes for
    QemuFwCfgAcpiPlatformDxe
  OvmfPkg/AcpiPlatformDxe: add [Un]RegisterAcpiTestSupport() skeletons
  OvmfPkg/AcpiPlatformDxe: fill in ACPI_TEST_SUPPORT at first
    Ready-To-Boot

 OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h               |  10 ++
 OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf          |   8 ++
 OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c            | 119 ++++++++++++++++++++
 OvmfPkg/AcpiPlatformDxe/EntryPoint.c                 |  24 +++-
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf |  10 ++
 OvmfPkg/Include/Guid/AcpiTestSupport.h               |  67 +++++++++++
 OvmfPkg/OvmfPkg.dec                                  |   6 +
 7 files changed, 239 insertions(+), 5 deletions(-)
 create mode 100644 OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c
 create mode 100644 OvmfPkg/Include/Guid/AcpiTestSupport.h

-- 
2.19.1.3.g30247aa5d201



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID
  2018-11-25 10:01 [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Laszlo Ersek
@ 2018-11-25 10:01 ` Laszlo Ersek
  2018-11-26 21:43   ` Philippe Mathieu-Daudé
  2018-12-03 17:09   ` Igor Mammedov
  2018-11-25 10:01 ` [PATCH 2/4] OvmfPkg/AcpiPlatformDxe: list missing lib classes for QemuFwCfgAcpiPlatformDxe Laszlo Ersek
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-25 10:01 UTC (permalink / raw)
  To: edk2-devel
  Cc: Anthony Perard, Ard Biesheuvel, Drew Jones, Igor Mammedov,
	Jordan Justen, Julien Grall, Philippe Mathieu-Daudé

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;
+} ACPI_TEST_SUPPORT;
+#pragma pack ()
+
+#endif // __ACPI_TEST_SUPPORT_H__
-- 
2.19.1.3.g30247aa5d201




^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] OvmfPkg/AcpiPlatformDxe: list missing lib classes for QemuFwCfgAcpiPlatformDxe
  2018-11-25 10:01 [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Laszlo Ersek
  2018-11-25 10:01 ` [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID Laszlo Ersek
@ 2018-11-25 10:01 ` Laszlo Ersek
  2018-11-25 10:01 ` [PATCH 3/4] OvmfPkg/AcpiPlatformDxe: add [Un]RegisterAcpiTestSupport() skeletons Laszlo Ersek
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-25 10:01 UTC (permalink / raw)
  To: edk2-devel
  Cc: Anthony Perard, Ard Biesheuvel, Drew Jones, Igor Mammedov,
	Jordan Justen, Julien Grall, Philippe Mathieu-Daudé

QemuFwCfgAcpiPlatformDxe consumes PcdPciDisableBusEnumeration via
AcpiPlatformEntryPoint() in "EntryPoint.c"; list the PcdLib class in the
[LibraryClasses] section of the INF file.

QemuFwCfgAcpiPlatformDxe calls CopyMem() at several places in
"QemuFwCfgAcpi.c"; list the BaseMemoryLib class too.

The other INF file, "AcpiPlatformDxe.inf", is fine already.

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/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
index 96fc17224133..f0fb3496e700 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
@@ -42,10 +42,12 @@ [Packages]
 
 [LibraryClasses]
   BaseLib
+  BaseMemoryLib
   DebugLib
   MemoryAllocationLib
   OrderedCollectionLib
+  PcdLib
   QemuFwCfgLib
   QemuFwCfgS3Lib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
-- 
2.19.1.3.g30247aa5d201




^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] OvmfPkg/AcpiPlatformDxe: add [Un]RegisterAcpiTestSupport() skeletons
  2018-11-25 10:01 [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Laszlo Ersek
  2018-11-25 10:01 ` [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID Laszlo Ersek
  2018-11-25 10:01 ` [PATCH 2/4] OvmfPkg/AcpiPlatformDxe: list missing lib classes for QemuFwCfgAcpiPlatformDxe Laszlo Ersek
@ 2018-11-25 10:01 ` Laszlo Ersek
  2018-11-25 10:01 ` [PATCH 4/4] OvmfPkg/AcpiPlatformDxe: fill in ACPI_TEST_SUPPORT at first Ready-To-Boot Laszlo Ersek
  2018-11-26 17:21 ` [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Ard Biesheuvel
  4 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-25 10:01 UTC (permalink / raw)
  To: edk2-devel
  Cc: Anthony Perard, Ard Biesheuvel, Drew Jones, Igor Mammedov,
	Jordan Justen, Julien Grall, Philippe Mathieu-Daudé

Introduce the PcdAcpiTestSupport Feature PCD to OvmfPkg. This PCD will
decide whether AcpiPlatformDxe populates the ACPI_TEST_SUPPORT structure
at the first Ready-To-Boot event.

(The Ready-To-Boot event is chosen because (a) it is bound to occur after
AcpiPlatformDxe installs the ACPI tables, and (b) it is guaranteed to
occur, namely when the boot manager launches the built-in UEFI shell.)

Introduce the RegisterAcpiTestSupport() and UnregisterAcpiTestSupport()
functions as skeletons, and call them dependent on the PCD, from the
driver's entry point function. Refactor the entry point function so the
registration can be rolled back on the failure path.

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                                  |  5 ++++
 OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf          |  4 +++
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf |  4 +++
 OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h               | 10 +++++++
 OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c            | 30 ++++++++++++++++++++
 OvmfPkg/AcpiPlatformDxe/EntryPoint.c                 | 24 ++++++++++++----
 6 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index e8c7d9423f43..00765a7995fa 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -155,13 +155,18 @@ [PcdsDynamic, PcdsDynamicEx]
 [PcdsFeatureFlag]
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderPciTranslation|TRUE|BOOLEAN|0x1c
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderMmioTranslation|FALSE|BOOLEAN|0x1d
 
   ## This feature flag enables SMM/SMRAM support. Note that it also requires
   #  such support from the underlying QEMU instance; if that support is not
   #  present, the firmware will reject continuing after a certain point.
   #
   #  The flag also acts as a general "security switch"; when TRUE, many
   #  components will change behavior, with the goal of preventing a malicious
   #  runtime OS from tampering with firmware structures (special memory ranges
   #  used by OVMF, the varstore pflash chip, LockBox etc).
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|FALSE|BOOLEAN|0x1e
+
+  ## Instruct AcpiPlatformDxe to expose the RSD PTR address(es) in the
+  #  MB-aligned ACPI_TEST_SUPPORT structure to the hypervisor. This is a
+  #  development / unit test aid; do not enable in production builds.
+  gUefiOvmfPkgTokenSpaceGuid.PcdAcpiTestSupport|FALSE|BOOLEAN|3
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
index 8440e7b343d8..9c5bfe767981 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -29,10 +29,11 @@ [Defines]
 [Sources]
   AcpiPlatform.c
   AcpiPlatform.h
+  AcpiTestSupport.c
   BootScript.c
   EntryPoint.c
   PciDecoding.c
   Qemu.c
   QemuFwCfgAcpi.c
   QemuLoader.h
   Xen.c
@@ -75,5 +76,8 @@ [Pcd]
   gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
 
+[FeaturePcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdAcpiTestSupport
+
 [Depex]
   gEfiAcpiTableProtocolGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
index f0fb3496e700..b2501fea9ca4 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
@@ -28,9 +28,10 @@ [Defines]
 
 [Sources]
   AcpiPlatform.h
+  AcpiTestSupport.c
   BootScript.c
   EntryPoint.c
   PciDecoding.c
   QemuFwCfgAcpi.c
   QemuFwCfgAcpiPlatform.c
   QemuLoader.h
@@ -62,5 +63,8 @@ [Guids]
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
 
+[FeaturePcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdAcpiTestSupport
+
 [Depex]
   gEfiAcpiTableProtocolGuid
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
index 83b981ee005d..4769b92621b9 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h
@@ -117,6 +117,16 @@ EFI_STATUS
 TransferS3ContextToBootScript (
   IN S3_CONTEXT *S3Context
   );
 
+VOID
+RegisterAcpiTestSupport (
+  VOID
+  );
+
+VOID
+UnregisterAcpiTestSupport (
+  VOID
+  );
+
 #endif
 
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c b/OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c
new file mode 100644
index 000000000000..462c8e64a4fd
--- /dev/null
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c
@@ -0,0 +1,30 @@
+/** @file
+  Register a Ready-To-Boot callback for populating the ACPI_TEST_SUPPORT
+  structure.
+
+  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 which 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.
+**/
+
+#include "AcpiPlatform.h"
+
+VOID
+RegisterAcpiTestSupport (
+  VOID
+  )
+{
+}
+
+VOID
+UnregisterAcpiTestSupport (
+  VOID
+  )
+{
+}
diff --git a/OvmfPkg/AcpiPlatformDxe/EntryPoint.c b/OvmfPkg/AcpiPlatformDxe/EntryPoint.c
index 1bfd31a0371a..d32fba4f22f4 100644
--- a/OvmfPkg/AcpiPlatformDxe/EntryPoint.c
+++ b/OvmfPkg/AcpiPlatformDxe/EntryPoint.c
@@ -61,36 +61,50 @@ EFIAPI
 AcpiPlatformEntryPoint (
   IN EFI_HANDLE         ImageHandle,
   IN EFI_SYSTEM_TABLE   *SystemTable
   )
 {
   EFI_STATUS Status;
   EFI_EVENT  RootBridgesConnected;
 
+  if (FeaturePcdGet (PcdAcpiTestSupport)) {
+    RegisterAcpiTestSupport ();
+  }
+
   //
   // If the platform doesn't support PCI, or PCI enumeration has been disabled,
   // install the tables at once, and let the entry point's return code reflect
   // the full functionality.
   //
   if (PcdGetBool (PcdPciDisableBusEnumeration)) {
     DEBUG ((EFI_D_INFO, "%a: PCI or its enumeration disabled, installing "
       "ACPI tables\n", __FUNCTION__));
-    return InstallAcpiTables (FindAcpiTableProtocol ());
+    Status = InstallAcpiTables (FindAcpiTableProtocol ());
+    if (EFI_ERROR (Status)) {
+      goto RollbackAcpiTestSupport;
+    }
+    return EFI_SUCCESS;
   }
 
   //
   // Otherwise, delay installing the ACPI tables until root bridges are
   // connected. The entry point's return status will only reflect the callback
   // setup. (Note that we're a DXE_DRIVER; our entry point function is invoked
   // strictly before BDS is entered and can connect the root bridges.)
   //
   Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
                   OnRootBridgesConnected, NULL /* Context */,
                   &gRootBridgesConnectedEventGroupGuid, &RootBridgesConnected);
-  if (!EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_INFO,
-      "%a: waiting for root bridges to be connected, registered callback\n",
-      __FUNCTION__));
+  if (EFI_ERROR (Status)) {
+    goto RollbackAcpiTestSupport;
   }
+  DEBUG ((DEBUG_INFO,
+    "%a: waiting for root bridges to be connected, registered callback\n",
+    __FUNCTION__));
+  return EFI_SUCCESS;
 
+RollbackAcpiTestSupport:
+  if (FeaturePcdGet (PcdAcpiTestSupport)) {
+    UnregisterAcpiTestSupport ();
+  }
   return Status;
 }
-- 
2.19.1.3.g30247aa5d201




^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] OvmfPkg/AcpiPlatformDxe: fill in ACPI_TEST_SUPPORT at first Ready-To-Boot
  2018-11-25 10:01 [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Laszlo Ersek
                   ` (2 preceding siblings ...)
  2018-11-25 10:01 ` [PATCH 3/4] OvmfPkg/AcpiPlatformDxe: add [Un]RegisterAcpiTestSupport() skeletons Laszlo Ersek
@ 2018-11-25 10:01 ` Laszlo Ersek
  2018-11-26 17:21 ` [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Ard Biesheuvel
  4 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-25 10:01 UTC (permalink / raw)
  To: edk2-devel
  Cc: Anthony Perard, Ard Biesheuvel, Drew Jones, Igor Mammedov,
	Jordan Justen, Julien Grall, Philippe Mathieu-Daudé

If the PcdAcpiTestSupport Feature PCD is set, allocate and fill the
ACPI_TEST_SUPPORT structure at a 1MB boundary.

This feature is a best-effort development / unit test helper, so even if
it is enabled at build time, and fails at boot time, it will not prevent
AcpiPlatformDxe from doing its job. For the same reason, error messages
are only logged at the DEBUG_WARN level.

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/AcpiPlatformDxe/AcpiPlatformDxe.inf          |  4 +
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf |  4 +
 OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c            | 89 ++++++++++++++++++++
 3 files changed, 97 insertions(+)

diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
index 9c5bfe767981..11ae3e2a1ac1 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
@@ -66,6 +66,10 @@ [Protocols]
   gEfiPciIoProtocolGuid                         # PROTOCOL SOMETIMES_CONSUMED
 
 [Guids]
+  gAcpiTestSupportGuid
+  gEfiAcpi10TableGuid
+  gEfiAcpi20TableGuid
+  gEfiEventReadyToBootGuid
   gEfiXenInfoGuid
   gRootBridgesConnectedEventGroupGuid
 
diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
index b2501fea9ca4..062f4d2fc50c 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
@@ -58,6 +58,10 @@ [Protocols]
   gEfiPciIoProtocolGuid                         # PROTOCOL SOMETIMES_CONSUMED
 
 [Guids]
+  gAcpiTestSupportGuid
+  gEfiAcpi10TableGuid
+  gEfiAcpi20TableGuid
+  gEfiEventReadyToBootGuid
   gRootBridgesConnectedEventGroupGuid
 
 [Pcd]
diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c b/OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c
index 462c8e64a4fd..26dd26cdc439 100644
--- a/OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c
+++ b/OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c
@@ -15,16 +15,105 @@
 
 #include "AcpiPlatform.h"
 
+#include <Guid/Acpi.h>
+#include <Guid/AcpiTestSupport.h>
+#include <Guid/EventGroup.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+STATIC volatile ACPI_TEST_SUPPORT *mAcpiTestSupport;
+STATIC EFI_EVENT                  mAcpiTestSupportEvent;
+
+STATIC
+VOID
+EFIAPI
+AcpiTestSupportOnReadyToBoot (
+  IN EFI_EVENT Event,
+  IN VOID      *Context
+  )
+{
+  VOID                          *Pages;
+  CONST VOID                    *Rsdp10;
+  CONST VOID                    *Rsdp20;
+  CONST EFI_CONFIGURATION_TABLE *ConfigTable;
+  CONST EFI_CONFIGURATION_TABLE *ConfigTablesEnd;
+  volatile EFI_GUID             *InverseSignature;
+  UINTN                         Idx;
+
+  Pages = AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof *mAcpiTestSupport),
+            SIZE_1MB);
+  if (Pages == NULL) {
+    DEBUG ((DEBUG_WARN, "%a: AllocateAlignedPages() failed\n", __FUNCTION__));
+    UnregisterAcpiTestSupport ();
+    return;
+  }
+
+  //
+  // Locate both gEfiAcpi10TableGuid and gEfiAcpi20TableGuid config tables in
+  // one go.
+  //
+  Rsdp10 = NULL;
+  Rsdp20 = NULL;
+  ConfigTable = gST->ConfigurationTable;
+  ConfigTablesEnd = gST->ConfigurationTable + gST->NumberOfTableEntries;
+  while ((Rsdp10 == NULL || Rsdp20 == NULL) && ConfigTable < ConfigTablesEnd) {
+    if (CompareGuid (&ConfigTable->VendorGuid, &gEfiAcpi10TableGuid)) {
+      Rsdp10 = ConfigTable->VendorTable;
+    } else if (CompareGuid (&ConfigTable->VendorGuid, &gEfiAcpi20TableGuid)) {
+      Rsdp20 = ConfigTable->VendorTable;
+    }
+    ++ConfigTable;
+  }
+
+  DEBUG ((DEBUG_VERBOSE, "%a: AcpiTestSupport=%p Rsdp10=%p Rsdp20=%p\n",
+    __FUNCTION__, Pages, Rsdp10, Rsdp20));
+
+  //
+  // Store the RSD PTR address(es) first, then the signature second.
+  //
+  mAcpiTestSupport = Pages;
+  mAcpiTestSupport->Rsdp10 = (UINTN)Rsdp10;
+  mAcpiTestSupport->Rsdp20 = (UINTN)Rsdp20;
+
+  MemoryFence();
+
+  InverseSignature = &mAcpiTestSupport->InverseSignatureGuid;
+  InverseSignature->Data1  = gAcpiTestSupportGuid.Data1;
+  InverseSignature->Data1 ^= MAX_UINT32;
+  InverseSignature->Data2  = gAcpiTestSupportGuid.Data2;
+  InverseSignature->Data2 ^= MAX_UINT16;
+  InverseSignature->Data3  = gAcpiTestSupportGuid.Data3;
+  InverseSignature->Data3 ^= MAX_UINT16;
+  for (Idx = 0; Idx < sizeof InverseSignature->Data4; ++Idx) {
+    InverseSignature->Data4[Idx]  = gAcpiTestSupportGuid.Data4[Idx];
+    InverseSignature->Data4[Idx] ^= MAX_UINT8;
+  }
+
+  UnregisterAcpiTestSupport ();
+}
+
 VOID
 RegisterAcpiTestSupport (
   VOID
   )
 {
+  EFI_STATUS Status;
+
+  Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+                  AcpiTestSupportOnReadyToBoot, NULL /* Context */,
+                  &gEfiEventReadyToBootGuid, &mAcpiTestSupportEvent);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN, "%a: CreateEventEx(): %r\n", __FUNCTION__, Status));
+  }
 }
 
 VOID
 UnregisterAcpiTestSupport (
   VOID
   )
 {
+  if (mAcpiTestSupportEvent != NULL) {
+    gBS->CloseEvent (mAcpiTestSupportEvent);
+    mAcpiTestSupportEvent = NULL;
+  }
 }
-- 
2.19.1.3.g30247aa5d201



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support
  2018-11-25 10:01 [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Laszlo Ersek
                   ` (3 preceding siblings ...)
  2018-11-25 10:01 ` [PATCH 4/4] OvmfPkg/AcpiPlatformDxe: fill in ACPI_TEST_SUPPORT at first Ready-To-Boot Laszlo Ersek
@ 2018-11-26 17:21 ` Ard Biesheuvel
  2018-11-27 11:02   ` Laszlo Ersek
  4 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2018-11-26 17:21 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: edk2-devel@lists.01.org, Anthony PERARD, Andrew Jones,
	Igor Mammedov, Jordan Justen, Julien Grall,
	Philippe Mathieu-Daudé

On Sun, 25 Nov 2018 at 11:02, Laszlo Ersek <lersek@redhat.com> wrote:
>
> Repo:   https://github.com/lersek/edk2.git
> Branch: acpi_test_support
>
> The feature is described in the first patch. Build OvmfPkg and
> ArmVirtPkg platforms with "--pcd PcdAcpiTestSupport=TRUE" to enable it.
>
> 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>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (4):
>   OvmfPkg: introduce ACPI Test Support data structure and GUID
>   OvmfPkg/AcpiPlatformDxe: list missing lib classes for
>     QemuFwCfgAcpiPlatformDxe
>   OvmfPkg/AcpiPlatformDxe: add [Un]RegisterAcpiTestSupport() skeletons
>   OvmfPkg/AcpiPlatformDxe: fill in ACPI_TEST_SUPPORT at first
>     Ready-To-Boot
>

I'm not crazy about scraping memory, but since this is a test feature,
and since the hypervisor can be trusted to only scrape regions
populated with non-secure DRAM, I can live with it.

Regarding the GUID: i thought about perhaps using the XOR of two known
GUIDs, but bitwise negation involving a all-ones GUID amounts to the
same thing in the end, so whatever :-)

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>  OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h               |  10 ++
>  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf          |   8 ++
>  OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c            | 119 ++++++++++++++++++++
>  OvmfPkg/AcpiPlatformDxe/EntryPoint.c                 |  24 +++-
>  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf |  10 ++
>  OvmfPkg/Include/Guid/AcpiTestSupport.h               |  67 +++++++++++
>  OvmfPkg/OvmfPkg.dec                                  |   6 +
>  7 files changed, 239 insertions(+), 5 deletions(-)
>  create mode 100644 OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c
>  create mode 100644 OvmfPkg/Include/Guid/AcpiTestSupport.h
>
> --
> 2.19.1.3.g30247aa5d201
>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID
  2018-11-25 10:01 ` [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID Laszlo Ersek
@ 2018-11-26 21:43   ` Philippe Mathieu-Daudé
  2018-11-27 11:23     ` Laszlo Ersek
  2018-12-03 17:09   ` Igor Mammedov
  1 sibling, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-26 21:43 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel
  Cc: Anthony Perard, Ard Biesheuvel, Drew Jones, Igor Mammedov,
	Jordan Justen, Julien Grall

Hi Laszlo,

On 25/11/18 11:01, Laszlo Ersek 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.

I applied/build your series, but keep failing trying to test it with QEMU :/
I modified a bit tests/bios-tables-test.c to use the OVMF.fd build for
the Q35 machine tests, but still pass the "OnRootBridgesConnected: root
bridges have been connected, installing ACPI tables".

Do you have a QEMU companion patch for this series?

> 
> 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
>   }


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support
  2018-11-26 17:21 ` [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Ard Biesheuvel
@ 2018-11-27 11:02   ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-27 11:02 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, Anthony PERARD, Andrew Jones,
	Igor Mammedov, Jordan Justen, Julien Grall,
	Philippe Mathieu-Daudé

On 11/26/18 18:21, Ard Biesheuvel wrote:
> On Sun, 25 Nov 2018 at 11:02, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: acpi_test_support
>>
>> The feature is described in the first patch. Build OvmfPkg and
>> ArmVirtPkg platforms with "--pcd PcdAcpiTestSupport=TRUE" to enable it.
>>
>> 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>
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (4):
>>   OvmfPkg: introduce ACPI Test Support data structure and GUID
>>   OvmfPkg/AcpiPlatformDxe: list missing lib classes for
>>     QemuFwCfgAcpiPlatformDxe
>>   OvmfPkg/AcpiPlatformDxe: add [Un]RegisterAcpiTestSupport() skeletons
>>   OvmfPkg/AcpiPlatformDxe: fill in ACPI_TEST_SUPPORT at first
>>     Ready-To-Boot
>>
> 
> I'm not crazy about scraping memory, but since this is a test feature,
> and since the hypervisor can be trusted to only scrape regions
> populated with non-secure DRAM, I can live with it.
> 
> Regarding the GUID: i thought about perhaps using the XOR of two known
> GUIDs, but bitwise negation involving a all-ones GUID amounts to the
> same thing in the end, so whatever :-)
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thanks :)

> 
>>  OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h               |  10 ++
>>  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf          |   8 ++
>>  OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c            | 119 ++++++++++++++++++++
>>  OvmfPkg/AcpiPlatformDxe/EntryPoint.c                 |  24 +++-
>>  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf |  10 ++
>>  OvmfPkg/Include/Guid/AcpiTestSupport.h               |  67 +++++++++++
>>  OvmfPkg/OvmfPkg.dec                                  |   6 +
>>  7 files changed, 239 insertions(+), 5 deletions(-)
>>  create mode 100644 OvmfPkg/AcpiPlatformDxe/AcpiTestSupport.c
>>  create mode 100644 OvmfPkg/Include/Guid/AcpiTestSupport.h
>>
>> --
>> 2.19.1.3.g30247aa5d201
>>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID
  2018-11-26 21:43   ` Philippe Mathieu-Daudé
@ 2018-11-27 11:23     ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-27 11:23 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, edk2-devel
  Cc: Anthony Perard, Ard Biesheuvel, Drew Jones, Igor Mammedov,
	Jordan Justen, Julien Grall

On 11/26/18 22:43, Philippe Mathieu-Daudé wrote:
> Hi Laszlo,
> 
> On 25/11/18 11:01, Laszlo Ersek 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.
> 
> I applied/build your series,

OK.

> but keep failing trying to test it with QEMU :/

Hm.

> I modified a bit tests/bios-tables-test.c to use the OVMF.fd build for
> the Q35 machine tests,

OK. The primary goal of this series is to enable QEMU developers to
extend bios-tables-test.c to "qemu-system-aarch64/virt"; but, indeed, I
intend the firmware feature to be platform-independent.

Instead, it's the QEMU test case that is supposed to know, intimately,
the subject physical memory map (that is: the set of locations in
guest-phys mem address space that are 1MB aligned and are actually
DRAM-backed). So where exactly to probe is up to QEMU / qtest.

> but still pass the "OnRootBridgesConnected: root
> bridges have been connected, installing ACPI tables".

Sorry, I don't understand this. What do you mean by "still pass"?

AcpiPlatformDxe will print the message you quote when OVMF's platform
boot manager library, in OvmfPkg/Library/PlatformBootManagerLib, signals
it. That's part of the BDS (Boot Device Selection) phase. Similarly,
ArmVirtQemu signals AcpiPlatformDxe from
"ArmVirtPkg/Library/PlatformBootManagerLib".

However, the event we're hooking in this patch set is different, and it
occurs later. It's called "Ready To Boot", and it is emitted / signaled
by the UEFI boot manager when it is about to launch a boot option. See
EFI_EVENT_GROUP_READY_TO_BOOT under EFI_BOOT_SERVICES.CreateEventEx() in
the UEFI spec.

Given that upstream OVMF and ArmVirt builds include / re-create a UEFI
boot option for the built-in UEFI shell, there's always something that
can be booted, even without disks / NICs. Thus, if you launch a
diskless/NIC-less guest with such fw images, the event will be signaled
(and the structure will be populated) no later than the built-in UEFI
shell is launched. The qtest case should simply wait for that.

> Do you have a QEMU companion patch for this series?

I totally don't; I'm not going to write it. :) This patch set is to
support the work of QEMU developers. And, clearly, I'm not going to push
it until someone says, "yes, I've got a working QEMU series that
interfaces with this".

I'm fairly certain this series works as intended, because I tested it as
follows: I booted a guest, waited for the firmware log to confirm that
the callback was called (the log also indicated the correctly aligned
address of the structure, and the RSDP1.0 / RSDP2.0 fields written to
the structure), and then I used the "xp" QEMU monitor commands (via HMP)
to print the guest RAM in the area, and to verify the GUID & the RSDP
fields.

Another possibility (for debugging the qtest work) is to dump the entire
guest RAM with the "dump-guest-memory" monitor command (in ELF format),
and to search the dump for the byte array in question (I think at least
"mcview" and "vbindiff" can search binary files for byte sequences).
Given the XOR trickery, you should have exactly one match, at a
MB-aligned address, assuming you waited long enough before dumping. (The
executable image of the DXE driver will not match.)

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID
  2018-11-25 10:01 ` [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID Laszlo Ersek
  2018-11-26 21:43   ` Philippe Mathieu-Daudé
@ 2018-12-03 17:09   ` Igor Mammedov
  2018-12-04 17:09     ` Laszlo Ersek
  1 sibling, 1 reply; 13+ messages in thread
From: Igor Mammedov @ 2018-12-03 17:09 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: edk2-devel, Anthony Perard, Ard Biesheuvel, Drew Jones,
	Jordan Justen, Julien Grall, Philippe Mathieu-Daudé

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__



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID
  2018-12-03 17:09   ` Igor Mammedov
@ 2018-12-04 17:09     ` Laszlo Ersek
  2018-12-26 20:24       ` Laszlo Ersek
  0 siblings, 1 reply; 13+ messages in thread
From: Laszlo Ersek @ 2018-12-04 17:09 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: edk2-devel, Anthony Perard, Ard Biesheuvel, Drew Jones,
	Jordan Justen, Julien Grall, Philippe Mathieu-Daudé

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__
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID
  2018-12-04 17:09     ` Laszlo Ersek
@ 2018-12-26 20:24       ` Laszlo Ersek
  2018-12-27  5:11         ` Igor Mammedov
  0 siblings, 1 reply; 13+ messages in thread
From: Laszlo Ersek @ 2018-12-26 20:24 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: Drew Jones, Jordan Justen, edk2-devel, Anthony Perard

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID
  2018-12-26 20:24       ` Laszlo Ersek
@ 2018-12-27  5:11         ` Igor Mammedov
  0 siblings, 0 replies; 13+ messages in thread
From: Igor Mammedov @ 2018-12-27  5:11 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: Drew Jones, Jordan Justen, edk2-devel, Anthony Perard

On Wed, 26 Dec 2018 21:24:44 +0100
Laszlo Ersek <lersek@redhat.com> wrote:

> Hi Igor,
[...] 
> 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.)
I'll CC you when I send QEMU patches (/me waiting till cleanup ones get merged first).

> 
> Thanks
> Laszlo



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-12-27  5:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-25 10:01 [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Laszlo Ersek
2018-11-25 10:01 ` [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID Laszlo Ersek
2018-11-26 21:43   ` Philippe Mathieu-Daudé
2018-11-27 11:23     ` Laszlo Ersek
2018-12-03 17:09   ` Igor Mammedov
2018-12-04 17:09     ` Laszlo Ersek
2018-12-26 20:24       ` Laszlo Ersek
2018-12-27  5:11         ` Igor Mammedov
2018-11-25 10:01 ` [PATCH 2/4] OvmfPkg/AcpiPlatformDxe: list missing lib classes for QemuFwCfgAcpiPlatformDxe Laszlo Ersek
2018-11-25 10:01 ` [PATCH 3/4] OvmfPkg/AcpiPlatformDxe: add [Un]RegisterAcpiTestSupport() skeletons Laszlo Ersek
2018-11-25 10:01 ` [PATCH 4/4] OvmfPkg/AcpiPlatformDxe: fill in ACPI_TEST_SUPPORT at first Ready-To-Boot Laszlo Ersek
2018-11-26 17:21 ` [PATCH 0/4] OvmfPkg, ArmVirtPkg: add ACPI Test Support Ard Biesheuvel
2018-11-27 11:02   ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox