Re: [PATCH 3/4] OvmfPkg/PlatformPei: extract memory type info defaults to PCDs

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 5/8/20 2:16 PM, Laszlo Ersek wrote:
> Some OvmfPkg modules already depend on "EmbeddedPkg.dec"; thus, replace
> the open-coded memory type info defaults in the source code with the
> EmbeddedPkg PCDs that stand for the same purpose. Consequently, platform
> builders can override these values with the "--pcd" option of "build",
> without source code updates.
> 
> While at it, sort the memory type names alphabetically.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2706
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   OvmfPkg/OvmfPkgIa32.dsc             | 12 +++++++++++
>   OvmfPkg/OvmfPkgIa32X64.dsc          | 13 ++++++++++++
>   OvmfPkg/OvmfPkgX64.dsc              | 12 +++++++++++
>   OvmfPkg/PlatformPei/PlatformPei.inf |  6 ++++++
>   OvmfPkg/PlatformPei/MemTypeInfo.c   | 21 +++++++++-----------
>   5 files changed, 52 insertions(+), 12 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 7c8b51f43b66..a1981f8085a6 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -532,6 +532,18 @@ [PcdsFixedAtBuild]
>     gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
>   !endif
>   
> +  #
> +  # The NumberOfPages values below are ad-hoc. They are updated sporadically at
> +  # best (please refer to git-blame for past updates). The values capture a set
> +  # of BIN hints that made sense at a particular time, for some (now likely
> +  # unknown) workloads / boot paths.
> +  #
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0x4
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0x8
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0x4
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|0x30
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|0x24
> +
>     #
>     # Network Pcds
>     #
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index a0596c44168c..c1950e6aa915 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -536,6 +536,19 @@ [PcdsFixedAtBuild]
>     gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
>   !endif
>   
> +[PcdsFixedAtBuild.IA32]
> +  #
> +  # The NumberOfPages values below are ad-hoc. They are updated sporadically at
> +  # best (please refer to git-blame for past updates). The values capture a set
> +  # of BIN hints that made sense at a particular time, for some (now likely
> +  # unknown) workloads / boot paths.
> +  #
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0x4
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0x8
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0x4
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|0x30
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|0x24
> +
>   [PcdsFixedAtBuild.X64]
>     #
>     # Network Pcds
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 2e764b6b7233..bc7e15f749ca 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -536,6 +536,18 @@ [PcdsFixedAtBuild]
>     gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
>   !endif
>   
> +  #
> +  # The NumberOfPages values below are ad-hoc. They are updated sporadically at
> +  # best (please refer to git-blame for past updates). The values capture a set
> +  # of BIN hints that made sense at a particular time, for some (now likely
> +  # unknown) workloads / boot paths.
> +  #
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0x4
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0x8
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0x4
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|0x30
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|0x24
> +
>     #
>     # Network Pcds
>     #
> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
> index e72ef7963d97..ff397b3ee9d7 100644
> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
> @@ -37,6 +37,7 @@ [Sources]
>     Xen.h
>   
>   [Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
>     MdePkg/MdePkg.dec
>     MdeModulePkg/MdeModulePkg.dec
>     SecurityPkg/SecurityPkg.dec
> @@ -105,6 +106,11 @@ [Pcd]
>   
>   [FixedPcd]
>     gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode
> +  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
>   
>   [FeaturePcd]
>     gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable
> diff --git a/OvmfPkg/PlatformPei/MemTypeInfo.c b/OvmfPkg/PlatformPei/MemTypeInfo.c
> index d287fb9d7df0..f3ce2b6865e1 100644
> --- a/OvmfPkg/PlatformPei/MemTypeInfo.c
> +++ b/OvmfPkg/PlatformPei/MemTypeInfo.c
> @@ -17,19 +17,16 @@
>   
>   #include "Platform.h"
>   
> -//
> -// The NumberOfPages values below are ad-hoc. They are updated sporadically at
> -// best (please refer to git-blame for past updates). The values capture a set
> -// of BIN hints that made sense at a particular time, for some (now likely
> -// unknown) workloads / boot paths.
> -//
> +#define MEMORY_TYPE_INFO_DEFAULT(Type) \
> +  { Type, FixedPcdGet32 (PcdMemoryType ## Type) }
> +
>   STATIC EFI_MEMORY_TYPE_INFORMATION mMemoryTypeInformation[] = {
> -  { EfiACPIMemoryNVS,       0x004 },
> -  { EfiACPIReclaimMemory,   0x008 },
> -  { EfiReservedMemoryType,  0x004 },
> -  { EfiRuntimeServicesData, 0x024 },
> -  { EfiRuntimeServicesCode, 0x030 },
> -  { EfiMaxMemoryType,       0x000 }
> +  MEMORY_TYPE_INFO_DEFAULT (EfiACPIMemoryNVS),
> +  MEMORY_TYPE_INFO_DEFAULT (EfiACPIReclaimMemory),
> +  MEMORY_TYPE_INFO_DEFAULT (EfiReservedMemoryType),
> +  MEMORY_TYPE_INFO_DEFAULT (EfiRuntimeServicesCode),
> +  MEMORY_TYPE_INFO_DEFAULT (EfiRuntimeServicesData),
> +  { EfiMaxMemoryType, 0 }
>   };
>   
>   STATIC
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>