Re: [edk2-devel] [PATCH 2/4] PrmPkg: Use new API to replace MtrrLibInitializeMtrrMask

["Michael Kubacki" <mikuback@linux.microsoft.com>]

Minor comment in code below.

Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>

On 6/7/2023 11:06 PM, Zhiguang Liu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3394
> 
> The function MtrrLibInitializeMtrrMask is a private function
> in MtrrLib.c from UefiCpuPkg, and it can be replace with new
> API GetMaxPlatformAddressBits.
> 
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>   .../PrmSampleHardwareAccessModule.c            | 18 ++----------------
>   .../PrmSampleHardwareAccessModule.inf          |  1 +
>   2 files changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.c b/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.c
> index 1a1e735029..398497c3a9 100644
> --- a/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.c
> +++ b/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.c
> @@ -13,6 +13,7 @@
>   #include <Library/BaseLib.h>
>   #include <Library/MtrrLib.h>
>   #include <Library/UefiLib.h>
> +#include <Library/CpuLib.h>
>   
>   #include <Register/Intel/ArchitecturalMsr.h>
>   #include <Register/Intel/Cpuid.h>
> @@ -37,21 +38,6 @@
>   //
>   extern CONST CHAR8  *mMtrrMemoryCacheTypeShortName[];
>   
> -/**
> -  Initializes the valid bits mask and valid address mask for MTRRs.
> -
> -  This function initializes the valid bits mask and valid address mask for MTRRs.
> -
> -  @param[out]  MtrrValidBitsMask     The mask for the valid bit of the MTRR
> -  @param[out]  MtrrValidAddressMask  The valid address mask for the MTRR
> -
> -**/
> -VOID
> -MtrrLibInitializeMtrrMask (
> -  OUT UINT64  *MtrrValidBitsMask,
> -  OUT UINT64  *MtrrValidAddressMask
> -  );
> -
>   /**
>     Convert variable MTRRs to a RAW MTRR_MEMORY_RANGE array.
>     One MTRR_MEMORY_RANGE element is created for each MTRR setting.
> @@ -151,7 +137,7 @@ AccessAllMtrrs (
>     MtrrGetAllMtrrs (&LocalMtrrs);
>     Mtrrs = &LocalMtrrs;
>   
> -  MtrrLibInitializeMtrrMask (&MtrrValidBitsMask, &MtrrValidAddressMask);
> +  GetMaxPlatformAddressBits (&MtrrValidBitsMask, &MtrrValidAddressMask);
>     Ranges[0].BaseAddress = 0;
>     Ranges[0].Length      = MtrrValidBitsMask + 1;
>     Ranges[0].Type        = MtrrGetDefaultMemoryType ();
> diff --git a/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf b/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf
> index 46d4a88185..b15da817c1 100644
> --- a/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf
> +++ b/PrmPkg/Samples/PrmSampleHardwareAccessModule/PrmSampleHardwareAccessModule.inf
> @@ -34,6 +34,7 @@
>     MtrrLib
>     UefiDriverEntryPoint
>     UefiLib
> +  CpuLib

Since the library class list was already sorted, can you please sort 
CpuLib within the list?

>   
>   [Depex]
>     TRUE