Re: [edk2] [RFC v4 05/13] OvmfPkg/PlatformPei: Set memory encryption PCD when SEV is enabled

[Brijesh Singh]

On 05/11/2017 09:37 AM, Laszlo Ersek wrote:


Please replace the last paragraph in the commit message with the
following (mostly based on
):


The patch sets the memory encryption PCD. The PCD is consumed by the
following edk2 modules, which manipulate page tables:

- PEI phase modules: CapsulePei, DxeIplPeim, S3Resume2Pei.

  CapsulePei is not used by OVMF. DxeIplPeim consumes the PCD at the
  end of the PEI phase, when it builds the initial page tables for the
  DXE core / DXE phase. S3Resume2Pei does not consume the PCD in its
  entry point function, only when DxeIplPeim branches to the S3 resume
  path at the end of the PEI phase, and calls S3Resume2Pei's
  EFI_PEI_S3_RESUME2_PPI.S3RestoreConfig2() member function.

  Therefore it is safe to set the PCD for these modules in PlatformPei.

- DXE phase modules: BootScriptExecutorDxe, CpuDxe, PiSmmCpuDxeSmm.

  They are all dispatched after the PEI phase, so setting the PCD for
  them in PlatformPei is safe. (BootScriptExecutorDxe is launched "for
  real" in the PEI phase during S3 resume, but it caches the PCD into a
  static variable when its entry point is originally invoked in DXE.)




I will update the patch description, thanks for the write up.

-Brijesh
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC v4 05/13] OvmfPkg/PlatformPei: Set memory encryption PCD when SEV is enabled

[Laszlo Ersek]

On 05/11/17 00:09, Brijesh Singh wrote:
> Secure Encrypted Virtualization (SEV) guest VMs have the concept of
> private and shared memory. Private memory is encrypted with the
> guest-specific key, while shared memory may be encrypted with hypervisor
> key.  Certain types of memory (namely instruction pages and guest page
> tables) are always treated as private memory by the hardware.
> For data memory, SEV guest VMs can choose which pages they would like
> to be private. The choice is done using the standard CPU page tables
> using the C-bit. When building the initial page table we mark all the
> memory as private.
> 
> The patch sets the memory encryption PCD, the PCD is used by DxeCore
> when building the initial page table.

Please replace the last paragraph in the commit message with the
following (mostly based on
):


The patch sets the memory encryption PCD. The PCD is consumed by the
following edk2 modules, which manipulate page tables:

- PEI phase modules: CapsulePei, DxeIplPeim, S3Resume2Pei.

  CapsulePei is not used by OVMF. DxeIplPeim consumes the PCD at the
  end of the PEI phase, when it builds the initial page tables for the
  DXE core / DXE phase. S3Resume2Pei does not consume the PCD in its
  entry point function, only when DxeIplPeim branches to the S3 resume
  path at the end of the PEI phase, and calls S3Resume2Pei's
  EFI_PEI_S3_RESUME2_PPI.S3RestoreConfig2() member function.

  Therefore it is safe to set the PCD for these modules in PlatformPei.

- DXE phase modules: BootScriptExecutorDxe, CpuDxe, PiSmmCpuDxeSmm.

  They are all dispatched after the PEI phase, so setting the PCD for
  them in PlatformPei is safe. (BootScriptExecutorDxe is launched "for
  real" in the PEI phase during S3 resume, but it caches the PCD into a
  static variable when its entry point is originally invoked in DXE.)


With that update,

Reviewed-by: Laszlo Ersek 

Thanks,
Laszlo


> 
> 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Brijesh Singh 
> ---
>  OvmfPkg/OvmfPkgIa32.dsc |  3 +
>  OvmfPkg/OvmfPkgIa32X64.dsc  |  3 +
>  OvmfPkg/OvmfPkgX64.dsc  |  3 +
>  OvmfPkg/PlatformPei/PlatformPei.inf |  3 +
>  OvmfPkg/PlatformPei/Platform.h  |  5 ++
>  OvmfPkg/PlatformPei/AmdSev.c| 62 
>  OvmfPkg/PlatformPei/Platform.c  |  1 +
>  7 files changed, 80 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 265f2d2b6d2f..139d54aa010e 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -530,6 +530,9 @@ [PcdsDynamicDefault]
>gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
>gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|5
>  
> +  # Set memory encryption mask
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0
> +
>  !if $(SMM_REQUIRE) == TRUE
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|10
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 3b50a2dd2d2f..9403f76ce862 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -538,6 +538,9 @@ [PcdsDynamicDefault]
>gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
>gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|5
>  
> +  # Set memory encryption mask
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0
> +
>  !if $(SMM_REQUIRE) == TRUE
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|10
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 82c73331aa9f..e137143f7afa 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -537,6 +537,9 @@ [PcdsDynamicDefault]
>gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
>gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|5
>  
> +  # Set memory encryption mask
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0
> +
>  !if $(SMM_REQUIRE) == TRUE
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|10
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
> b/OvmfPkg/PlatformPei/PlatformPei.inf
> index 53c6dd445a0e..a9a7a76c7325 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -29,6 +29,7 @@ [Defines]
>  #
>  
>  [Sources]
> +  AmdSev.c
>Cmos.c
>FeatureControl.c
>Fv.c
> @@ -60,6 +61,7 @@ [LibraryClasses]
>QemuFwCfgLib
>QemuFwCfgS3Lib
>MtrrLib
> +  MemEncryptSevLib
>PcdLib
>  
>  [Pcd]
> @@ -94,6 +96,7 @@ [Pcd]
>