OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 -------- 1 file changed, 8 deletions(-)
The SEV-ES bit of Fn800-001F[EAX] - Bit 3 is used for a host to
determine support for running SEV-ES guests. It should not be checked by
a guest to determine if it is running under SEV-ES. The guest should use
the SEV_STATUS MSR Bit 1 to determine if SEV-ES is enabled. This check
was not part of the original SEV-ES support and was added in
a91b700e38. Removing the check makes this code consistent with the
Linux kernel
Fixes: a91b700e38 (Ovmf/ResetVector: Simplify and consolidate the SEV features checks)
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Marc Orr <marcorr@google.com>
Signed-off-by: Peter Gonda <pgonda@google.com>
---
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 --------
1 file changed, 8 deletions(-)
diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 1f827da3b9..77692db27e 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -265,14 +265,6 @@ CheckSevFeatures:
; Set the work area header to indicate that the SEV is enabled
mov byte[WORK_AREA_GUEST_TYPE], 1
- ; Check for SEV-ES memory encryption feature:
- ; CPUID Fn8000_001F[EAX] - Bit 3
- ; CPUID raises a #VC exception if running as an SEV-ES guest
- mov eax, 0x8000001f
- cpuid
- bt eax, 3
- jnc GetSevEncBit
-
; Check if SEV-ES is enabled
; MSR_0xC0010131 - Bit 1 (SEV-ES enabled)
mov ecx, SEV_STATUS_MSR
--
2.34.1.575.g55b058a8bb-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#85664): https://edk2.groups.io/g/devel/message/85664
Mute This Topic: https://groups.io/mt/88400388/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Tue, 31 May 2022 at 16:40, Peter Gonda <pgonda@google.com> wrote: > > The SEV-ES bit of Fn800-001F[EAX] - Bit 3 is used for a host to > determine support for running SEV-ES guests. It should not be checked by > a guest to determine if it is running under SEV-ES. The guest should use > the SEV_STATUS MSR Bit 1 to determine if SEV-ES is enabled. This check > was not part of the original SEV-ES support and was added in > a91b700e38. Removing the check makes this code consistent with the > Linux kernel > > Fixes: a91b700e38 (Ovmf/ResetVector: Simplify and consolidate the SEV features checks) > > Cc: James Bottomley <jejb@linux.ibm.com> > Cc: Min Xu <min.m.xu@intel.com> > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Erdem Aktas <erdemaktas@google.com> > Cc: Brijesh Singh <brijesh.singh@amd.com> > Cc: Erdem Aktas <erdemaktas@google.com> > Cc: Marc Orr <marcorr@google.com> > Signed-off-by: Peter Gonda <pgonda@google.com> > Acked-by: Tom Lendacky <thomas.lendacky@amd.com> > > --- > OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm > index 1f827da3b9..77692db27e 100644 > --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm > +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm > @@ -265,14 +265,6 @@ CheckSevFeatures: > ; Set the work area header to indicate that the SEV is enabled > mov byte[WORK_AREA_GUEST_TYPE], 1 > > - ; Check for SEV-ES memory encryption feature: > - ; CPUID Fn8000_001F[EAX] - Bit 3 > - ; CPUID raises a #VC exception if running as an SEV-ES guest > - mov eax, 0x8000001f > - cpuid > - bt eax, 3 > - jnc GetSevEncBit > - > ; Check if SEV-ES is enabled > ; MSR_0xC0010131 - Bit 1 (SEV-ES enabled) > mov ecx, SEV_STATUS_MSR Thanks Peter, I have queued this up. I did wonder, though: the only remaining reference to GetSevEncBit is a conditional branch that just precedes the label itself. This appears to be a leftover from commit 63c50d3ff2854a76 ("OvmfPkg/ResetVector: cache the SEV status MSR value in workarea") but it looks a bit dodgy. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90126): https://edk2.groups.io/g/devel/message/90126 Mute This Topic: https://groups.io/mt/91473916/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 6/1/22 07:25, Ard Biesheuvel wrote: > On Tue, 31 May 2022 at 16:40, Peter Gonda <pgonda@google.com> wrote: >> >> The SEV-ES bit of Fn800-001F[EAX] - Bit 3 is used for a host to >> determine support for running SEV-ES guests. It should not be checked by >> a guest to determine if it is running under SEV-ES. The guest should use >> the SEV_STATUS MSR Bit 1 to determine if SEV-ES is enabled. This check >> was not part of the original SEV-ES support and was added in >> a91b700e38. Removing the check makes this code consistent with the >> Linux kernel >> >> Fixes: a91b700e38 (Ovmf/ResetVector: Simplify and consolidate the SEV features checks) >> >> Cc: James Bottomley <jejb@linux.ibm.com> >> Cc: Min Xu <min.m.xu@intel.com> >> Cc: Jiewen Yao <jiewen.yao@intel.com> >> Cc: Tom Lendacky <thomas.lendacky@amd.com> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Erdem Aktas <erdemaktas@google.com> >> Cc: Brijesh Singh <brijesh.singh@amd.com> >> Cc: Erdem Aktas <erdemaktas@google.com> >> Cc: Marc Orr <marcorr@google.com> >> Signed-off-by: Peter Gonda <pgonda@google.com> >> Acked-by: Tom Lendacky <thomas.lendacky@amd.com> >> >> --- >> OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 -------- >> 1 file changed, 8 deletions(-) >> >> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm >> index 1f827da3b9..77692db27e 100644 >> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm >> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm >> @@ -265,14 +265,6 @@ CheckSevFeatures: >> ; Set the work area header to indicate that the SEV is enabled >> mov byte[WORK_AREA_GUEST_TYPE], 1 >> >> - ; Check for SEV-ES memory encryption feature: >> - ; CPUID Fn8000_001F[EAX] - Bit 3 >> - ; CPUID raises a #VC exception if running as an SEV-ES guest >> - mov eax, 0x8000001f >> - cpuid >> - bt eax, 3 >> - jnc GetSevEncBit >> - >> ; Check if SEV-ES is enabled >> ; MSR_0xC0010131 - Bit 1 (SEV-ES enabled) >> mov ecx, SEV_STATUS_MSR > > Thanks Peter, I have queued this up. > > I did wonder, though: the only remaining reference to GetSevEncBit is > a conditional branch that just precedes the label itself. This appears > to be a leftover from commit 63c50d3ff2854a76 ("OvmfPkg/ResetVector: > cache the SEV status MSR value in workarea") but it looks a bit dodgy. Yes, it looks like the rdmsr and the GetSevEncBit: label can all be removed since the MSR value is now cached (a few lines above) and used for checks. Thanks, Tom -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#90127): https://edk2.groups.io/g/devel/message/90127 Mute This Topic: https://groups.io/mt/91473916/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 1/13/22 10:30 AM, Peter Gonda wrote: > The SEV-ES bit of Fn800-001F[EAX] - Bit 3 is used for a host to > determine support for running SEV-ES guests. It should not be checked by > a guest to determine if it is running under SEV-ES. The guest should use > the SEV_STATUS MSR Bit 1 to determine if SEV-ES is enabled. This check > was not part of the original SEV-ES support and was added in > a91b700e38. Removing the check makes this code consistent with the > Linux kernel > > Fixes: a91b700e38 (Ovmf/ResetVector: Simplify and consolidate the SEV features checks) > Cc: James Bottomley <jejb@linux.ibm.com> > Cc: Min Xu <min.m.xu@intel.com> > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Erdem Aktas <erdemaktas@google.com> > Cc: Brijesh Singh <brijesh.singh@amd.com> > Cc: Erdem Aktas <erdemaktas@google.com> > Cc: Marc Orr <marcorr@google.com> > Signed-off-by: Peter Gonda <pgonda@google.com> Acked-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm > index 1f827da3b9..77692db27e 100644 > --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm > +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm > @@ -265,14 +265,6 @@ CheckSevFeatures: > ; Set the work area header to indicate that the SEV is enabled > mov byte[WORK_AREA_GUEST_TYPE], 1 > > - ; Check for SEV-ES memory encryption feature: > - ; CPUID Fn8000_001F[EAX] - Bit 3 > - ; CPUID raises a #VC exception if running as an SEV-ES guest > - mov eax, 0x8000001f > - cpuid > - bt eax, 3 > - jnc GetSevEncBit > - > ; Check if SEV-ES is enabled > ; MSR_0xC0010131 - Bit 1 (SEV-ES enabled) > mov ecx, SEV_STATUS_MSR > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85824): https://edk2.groups.io/g/devel/message/85824 Mute This Topic: https://groups.io/mt/88400388/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.