Re: [Patch 3/4] QuarkSocPkg/SmmAccessDxe: Set region to UC on SMRAM close

["Steele, Kelly" <kelly.steele@intel.com>]

Reviewed-by: Kelly Steele <kelly.steele@intel.com>




> -----Original Message-----
> From: Kinney, Michael D
> Sent: April 25, 2019 10:54
> To: devel@edk2.groups.io
> Cc: Steele, Kelly <kelly.steele@intel.com>
> Subject: [Patch 3/4] QuarkSocPkg/SmmAccessDxe: Set region to UC on SMRAM
> close
> 
> The following commit removed the unconditional UC setting
> just prior to closing the SMRAM region.  This is a correct
> change for most platforms.
> 
> https://github.com/tianocore/edk2/commit/bfc87aa78e77ed15b09d1b4499c5e
> ab63e8842bb
> 
> The Quark platforms still require this UC setting, so move
> the UC setting into the Quark specific SMM Access Protocol
> when the Close() service is called.
> 
> Cc: Kelly Steele <kelly.steele@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  .../Smm/Dxe/SmmAccessDxe/SmmAccess.inf         |  3 ++-
>  .../Smm/Dxe/SmmAccessDxe/SmmAccessDriver.c     | 18 +++++++++++++++++-
>  .../Smm/Dxe/SmmAccessDxe/SmmAccessDriver.h     |  3 ++-
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git
> a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccess.inf
> b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccess.inf
> index db916f686a..405e9eb7fd 100644
> ---
> a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccess.inf
> +++
> b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccess.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  # Component description file for SmmAccess module
>  #
> -# Copyright (c) 2013-2015 Intel Corporation.
> +# Copyright (c) 2013-2019 Intel Corporation.
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -34,6 +34,7 @@ [LibraryClasses]
>    S3BootScriptLib
>    UefiDriverEntryPoint
>    UefiBootServicesTableLib
> +  DxeServicesTableLib
>    PcdLib
>    SmmLib
> 
> diff --git
> a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDrive
> r.c
> b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDrive
> r.c
> index 6148dea1b4..205f51ddb0 100644
> ---
> a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDrive
> r.c
> +++
> b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDrive
> r.c
> @@ -2,7 +2,7 @@
>  This is the driver that publishes the SMM Access Protocol
>  instance for the Tylersburg chipset.
> 
> -Copyright (c) 2013-2015 Intel Corporation.
> +Copyright (c) 2013-2019 Intel Corporation.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -221,6 +221,7 @@ Returns:
> 
>  --*/
>  {
> +  EFI_STATUS              Status;
>    SMM_ACCESS_PRIVATE_DATA *SmmAccess;
>    BOOLEAN                 OpenState;
>    UINTN                   Index;
> @@ -239,6 +240,21 @@ Returns:
>      return EFI_DEVICE_ERROR;
>    }
> 
> +  //
> +  // Reset SMRAM cacheability to UC
> +  //
> +  for (Index = 0; Index < mSmmAccess.NumberRegions; Index++) {
> +    DEBUG ((DEBUG_INFO, "SmmAccess->Close: Set to UC Base=%016lx
> Size=%016lx\n", SmmAccess->SmramDesc[Index].CpuStart, SmmAccess-
> >SmramDesc[Index].PhysicalSize));
> +    Status = gDS->SetMemorySpaceAttributes(
> +                    SmmAccess->SmramDesc[Index].CpuStart,
> +                    SmmAccess->SmramDesc[Index].PhysicalSize,
> +                    EFI_MEMORY_UC
> +                    );
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_WARN, "SmmAccess: Failed to reset SMRAM window to
> EFI_MEMORY_UC\n"));
> +    }
> +  }
> +
>    //
>    // Close TSEG
>    //
> diff --git
> a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDrive
> r.h
> b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDrive
> r.h
> index 80f73ba0e3..aca169d3e2 100644
> ---
> a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDrive
> r.h
> +++
> b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDrive
> r.h
> @@ -3,7 +3,7 @@ Header file for SMM Access Driver.
> 
>  This file includes package header files, library classes and protocol, PPI & GUID
> definitions.
> 
> -Copyright (c) 2013-2015 Intel Corporation.
> +Copyright (c) 2013-2019 Intel Corporation.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
> @@ -21,6 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/UefiDriverEntryPoint.h>
>  #include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DxeServicesTableLib.h>
>  #include <Library/PcdLib.h>
> 
>  //
> --
> 2.21.0.windows.1