[PATCH v1 0/5] Abstract the detect/enable/disable of several x86 features

[PATCH v1 0/5] Abstract the detect/enable/disable of several x86 features

[Joseph Shifflett]

This series abstracts accesses to various x86 features.  Existing code used Intel
specific MSR accesses to detect support and to enable/disable features.

Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Kelly Steele <kelly.steele@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>

Joseph Shifflett (5):
  UefiCpuPkg: SmmCpuFeaturesLib.h: Abstact processor features
  UefiCpuPkg/SmmCpuFeaturesLib: Abstact processor features
  QuarkSocPkg/SmmCpuFeaturesLib: Abstact processor features
  OvmfPkg/SmmCpuFeaturesLib: Abstact processor features
  UefiCpuPkg/PiSmmCpuDxeSmm: Abstact processor features

 UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h                              | 37 ++++++++++
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c                       | 44 +++++++++++
 QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 44 +++++++++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c                    | 78 ++++++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c                                       | 13 +---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                                      | 18 +----
 6 files changed, 208 insertions(+), 26 deletions(-)

-- 
2.7.4

[PATCH v1 1/5] UefiCpuPkg: SmmCpuFeaturesLib.h: Abstact processor features

[Joseph Shifflett]

Create new functions to abstract how XD/NX is detected, enabled, and
disabled.  Also, create a new function to determine if Branch Trace
Storage is supported.  Existing code is specific to Intel processors.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Joseph Shifflett <joseph.shifflett@hpe.com>
---
 UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h | 37 ++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h b/UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h
index 4478003467c8..48b4565b49bb 100644
--- a/UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h
@@ -2,6 +2,7 @@
 Library that provides CPU specific functions to support the PiSmmCpuDxeSmm module.
 
 Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -398,4 +399,40 @@ SmmCpuFeaturesAllocatePageTableMemory (
   IN UINTN           Pages
   );
 
