Re: [PATCH v2] MinPlatformPkg: Support TCO base locked by FSP

["Kubacki, Michael A" <michael.a.kubacki@intel.com>]

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

> -----Original Message-----
> From: Chiu, Chasel
> Sent: Wednesday, January 16, 2019 11:16 PM
> To: edk2-devel@lists.01.org
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
> Subject: [PATCH v2] MinPlatformPkg: Support TCO base locked by FSP
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1457
> 
> Per security recommendation TCO Base should be initialized and locked by
> FSP and MinPlatform should support both TCO Base locked and not locked
> scenarios.
> 
> Cc: Michael A Kubacki <michael.a.kubacki@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  Silicon/Intel/KabylakeSiliconPkg/Library/SiliconInitLib/SiliconInitPreMem.c
> |  8 +++++---
> 
> Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiDxeSmmPchCycleDecodingLi
> b/PchCycleDecodingLib.c | 48
> ++++++++++++++++++++++++++++++++++++++++--------
>  Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/PchCycleDecodingLib.h
> | 18 +++++++++++++++++-
>  3 files changed, 62 insertions(+), 12 deletions(-)
> 
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/Library/SiliconInitLib/SiliconInitPreMem.c
> b/Silicon/Intel/KabylakeSiliconPkg/Library/SiliconInitLib/SiliconInitPreMem.c
> index 616584ffe7..bb21872e1e 100644
> ---
> a/Silicon/Intel/KabylakeSiliconPkg/Library/SiliconInitLib/SiliconInitPreMem.c
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Library/SiliconInitLib/SiliconIni
> +++ tPreMem.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Source code file for Platform Init Pre-Memory PEI module
> 
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials are licensed and made
> available under  the terms and conditions of the BSD License that
> accompanies this distribution.
>  The full text of the license may be found at @@ -80,9 +80,11 @@
> EarlySiliconInit (
>    PchPwrmBaseSet (PCH_PWRM_BASE_ADDRESS);
> 
>    ///
> -  /// Program TCO BASE
> +  /// Program TCO BASE if it is present and not locked
>    ///
> -  PchTcoBaseSet (PcdGet16 (PcdTcoBaseAddress));
> +  if (PchIsTcoBaseSetValid ()) {
> +    PchTcoBaseSet (PcdGet16 (PcdTcoBaseAddress));  }
> 
>    ///
>    /// LPC I/O Configuration
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiDxeSmmPchCycleDecodin
> gLib/PchCycleDecodingLib.c
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiDxeSmmPchCycleDecodin
> gLib/PchCycleDecodingLib.c
> index 68b0b5dd4b..d7e91f947b 100644
> ---
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiDxeSmmPchCycleDecodin
> gLib/PchCycleDecodingLib.c
> +++
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiDxeSmmPchCycleDeco
> +++ dingLib/PchCycleDecodingLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>    PCH cycle deocding configuration and query library.
> 
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials are licensed and made
> available under  the terms and conditions of the BSD License that
> accompanies this distribution.
>  The full text of the license may be found at @@ -306,6 +306,36 @@
> PchPwrmBaseGet (  }
> 
>  /**
> +  Check if TCO Base register is present and unlocked.
> +  This should be called before calling PchTcoBaseSet ()
> +
> +  @retval BOOLEAN                       FALSE = Either TCO base is locked or Smbus
> not present
> +                                        TRUE  = TCO base is not locked
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +PchIsTcoBaseSetValid (
> +  VOID
> +  )
> +{
> +  UINTN                                 SmbusBase;
> +
> +  SmbusBase = MmPciBase (
> +                DEFAULT_PCI_BUS_NUMBER_PCH,
> +                PCI_DEVICE_NUMBER_PCH_SMBUS,
> +                PCI_FUNCTION_NUMBER_PCH_SMBUS
> +                );
> +  if (MmioRead16 (SmbusBase) == 0xFFFF) {
> +    return FALSE;
> +  }
> +  //
> +  // Verify TCO base is not locked.
> +  //
> +  return ((MmioRead8 (SmbusBase + R_PCH_SMBUS_TCOCTL) &
> +B_PCH_SMBUS_TCOCTL_TCO_BASE_LOCK) == 0); }
> +
> +/**
>    Set PCH TCO base address.
>    This cycle decoding is allowed to set when DMIC.SRL is 0.
>    Programming steps:
> @@ -318,7 +348,8 @@ PchPwrmBaseGet (
> 
>    @retval EFI_SUCCESS                   Successfully completed.
>    @retval EFI_INVALID_PARAMETER         Invalid base address passed.
> -  @retval EFI_UNSUPPORTED               DMIC.SRL is set.
> +  @retval EFI_UNSUPPORTED               DMIC.SRL is set, or Smbus device not
> present
> +  @retval EFI_DEVICE_ERROR              TCO Base register is locked already
>  **/
>  EFI_STATUS
>  EFIAPI
> @@ -353,16 +384,17 @@ PchTcoBaseSet (
>    //
>    // Verify TCO base is not locked.
>    //
> -  if ((MmioRead8 (SmbusBase + R_PCH_SMBUS_TCOCTL) &
> B_PCH_SMBUS_TCOCTL_TCO_BASE_LOCK) != 0) {
> +  if (!PchIsTcoBaseSetValid ()) {
>      ASSERT (FALSE);
>      return EFI_DEVICE_ERROR;
>    }
>    //
>    // Disable TCO in SMBUS Device first before changing base address.
> +  // Byte access to not touch the TCO_BASE_LOCK bit
>    //
> -  MmioAnd16 (
> -    SmbusBase + R_PCH_SMBUS_TCOCTL,
> -    (UINT16) ~B_PCH_SMBUS_TCOCTL_TCO_BASE_EN
> +  MmioAnd8 (
> +    SmbusBase + R_PCH_SMBUS_TCOCTL + 1,
> +    (UINT8) ~(B_PCH_SMBUS_TCOCTL_TCO_BASE_EN >> 8)
>      );
>    //
>    // Program TCO in SMBUS Device
> @@ -373,11 +405,11 @@ PchTcoBaseSet (
>      Address
>      );
>    //
> -  // Enable TCO in SMBUS Device
> +  // Enable TCO in SMBUS Device and lock TCO BASE
>    //
>    MmioOr16 (
>      SmbusBase + R_PCH_SMBUS_TCOCTL,
> -    B_PCH_SMBUS_TCOCTL_TCO_BASE_EN
> +    B_PCH_SMBUS_TCOCTL_TCO_BASE_EN |
> B_PCH_SMBUS_TCOCTL_TCO_BASE_LOCK
>      );
>    //
>    // Program "TCO Base Address" PCR[DMI] + 2778h[15:5, 1] to [SMBUS PCI
> offset 50h[15:5], 1].
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/PchCycleDecodingLib.
> h
> b/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/PchCycleDecodingLib
> .h
> index 30ad2713b5..830fdf5abf 100644
> ---
> a/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/PchCycleDecodingLib.
> h
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/PchCycleDecod
> +++ ingLib.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Header file for PchCycleDecodingLib.
> 
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials are licensed and made
> available under  the terms and conditions of the BSD License that
> accompanies this distribution.
>  The full text of the license may be found at @@ -87,6 +87,20 @@
> PchPwrmBaseGet (
>    );
> 
>  /**
> +  Check if TCO Base register is present and unlocked.
> +  This should be called before calling PchTcoBaseSet ()
> +
> +  @retval BOOLEAN                       FALSE = Either TCO base is locked or Smbus
> not present
> +                                        TRUE  = TCO base is not locked
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +PchIsTcoBaseSetValid (
> +  VOID
> +  );
> +
> +/**
>    Set PCH TCO base address.
>    This cycle decoding is allowed to set when DMIC.SRL is 0.
>    Programming steps:
> @@ -99,6 +113,8 @@ PchPwrmBaseGet (
> 
>    @retval EFI_SUCCESS                   Successfully completed.
>    @retval EFI_INVALID_PARAMETER         Invalid base address passed.
> +  @retval EFI_UNSUPPORTED               DMIC.SRL is set, or Smbus device not
> present
> +  @retval EFI_DEVICE_ERROR              TCO Base register is locked already
>  **/
>  EFI_STATUS
>  EFIAPI
> --
> 2.13.3.windows.1