[edk2-devel] [PATCH v7 25/31] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled

Ni, Ray ray.ni at intel.com
Tue Sep 14 02:25:18 UTC 2021


The comments don't apply to this patch only.
To be clear, it would be great that you can do a cleanup of existing code to try best separating the SEV flow from the common flow.

-----Original Message-----
From: Ni, Ray 
Sent: Tuesday, September 14, 2021 10:24 AM
To: Brijesh Singh <brijesh.singh at amd.com>; devel at edk2.groups.io
Cc: James Bottomley <jejb at linux.ibm.com>; Xu, Min M <min.m.xu at intel.com>; Yao, Jiewen <jiewen.yao at intel.com>; Tom Lendacky <thomas.lendacky at amd.com>; Justen, Jordan L <jordan.l.justen at intel.com>; Ard Biesheuvel <ardb+tianocore at kernel.org>; Erdem Aktas <erdemaktas at google.com>; Michael Roth <Michael.Roth at amd.com>; Gerd Hoffmann <kraxel at redhat.com>; Michael Roth <michael.roth at amd.com>; Dong, Eric <eric.dong at intel.com>; Kumar, Rahul1 <Rahul1.Kumar at intel.com>
Subject: RE: [PATCH v7 25/31] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled

Hi Brijesh,
Can you please separate the SEV logic in separate functions in separate files?
These are not x86 common logics. With more and more SEV specific logics added, I want to keep the common flow clean.

Thanks,
Ray

-----Original Message-----
From: Brijesh Singh <brijesh.singh at amd.com>
Sent: Tuesday, September 14, 2021 2:20 AM
To: devel at edk2.groups.io
Cc: James Bottomley <jejb at linux.ibm.com>; Xu, Min M <min.m.xu at intel.com>; Yao, Jiewen <jiewen.yao at intel.com>; Tom Lendacky <thomas.lendacky at amd.com>; Justen, Jordan L <jordan.l.justen at intel.com>; Ard Biesheuvel <ardb+tianocore at kernel.org>; Erdem Aktas <erdemaktas at google.com>; Michael Roth <Michael.Roth at amd.com>; Gerd Hoffmann <kraxel at redhat.com>; Brijesh Singh <brijesh.singh at amd.com>; Michael Roth <michael.roth at amd.com>; Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Kumar, Rahul1 <rahul1.kumar at intel.com>
Subject: [PATCH v7 25/31] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

An SEV-SNP guest requires that the physical address of the GHCB must be registered with the hypervisor before using it. See the GHCB specification section 2.3.2 for more details.

Cc: Michael Roth <michael.roth at amd.com>
Cc: Eric Dong <eric.dong at intel.com>
Cc: Ray Ni <ray.ni at intel.com>
Cc: Rahul Kumar <rahul1.kumar at intel.com>
Cc: James Bottomley <jejb at linux.ibm.com>
Cc: Min Xu <min.m.xu at intel.com>
Cc: Jiewen Yao <jiewen.yao at intel.com>
Cc: Tom Lendacky <thomas.lendacky at amd.com>
Cc: Jordan Justen <jordan.l.justen at intel.com>
Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
Cc: Erdem Aktas <erdemaktas at google.com>
Cc: Gerd Hoffmann <kraxel at redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.h          |  2 +
 UefiCpuPkg/Library/MpInitLib/MpLib.c          |  2 +
 UefiCpuPkg/Library/MpInitLib/MpEqu.inc        |  1 +
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 53 +++++++++++++++++++
 4 files changed, 58 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 388ebef7b0dc..56d6d703d8b0 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -219,6 +219,7 @@ typedef struct {
   //
   BOOLEAN               Enable5LevelPaging;
   BOOLEAN               SevEsIsEnabled;
+  BOOLEAN               SevSnpIsEnabled;
   UINTN                 GhcbBase;
 } MP_CPU_EXCHANGE_INFO;
 
