* [PATCH v2] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check
@ 2022-01-13 16:30 Peter Gonda
2022-01-19 17:03 ` Lendacky, Thomas
2022-06-01 12:25 ` [PATCH V3] " Ard Biesheuvel
0 siblings, 2 replies; 4+ messages in thread
From: Peter Gonda @ 2022-01-13 16:30 UTC (permalink / raw)
To: devel
Cc: Peter Gonda, James Bottomley, Min Xu, Jiewen Yao, Tom Lendacky,
Jordan Justen, Ard Biesheuvel, Laszlo Ersek, Erdem Aktas,
Brijesh Singh, Marc Orr
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check
2022-01-13 16:30 [PATCH v2] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check Peter Gonda
@ 2022-01-19 17:03 ` Lendacky, Thomas
2022-06-01 12:25 ` [PATCH V3] " Ard Biesheuvel
1 sibling, 0 replies; 4+ messages in thread
From: Lendacky, Thomas @ 2022-01-19 17:03 UTC (permalink / raw)
To: Peter Gonda, devel
Cc: James Bottomley, Min Xu, Jiewen Yao, Jordan Justen,
Ard Biesheuvel, Laszlo Ersek, Erdem Aktas, Brijesh Singh,
Marc Orr
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V3] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check
2022-01-13 16:30 [PATCH v2] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check Peter Gonda
2022-01-19 17:03 ` Lendacky, Thomas
@ 2022-06-01 12:25 ` Ard Biesheuvel
2022-06-01 13:16 ` Lendacky, Thomas
1 sibling, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2022-06-01 12:25 UTC (permalink / raw)
To: Peter Gonda
Cc: edk2-devel-groups-io, James Bottomley, Min Xu, Jiewen Yao,
Tom Lendacky, Jordan Justen, Laszlo Ersek, Erdem Aktas,
Brijesh Singh, Marc Orr, Gerd Hoffmann
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V3] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check
2022-06-01 12:25 ` [PATCH V3] " Ard Biesheuvel
@ 2022-06-01 13:16 ` Lendacky, Thomas
0 siblings, 0 replies; 4+ messages in thread
From: Lendacky, Thomas @ 2022-06-01 13:16 UTC (permalink / raw)
To: Ard Biesheuvel, Peter Gonda
Cc: edk2-devel-groups-io, James Bottomley, Min Xu, Jiewen Yao,
Jordan Justen, Laszlo Ersek, Erdem Aktas, Brijesh Singh, Marc Orr,
Gerd Hoffmann
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-01 13:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-13 16:30 [PATCH v2] OvmfPkg/ResetVector: Removing SEV-ES CPUID bit check Peter Gonda
2022-01-19 17:03 ` Lendacky, Thomas
2022-06-01 12:25 ` [PATCH V3] " Ard Biesheuvel
2022-06-01 13:16 ` Lendacky, Thomas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox