Re: [PATCH V4 06/10] MdeModulePkg VariableInfo: Always consider RT DXE and SMM stats

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: Kubacki, Michael A
> Sent: Tuesday, October 15, 2019 7:30 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo Ersek; Gao, Liming; Kinney,
> Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao, Jiewen
> Subject: [PATCH V4 06/10] MdeModulePkg VariableInfo: Always consider RT
> DXE and SMM stats
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2220
> 
> The current VariableInfo application only checks for variable
> statistics from SMM if the variable information entries are
> not present in the UEFI System Configuration table as published
> by the DXE UEFI variable driver (VariableRuntimeDxe).
> 
> This change first checks for variable information entries in the
> UEFI System Configuration but always checks for entries in SMM
> as well. If the SMM variable driver is not present, an instance of
> EFI_SMM_VARIABLE_PROTOCOL will not be found and the search for
> SMM variable statistics will be aborted (an SW SMI to get variable
> statistics will not be triggered).
> 
> In the case variable statistics are provided by both a Runtime DXE
> driver (e.g. VariableSmmRuntimeDxe) and a SMM driver (VariableSmm),
> this change will clearly identify statistics from each respective
> driver.


You can keep my previous ACK tag in V2 series.

Best Regards,
Hao Wu


> 
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> ---
>  MdeModulePkg/Application/VariableInfo/VariableInfo.c | 37 ++++++++++--
> --------
>  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/MdeModulePkg/Application/VariableInfo/VariableInfo.c
> b/MdeModulePkg/Application/VariableInfo/VariableInfo.c
> index f213471e9a..c04ba18213 100644
> --- a/MdeModulePkg/Application/VariableInfo/VariableInfo.c
> +++ b/MdeModulePkg/Application/VariableInfo/VariableInfo.c
> @@ -3,7 +3,7 @@
>    this utility will print out the statistics information. You can use console
>    redirection to capture the data.
> 
> -  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -126,7 +126,7 @@ PrintInfoFromSmm (
>    ASSERT (CommBuffer != NULL);
>    ZeroMem (CommBuffer, RealCommSize);
> 
> -  Print (L"Non-Volatile SMM Variables:\n");
> +  Print (L"SMM Driver Non-Volatile Variables:\n");
>    do {
>      CommSize = RealCommSize;
>      Status = GetVariableStatisticsData (CommBuffer, &CommSize);
> @@ -155,7 +155,7 @@ PrintInfoFromSmm (
>      }
>    } while (TRUE);
> 
> -  Print (L"Volatile SMM Variables:\n");
> +  Print (L"SMM Driver Volatile Variables:\n");
>    ZeroMem (CommBuffer, RealCommSize);
>    do {
>      CommSize = RealCommSize;
> @@ -207,24 +207,18 @@ UefiMain (
>    IN EFI_SYSTEM_TABLE  *SystemTable
>    )
>  {
> -  EFI_STATUS            Status;
> +  EFI_STATUS            RuntimeDxeStatus;
> +  EFI_STATUS            SmmStatus;
>    VARIABLE_INFO_ENTRY   *VariableInfo;
>    VARIABLE_INFO_ENTRY   *Entry;
> 
> -  Status = EfiGetSystemConfigurationTable (&gEfiVariableGuid, (VOID
> **)&Entry);
> -  if (EFI_ERROR (Status) || (Entry == NULL)) {
> -    Status = EfiGetSystemConfigurationTable
> (&gEfiAuthenticatedVariableGuid, (VOID **)&Entry);
> +  RuntimeDxeStatus = EfiGetSystemConfigurationTable (&gEfiVariableGuid,
> (VOID **) &Entry);
> +  if (EFI_ERROR (RuntimeDxeStatus) || (Entry == NULL)) {
> +    RuntimeDxeStatus = EfiGetSystemConfigurationTable
> (&gEfiAuthenticatedVariableGuid, (VOID **) &Entry);
>    }
> 
> -  if (EFI_ERROR (Status) || (Entry == NULL)) {
> -    Status = PrintInfoFromSmm ();
> -    if (!EFI_ERROR (Status)) {
> -      return Status;
> -    }
> -  }
> -
> -  if (!EFI_ERROR (Status) && (Entry != NULL)) {
> -    Print (L"Non-Volatile EFI Variables:\n");
> +  if (!EFI_ERROR (RuntimeDxeStatus) && (Entry != NULL)) {
> +    Print (L"Runtime DXE Driver Non-Volatile EFI Variables:\n");
>      VariableInfo = Entry;
>      do {
>        if (!VariableInfo->Volatile) {
> @@ -242,7 +236,7 @@ UefiMain (
>        VariableInfo = VariableInfo->Next;
>      } while (VariableInfo != NULL);
> 
> -    Print (L"Volatile EFI Variables:\n");
> +    Print (L"Runtime DXE Driver Volatile EFI Variables:\n");
>      VariableInfo = Entry;
>      do {
>        if (VariableInfo->Volatile) {
> @@ -258,14 +252,19 @@ UefiMain (
>        }
>        VariableInfo = VariableInfo->Next;
>      } while (VariableInfo != NULL);
> +  }
> 
> -  } else {
> +  SmmStatus = PrintInfoFromSmm ();
> +
> +  if (EFI_ERROR (RuntimeDxeStatus) && EFI_ERROR (SmmStatus)) {
>      Print (L"Warning: Variable Dxe/Smm driver doesn't enable the feature of
> statistical information!\n");
>      Print (L"If you want to see this info, please:\n");
>      Print (L"  1. Set PcdVariableCollectStatistics as TRUE\n");
>      Print (L"  2. Rebuild Variable Dxe/Smm driver\n");
>      Print (L"  3. Run \"VariableInfo\" cmd again\n");
> +
> +    return EFI_NOT_FOUND;
>    }
> 
> -  return Status;
> +  return EFI_SUCCESS;
>  }
> --
> 2.16.2.windows.1