[PATCH] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check

[PATCH] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check

[pgonda]

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.

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: Marc Orr <marcorr@google.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jim Mattson <jmattson@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.448.ga2b2bfdf31-goog

Re: [PATCH] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check

[Lendacky, Thomas]

On 1/7/22 11:04 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.

Worth mentioning in the commit message that this check wasn't part of the 
original SEV-ES support (Fixes: a91b700e385e7484ab7286b3ba7ea2efbd59480e 
tag?), so this is really a compatibility thing, and that this makes the 
check consistent with the Linux kernel.

Thanks,
Tom

> 
> 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: Marc Orr <marcorr@google.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Jim Mattson <jmattson@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
> 

Re: [PATCH] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check

[Peter Gonda]

On Fri, Jan 7, 2022 at 3:54 PM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 1/7/22 11:04 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.
>
> Worth mentioning in the commit message that this check wasn't part of the
> original SEV-ES support (Fixes: a91b700e385e7484ab7286b3ba7ea2efbd59480e
> tag?), so this is really a compatibility thing, and that this makes the
> check consistent with the Linux kernel.

Sure I update the commit message in the V2 with this info and add the
Fixes tag. Do I need a (Fixes:
b461d67639f2deced77e9bb967d014b7cfcd75f8) tag too? Since the Check was
moved between files in that commit?

Re: [PATCH] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check

[Lendacky, Thomas]

On 1/10/22 9:29 AM, Peter Gonda wrote:
> On Fri, Jan 7, 2022 at 3:54 PM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>>
>> On 1/7/22 11:04 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.
>>
>> Worth mentioning in the commit message that this check wasn't part of the
>> original SEV-ES support (Fixes: a91b700e385e7484ab7286b3ba7ea2efbd59480e
>> tag?), so this is really a compatibility thing, and that this makes the
>> check consistent with the Linux kernel.
> 
> Sure I update the commit message in the V2 with this info and add the
> Fixes tag. Do I need a (Fixes:
> b461d67639f2deced77e9bb967d014b7cfcd75f8) tag too? Since the Check was
> moved between files in that commit?

I don't think so, but that's just my opinion.

Thanks,
Tom

> 

Re: [PATCH] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check

[Peter Gonda]

On Mon, Jan 10, 2022 at 11:18 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 1/10/22 9:29 AM, Peter Gonda wrote:
> > On Fri, Jan 7, 2022 at 3:54 PM Tom Lendacky <thomas.lendacky@amd.com> wrote:
> >>
> >> On 1/7/22 11:04 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.
> >>
> >> Worth mentioning in the commit message that this check wasn't part of the
> >> original SEV-ES support (Fixes: a91b700e385e7484ab7286b3ba7ea2efbd59480e
> >> tag?), so this is really a compatibility thing, and that this makes the
> >> check consistent with the Linux kernel.
> >
> > Sure I update the commit message in the V2 with this info and add the
> > Fixes tag. Do I need a (Fixes:
> > b461d67639f2deced77e9bb967d014b7cfcd75f8) tag too? Since the Check was
> > moved between files in that commit?
>
> I don't think so, but that's just my opinion.

Thanks. Sent V2 with updates to commit and fixes tag.

>
> Thanks,
> Tom
>
> >