Here I will add some debug infomation about the reboot issue. on the master branch, I added some logs, see below:
https://1drv.ms/u/s!As-Ec5SPH0fuimANnT5FPm8cmeM6 --- .../Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c index e2fd109d12..8ab5849386 100644 --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c @@ -44,6 +44,7 @@ InternalMemEncryptSevStatus ( ReadSevMsr = FALSE; SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase); + DEBUG ((DEBUG_INFO, "SevEsWorkAera: %p, SevEsWorkArea->EncryptionMask: %lu\n", SevEsWorkArea, SevEsWorkArea ? SevEsWorkArea->EncryptionMask : 0)); if (SevEsWorkArea != NULL && SevEsWorkArea->EncryptionMask != 0) { // // The MSR has been read before, so it is safe to read it again and avoid @@ -71,7 +72,9 @@ InternalMemEncryptSevStatus ( // // Check MSR_0xC0010131 Bit 0 (Sev Enabled) // + DEBUG ((DEBUG_INFO, "Begin read msr\n")); Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS); + DEBUG ((DEBUG_INFO, "End read msr\n")); if (Msr.Bits.SevBit) { mSevStatus = TRUE; } -- 2.25.1 Then rebuild the ovmf package, launch qemu with windows 10 guest os installed, repeat reboot win10 guest os 4-10 times, then there may got reboot fail see this screenshot: https://1drv.ms/u/s!As-Ec5SPH0fuil9hsVV_ooZG0mcw?e=xgfJoL From: qi zhou <atm...@outlook.com> Sent: Thursday, November 25, 2021 21:12 To: devel@edk2.groups.io <devel@edk2.groups.io> Cc: brijesh.si...@amd.com <brijesh.si...@amd.com>; erdemak...@google.com <erdemak...@google.com>; j...@linux.ibm.com <j...@linux.ibm.com>; jiewen....@intel.com <jiewen....@intel.com>; min.m...@intel.com <min.m...@intel.com>; thomas.lenda...@amd.com <thomas.lenda...@amd.com> Subject: [PATCH] OvmfPkg/MemEncryptSevLib: check CPUID when read msr during PEI phase >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) { // - 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; + } } } -- 2.25.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#84086): https://edk2.groups.io/g/devel/message/84086 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] -=-=-=-=-=-=-=-=-=-=-=-
