[edk2-devel] [PATCH] OvmfPkg/ResetVector: Fix SNP CPUID table processing results for ECX/EDX

[edk2-devel] [PATCH] OvmfPkg/ResetVector: Fix SNP CPUID table processing results for ECX/EDX

[Lendacky, Thomas via groups.io]

The current support within the boot SNP CPUID table processing mistakenly
swaps the ECX and EDX results. It does not have an effect at this time
because current CPUID results checking does not check ECX or EDX. However,
any future CPUID checks that need to check ECX or EDX may have erroneous
behavior.

Fix the assembler code to save ECX and EDX to the proper locations.

Fixes: 34819f2caccb ("OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values")
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 OvmfPkg/ResetVector/Ia32/AmdSev.asm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 3abc8300c403..043c88a7abbe 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -395,9 +395,9 @@ SnpCpuidEntryFound:
     mov     [esp + VC_CPUID_RESULT_EAX], eax
     mov     eax, [ecx + SNP_CPUID_ENTRY_EBX]
     mov     [esp + VC_CPUID_RESULT_EBX], eax
-    mov     eax, [ecx + SNP_CPUID_ENTRY_EDX]
-    mov     [esp + VC_CPUID_RESULT_ECX], eax
     mov     eax, [ecx + SNP_CPUID_ENTRY_ECX]
+    mov     [esp + VC_CPUID_RESULT_ECX], eax
+    mov     eax, [ecx + SNP_CPUID_ENTRY_EDX]
     mov     [esp + VC_CPUID_RESULT_EDX], eax
     jmp     VmmDoneSnpCpuid
 
-- 
2.42.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115066): https://edk2.groups.io/g/devel/message/115066
Mute This Topic: https://groups.io/mt/104126108/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] OvmfPkg/ResetVector: Fix SNP CPUID table processing results for ECX/EDX

[Roth, Michael via groups.io]

On Fri, Feb 02, 2024 at 01:18:37PM -0600, Tom Lendacky wrote:
> The current support within the boot SNP CPUID table processing mistakenly
> swaps the ECX and EDX results. It does not have an effect at this time
> because current CPUID results checking does not check ECX or EDX. However,
> any future CPUID checks that need to check ECX or EDX may have erroneous
> behavior.
> 
> Fix the assembler code to save ECX and EDX to the proper locations.
> 
> Fixes: 34819f2caccb ("OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values")
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>

Reviewed-by: Michael Roth <michael.roth@amd.com>

Thanks for the fix. Not sure how this slipped by me =\

-Mike

> ---
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 3abc8300c403..043c88a7abbe 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -395,9 +395,9 @@ SnpCpuidEntryFound:
>      mov     [esp + VC_CPUID_RESULT_EAX], eax
>      mov     eax, [ecx + SNP_CPUID_ENTRY_EBX]
>      mov     [esp + VC_CPUID_RESULT_EBX], eax
> -    mov     eax, [ecx + SNP_CPUID_ENTRY_EDX]
> -    mov     [esp + VC_CPUID_RESULT_ECX], eax
>      mov     eax, [ecx + SNP_CPUID_ENTRY_ECX]
> +    mov     [esp + VC_CPUID_RESULT_ECX], eax
> +    mov     eax, [ecx + SNP_CPUID_ENTRY_EDX]
>      mov     [esp + VC_CPUID_RESULT_EDX], eax
>      jmp     VmmDoneSnpCpuid
>  
> -- 
> 2.42.0
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115070): https://edk2.groups.io/g/devel/message/115070
Mute This Topic: https://groups.io/mt/104126108/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] OvmfPkg/ResetVector: Fix SNP CPUID table processing results for ECX/EDX

[Ard Biesheuvel]

On Fri, 2 Feb 2024 at 22:55, Michael Roth <michael.roth@amd.com> wrote:
>
> On Fri, Feb 02, 2024 at 01:18:37PM -0600, Tom Lendacky wrote:
> > The current support within the boot SNP CPUID table processing mistakenly
> > swaps the ECX and EDX results. It does not have an effect at this time
> > because current CPUID results checking does not check ECX or EDX. However,
> > any future CPUID checks that need to check ECX or EDX may have erroneous
> > behavior.
> >
> > Fix the assembler code to save ECX and EDX to the proper locations.
> >
> > Fixes: 34819f2caccb ("OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values")
> > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>
> Reviewed-by: Michael Roth <michael.roth@amd.com>
>
> Thanks for the fix. Not sure how this slipped by me =\
>

Merged as #5341

Thanks,


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115075): https://edk2.groups.io/g/devel/message/115075
Mute This Topic: https://groups.io/mt/104126108/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-