From f0ed194236b1fe55199ee82c014b70119ee3f227 Mon Sep 17 00:00:00 2001
From: Michael Roth <michael.roth@amd.com>
Date: Thu, 2 May 2024 13:49:26 +0200
Subject: OvmfPkg: Don't make APIC MMIO accesses with encryption bit set

For the most part, OVMF will clear the encryption bit for MMIO regions,
but there is currently one known exception during SEC when the APIC
base address is accessed via MMIO with the encryption bit set for
SEV-ES/SEV-SNP guests. In the case of SEV-SNP, this requires special
handling on the hypervisor side which may not be available in the
future[1], so make the necessary changes in the SEC-configured page
table to clear the encryption bit for 4K region containing the APIC
base address.

[1] https://lore.kernel.org/lkml/20240208002420.34mvemnzrwwsaesw@amd.com/#t

Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jianyong Wu <jianyong.wu@arm.com>
Cc: Anatol Belski <anbelski@linux.microsoft.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/Sec/AmdSev.c    | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
 OvmfPkg/Sec/AmdSev.h    | 14 ++++++++++++
 OvmfPkg/Sec/SecMain.c   |  1 +
 OvmfPkg/Sec/SecMain.inf |  3 +++
 4 files changed, 76 insertions(+)

(limited to 'OvmfPkg/Sec')

diff --git a/OvmfPkg/Sec/AmdSev.c b/OvmfPkg/Sec/AmdSev.c
index 520b125132..89fba2fd18 100644
--- a/OvmfPkg/Sec/AmdSev.c
+++ b/OvmfPkg/Sec/AmdSev.c
@@ -8,7 +8,10 @@
 **/
 
 #include <Library/BaseLib.h>
+#include <Library/CpuLib.h>
+#include <Library/CpuPageTableLib.h>
 #include <Library/DebugLib.h>
+#include <Library/LocalApicLib.h>
 #include <Library/MemEncryptSevLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Register/Amd/Ghcb.h>
@@ -301,3 +304,58 @@ SecValidateSystemRam (
     MemEncryptSevSnpPreValidateSystemRam (Start, EFI_SIZE_TO_PAGES ((UINTN)(End - Start)));
   }
 }
+
+/**
+  Map known MMIO regions unencrypted if SEV-ES is active.
+
+  During early booting, page table entries default to having the encryption bit
+  set for SEV-ES/SEV-SNP guests. In cases where there is MMIO to an address, the
+  encryption bit should be cleared. Clear it here for any known MMIO accesses
+  during SEC, which is currently just the APIC base address.
+
+**/
+VOID
+SecMapApicBaseUnencrypted (
+  VOID
+  )
+{
+  PHYSICAL_ADDRESS    Cr3;
+  UINT64              ApicAddress;
+  VOID                *Buffer;
+  UINTN               BufferSize;
+  IA32_MAP_ATTRIBUTE  MapAttribute;
+  IA32_MAP_ATTRIBUTE  MapMask;
+  RETURN_STATUS       Status;
+
+  if (!SevEsIsEnabled ()) {
+    return;
+  }
+
+  ApicAddress = (UINT64)GetLocalApicBaseAddress ();
+  Buffer      = (VOID *)(UINTN)FixedPcdGet32 (PcdOvmfSecApicPageTableBase);
+  Cr3         = AsmReadCr3 ();
+
+  MapAttribute.Uint64         = ApicAddress;
+  MapAttribute.Bits.Present   = 1;
+  MapAttribute.Bits.ReadWrite = 1;
+  MapMask.Uint64              = MAX_UINT64;
+  BufferSize                  = SIZE_4KB;
+
+  Status = PageTableMap (
+             (UINTN *)&Cr3,
+             Paging4Level,
+             Buffer,
+             &BufferSize,
+             ApicAddress,
+             SIZE_4KB,
+             &MapAttribute,
+             &MapMask,
+             NULL
+             );
+  if (RETURN_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed to map APIC MMIO region as unencrypted: %d\n", Status));
+    ASSERT (FALSE);
+  }
+
+  CpuFlushTlb ();
+}
diff --git a/OvmfPkg/Sec/AmdSev.h b/OvmfPkg/Sec/AmdSev.h
index f75877096e..c5ab0d5a0b 100644
--- a/OvmfPkg/Sec/AmdSev.h
+++ b/OvmfPkg/Sec/AmdSev.h
@@ -91,4 +91,18 @@ SevSnpIsEnabled (
   VOID
   );
 
+/**
+  Map MMIO regions unencrypted if SEV-ES is active.
+
+  During early booting, page table entries default to having the encryption bit
+  set for SEV-ES/SEV-SNP guests. In cases where there is MMIO to an address, the
+  encryption bit should be cleared. Clear it here for any known MMIO accesses
+  during SEC, which is currently just the APIC base address.
+
+**/
+VOID
+SecMapApicBaseUnencrypted (
+  VOID
+  );
+
 #endif
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index a30d4ce09e..60dfa61842 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -938,6 +938,7 @@ SecCoreStartupWithStack (
   // interrupts before initializing the Debug Agent and the debug timer is
   // enabled.
   //
+  SecMapApicBaseUnencrypted ();
   InitializeApicTimer (0, MAX_UINT32, TRUE, 5);
   DisableApicTimerInterrupt ();
 
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index dca932a474..88c2d3fb6d 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -55,6 +55,7 @@
   MemEncryptSevLib
   CpuExceptionHandlerLib
   CcProbeLib
+  CpuPageTableLib
 
 [Ppis]
   gEfiTemporaryRamSupportPpiGuid                # PPI ALWAYS_PRODUCED
@@ -83,6 +84,8 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
   gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecApicPageTableBase
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecApicPageTableSize
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
-- 
cgit v1.2.3