[edk2-devel] [PATCH v12 26/32] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled

Brijesh Singh via groups.io posted 32 patches 4 years, 1 month ago
There is a newer version of this series
[edk2-devel] [PATCH v12 26/32] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled
Posted by Brijesh Singh via groups.io 4 years, 1 month ago
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@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.h         |  2 +
 UefiCpuPkg/Library/MpInitLib/MpLib.c         |  2 +
 UefiCpuPkg/Library/MpInitLib/MpEqu.inc       |  1 +
 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 54 ++++++++++++++++++++
 4 files changed, 59 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 2107f3f705a2..45bc1de23e3c 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -222,6 +222,7 @@ typedef struct {
   //
   BOOLEAN               Enable5LevelPaging;
   BOOLEAN               SevEsIsEnabled;
+  BOOLEAN               SevSnpIsEnabled;
   UINTN                 GhcbBase;
 } MP_CPU_EXCHANGE_INFO;
 
@@ -291,6 +292,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 9109607c87a9..ab838cbc0ff6 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -889,6 +889,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;
 
   //
@@ -1817,6 +1818,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/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
index 0ccafe25eca4..0034920b2f6b 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
@@ -15,6 +15,57 @@
 
 %define SIZE_4KB    0x1000
 
+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:
+    OneTimeCallRet    RegisterGhcbGpa
+
 ;
 ; The function checks whether SEV-ES is enabled, if enabled
 ; then setup the GHCB page.
@@ -39,6 +90,9 @@ SevEsSetupGhcb:
     mov        rdx, rax
     shr        rdx, 32
     mov        rcx, 0xc0010130
+
+    OneTimeCall RegisterGhcbGpa
+
     wrmsr
 
 SevEsSetupGhcbExit:
-- 
2.25.1



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


Re: [edk2-devel] [PATCH v12 26/32] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled
Posted by Ni, Ray 4 years, 1 month ago
1 comment:

Can you please group the SevEsIsEnabled/SevSnpIsEnabled to a "2 boolean" struct or
just one UINT8 field "SevEsEnable"?

Through this way, MpLib.c can know less knowledge of SEV-ES.
(I appreciate your effort to group the SEV-ES logic to separate files😊)


> 
>    BOOLEAN                        SevEsIsEnabled;
> +  BOOLEAN                        SevSnpIsEnabled;
>    UINTN                          SevEsAPBuffer;
>    UINTN                          SevEsAPResetStackStart;

> 
>    ExchangeInfo->SevEsIsEnabled  = CpuMpData->SevEsIsEnabled;
> +  ExchangeInfo->SevSnpIsEnabled = CpuMpData->SevSnpIsEnabled;
>    ExchangeInfo->GhcbBase        = (UINTN) CpuMpData->GhcbBase;
> 

>    InitializeSpinLock(&CpuMpData->MpLock);
>    CpuMpData->SevEsIsEnabled = ConfidentialComputingGuestHas (CCAttrAmdSevEs);
> +  CpuMpData->SevSnpIsEnabled = ConfidentialComputingGuestHas (CCAttrAmdSevSnp);
>    CpuMpData->SevEsAPBuffer  = (UINTN) -1;

>    .Enable5LevelPaging:           CTYPE_BOOLEAN 1
>    .SevEsIsEnabled:               CTYPE_BOOLEAN 1
> +  .SevSnpIsEnabled               CTYPE_BOOLEAN 1
>    .GhcbBase:                     CTYPE_UINTN 1




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


Re: [edk2-devel] [PATCH v12 26/32] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled
Posted by Brijesh Singh via groups.io 4 years, 1 month ago
Hi Ray,

Thanks you for all your comments.

On 11/11/21 7:48 PM, Ni, Ray wrote:
> 1 comment:
> 
> Can you please group the SevEsIsEnabled/SevSnpIsEnabled to a "2 boolean" struct or
> just one UINT8 field "SevEsEnable"?
> 

I think using the SevEsEnabled will create a bit more confusion. I can 
certainly follow up patch to combining the fields in structure after 
this code is merged. I am thinking is we need is actually pass the full 
CCAttribute in the CpuMetaData, use that to determine the type of the 
guest. That will require me looking at Min's TDX series and see what I 
can do to come up with an approach that works for all CC types and keep 
the code separate.

With that said, if I can get your Ack on what we have then it will be great.

thanks

> Through this way, MpLib.c can know less knowledge of SEV-ES.
> (I appreciate your effort to group the SEV-ES logic to separate files😊)
> 
> 
>>
>>     BOOLEAN                        SevEsIsEnabled;
>> +  BOOLEAN                        SevSnpIsEnabled;
>>     UINTN                          SevEsAPBuffer;
>>     UINTN                          SevEsAPResetStackStart;
> 
>>
>>     ExchangeInfo->SevEsIsEnabled  = CpuMpData->SevEsIsEnabled;
>> +  ExchangeInfo->SevSnpIsEnabled = CpuMpData->SevSnpIsEnabled;
>>     ExchangeInfo->GhcbBase        = (UINTN) CpuMpData->GhcbBase;
>>
> 
>>     InitializeSpinLock(&CpuMpData->MpLock);
>>     CpuMpData->SevEsIsEnabled = ConfidentialComputingGuestHas (CCAttrAmdSevEs);
>> +  CpuMpData->SevSnpIsEnabled = ConfidentialComputingGuestHas (CCAttrAmdSevSnp);
>>     CpuMpData->SevEsAPBuffer  = (UINTN) -1;
> 
>>     .Enable5LevelPaging:           CTYPE_BOOLEAN 1
>>     .SevEsIsEnabled:               CTYPE_BOOLEAN 1
>> +  .SevSnpIsEnabled               CTYPE_BOOLEAN 1
>>     .GhcbBase:                     CTYPE_UINTN 1
> 
> 


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


Re: [edk2-devel] [PATCH v12 26/32] UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is enabled
Posted by Ni, Ray 4 years ago
> I think using the SevEsEnabled will create a bit more confusion. I can
> certainly follow up patch to combining the fields in structure after
> this code is merged. 

With what you said, Acked-by: Ray Ni <ray.ni@Intel.com>



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