On 11/29/21 1:04 PM, Tom Lendacky wrote:
On 11/25/21 7:12 AM, qi zhou wrote:
From 5b10265fa5c7b5ca728b4f18488089de6535ed28 Mon Sep 17 00:00:00 2001
From: Qi Zhou <atm...@outlook.com>
Date: Thu, 25 Nov 2021 20:25:55 +0800
Subject: [PATCH] OvmfPkg/MemEncryptSevLib: check CPUID when read msr
during
PEI phase
Tested on Intel Platform, It is like 'SEV-ES work area' can be
modified by
os(Windows etc), and will not restored on reboot, the
SevEsWorkArea->EncryptionMask may have a random value after reboot.
then it
may casue fail on reboot. The msr bits already cached by
mSevStatusChecked,
there is no need to try cache again in PEI phase.
Signed-off-by: Qi Zhou <atm...@outlook.com>
---
.../PeiMemEncryptSevLibInternal.c | 55 +++++++------------
1 file changed, 19 insertions(+), 36 deletions(-)
diff --git
a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
index e2fd109d12..0819f50669 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c
@@ -38,49 +38,32 @@ InternalMemEncryptSevStatus (
UINT32 RegEax;
MSR_SEV_STATUS_REGISTER Msr;
CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
- BOOLEAN ReadSevMsr;
- SEC_SEV_ES_WORK_AREA *SevEsWorkArea;
- ReadSevMsr = FALSE;
-
- SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32
(PcdSevEsWorkAreaBase);
- if (SevEsWorkArea != NULL && SevEsWorkArea->EncryptionMask != 0) {
- //
- // The MSR has been read before, so it is safe to read it again
and avoid
- // having to validate the CPUID information.
+ //
+ // Check if memory encryption leaf exist
+ //
+ AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
+ if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
What is missing in the original patch set is that now with the common
work area we need to check the Guest Type before accessing the SevEs
workarea type. I have a patch in my wip to cleanup the SEV feature
detection check and patiently waiting for the SEV-SNP series to land so
that I can submit other patches.
You need something like IsSevGuest() before accessing the SevEs
workarea, see how its done for the SEC.
https://github.com/AMDESE/ovmf/blob/snp-v13/OvmfPkg/Sec/AmdSev.c#L234
In my WIP I am moving that to common BaseMemEncryptLib.
thanks
This now defeats the purpose of the workarea the already validated CPUID
information. This CPUID information will now require validating.
Wouldn't the best thing be to clear the workarea in the early boot code?
Thanks,
Tom
//
- ReadSevMsr = TRUE;
- } else {
+ // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
//
- // Check if memory encryption leaf exist
- //
- AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
- if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
+ AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL,
NULL);
+
+ if (Eax.Bits.SevBit) {
//
- // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
+ // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
//
- AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL,
NULL, NULL);
-
- if (Eax.Bits.SevBit) {
- ReadSevMsr = TRUE;
+ Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
+ if (Msr.Bits.SevBit) {
+ mSevStatus = TRUE;
}
- }
- }
-
- if (ReadSevMsr) {
- //
- // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
- //
- Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
- if (Msr.Bits.SevBit) {
- mSevStatus = TRUE;
- }
- //
- // Check MSR_0xC0010131 Bit 1 (Sev-Es Enabled)
- //
- if (Msr.Bits.SevEsBit) {
- mSevEsStatus = TRUE;
+ //
+ // Check MSR_0xC0010131 Bit 1 (Sev-Es Enabled)
+ //
+ if (Msr.Bits.SevEsBit) {
+ mSevEsStatus = TRUE;
+ }
}
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84135): https://edk2.groups.io/g/devel/message/84135
Mute This Topic: https://groups.io/mt/87301748/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
