With SEV-SNP, the SEV-ES save area for a vCPU should be unique to that
vCPU. After commit 3323359a811a, the VMSA allocation was re-used, but when
sorting the CPUs by APIC ID, the save area was not updated to follow the
original CPU. Similar to the StartupApSignal address, the SevEsSaveArea
address should be updated when sorting the CPUs.

This does not cause an issue at this time because all APs are in HLT state
and then are (re)started at the same time, with the same VMSA contents.
However, this should be fixed to account for any change in future
behavior.

Fixes: 3323359a811a ("UefiCpuPkg/MpInitLib: Reuse VMSA allocation to ...")
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9a6ec5db5ce9..a41b9e5701af 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -370,6 +370,7 @@ SortApicId (
   UINT32           ApCount;
   CPU_INFO_IN_HOB  *CpuInfoInHob;
   volatile UINT32  *StartupApSignal;
+  VOID             *SevEsSaveArea;
 
   ApCount      = CpuMpData->CpuCount - 1;
   CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
@@ -397,12 +398,17 @@ SortApicId (
         CopyMem (&CpuInfoInHob[Index1], &CpuInfo, sizeof (CPU_INFO_IN_HOB));
 
         //
-        // Also exchange the StartupApSignal.
+        // Also exchange the StartupApSignal and SevEsSaveArea.
         //
         StartupApSignal                            = CpuMpData->CpuData[Index3].StartupApSignal;
         CpuMpData->CpuData[Index3].StartupApSignal =
           CpuMpData->CpuData[Index1].StartupApSignal;
         CpuMpData->CpuData[Index1].StartupApSignal = StartupApSignal;
+
+        SevEsSaveArea                            = CpuMpData->CpuData[Index3].SevEsSaveArea;
+        CpuMpData->CpuData[Index3].SevEsSaveArea =
+          CpuMpData->CpuData[Index1].SevEsSaveArea;
+        CpuMpData->CpuData[Index1].SevEsSaveArea = SevEsSaveArea;
       }
     }
 
-- 
2.42.0



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

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
> -----Original Message-----
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Sent: Tuesday, November 7, 2023 6:46 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>;
> Ard Biesheuvel <ardb+tianocore@kernel.org>; Michael Roth
> <michael.roth@amd.com>
> Subject: [PATCH 2/2] UefiCpuPkg/MpInitLib: Copy SEV-ES save area pointer
> during APIC ID sorting
> 
> With SEV-SNP, the SEV-ES save area for a vCPU should be unique to that
> vCPU. After commit 3323359a811a, the VMSA allocation was re-used, but
> when
> sorting the CPUs by APIC ID, the save area was not updated to follow the
> original CPU. Similar to the StartupApSignal address, the SevEsSaveArea
> address should be updated when sorting the CPUs.
> 
> This does not cause an issue at this time because all APs are in HLT state
> and then are (re)started at the same time, with the same VMSA contents.
> However, this should be fixed to account for any change in future
> behavior.
> 
> Fixes: 3323359a811a ("UefiCpuPkg/MpInitLib: Reuse VMSA allocation to ...")
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 9a6ec5db5ce9..a41b9e5701af 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -370,6 +370,7 @@ SortApicId (
>    UINT32           ApCount;
>    CPU_INFO_IN_HOB  *CpuInfoInHob;
>    volatile UINT32  *StartupApSignal;
> +  VOID             *SevEsSaveArea;
> 
>    ApCount      = CpuMpData->CpuCount - 1;
>    CpuInfoInHob = (CPU_INFO_IN_HOB
> *)(UINTN)CpuMpData->CpuInfoInHob;
> @@ -397,12 +398,17 @@ SortApicId (
>          CopyMem (&CpuInfoInHob[Index1], &CpuInfo, sizeof
> (CPU_INFO_IN_HOB));
> 
>          //
> -        // Also exchange the StartupApSignal.
> +        // Also exchange the StartupApSignal and SevEsSaveArea.
>          //
>          StartupApSignal                            =
> CpuMpData->CpuData[Index3].StartupApSignal;
>          CpuMpData->CpuData[Index3].StartupApSignal =
>            CpuMpData->CpuData[Index1].StartupApSignal;
>          CpuMpData->CpuData[Index1].StartupApSignal = StartupApSignal;
> +
> +        SevEsSaveArea                            =
> CpuMpData->CpuData[Index3].SevEsSaveArea;
> +        CpuMpData->CpuData[Index3].SevEsSaveArea =
> +          CpuMpData->CpuData[Index1].SevEsSaveArea;
> +        CpuMpData->CpuData[Index1].SevEsSaveArea = SevEsSaveArea;
>        }
>      }
> 
> --
> 2.42.0



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