[edk2-devel] [PATCH RFC v2 09/28] OvmfPkg/VmgExitLib: Allow PMBASE register access in Dxe phase

[Brijesh Singh]

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Commit 85b8eac59b8c5bd9c7eb9afdb64357ce1aa2e803 added support to ensure
that MMIO is only performed against the un-encrypted memory. If MMIO
is performed against encrypted memory, a #GP is raised.

The VmgExitLib library depends on ApicTimerLib to get the APIC base
address so that it can exclude the APIC range from the un-encrypted
check. The OvmfPkg provides ApicTimerLib for the DXE phase. The
constructor AcpiTimerLibConstructor() used in the ApicTimerLib uses
the PciRead to get the PMBASE register. The PciRead() will cause an
MMIO access.

The AmdSevDxe driver clears the memory encryption attribute from the
MMIO ranges. However, if VmgExitLib is linked to AmdSevDxe driver then the
AcpiTimerLibConstructor() will be called before AmdSevDxe driver can
clear the encryption attributes for the MMIO regions.

Exclude the PMBASE register from the encrypted check so that we
can link VmgExitLib to the MemEncryptSevLib; which gets linked to
AmdSevDxe driver.

Cc: James Bottomley <jejb at linux.ibm.com>
Cc: Min Xu <min.m.xu at intel.com>
Cc: Jiewen Yao <jiewen.yao at intel.com>
Cc: Tom Lendacky <thomas.lendacky at amd.com>
Cc: Jordan Justen <jordan.l.justen at intel.com>
Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
Cc: Laszlo Ersek <lersek at redhat.com>
Cc: Erdem Aktas <erdemaktas at google.com>
Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
---
 OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf  |  4 ++
 OvmfPkg/Library/VmgExitLib/VmgExitLib.inf     |  7 +++
 OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 45 ++++++++++++++++++++
 3 files changed, 56 insertions(+)

diff --git a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
index e6f6ea7972..22435a0590 100644
--- a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
@@ -27,6 +27,7 @@
   SecVmgExitVcHandler.c
 
 [Packages]
+  MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
@@ -42,4 +43,7 @@
 [FixedPcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
 
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
index c66c68726c..d3175c260e 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
@@ -27,6 +27,7 @@
   PeiDxeVmgExitVcHandler.c
 
 [Packages]
+  MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
@@ -37,4 +38,10 @@
   DebugLib
   LocalApicLib
   MemEncryptSevLib
+  PcdLib
 
+[FixedPcd]
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index 24259060fd..01ac5d8c19 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -14,7 +14,10 @@
 #include <Library/VmgExitLib.h>
 #include <Register/Amd/Msr.h>
 #include <Register/Intel/Cpuid.h>
+#include <IndustryStandard/Q35MchIch9.h>
+#include <IndustryStandard/I440FxPiix4.h>
 #include <IndustryStandard/InstructionParsing.h>
+#include <Library/PcdLib.h>
 
 #include "VmgExitVcHandler.h"
 
@@ -596,6 +599,40 @@ UnsupportedExit (
   return Status;
 }
 
+STATIC
+BOOLEAN
+IsPmbaBaseAddress (
+  IN  UINTN     Address
+  )
+{
+  UINT16 HostBridgeDevId;
+  UINTN Pmba;
+
+  //
+  // Query Host Bridge DID to determine platform type
+  //
+  HostBridgeDevId = PcdGet16 (PcdOvmfHostBridgePciDevId);
+  switch (HostBridgeDevId) {
+    case INTEL_82441_DEVICE_ID:
+      Pmba = POWER_MGMT_REGISTER_PIIX4 (PIIX4_PMBA);
+      break;
+    case INTEL_Q35_MCH_DEVICE_ID:
+      Pmba = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE);
+      //
+      // Add the MMCONFIG base address to get the Pmba base access address
+      //
+      Pmba += FixedPcdGet64 (PcdPciExpressBaseAddress);
+      break;
+    default:
+      return FALSE;
+  }
+
+  // Round up the offset to page size
+  Pmba = Pmba & ~(SIZE_4KB - 1);
+
+  return (Address == Pmba);
+}
+
 /**
   Validate that the MMIO memory access is not to encrypted memory.
 
@@ -640,6 +677,14 @@ ValidateMmioMemory (
     return 0;
   }
 
+  //
+  // Allow PMBASE accesses (which will have the encryption bit set before
+  // AmdSevDxe runs in the DXE phase)
+  //
+  if (IsPmbaBaseAddress (Address)) {
+    return 0;
+  }
+
   //
   // Any state other than unencrypted is an error, issue a #GP.
   //
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74636): https://edk2.groups.io/g/devel/message/74636
Mute This Topic: https://groups.io/mt/82479056/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-