Re: [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask

[Laszlo Ersek <lersek@redhat.com>]

On 02/07/17 20:53, Leo Duran wrote:
> From: Brijesh Singh <brijesh.singh@amd.com>
> 
> This dynamic PCD holds the address mask for page table entries when memory
> encryption is enabled on AMD processors supporting the Secure Encrypted
> Virtualization (SEV) feature.
> 
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leo Duran <leo.duran@amd.com>
> ---
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          |  5 ++++-
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 18 ++++++++++--------
>  MdeModulePkg/MdeModulePkg.dec                    |  8 ++++++++
>  3 files changed, 22 insertions(+), 9 deletions(-)

[snip]

> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index 273cd7e..207384f 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -6,6 +6,8 @@
>  # Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
>  # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
>  # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +#
>  # This program and the accompanying materials are licensed and made available under
>  # the terms and conditions of the BSD License that accompanies this distribution.
>  # The full text of the license may be found at
> @@ -1738,5 +1740,11 @@ [PcdsDynamic, PcdsDynamicEx]
>    # @Prompt If there is any test key used by the platform.
>    gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed|FALSE|BOOLEAN|0x00030003
>  
> +  ## This dynamic PCD holds the address mask for page table entries when memory encryption is
> +  #  enabled on AMD processors supporting the Secure Encrypted Virtualization (SEV) feature.
> +  #  This mask should be applied when creating 1:1 virtual to physical mapping tables.
> +  #
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0|UINT64|0x00030004
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>    MdeModulePkgExtra.uni
> 

I think this PCD should be added to the following section of the DEC file:

  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]

Although OVMF will use the PCD as a dynamic one, I see no reason why the
DEC file should prohibit setting the PCD as a fixed one.

Looking at other PCDs in the

  [PcdsDynamic, PcdsDynamicEx]

section, most of those seem to stand for data that are impossible to
determine at build time. Is the new PCD impossible to determine at build
time, for any platform at all?

... Of course, if it can be proved that any given platform will either
(a) set this new PCD dynamically, or
(b) not even *include* the PCD -- with a dynamic default value -- in
    its platform DSC file,
then this patch can work too.

Thanks
Laszlo