[edk2-devel] [PATCH v5 3/4] OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall

Ashish Kalra via groups.io posted 4 patches 4 years, 7 months ago
There is a newer version of this series
[edk2-devel] [PATCH v5 3/4] OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall
Posted by Ashish Kalra via groups.io 4 years, 7 months ago
From: Ashish Kalra <ashish.kalra@amd.com>

Mark the SEC GHCB page (that is mapped as unencrypted in
ResetVector code) in the hypervisor page status tracking.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 OvmfPkg/PlatformPei/AmdSev.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index a8bf610022..1ec0de48fe 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -52,6 +52,15 @@ AmdSevEsInitialize (
   PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
   ASSERT_RETURN_ERROR (PcdStatus);
 
+  //
+  // GHCB_BASE setup during reset-vector needs to be marked as
+  // decrypted in the hypervisor page encryption bitmap.
+  //
+  SetMemoryEncDecHypercall3 (FixedPcdGet32 (PcdOvmfSecGhcbBase),
+    EFI_SIZE_TO_PAGES(FixedPcdGet32 (PcdOvmfSecGhcbSize)),
+    KVM_MAP_GPA_RANGE_DECRYPTED
+    );
+
   //
   // Allocate GHCB and per-CPU variable pages.
   //   Since the pages must survive across the UEFI to OS transition
-- 
2.17.1



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


Re: [edk2-devel] [PATCH v5 3/4] OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall
Posted by Lendacky, Thomas via groups.io 4 years, 6 months ago
On 7/8/21 9:08 AM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Mark the SEC GHCB page (that is mapped as unencrypted in
> ResetVector code) in the hypervisor page status tracking.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  OvmfPkg/PlatformPei/AmdSev.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
> index a8bf610022..1ec0de48fe 100644
> --- a/OvmfPkg/PlatformPei/AmdSev.c
> +++ b/OvmfPkg/PlatformPei/AmdSev.c
> @@ -52,6 +52,15 @@ AmdSevEsInitialize (
>    PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
>    ASSERT_RETURN_ERROR (PcdStatus);
>  
> +  //
> +  // GHCB_BASE setup during reset-vector needs to be marked as

s/GHCB_BASE/The SEC Ghcb/

> +  // decrypted in the hypervisor page encryption bitmap.

Is the "hypervisor page encryption bitmap" valid anymore? This gets passed
up to userspace now, right?

You should go through all the patches to be sure you aren't talking about
a bitmap anymore and just state that you're updating the encryption state
with the hypervisor.

> +  //
> +  SetMemoryEncDecHypercall3 (FixedPcdGet32 (PcdOvmfSecGhcbBase),

The first argument needs to be moved down to a line of its own and
indented like the following arguments.

> +    EFI_SIZE_TO_PAGES(FixedPcdGet32 (PcdOvmfSecGhcbSize)),
> +    KVM_MAP_GPA_RANGE_DECRYPTED

Ah, now I see this #define used, but you should be passing a 0 or 1,
right? This happens to evaluate to 0, but it's the wrong way to call this
function.

Thanks,
Tom

> +    );
> +
>    //
>    // Allocate GHCB and per-CPU variable pages.
>    //   Since the pages must survive across the UEFI to OS transition
> 


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


Re: [edk2-devel] [PATCH v5 3/4] OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall
Posted by Ashish Kalra via groups.io 4 years, 6 months ago
Hello Tom,

On Fri, Jul 16, 2021 at 09:22:20AM -0500, Tom Lendacky wrote:
> On 7/8/21 9:08 AM, Ashish Kalra wrote:
> > From: Ashish Kalra <ashish.kalra@amd.com>
> > 
> > Mark the SEC GHCB page (that is mapped as unencrypted in
> > ResetVector code) in the hypervisor page status tracking.
> > 
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> > ---
> >  OvmfPkg/PlatformPei/AmdSev.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
> > index a8bf610022..1ec0de48fe 100644
> > --- a/OvmfPkg/PlatformPei/AmdSev.c
> > +++ b/OvmfPkg/PlatformPei/AmdSev.c
> > @@ -52,6 +52,15 @@ AmdSevEsInitialize (
> >    PcdStatus = PcdSetBoolS (PcdSevEsIsEnabled, TRUE);
> >    ASSERT_RETURN_ERROR (PcdStatus);
> >  
> > +  //
> > +  // GHCB_BASE setup during reset-vector needs to be marked as
> 
> s/GHCB_BASE/The SEC Ghcb/
> 
> > +  // decrypted in the hypervisor page encryption bitmap.
> 
> Is the "hypervisor page encryption bitmap" valid anymore? This gets passed
> up to userspace now, right?
> 
> You should go through all the patches to be sure you aren't talking about
> a bitmap anymore and just state that you're updating the encryption state
> with the hypervisor.
> 

Ok, i will fix that.

> > +  //
> > +  SetMemoryEncDecHypercall3 (FixedPcdGet32 (PcdOvmfSecGhcbBase),
> 
> The first argument needs to be moved down to a line of its own and
> indented like the following arguments.
> 
> > +    EFI_SIZE_TO_PAGES(FixedPcdGet32 (PcdOvmfSecGhcbSize)),
> > +    KVM_MAP_GPA_RANGE_DECRYPTED
> 
> Ah, now I see this #define used, but you should be passing a 0 or 1,
> right? This happens to evaluate to 0, but it's the wrong way to call this
> function.
> 

Ok.

Thanks,
Ashish

> > +    );
> > +
> >    //
> >    // Allocate GHCB and per-CPU variable pages.
> >    //   Since the pages must survive across the UEFI to OS transition
> > 


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