Re: [PATCH RFC v2 07/28] OvmfPkg: Use MemEncryptSevClearMmioPageEncMask() to clear EncMask from Mmio


Laszlo Ersek
 

On 04/30/21 13:51, Brijesh Singh wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Use the MemEncryptSevClearMmioPageEncMask() to clear memory encryption mask
for the Mmio address range from the current page table context.

Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Erdem Aktas <erdemaktas@...>
Signed-off-by: Brijesh Singh <brijesh.singh@...>
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 10 ++++------
OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c | 5 ++---
OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c | 5 ++---
3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 689bfb376d..80831b81fa 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -53,11 +53,10 @@ AmdSevDxeEntryPoint (
Desc = &AllDescMap[Index];
if (Desc->GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo ||
Desc->GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
- Status = MemEncryptSevClearPageEncMask (
+ Status = MemEncryptSevClearMmioPageEncMask (
0,
Desc->BaseAddress,
- EFI_SIZE_TO_PAGES (Desc->Length),
- FALSE
+ EFI_SIZE_TO_PAGES (Desc->Length)
);
ASSERT_EFI_ERROR (Status);
}
@@ -73,11 +72,10 @@ AmdSevDxeEntryPoint (
// the range.
//
if (PcdGet16 (PcdOvmfHostBridgePciDevId) == INTEL_Q35_MCH_DEVICE_ID) {
- Status = MemEncryptSevClearPageEncMask (
+ Status = MemEncryptSevClearMmioPageEncMask (
0,
FixedPcdGet64 (PcdPciExpressBaseAddress),
- EFI_SIZE_TO_PAGES (SIZE_256MB),
- FALSE
+ EFI_SIZE_TO_PAGES (SIZE_256MB)
);

ASSERT_EFI_ERROR (Status);
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
index 1f285e0083..ab40087a84 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
@@ -205,11 +205,10 @@ MarkIoMemoryRangeForRuntimeAccess (
// memory range.
//
if (MemEncryptSevIsEnabled ()) {
- Status = MemEncryptSevClearPageEncMask (
+ Status = MemEncryptSevClearMmioPageEncMask (
0,
BaseAddress,
- EFI_SIZE_TO_PAGES (Length),
- FALSE
+ EFI_SIZE_TO_PAGES (Length)
);
ASSERT_EFI_ERROR (Status);
}
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
index 7eb80bfeff..ea75b489c7 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashSmm.c
@@ -38,11 +38,10 @@ QemuFlashBeforeProbe (
// C-bit on flash ranges from SMM page table.
//

- Status = MemEncryptSevClearPageEncMask (
+ Status = MemEncryptSevClearMmioPageEncMask (
0,
BaseAddress,
- EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount),
- FALSE
+ EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount)
);
ASSERT_EFI_ERROR (Status);
}
The contents of this patch are sound, but they are incomplete, and
incorrectly structured too.

(1) Please provide a separate patch for each modified module.

(2) You missed the MemEncryptSevClearPageEncMask() call in
TpmMmioSevDecryptPeimEntryPoint()
[OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c] -- probably
because you worked on this series in parallel with Tom working on the
SEV-ES TPM fixes.

In the end, this patch should be split into three patches (because the
change is needed for three modules).

Thanks
Laszlo

Join devel@edk2.groups.io to automatically receive all group messages.