On 05/27/21 01:10, Brijesh Singh wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275 > > Create a function that can be used to determine if VM is running as an > SEV-SNP guest. > > Cc: James Bottomley <j...@linux.ibm.com> > Cc: Min Xu <min.m...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Tom Lendacky <thomas.lenda...@amd.com> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Erdem Aktas <erdemak...@google.com> > Signed-off-by: Brijesh Singh <brijesh.si...@amd.com> > --- > OvmfPkg/Include/Library/MemEncryptSevLib.h | 12 +++++++++ > .../DxeMemEncryptSevLibInternal.c | 27 +++++++++++++++++++ > .../PeiMemEncryptSevLibInternal.c | 27 +++++++++++++++++++ > .../SecMemEncryptSevLibInternal.c | 19 +++++++++++++ > 4 files changed, 85 insertions(+) > > diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h > b/OvmfPkg/Include/Library/MemEncryptSevLib.h > index 76d06c206c8b..2425d8ba0a36 100644 > --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h > +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h > @@ -66,6 +66,18 @@ typedef enum { > MemEncryptSevAddressRangeError, > } MEM_ENCRYPT_SEV_ADDRESS_RANGE_STATE; > > +/** > + Returns a boolean to indicate whether SEV-SNP is enabled > + > + @retval TRUE SEV-SNP is enabled > + @retval FALSE SEV-SNP is not enabled > +**/ > +BOOLEAN > +EFIAPI > +MemEncryptSevSnpIsEnabled ( > + VOID > + ); > + > /** > Returns a boolean to indicate whether SEV-ES is enabled. > > diff --git > a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c > b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c > index 2816f859a0c4..057129723824 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c > @@ -19,6 +19,7 @@ > > STATIC BOOLEAN mSevStatus = FALSE; > STATIC BOOLEAN mSevEsStatus = FALSE; > +STATIC BOOLEAN mSevSnpStatus = FALSE; > STATIC BOOLEAN mSevStatusChecked = FALSE; > > STATIC UINT64 mSevEncryptionMask = 0; > @@ -82,11 +83,37 @@ InternalMemEncryptSevStatus ( > if (Msr.Bits.SevEsBit) { > mSevEsStatus = TRUE; > } > + > + // > + // Check MSR_0xC0010131 Bit 2 (Sev-Snp Enabled) > + // > + if (Msr.Bits.SevSnpBit) { > + mSevSnpStatus = TRUE; > + } > } > > mSevStatusChecked = TRUE; > } > > +/** > + Returns a boolean to indicate whether SEV-SNP is enabled. > + > + @retval TRUE SEV-SNP is enabled > + @retval FALSE SEV-SNP is not enabled > +**/ > +BOOLEAN > +EFIAPI > +MemEncryptSevSnpIsEnabled ( > + VOID > + ) > +{ > + if (!mSevStatusChecked) { > + InternalMemEncryptSevStatus (); > + } > + > + return mSevSnpStatus; > +} > + > /** > Returns a boolean to indicate whether SEV-ES is enabled. > > diff --git > a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c > b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c > index e2fd109d120f..b561f211f577 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c > @@ -19,6 +19,7 @@ > > STATIC BOOLEAN mSevStatus = FALSE; > STATIC BOOLEAN mSevEsStatus = FALSE; > +STATIC BOOLEAN mSevSnpStatus = FALSE; > STATIC BOOLEAN mSevStatusChecked = FALSE; > > STATIC UINT64 mSevEncryptionMask = 0; > @@ -82,11 +83,37 @@ InternalMemEncryptSevStatus ( > if (Msr.Bits.SevEsBit) { > mSevEsStatus = TRUE; > } > + > + // > + // Check MSR_0xC0010131 Bit 2 (Sev-Snp Enabled) > + // > + if (Msr.Bits.SevSnpBit) { > + mSevSnpStatus = TRUE; > + } > } > > mSevStatusChecked = TRUE; > } > > +/** > + Returns a boolean to indicate whether SEV-SNP is enabled. > + > + @retval TRUE SEV-SNP is enabled > + @retval FALSE SEV-SNP is not enabled > +**/ > +BOOLEAN > +EFIAPI > +MemEncryptSevSnpIsEnabled ( > + VOID > + ) > +{ > + if (!mSevStatusChecked) { > + InternalMemEncryptSevStatus (); > + } > + > + return mSevSnpStatus; > +} > + > /** > Returns a boolean to indicate whether SEV-ES is enabled. > > diff --git > a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c > b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c > index 56d8f3f3183f..69852779e2ff 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c > @@ -62,6 +62,25 @@ InternalMemEncryptSevStatus ( > return ReadSevMsr ? AsmReadMsr32 (MSR_SEV_STATUS) : 0; > } > > +/** > + Returns a boolean to indicate whether SEV-SNP is enabled. > + > + @retval TRUE SEV-SNP is enabled > + @retval FALSE SEV-SNP is not enabled > +**/ > +BOOLEAN > +EFIAPI > +MemEncryptSevSnpIsEnabled ( > + VOID > + ) > +{ > + MSR_SEV_STATUS_REGISTER Msr; > + > + Msr.Uint32 = InternalMemEncryptSevStatus (); > + > + return Msr.Bits.SevSnpBit ? TRUE : FALSE;
According to the edk2 coding style, this should be written as: return (BOOLEAN)(Msr.Bits.SevSnpBit != 0); primarily because "Msr.Bits.SevSnpBit" does not have type BOOLEAN, so it should not be used stand-alone in a logical context -- instead, it should be compared with zero explicitly. However, I see that the other two functions MemEncryptSevEsIsEnabled() and MemEncryptSevIsEnabled() in this library instance use the same pattern. I agree consistency is more important here. And I don't think we should add more patches just for updating the style there. These functions are closely located together in the same source file; if we ever update the style, it's not a big deal to update them together. Reviewed-by: Laszlo Ersek <ler...@redhat.com> Thanks Laszlo > +} > + > /** > Returns a boolean to indicate whether SEV-ES is enabled. > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76074): https://edk2.groups.io/g/devel/message/76074 Mute This Topic: https://groups.io/mt/83113763/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-