[edk2-devel] [PATCH v7 09/31] OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest

Brijesh Singh via groups.io posted 31 patches 4 years, 5 months ago
There is a newer version of this series
[edk2-devel] [PATCH v7 09/31] OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
Posted by Brijesh Singh via groups.io 4 years, 5 months ago
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

The SEV-SNP guest requires that GHCB GPA must be registered before using.
See the GHCB specification section 2.3.2 for more details.

Cc: Michael Roth <michael.roth@amd.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: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 OvmfPkg/Sec/AmdSev.c | 137 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 137 insertions(+)

diff --git a/OvmfPkg/Sec/AmdSev.c b/OvmfPkg/Sec/AmdSev.c
index 7f74e8bfe88e..7d4d7cc8a07c 100644
--- a/OvmfPkg/Sec/AmdSev.c
+++ b/OvmfPkg/Sec/AmdSev.c
@@ -48,6 +48,125 @@ SevEsProtocolFailure (
   CpuDeadLoop ();
 }
 
+/**
+  Determine if SEV-SNP is active.
+
+  @retval TRUE   SEV-SNP is enabled
+  @retval FALSE  SEV-SNP is not enabled
+
+**/
+STATIC
+BOOLEAN
+SevSnpIsEnabled (
+  VOID
+  )
+{
+  MSR_SEV_STATUS_REGISTER           Msr;
+
+  //
+  // Read the SEV_STATUS MSR to determine whether SEV-SNP is active.
+  //
+  Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
+
+  //
+  // Check MSR_0xC0010131 Bit 2 (Sev-Snp Enabled)
+  //
+  if (Msr.Bits.SevSnpBit) {
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+/**
+ Register the GHCB GPA
+
+*/
+STATIC
+VOID
+SevSnpGhcbRegister (
+  UINTN   Address
+  )
+{
+  MSR_SEV_ES_GHCB_REGISTER  Msr;
+  MSR_SEV_ES_GHCB_REGISTER  CurrentMsr;
+  EFI_PHYSICAL_ADDRESS      GuestFrameNumber;
+
+  GuestFrameNumber = Address >> EFI_PAGE_SHIFT;
+
+  //
+  // Save the current MSR Value
+  //
+  CurrentMsr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+  //
+  // Use the GHCB MSR Protocol to request to register the GPA.
+  //
+  Msr.GhcbPhysicalAddress = 0;
+  Msr.GhcbGpaRegister.Function = GHCB_INFO_GHCB_GPA_REGISTER_REQUEST;
+  Msr.GhcbGpaRegister.GuestFrameNumber = GuestFrameNumber;
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
+
+  AsmVmgExit ();
+
+  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+  //
+  // If hypervisor responded with a different GPA than requested then fail.
+  //
+  if ((Msr.GhcbGpaRegister.Function != GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE) ||
+      (Msr.GhcbGpaRegister.GuestFrameNumber != GuestFrameNumber)) {
+    SevEsProtocolFailure (GHCB_TERMINATE_GHCB_GENERAL);
+  }
+
+  //
+  // Restore the MSR
+  //
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, CurrentMsr.GhcbPhysicalAddress);
+}
+
+/**
+ Verify that Hypervisor supports the SNP feature.
+
+ */
+STATIC
+BOOLEAN
+HypervisorSnpFeatureCheck (
+  VOID
+  )
+{
+  MSR_SEV_ES_GHCB_REGISTER  Msr;
+  MSR_SEV_ES_GHCB_REGISTER  CurrentMsr;
+
+  //
+  // Save the current MSR Value
+  //
+  CurrentMsr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+  //
+  // Use the GHCB MSR Protocol to query the hypervisor capabilities
+  //
+  Msr.GhcbPhysicalAddress = 0;
+  Msr.GhcbHypervisorFeatures.Function = GHCB_HYPERVISOR_FEATURES_REQUEST;
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
+
+  AsmVmgExit ();
+
+  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+
+  if ((Msr.GhcbHypervisorFeatures.Function != GHCB_HYPERVISOR_FEATURES_RESPONSE) ||
+      (!(Msr.GhcbHypervisorFeatures.Features & GHCB_HV_FEATURES_SNP))) {
+    return FALSE;
+  }
+
+  //
+  // Restore the MSR
+  //
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, CurrentMsr.GhcbPhysicalAddress);
+
+  return TRUE;
+}
+
 /**
   Validate the SEV-ES/GHCB protocol level.
 
@@ -88,6 +207,24 @@ SevEsProtocolCheck (
     SevEsProtocolFailure (GHCB_TERMINATE_GHCB_PROTOCOL);
   }
 
+  //
+  // We cannot use the MemEncryptSevSnpIsEnabled () because the
+  // ProcessLibraryConstructorList () is not called yet.
+  //
+  if (SevSnpIsEnabled ()) {
+    //
+    // Check if hypervisor supports the SNP feature
+    //
+    if (!HypervisorSnpFeatureCheck ()) {
+      SevEsProtocolFailure (GHCB_TERMINATE_GHCB_PROTOCOL);
+    }
+
+    //
+    // SEV-SNP guest requires that GHCB GPA must be registered before using it.
+    //
+    SevSnpGhcbRegister (FixedPcdGet32 (PcdOvmfSecGhcbBase));
+  }
+
   //
   // SEV-ES protocol checking succeeded, set the initial GHCB address
   //
-- 
2.17.1



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


Re: [edk2-devel] [PATCH v7 09/31] OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
Posted by Erdem Aktas via groups.io 4 years, 4 months ago
On Mon, Sep 13, 2021 at 9:20 PM Brijesh Singh <brijesh.singh@amd.com> wrote:
> +*/
> +STATIC
> +VOID
> +SevSnpGhcbRegister (
> +  UINTN   Address
> +  )
> +{
> +  MSR_SEV_ES_GHCB_REGISTER  Msr;
> +  MSR_SEV_ES_GHCB_REGISTER  CurrentMsr;
> +  EFI_PHYSICAL_ADDRESS      GuestFrameNumber;
> +
> +  GuestFrameNumber = Address >> EFI_PAGE_SHIFT;
> +
> +  //
> +  // Save the current MSR Value
> +  //
> +  CurrentMsr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);

