A full-featured SEV-SNP guest will not rely on the AP jump table, and
will instead use the AP Creation interface defined by the GHCB. However,
a guest is still allowed to use the AP jump table if desired.

However, unlike with SEV-ES guests, SEV-SNP guests should not
store/retrieve the jump table address via GHCB requests to the
hypervisor, they should instead store/retrieve it via the SEV-SNP
secrets page. Implement the store side of this for OVMF.

Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index e1cd0b3500..d8cfddcd82 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -80,3 +80,4 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                      ## CONSUMES

   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                           ## CONSUMES

   gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr           ## CONSUMES

+  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress                     ## CONSUMES

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 60d14a5a0e..4d6f7643db 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -15,6 +15,7 @@
 #include <Library/VmgExitLib.h>

 #include <Register/Amd/Fam17Msr.h>

 #include <Register/Amd/Ghcb.h>

+#include <Register/Amd/SnpSecretsPage.h>

 

 #include <Protocol/Timer.h>

 

@@ -216,6 +217,15 @@ GetSevEsAPMemory (
 

   DEBUG ((DEBUG_INFO, "Dxe: SevEsAPMemory = %lx\n", (UINTN)StartAddress));

 

+  if (ConfidentialComputingGuestHas (CCAttrAmdSevSnp)) {

+    SNP_SECRETS_PAGE  *Secrets;

+

+    Secrets                       = (SNP_SECRETS_PAGE *)(INTN)PcdGet64 (PcdSevSnpSecretsAddress);

+    Secrets->OsArea.ApJumpTablePa = (UINT64)(UINTN)StartAddress;

+

+    return (UINTN)StartAddress;

+  }

+

   //

   // Save the SevEsAPMemory as the AP jump table.

   //

-- 
2.25.1



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

On 5/20/22 10:27, Michael Roth wrote:
> A full-featured SEV-SNP guest will not rely on the AP jump table, and
> will instead use the AP Creation interface defined by the GHCB. However,
> a guest is still allowed to use the AP jump table if desired.
> 
> However, unlike with SEV-ES guests, SEV-SNP guests should not
> store/retrieve the jump table address via GHCB requests to the
> hypervisor, they should instead store/retrieve it via the SEV-SNP
> secrets page. Implement the store side of this for OVMF.
> 
> Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>   UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
>   UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       | 10 ++++++++++
>   2 files changed, 11 insertions(+)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> index e1cd0b3500..d8cfddcd82 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> @@ -80,3 +80,4 @@
>     gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                      ## CONSUMES
>     gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase                           ## CONSUMES
>     gEfiMdePkgTokenSpaceGuid.PcdConfidentialComputingGuestAttr           ## CONSUMES
> +  gEfiMdePkgTokenSpaceGuid.PcdSevSnpSecretsAddress                     ## CONSUMES
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index 60d14a5a0e..4d6f7643db 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -15,6 +15,7 @@
>   #include <Library/VmgExitLib.h>
>   #include <Register/Amd/Fam17Msr.h>
>   #include <Register/Amd/Ghcb.h>
> +#include <Register/Amd/SnpSecretsPage.h>
>   
>   #include <Protocol/Timer.h>
>   
> @@ -216,6 +217,15 @@ GetSevEsAPMemory (
>   
>     DEBUG ((DEBUG_INFO, "Dxe: SevEsAPMemory = %lx\n", (UINTN)StartAddress));
>   
> +  if (ConfidentialComputingGuestHas (CCAttrAmdSevSnp)) {
> +    SNP_SECRETS_PAGE  *Secrets;
> +
> +    Secrets                       = (SNP_SECRETS_PAGE *)(INTN)PcdGet64 (PcdSevSnpSecretsAddress);
> +    Secrets->OsArea.ApJumpTablePa = (UINT64)(UINTN)StartAddress;
> +
> +    return (UINTN)StartAddress;
> +  }
> +

Just a nit, but I probably would have still put this under the comment 
below, because you are still saving the SevEsAPMemory as the AP jump table.

It might look cleaner with the non-SNP path as the else and have the 
single return.

Probably not worth another version, but up to you.

Thanks,
Tom

>     //
>     // Save the SevEsAPMemory as the AP jump table.
>     //


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