@@ -288,6 +289,7 @@ struct _CPU_MP_DATA {
   BOOLEAN                        WakeUpByInitSipiSipi;
 
   BOOLEAN                        SevEsIsEnabled;
+  BOOLEAN                        SevSnpIsEnabled;
   UINTN                          SevEsAPBuffer;
   UINTN                          SevEsAPResetStackStart;
   CPU_MP_DATA                    *NewCpuMpData;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index bfef1237f452..365c0ff24ebe 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1040,6 +1040,7 @@ FillExchangeInfoData (
   DEBUG ((DEBUG_INFO, "%a: 5-Level Paging = %d\n", gEfiCallerBaseName, ExchangeInfo->Enable5LevelPaging));
 
   ExchangeInfo->SevEsIsEnabled  = CpuMpData->SevEsIsEnabled;
+  ExchangeInfo->SevSnpIsEnabled = CpuMpData->SevSnpIsEnabled;
   ExchangeInfo->GhcbBase        = (UINTN) CpuMpData->GhcbBase;
 
   //
@@ -2033,6 +2034,7 @@ MpInitLibInitialize (
   CpuMpData->CpuInfoInHob     = (UINT64) (UINTN) (CpuMpData->CpuData + MaxLogicalProcessorNumber);
   InitializeSpinLock(&CpuMpData->MpLock);
   CpuMpData->SevEsIsEnabled = ConfidentialComputingGuestHas (CCAttrAmdSevEs);
+  CpuMpData->SevSnpIsEnabled = ConfidentialComputingGuestHas 
+ (CCAttrAmdSevSnp);
   CpuMpData->SevEsAPBuffer  = (UINTN) -1;
   CpuMpData->GhcbBase       = PcdGet64 (PcdGhcbBase);
 
diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
index 2e9368a374a4..01668638f245 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
+++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
@@ -92,6 +92,7 @@ struc MP_CPU_EXCHANGE_INFO
   .ModeHighSegment:              CTYPE_UINT16 1
   .Enable5LevelPaging:           CTYPE_BOOLEAN 1
   .SevEsIsEnabled:               CTYPE_BOOLEAN 1
+  .SevSnpIsEnabled               CTYPE_BOOLEAN 1
   .GhcbBase:                     CTYPE_UINTN 1
 endstruc
 
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 50df802d1fca..018ebe74bf5f 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -194,6 +194,59 @@ LongModeStart:
     mov        rdx, rax
     shr        rdx, 32
     mov        rcx, 0xc0010130
+
+    ;
+    ; If its an SEV-SNP guest then register the GHCB GPA
+    ;
+RegisterGhcbGpa:
+    ;
+    ; Register GHCB GPA when SEV-SNP is enabled
+    ;
+    lea        edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevSnpIsEnabled)]
+    cmp        byte [edi], 1        ; SevSnpIsEnabled
+    jne        RegisterGhcbGpaDone
+
+    ; Save the rdi and rsi to used for later comparison
+    push       rdi
+    push       rsi
+    mov        edi, eax
+    mov        esi, edx
+    or         eax, 18              ; Ghcb registration request
+    wrmsr
+    rep vmmcall
+    rdmsr
+    mov        r12, rax
+    and        r12, 0fffh
+    cmp        r12, 19              ; Ghcb registration response
+    jne        GhcbGpaRegisterFailure
+
+    ; Verify that GPA is not changed
+    and        eax, 0fffff000h
+    cmp        edi, eax
+    jne        GhcbGpaRegisterFailure
+    cmp        esi, edx
+    jne        GhcbGpaRegisterFailure
+    pop        rsi
+    pop        rdi
+    jmp        RegisterGhcbGpaDone
+
+    ;
+    ; Request the guest termination
+    ;
+GhcbGpaRegisterFailure:
+    xor        edx, edx
+    mov        eax, 256             ; GHCB terminate
+    wrmsr
+    rep vmmcall
+
+    ; We should not return from the above terminate request, but if we do
+    ; then enter into the hlt loop.
+DoHltLoop:
+    cli
+    hlt
+    jmp        DoHltLoop
+
+RegisterGhcbGpaDone:
     wrmsr
     jmp        CProcedureInvoke
 
--
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80635): https://edk2.groups.io/g/devel/message/80635
Mute This Topic: https://groups.io/mt/85582715/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-






More information about the edk2-devel-archive mailing list