We are backing the current MSR value but when was it initialized
before ? Also is not this function supposed to set the Address as the
GHCB address? If it is, do we care about the old value?


> +  // Restore the MSR
> +  //
> +  AsmWriteMsr64 (MSR_SEV_ES_GHCB, CurrentMsr.GhcbPhysicalAddress);

Why are we restoring the old value? I may have misunderstood but I
thought this function will set Address as the new GHCB address?

Thanks
-Erdem


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


Re: [edk2-devel] [PATCH v7 09/31] OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
Posted by Brijesh Singh via groups.io 4 years, 4 months ago

On 9/15/21 12:08 PM, Erdem Aktas wrote:
> On Mon, Sep 13, 2021 at 9:20 PM Brijesh Singh <brijesh.singh@amd.com> wrote:
>> +*/
>> +STATIC
>> +VOID
>> +SevSnpGhcbRegister (
>> +  UINTN   Address
>> +  )
>> +{
>> +  MSR_SEV_ES_GHCB_REGISTER  Msr;
>> +  MSR_SEV_ES_GHCB_REGISTER  CurrentMsr;
>> +  EFI_PHYSICAL_ADDRESS      GuestFrameNumber;
>> +
>> +  GuestFrameNumber = Address >> EFI_PAGE_SHIFT;
>> +
>> +  //
>> +  // Save the current MSR Value
>> +  //
>> +  CurrentMsr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
> 
> We are backing the current MSR value but when was it initialized
> before ? Also is not this function supposed to set the Address as the
> GHCB address? If it is, do we care about the old value?
> 

Good point, there is no reason to read and restore the old GHCB, I will 
remove it in next version. The function does not set this as a GHCB 
address, it send request to hypervisor saying that it would like to use 
this address. If hypervisor is not okay with the address then it may 
recommend something else. We don't support working with the hypervisor 
preferred address. Setting the GHCB address code is common between Snp 
and Es but checking with hypervisor whether its okay to use is new in 
the GHCBv2 and is SNP specific.


> 
>> +  // Restore the MSR
>> +  //
>> +  AsmWriteMsr64 (MSR_SEV_ES_GHCB, CurrentMsr.GhcbPhysicalAddress);
> 
> Why are we restoring the old value? I may have misunderstood but I
> thought this function will set Address as the new GHCB address?
> 
> Thanks
> -Erdem
> 


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


Re: [edk2-devel] [PATCH v7 09/31] OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
Posted by Gerd Hoffmann 4 years, 4 months ago
  Hi,

> Good point, there is no reason to read and restore the old GHCB, I will
> remove it in next version. The function does not set this as a GHCB address,
> it send request to hypervisor saying that it would like to use this address.
> If hypervisor is not okay with the address then it may recommend something
> else. We don't support working with the hypervisor preferred address.
> Setting the GHCB address code is common between Snp and Es but checking with
> hypervisor whether its okay to use is new in the GHCBv2 and is SNP specific.

A comment explaining those GHCBv1 vs. GHCBv2 differences would be great.

thanks,
  Gerd



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


Re: [edk2-devel] [PATCH v7 09/31] OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
Posted by Brijesh Singh via groups.io 4 years, 4 months ago
On 9/16/21 3:30 AM, Gerd Hoffmann wrote:
>   Hi,
>
>> Good point, there is no reason to read and restore the old GHCB, I will
>> remove it in next version. The function does not set this as a GHCB address,
>> it send request to hypervisor saying that it would like to use this address.
>> If hypervisor is not okay with the address then it may recommend something
>> else. We don't support working with the hypervisor preferred address.
>> Setting the GHCB address code is common between Snp and Es but checking with
>> hypervisor whether its okay to use is new in the GHCBv2 and is SNP specific.
> A comment explaining those GHCBv1 vs. GHCBv2 differences would be great.

Sure, I will add comment.

thanks


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