CPUID leaf 0xD sub-leafs 0x0 and 0x1 contain cumulative sizes for the
enabled XSave areas. Those sizes are calculated by tallying up all the
other sub-leafs that contain per-area size information for XSave areas
that are currently enabled in XCr0/XSS. The current check has the logic
inverted. Fix that.

This doesn't seem to cause problems currently, but could in the future
if OVMF made more extensive use of XSave areas. It was noticed while
implementing SNP-related tests for KVM Unit Tests, which re-uses the
OVMF #VC handler in some cases.

Reported-by: Pavan Kumar Paluri <papaluri@amd.com>
Cc: Pavan Kumar Paluri <papaluri@amd.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
index 985e547977..cd117d5a31 100644
--- a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
+++ b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
@@ -1678,9 +1678,7 @@ GetCpuidXSaveSize (
   for (Idx = 0; Idx < CpuidInfo->Count; Idx++) {

     SEV_SNP_CPUID_FUNCTION  *CpuidFn = &CpuidInfo->function[Idx];

 

-    if (!((CpuidFn->EaxIn == 0xD) &&

-          ((CpuidFn->EcxIn == 0) || (CpuidFn->EcxIn == 1))))

-    {

+    if (!((CpuidFn->EaxIn == 0xD) && (CpuidFn->EcxIn > 1))) {

       continue;

     }

 

-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97704): https://edk2.groups.io/g/devel/message/97704
Mute This Topic: https://groups.io/mt/95815542/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Acked-by: Jiewen Yao <jiewen.yao@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Roth,
> Michael via groups.io
> Sent: Thursday, December 22, 2022 12:07 AM
> To: devel@edk2.groups.io
> Cc: Tom Lendacky <thomas.lendacky@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Pavan Kumar Paluri <papaluri@amd.com>
> Subject: [edk2-devel] [PATCH 3/4] OvmfPkg/CcExitLib: Fix SEV-SNP XSave area
> size calculation
> 
> CPUID leaf 0xD sub-leafs 0x0 and 0x1 contain cumulative sizes for the
> enabled XSave areas. Those sizes are calculated by tallying up all the
> other sub-leafs that contain per-area size information for XSave areas
> that are currently enabled in XCr0/XSS. The current check has the logic
> inverted. Fix that.
> 
> This doesn't seem to cause problems currently, but could in the future
> if OVMF made more extensive use of XSave areas. It was noticed while
> implementing SNP-related tests for KVM Unit Tests, which re-uses the
> OVMF #VC handler in some cases.
> 
> Reported-by: Pavan Kumar Paluri <papaluri@amd.com>
> Cc: Pavan Kumar Paluri <papaluri@amd.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>  OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> index 985e547977..cd117d5a31 100644
> --- a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> +++ b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> @@ -1678,9 +1678,7 @@ GetCpuidXSaveSize (
>    for (Idx = 0; Idx < CpuidInfo->Count; Idx++) {
> 
>      SEV_SNP_CPUID_FUNCTION  *CpuidFn = &CpuidInfo->function[Idx];
> 
> 
> 
> -    if (!((CpuidFn->EaxIn == 0xD) &&
> 
> -          ((CpuidFn->EcxIn == 0) || (CpuidFn->EcxIn == 1))))
> 
> -    {
> 
> +    if (!((CpuidFn->EaxIn == 0xD) && (CpuidFn->EcxIn > 1))) {
> 
>        continue;
> 
>      }
> 
> 
> 
> --
> 2.25.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98081): https://edk2.groups.io/g/devel/message/98081
Mute This Topic: https://groups.io/mt/95815542/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 12/21/22 10:06, Michael Roth wrote:
> CPUID leaf 0xD sub-leafs 0x0 and 0x1 contain cumulative sizes for the
> enabled XSave areas. Those sizes are calculated by tallying up all the
> other sub-leafs that contain per-area size information for XSave areas
> that are currently enabled in XCr0/XSS. The current check has the logic
> inverted. Fix that.
> 
> This doesn't seem to cause problems currently, but could in the future
> if OVMF made more extensive use of XSave areas. It was noticed while
> implementing SNP-related tests for KVM Unit Tests, which re-uses the
> OVMF #VC handler in some cases.
> 
> Reported-by: Pavan Kumar Paluri <papaluri@amd.com>
> Cc: Pavan Kumar Paluri <papaluri@amd.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> index 985e547977..cd117d5a31 100644
> --- a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> +++ b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> @@ -1678,9 +1678,7 @@ GetCpuidXSaveSize (
>     for (Idx = 0; Idx < CpuidInfo->Count; Idx++) {
>       SEV_SNP_CPUID_FUNCTION  *CpuidFn = &CpuidInfo->function[Idx];
>   
> -    if (!((CpuidFn->EaxIn == 0xD) &&
> -          ((CpuidFn->EcxIn == 0) || (CpuidFn->EcxIn == 1))))
> -    {
> +    if (!((CpuidFn->EaxIn == 0xD) && (CpuidFn->EcxIn > 1))) {
>         continue;
>       }
>   


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97693): https://edk2.groups.io/g/devel/message/97693
Mute This Topic: https://groups.io/mt/95809908/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-