Re: [edk2] [PATCH v2] MinPlatformPkg: Support TCO base locked by FSP
Reviewed-by: Michael Kubacki
> -Original Message-
> From: Chiu, Chasel
> Sent: Wednesday, January 16, 2019 11:16 PM
> To: edk2-devel@lists.01.org
> Cc: Kubacki, Michael A ; Yao, Jiewen
> ; Chiu, Chasel
> 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
> Cc: Jiewen Yao
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu
> ---
> 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.
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
> 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.
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
> 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) == 0x) {
> +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
[edk2] [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
Cc: Jiewen Yao
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu
---
Silicon/Intel/KabylakeSiliconPkg/Library/SiliconInitLib/SiliconInitPreMem.c
| 8 +---
Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiDxeSmmPchCycleDecodingLib/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/SiliconInitPreMem.c
@@ -1,7 +1,7 @@
/** @file
Source code file for Platform Init Pre-Memory PEI module
-Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
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/PeiDxeSmmPchCycleDecodingLib/PchCycleDecodingLib.c
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiDxeSmmPchCycleDecodingLib/PchCycleDecodingLib.c
index 68b0b5dd4b..d7e91f947b 100644
---
a/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiDxeSmmPchCycleDecodingLib/PchCycleDecodingLib.c
+++
b/Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiDxeSmmPchCycleDecodingLib/PchCycleDecodingLib.c
@@ -1,7 +1,7 @@
/** @file
PCH cycle deocding configuration and query library.
-Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
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) == 0x) {
+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