+/**
+  This API provides a method to determine if XD/NX support has been forced off in
+  the non-SMM execution environment.  It will enable XD/NX support while in SMM
+
+  @retval TRUE   XD/NX was disabled when runningn in the non-SMM execution environment
+  @retval FALSE  XD/NX was enabled when runningn in the non-SMM execution environment
+
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesCheckAndEnableXdSupport (
+  VOID
+  );
+
+/**
+  This API provides a method to disable XD/NX support before exiting SMM
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesDisableXdSupport (
+  VOID
+  );
+
+/**
+  This API determines if Branch Trace Storage Support is currently available
+
+  @retval TRUE   BTS is available
+  @retval FALSE  BTS is disabled
+
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesConfirmBranchTraceStorageSupport (
+  VOID
+  );
+
 #endif
-- 
2.7.4

[PATCH v1 2/5] UefiCpuPkg/SmmCpuFeaturesLib: Abstact processor features

[Joseph Shifflett]

Create new functions to abstract how XD/NX is detected, enabled, and
disabled.  Also, create a new function to determine if Branch Trace
Storage is supported.  Existing code is specific to Intel processors.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Joseph Shifflett <joseph.shifflett@hpe.com>
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 78 ++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 1754f2df5f57..f7d1223ddf55 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -2,6 +2,7 @@
 The CPU specific programming for PiSmmCpuDxeSmm module.
 
 Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -20,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/MemoryAllocationLib.h>
 #include <Library/DebugLib.h>
 #include <Register/Cpuid.h>
+#include <Register/Msr.h>
 #include <Register/SmramSaveStateMap.h>
 
 //
@@ -673,3 +675,79 @@ SmmCpuFeaturesAllocatePageTableMemory (
   return NULL;
 }
 
+/**
+  This API provides a method to determine if XD/NX support has been forced off in
+  the non-SMM execution environment.  It will enable XD/NX support while in SMM
+
+  @retval TRUE   XD/NX was disabled when runningn in the non-SMM execution environment
+  @retval FALSE  XD/NX was enabled when runningn in the non-SMM execution environment
+
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesCheckAndEnableXdSupport (
+  VOID
+  )
+{
+  BOOLEAN                        XdDisableFlag;
+  MSR_IA32_MISC_ENABLE_REGISTER  MiscEnableMsr;
+
+  XdDisableFlag = FALSE;
+
+  MiscEnableMsr.Uint64 = AsmReadMsr64 (MSR_IA32_MISC_ENABLE);
+  if (MiscEnableMsr.Bits.XD == 1) {
+    XdDisableFlag = TRUE;
+    MiscEnableMsr.Bits.XD = 0;
+    AsmWriteMsr64 (MSR_IA32_MISC_ENABLE, MiscEnableMsr.Uint64);
+  }
+  return XdDisableFlag;
+}
+
+/**
+  This API provides a method to disable XD/NX support before exiting SMM
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesDisableXdSupport (
+  VOID
+  )
+{
+  MSR_IA32_MISC_ENABLE_REGISTER  MiscEnableMsr;
+
+  MiscEnableMsr.Uint64 = AsmReadMsr64 (MSR_IA32_MISC_ENABLE);
+  MiscEnableMsr.Bits.XD = 1;
+  AsmWriteMsr64 (MSR_IA32_MISC_ENABLE, MiscEnableMsr.Uint64);
+}
+
+/**
+  This API determines if Branch Trace Storage Support is currently available
+
+  @retval TRUE   BTS is available
+  @retval FALSE  BTS is disabled
+
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesConfirmBranchTraceStorageSupport (
+  VOID
+  )
+{
+  MSR_IA32_MISC_ENABLE_REGISTER  MiscEnableMsr;
+
+  //
+  // Per IA32 manuals:
+  // When CPUID.1:EDX[21] is set, the following BTS facilities are available:
+  // 1. The BTS_UNAVAILABLE flag in the IA32_MISC_ENABLE MSR indicates the
+  //    availability of the BTS facilities, including the ability to set the BTS and
+  //    BTINT bits in the MSR_DEBUGCTLA MSR.
+  // 2. The IA32_DS_AREA MSR can be programmed to point to the DS save area.
+  //
+  MiscEnableMsr.Uint64 = AsmReadMsr64 (MSR_IA32_MISC_ENABLE);
+  if (MiscEnableMsr.Bits.BTS == 1) {
+    //
+    // BTS facilities is not supported if MSR_IA32_MISC_ENABLE.BTS bit is set.
+    //
+    return FALSE;
+  }
+  return TRUE;
+}
-- 
2.7.4

[PATCH v1 3/5] QuarkSocPkg/SmmCpuFeaturesLib: Abstact processor features

[Joseph Shifflett]

Create new functions to abstract how XD/NX is detected, enabled, and
disabled.  Also, create a new function to determine if Branch Trace
Storage is supported.  Existing code is specific to Intel processors.

This provides NULL implementations of the new functions.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Joseph Shifflett <joseph.shifflett@hpe.com>
---
 QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 44 ++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 54eeac6d19a3..ddd667cc993f 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -2,6 +2,7 @@
 The Quark CPU specific programming for PiSmmCpuDxeSmm module.
 
 Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -450,3 +451,46 @@ SmmCpuFeaturesAllocatePageTableMemory (
 {
   return NULL;
 }
+/**
+  This API provides a method to determine if XD/NX support has been forced off in
+  the non-SMM execution environment.  It will enable XD/NX support while in SMM
+
+  @retval TRUE   XD/NX was disabled when runningn in the non-SMM execution environment
+  @retval FALSE  XD/NX was enabled when runningn in the non-SMM execution environment
+
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesCheckAndEnableXdSupport (
+  VOID
+  )
+{
+  return FALSE;
+}
+
+/**
+  This API provides a method to disable XD/NX support before exiting SMM
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesDisableXdSupport (
+  VOID
+  )
+{
+}
+
+/**
+  This API determines if Branch Trace Storage Support is currently available
+
+  @retval TRUE   BTS is available
+  @retval FALSE  BTS is disabled
+
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesConfirmBranchTraceStorageSupport (
+  VOID
+  )
+{
+  return FALSE;
+}
-- 
2.7.4

[PATCH v1 4/5] OvmfPkg/SmmCpuFeaturesLib: Abstact processor features

[Joseph Shifflett]

Create new functions to abstract how XD/NX is detected, enabled, and
disabled.  Also, create a new function to determine if Branch Trace
Storage is supported.  Existing code is specific to Intel processors.

This provides NULL implementations of the new functions.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Joseph Shifflett <joseph.shifflett@hpe.com>
---
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 44 ++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index a307f64c9c61..4c8c7fb7b25f 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -2,6 +2,7 @@
 The CPU specific programming for PiSmmCpuDxeSmm module.
 
 Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -832,3 +833,46 @@ SmmCpuFeaturesAllocatePageTableMemory (
   return NULL;
 }
 
+/**
+  This API provides a method to determine if XD/NX support has been forced off in
+  the non-SMM execution environment.  It will enable XD/NX support while in SMM
+
+  @retval TRUE   XD/NX was disabled when runningn in the non-SMM execution environment
+  @retval FALSE  XD/NX was enabled when runningn in the non-SMM execution environment
+
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesCheckAndEnableXdSupport (
+  VOID
+  )
+{
+  return FALSE;
+}
+
+/**
+  This API provides a method to disable XD/NX support before exiting SMM
+**/
+VOID
+EFIAPI
+SmmCpuFeaturesDisableXdSupport (
+  VOID
+  )
+{
+}
+
+/**
+  This API determines if Branch Trace Storage Support is currently available
+
+  @retval TRUE   BTS is available
+  @retval FALSE  BTS is disabled
+
+**/
+BOOLEAN
+EFIAPI
+SmmCpuFeaturesConfirmBranchTraceStorageSupport (
+  VOID
+  )
+{
+  return FALSE;
+}
-- 
2.7.4

