Re: [edk2-devel] [PATCH 11/12] OvmfPkg/PlatformPei: Reserve GHCB backup pages if S3 is supported

["Laszlo Ersek" <lersek@redhat.com>]

On 12/15/20 21:51, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3108
> 
> Protect the GHCB backup pages used by an SEV-ES guest when S3 is
> supported.
> 
> Regarding the lifecycle of the GHCB backup pages:
>   PcdOvmfSecGhcbBackupBase
> 
> (a) when and how it is initialized after first boot of the VM
> 
>   If SEV-ES is enabled, the GHCB backup pages when a nested #VC is
>   received during the SEC phase
>   [OvmfPkg/Library/VmgExitLib/SecVmgExitVcHandler.c].

(1) This sentence appears to miss a verb.

With that fixed:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> 
> (b) how it is protected from memory allocations during DXE
> 
>   If S3 and SEV-ES are enabled, then InitializeRamRegions()
>   [OvmfPkg/PlatformPei/MemDetect.c] protects the ranges with an AcpiNVS
>   memory allocation HOB, in PEI.
> 
>   If S3 is disabled, then these ranges are not protected. PEI switches to
>   the GHCB backup pages in permanent PEI memory and DXE will use these
>   PEI GHCB backup pages, so we don't have to preserve
>   PcdOvmfSecGhcbBackupBase.
> 
> (c) how it is protected from the OS
> 
>   If S3 is enabled, then (b) reserves it from the OS too.
> 
>   If S3 is disabled, then the range needs no protection.
> 
> (d) how it is accessed on the S3 resume path
> 
>   It is rewritten same as in (a), which is fine because (b) reserved it.
> 
> (e) how it is accessed on the warm reset path
> 
>   It is rewritten same as in (a).
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/PlatformPei/PlatformPei.inf | 2 ++
>  OvmfPkg/PlatformPei/MemDetect.c     | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
> index c53be2f4925c..6ef77ba7bb21 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -118,6 +118,8 @@ [FixedPcd]
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
>    gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
>    gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
>  
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> index ffbbef891a11..c08aa2e45a53 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -888,6 +888,11 @@ InitializeRamRegions (
>          (UINT64)(UINTN) PcdGet32 (PcdOvmfSecGhcbSize),
>          EfiACPIMemoryNVS
>          );
> +      BuildMemoryAllocationHob (
> +        (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfSecGhcbBackupBase),
> +        (UINT64)(UINTN) PcdGet32 (PcdOvmfSecGhcbBackupSize),
> +        EfiACPIMemoryNVS
> +        );
>      }
>  #endif
>    }
>