Re: [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/MtrrLib: Remove unnecessary API MtrrGetVariableMtrr()

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

On 07/13/20 10:12, Ni, Ray wrote:
> MtrrGetVariableMtrr() returns all the variable MTRR settings.
> But in fact MtrrGetAllMtrrs() and
> MtrrGetMemoryAttributeInVariableMtrr() are used by callers to get the
> MTRR settings. The former one returns both the fixed and variable
> MTRR settings.
> 
> The patch removes the necessary API MtrrGetVariableMtrr() to simplify

(1) I think we should replace "necessary" with "unnecessary" in the
commit message.

with that fixed:

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


> the MtrrLib API.
> 
> There is no code in edk2 and edk2-platforms repo that calls
> MtrrGetVariableMtrr().
> 
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> ---
>  UefiCpuPkg/Include/Library/MtrrLib.h | 17 +----------------
>  UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 27 +--------------------------
>  2 files changed, 2 insertions(+), 42 deletions(-)
> 
> diff --git a/UefiCpuPkg/Include/Library/MtrrLib.h b/UefiCpuPkg/Include/Library/MtrrLib.h
> index 95ffbc8bf1..cfe3c0ab59 100644
> --- a/UefiCpuPkg/Include/Library/MtrrLib.h
> +++ b/UefiCpuPkg/Include/Library/MtrrLib.h
> @@ -1,7 +1,7 @@
>  /** @file
>    MTRR setting library
>  
> -  Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2008 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -185,21 +185,6 @@ MtrrGetMemoryAttribute (
>    );
>  
>  
> -/**
> -  This function will get the raw value in variable MTRRs
> -
> -  @param[out]  VariableSettings   A buffer to hold variable MTRRs content.
> -
> -  @return The buffer point to MTRR_VARIABLE_SETTINGS in which holds the content of the variable MTRR
> -
> -**/
> -MTRR_VARIABLE_SETTINGS*
> -EFIAPI
> -MtrrGetVariableMtrr (
> -  OUT MTRR_VARIABLE_SETTINGS         *VariableSettings
> -  );
> -
> -
>  /**
>    This function sets variable MTRRs
>  
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index dfa848022b..f4a10edc87 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -5,7 +5,7 @@
>      Most of services in this library instance are suggested to be invoked by BSP only,
>      except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting to APs.
>  
> -  Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2008 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -456,31 +456,6 @@ MtrrGetVariableMtrrWorker (
>    return  VariableSettings;
>  }
>  
> -/**
> -  This function will get the raw value in variable MTRRs
> -
> -  @param[out]  VariableSettings   A buffer to hold variable MTRRs content.
> -
> -  @return The VariableSettings input pointer
> -
> -**/
> -MTRR_VARIABLE_SETTINGS*
> -EFIAPI
> -MtrrGetVariableMtrr (
> -  OUT MTRR_VARIABLE_SETTINGS         *VariableSettings
> -  )
> -{
> -  if (!IsMtrrSupported ()) {
> -    return VariableSettings;
> -  }
> -
> -  return MtrrGetVariableMtrrWorker (
> -           NULL,
> -           GetVariableMtrrCountWorker (),
> -           VariableSettings
> -           );
> -}
> -
>  /**
>    Programs fixed MTRRs registers.
>  
>