[PATCH v1 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Abstact processor features

[Joseph Shifflett]

Use newly created functions that abstract how XD/NX is detected,
enabled, and disabled.  Also, use a new function to determine if
Branch Trace Storage is supported.  Existing code is specific
to Intel processors.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Joseph Shifflett <joseph.shifflett@hpe.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  | 13 +++----------
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 18 ++----------------
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 12466ef5de1f..d116ffd39c20 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -2,6 +2,7 @@
 SMM MP service implementation
 
 Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -1023,7 +1024,6 @@ SmiRendezvous (
   UINTN                          Index;
   UINTN                          Cr2;
   BOOLEAN                        XdDisableFlag;
-  MSR_IA32_MISC_ENABLE_REGISTER  MiscEnableMsr;
 
   //
   // Save Cr2 because Page Fault exception in SMM may override its value
@@ -1087,12 +1087,7 @@ SmiRendezvous (
     //
     XdDisableFlag = FALSE;
     if (mXdSupported) {
-      MiscEnableMsr.Uint64 = AsmReadMsr64 (MSR_IA32_MISC_ENABLE);
-      if (MiscEnableMsr.Bits.XD == 1) {
-        XdDisableFlag = TRUE;
-        MiscEnableMsr.Bits.XD = 0;
-        AsmWriteMsr64 (MSR_IA32_MISC_ENABLE, MiscEnableMsr.Uint64);
-      }
+      XdDisableFlag = SmmCpuFeaturesCheckAndEnableXdSupport ();
       ActivateXd ();
     }
 
@@ -1182,9 +1177,7 @@ SmiRendezvous (
     // Restore XD
     //
     if (XdDisableFlag) {
-      MiscEnableMsr.Uint64 = AsmReadMsr64 (MSR_IA32_MISC_ENABLE);
-      MiscEnableMsr.Bits.XD = 1;
-      AsmWriteMsr64 (MSR_IA32_MISC_ENABLE, MiscEnableMsr.Uint64);
+      SmmCpuFeaturesDisableXdSupport ();
     }
   }
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 329574ebb24e..47793b27f2f1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -2,6 +2,7 @@
 Enable SMM profile.
 
 Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -933,7 +934,6 @@ CheckFeatureSupported (
 {
   UINT32                         RegEax;
   UINT32                         RegEdx;
-  MSR_IA32_MISC_ENABLE_REGISTER  MiscEnableMsr;
 
   if (mXdSupported) {
     AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
@@ -956,21 +956,7 @@ CheckFeatureSupported (
   if (mBtsSupported) {
     AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, &RegEdx);
     if ((RegEdx & CPUID1_EDX_BTS_AVAILABLE) != 0) {
-      //
-      // Per IA32 manuals:
-      // When CPUID.1:EDX[21] is set, the following BTS facilities are available:
-      // 1. The BTS_UNAVAILABLE flag in the IA32_MISC_ENABLE MSR indicates the
-      //    availability of the BTS facilities, including the ability to set the BTS and
-      //    BTINT bits in the MSR_DEBUGCTLA MSR.
-      // 2. The IA32_DS_AREA MSR can be programmed to point to the DS save area.
-      //
-      MiscEnableMsr.Uint64 = AsmReadMsr64 (MSR_IA32_MISC_ENABLE);
-      if (MiscEnableMsr.Bits.BTS == 1) {
-        //
-        // BTS facilities is not supported if MSR_IA32_MISC_ENABLE.BTS bit is set.
-        //
-        mBtsSupported = FALSE;
-      }
+      mBtsSupported = SmmCpuFeaturesConfirmBranchTraceStorageSupport ();
     }
   }
 }
-- 
2.7.4

Re: [PATCH v1 1/5] UefiCpuPkg: SmmCpuFeaturesLib.h: Abstact processor features

[Laszlo Ersek]

On 09/08/16 06:30, Joseph Shifflett wrote:
> Create new functions to abstract how XD/NX is detected, enabled, and
> disabled.  Also, create a new function to determine if Branch Trace
> Storage is supported.  Existing code is specific to Intel processors.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Joseph Shifflett <joseph.shifflett@hpe.com>
> ---
>  UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h | 37 ++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h b/UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h
> index 4478003467c8..48b4565b49bb 100644
> --- a/UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h
> +++ b/UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h
> @@ -2,6 +2,7 @@
>  Library that provides CPU specific functions to support the PiSmmCpuDxeSmm module.
>  
>  Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD License
>  which accompanies this distribution.  The full text of the license may be found at
> @@ -398,4 +399,40 @@ SmmCpuFeaturesAllocatePageTableMemory (
>    IN UINTN           Pages
>    );
>  
> +/**
> +  This API provides a method to determine if XD/NX support has been forced off in
> +  the non-SMM execution environment.  It will enable XD/NX support while in SMM
> +
> +  @retval TRUE   XD/NX was disabled when runningn in the non-SMM execution environment
> +  @retval FALSE  XD/NX was enabled when runningn in the non-SMM execution environment

Typo: "runningn".

Other than that, I'll let the UefiCpuPkg maintainers review these
interface additions.

Thanks
Laszlo

> +
> +**/
> +BOOLEAN
> +EFIAPI
> +SmmCpuFeaturesCheckAndEnableXdSupport (
> +  VOID
> +  );
> +
> +/**
> +  This API provides a method to disable XD/NX support before exiting SMM
> +**/
> +VOID
> +EFIAPI
> +SmmCpuFeaturesDisableXdSupport (
> +  VOID
> +  );
> +
> +/**
> +  This API determines if Branch Trace Storage Support is currently available
> +
> +  @retval TRUE   BTS is available
> +  @retval FALSE  BTS is disabled
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +SmmCpuFeaturesConfirmBranchTraceStorageSupport (
> +  VOID
> +  );
> +
>  #endif
> 

Re: [PATCH v1 0/5] Abstract the detect/enable/disable of several x86 features

[Fan, Jeff]

Hi, Joseph

Abstracting the detect/enable operation on CPU SMM features is good idea on UefiCpuPkg.

Actually, we are planning to abstract as more as possible CPU features in one new library in UefiCpuPKg. It not only includes CPU SMM features, but also includes the non-SMM features.

We will work on it in the next a couple of months. We'd like to share some information here and I'd like to get the input from you. Does it make sense for you?

Thanks!
Jeff 

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Joseph Shifflett
Sent: Thursday, September 08, 2016 12:30 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D; Justen, Jordan L; Fan, Jeff; Laszlo Ersek
Subject: [edk2] [PATCH v1 0/5] Abstract the detect/enable/disable of several x86 features

This series abstracts accesses to various x86 features.  Existing code used Intel specific MSR accesses to detect support and to enable/disable features.

Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Kelly Steele <kelly.steele@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>

Joseph Shifflett (5):
  UefiCpuPkg: SmmCpuFeaturesLib.h: Abstact processor features
  UefiCpuPkg/SmmCpuFeaturesLib: Abstact processor features
  QuarkSocPkg/SmmCpuFeaturesLib: Abstact processor features
  OvmfPkg/SmmCpuFeaturesLib: Abstact processor features
  UefiCpuPkg/PiSmmCpuDxeSmm: Abstact processor features

 UefiCpuPkg/Include/Library/SmmCpuFeaturesLib.h                              | 37 ++++++++++
 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c                       | 44 +++++++++++
 QuarkSocPkg/QuarkNorthCluster/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 44 +++++++++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c                    | 78 ++++++++++++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c                                       | 13 +---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                                      | 18 +----
 6 files changed, 208 insertions(+), 26 deletions(-)

--
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v1 4/5] OvmfPkg/SmmCpuFeaturesLib: Abstact processor features

[Laszlo Ersek]

Adding Paolo.

Paolo, here's the root of the thread:

https://lists.01.org/pipermail/edk2-devel/2016-September/001500.html

On 09/08/16 06:30, Joseph Shifflett wrote:
> Create new functions to abstract how XD/NX is detected, enabled, and
> disabled.  Also, create a new function to determine if Branch Trace
> Storage is supported.  Existing code is specific to Intel processors.
> 
> This provides NULL implementations of the new functions.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Joseph Shifflett <joseph.shifflett@hpe.com>
> ---
>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 44 ++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> index a307f64c9c61..4c8c7fb7b25f 100644
> --- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> +++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> @@ -2,6 +2,7 @@
>  The CPU specific programming for PiSmmCpuDxeSmm module.
>  
>  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD License
>  which accompanies this distribution.  The full text of the license may be found at
> @@ -832,3 +833,46 @@ SmmCpuFeaturesAllocatePageTableMemory (
>    return NULL;
>  }
>  
> +/**
> +  This API provides a method to determine if XD/NX support has been forced off in
> +  the non-SMM execution environment.  It will enable XD/NX support while in SMM
> +
> +  @retval TRUE   XD/NX was disabled when runningn in the non-SMM execution environment
> +  @retval FALSE  XD/NX was enabled when runningn in the non-SMM execution environment
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +SmmCpuFeaturesCheckAndEnableXdSupport (
> +  VOID
> +  )
> +{
> +  return FALSE;
> +}
> +
> +/**
> +  This API provides a method to disable XD/NX support before exiting SMM
> +**/
> +VOID
> +EFIAPI
> +SmmCpuFeaturesDisableXdSupport (
> +  VOID
> +  )
> +{
> +}
> +
> +/**
> +  This API determines if Branch Trace Storage Support is currently available
> +
> +  @retval TRUE   BTS is available
> +  @retval FALSE  BTS is disabled
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +SmmCpuFeaturesConfirmBranchTraceStorageSupport (
> +  VOID
> +  )
> +{
> +  return FALSE;
> +}
> 

I think it would be best to check these with Paolo, as he authored
OvmfPkg's SmmCpuFeaturesLib.

* The XD manipulation (wrt. the first two functions) is definitely
supported by KVM, so at least for those functions, I think the same
functionality as in the UefiCpuPkg library instance would be
appropriate. (See some additional pondering at [1] below.)

* For the last function, I think KVM does not implement the branch
recording MSRs. I also don't know if it even implements the "feature
query" MSR for the branch recording feature. Here too, the UefiCpuPkg
variant might be more appropriate for OvmfPkg than a constant FALSE --
please consult with Paolo.


Most importantly (I should have asked this first): what are you going to
use these functions for? I assume you are abstracting PiSmmCpuDxeSmm
from Ia32/X64, bringing it closer to ARM/AARCH64. It would be nice to
see the exact actions in PiSmmCpuDxeSmm that you plan to rebase on these
new library APIs.

If you post a version 2 of the patch set,
- please spell out the intended *concrete* use of each function in the
commit message of patch #1 (that is, where you modify the library class
header),
- please CC Paolo on the entire set.

(

[1] I recall that there was some nastiness wrt. the AMD SMM save state
area *not* saving and restoring XD state when entering and leaving SMM
*on Ia32*, but correctly saving and restoring XD state when doing the
same on X64.

Therefore our current recommendation for running a 32-bit OVMF binary
(that was built with -D SMM_REQUIRE) on qemu-system-i386 is to pass

  -cpu XXXX,-nx

because if XD is presented to the guest, then SMM will break. IOW,
currently we ask the QEMU user to hide the XD processor feature on
32-bit processors.

I don't know if we could get rid of the user-side tweak with the help of
this library. I guess you might be able to add MDE_CPU_IA32 vs.
MDE_CPU_X64 specific code to the first two functions in OvmfPkg;
reporting "XD invariably absent" in the MDE_CPU_IA32 case. However I'm
worried that some other component in the firmware would still query
MSR_IA32_MISC_ENABLE, and then be inconsistent with the constant retval
from here.

Thus we'll likely have to keep the -nx tweak in OvmfPkg/README anyway,
and in turn these functions should not be specific to Ia32/X64 in OvmfPkg.

I believe the first two functions should just copy the UefiCpuPkg
implementation, and possibly even the third one. But, check with Paolo
please.

)

Thanks!
Laszlo