* [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: SMM profile and static paging mutual exclusion
@ 2017-12-07 10:47 Star Zeng
2017-12-07 11:02 ` Dong, Eric
2017-12-07 11:32 ` Laszlo Ersek
0 siblings, 2 replies; 4+ messages in thread
From: Star Zeng @ 2017-12-07 10:47 UTC (permalink / raw)
To: edk2-devel
Cc: Star Zeng, Jiewen Yao, Eric Dong, Ruiyu Ni, Jian J Wang,
Laszlo Ersek
SMM profile and static paging could not enabled at the same time,
this patch is add check and comments to make sure it.
Similar comments are also added for the case of static paging and
heap guard for SMM.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 9 +++++++++
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 13 ++++++++++---
UefiCpuPkg/UefiCpuPkg.dec | 6 +++++-
UefiCpuPkg/UefiCpuPkg.uni | 10 ++++++++--
4 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 6e1ffe7c6287..939ac25a506c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -205,6 +205,15 @@ SetPageTableAttributes (
// BIT3: SMM pool guard enabled
//
if ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
+ DEBUG ((DEBUG_INFO, "Don't mark page table as read-only as heap guard is enabled\n"));
+ return ;
+ }
+
+ //
+ // Don't mark page table as read-only if SMM profile is enabled.
+ //
+ if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
+ DEBUG ((DEBUG_INFO, "Don't mark page table as read-only as SMM profile is enabled\n"));
return ;
}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 6478c6c3e355..0fe944fc18cc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -919,17 +919,24 @@ SetPageTableAttributes (
//
// Don't do this if
// - no static page table; or
- // - SMM heap guard feature enabled
+ // - SMM heap guard feature enabled; or
// BIT2: SMM page guard enabled
// BIT3: SMM pool guard enabled
+ // - SMM profile feature enabled
//
if (!mCpuSmmStaticPageTable ||
- (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
+ ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) ||
+ FeaturePcdGet (PcdCpuSmmProfileEnable)) {
//
- // Static paging and heap guard should not be enabled at the same time.
+ // Static paging and heap guard could not be enabled at the same time.
//
ASSERT (!(mCpuSmmStaticPageTable &&
(PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0));
+
+ //
+ // Static paging and SMM profile could not be enabled at the same time.
+ //
+ ASSERT (!(mCpuSmmStaticPageTable && FeaturePcdGet (PcdCpuSmmProfileEnable)));
return ;
}
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index d2965ba14c2d..36205ab63796 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -84,6 +84,7 @@ [Protocols]
[PcdsFeatureFlag]
## Indicates if SMM Profile will be enabled.
# If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged.
+ # It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable).
# This PCD is only for validation purpose. It should be set to false in production.<BR><BR>
# TRUE - SMM Profile will be enabled.<BR>
# FALSE - SMM Profile will be disabled.<BR>
@@ -225,8 +226,11 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate|0|UINT8|0x00000007
## Indicates if SMM uses static page table.
- # If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.<BR><BR>
+ # If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.
# This flag only impacts X64 build, because SMM alway builds static page table for IA32.
+ # It could not be enabled at the same time with SMM profile feature (PcdCpuSmmProfileEnable).
+ # It could not be enabled also at the same time with heap guard feature for SMM
+ # (PcdHeapGuardPropertyMask in MdeModulePkg).<BR><BR>
# TRUE - SMM uses static page table for all memory.<BR>
# FALSE - SMM uses static page table for below 4G memory and use on-demand paging for above 4G memory.<BR>
# @Prompt Use static page table for all memory in SMM.
diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index 9472b185e46e..013d2870a682 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -53,7 +53,10 @@
#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_PROMPT #language en-US "Enable SMM Profile"
-#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_HELP #language en-US "Indicates if SMM Profile will be enabled. If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged. This PCD is only for validation purpose. It should be set to false in production.<BR><BR>\n"
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_HELP #language en-US "Indicates if SMM Profile will be enabled.\n"
+ "If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged.\n"
+ "It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable).\n"
+ "This PCD is only for validation purpose. It should be set to false in production.<BR><BR>\n"
"TRUE - SMM Profile will be enabled.<BR>\n"
"FALSE - SMM Profile will be disabled.<BR>"
@@ -150,8 +153,11 @@
#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmStaticPageTable_PROMPT #language en-US "Use static page table for all memory in SMM."
#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmStaticPageTable_HELP #language en-US "Indicates if SMM uses static page table.\n"
- "If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.<BR><BR>\n"
+ "If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.\n"
"This flag only impacts X64 build, because SMM alway builds static page table for IA32.\n"
+ "It could not be enabled at the same time with SMM profile feature (PcdCpuSmmProfileEnable).\n"
+ "It could not be enabled also at the same time with heap guard feature for SMM\n"
+ "(PcdHeapGuardPropertyMask in MdeModulePkg).<BR><BR>\n"
"TRUE - SMM uses static page table for all memory.<BR>\n"
"FALSE - SMM uses static page table for below 4G memory and use on-demand paging for above 4G memory.<BR>"
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: SMM profile and static paging mutual exclusion
2017-12-07 10:47 [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: SMM profile and static paging mutual exclusion Star Zeng
@ 2017-12-07 11:02 ` Dong, Eric
2017-12-07 11:32 ` Laszlo Ersek
1 sibling, 0 replies; 4+ messages in thread
From: Dong, Eric @ 2017-12-07 11:02 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org
Cc: Ni, Ruiyu, Yao, Jiewen, Laszlo Ersek, Zeng, Star
Star,
Reviewed-by: Eric Dong <eric.dong@intel.com>
Below typo you can change when you push the code.
+ # It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable).
" SMM profile feature " should be "Static page table feature"?
Thanks,
Eric
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star Zeng
Sent: Thursday, December 7, 2017 6:48 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu; Dong, Eric; Yao, Jiewen; Laszlo Ersek; Zeng, Star
Subject: [edk2] [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: SMM profile and static paging mutual exclusion
SMM profile and static paging could not enabled at the same time, this patch is add check and comments to make sure it.
Similar comments are also added for the case of static paging and heap guard for SMM.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 9 +++++++++ UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 13 ++++++++++---
UefiCpuPkg/UefiCpuPkg.dec | 6 +++++-
UefiCpuPkg/UefiCpuPkg.uni | 10 ++++++++--
4 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 6e1ffe7c6287..939ac25a506c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -205,6 +205,15 @@ SetPageTableAttributes (
// BIT3: SMM pool guard enabled
//
if ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
+ DEBUG ((DEBUG_INFO, "Don't mark page table as read-only as heap guard is enabled\n"));
+ return ;
+ }
+
+ //
+ // Don't mark page table as read-only if SMM profile is enabled.
+ //
+ if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
+ DEBUG ((DEBUG_INFO, "Don't mark page table as read-only as SMM
+ profile is enabled\n"));
return ;
}
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 6478c6c3e355..0fe944fc18cc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -919,17 +919,24 @@ SetPageTableAttributes (
//
// Don't do this if
// - no static page table; or
- // - SMM heap guard feature enabled
+ // - SMM heap guard feature enabled; or
// BIT2: SMM page guard enabled
// BIT3: SMM pool guard enabled
+ // - SMM profile feature enabled
//
if (!mCpuSmmStaticPageTable ||
- (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
+ ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) ||
+ FeaturePcdGet (PcdCpuSmmProfileEnable)) {
//
- // Static paging and heap guard should not be enabled at the same time.
+ // Static paging and heap guard could not be enabled at the same time.
//
ASSERT (!(mCpuSmmStaticPageTable &&
(PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0));
+
+ //
+ // Static paging and SMM profile could not be enabled at the same time.
+ //
+ ASSERT (!(mCpuSmmStaticPageTable && FeaturePcdGet
+ (PcdCpuSmmProfileEnable)));
return ;
}
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index d2965ba14c2d..36205ab63796 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -84,6 +84,7 @@ [Protocols]
[PcdsFeatureFlag]
## Indicates if SMM Profile will be enabled.
# If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged.
+ # It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable).
# This PCD is only for validation purpose. It should be set to false in production.<BR><BR>
# TRUE - SMM Profile will be enabled.<BR>
# FALSE - SMM Profile will be disabled.<BR>
@@ -225,8 +226,11 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate|0|UINT8|0x00000007
## Indicates if SMM uses static page table.
- # If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.<BR><BR>
+ # If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.
# This flag only impacts X64 build, because SMM alway builds static page table for IA32.
+ # It could not be enabled at the same time with SMM profile feature (PcdCpuSmmProfileEnable).
+ # It could not be enabled also at the same time with heap guard
+ feature for SMM # (PcdHeapGuardPropertyMask in
+ MdeModulePkg).<BR><BR>
# TRUE - SMM uses static page table for all memory.<BR>
# FALSE - SMM uses static page table for below 4G memory and use on-demand paging for above 4G memory.<BR>
# @Prompt Use static page table for all memory in SMM.
diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni index 9472b185e46e..013d2870a682 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -53,7 +53,10 @@
#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_PROMPT #language en-US "Enable SMM Profile"
-#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_HELP #language en-US "Indicates if SMM Profile will be enabled. If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged. This PCD is only for validation purpose. It should be set to false in production.<BR><BR>\n"
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_HELP #language en-US "Indicates if SMM Profile will be enabled.\n"
+ "If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged.\n"
+ "It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable).\n"
+ "This PCD is only for validation purpose. It should be set to false in production.<BR><BR>\n"
"TRUE - SMM Profile will be enabled.<BR>\n"
"FALSE - SMM Profile will be disabled.<BR>"
@@ -150,8 +153,11 @@
#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmStaticPageTable_PROMPT #language en-US "Use static page table for all memory in SMM."
#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmStaticPageTable_HELP #language en-US "Indicates if SMM uses static page table.\n"
- "If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.<BR><BR>\n"
+ "If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.\n"
"This flag only impacts X64 build, because SMM alway builds static page table for IA32.\n"
+ "It could not be enabled at the same time with SMM profile feature (PcdCpuSmmProfileEnable).\n"
+ "It could not be enabled also at the same time with heap guard feature for SMM\n"
+ "(PcdHeapGuardPropertyMask in MdeModulePkg).<BR><BR>\n"
"TRUE - SMM uses static page table for all memory.<BR>\n"
"FALSE - SMM uses static page table for below 4G memory and use on-demand paging for above 4G memory.<BR>"
--
2.7.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: SMM profile and static paging mutual exclusion
2017-12-07 10:47 [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: SMM profile and static paging mutual exclusion Star Zeng
2017-12-07 11:02 ` Dong, Eric
@ 2017-12-07 11:32 ` Laszlo Ersek
2017-12-08 1:03 ` Zeng, Star
1 sibling, 1 reply; 4+ messages in thread
From: Laszlo Ersek @ 2017-12-07 11:32 UTC (permalink / raw)
To: Star Zeng, edk2-devel; +Cc: Jiewen Yao, Eric Dong, Ruiyu Ni, Jian J Wang
Hi Star,
On 12/07/17 11:47, Star Zeng wrote:
> SMM profile and static paging could not enabled at the same time,
> this patch is add check and comments to make sure it.
>
> Similar comments are also added for the case of static paging and
> heap guard for SMM.
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 9 +++++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 13 ++++++++++---
> UefiCpuPkg/UefiCpuPkg.dec | 6 +++++-
> UefiCpuPkg/UefiCpuPkg.uni | 10 ++++++++--
> 4 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> index 6e1ffe7c6287..939ac25a506c 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> @@ -205,6 +205,15 @@ SetPageTableAttributes (
> // BIT3: SMM pool guard enabled
> //
> if ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
> + DEBUG ((DEBUG_INFO, "Don't mark page table as read-only as heap guard is enabled\n"));
> + return ;
> + }
> +
> + //
> + // Don't mark page table as read-only if SMM profile is enabled.
> + //
> + if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
> + DEBUG ((DEBUG_INFO, "Don't mark page table as read-only as SMM profile is enabled\n"));
> return ;
> }
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 6478c6c3e355..0fe944fc18cc 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -919,17 +919,24 @@ SetPageTableAttributes (
> //
> // Don't do this if
> // - no static page table; or
> - // - SMM heap guard feature enabled
> + // - SMM heap guard feature enabled; or
> // BIT2: SMM page guard enabled
> // BIT3: SMM pool guard enabled
> + // - SMM profile feature enabled
> //
> if (!mCpuSmmStaticPageTable ||
> - (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
> + ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) ||
> + FeaturePcdGet (PcdCpuSmmProfileEnable)) {
> //
> - // Static paging and heap guard should not be enabled at the same time.
> + // Static paging and heap guard could not be enabled at the same time.
> //
> ASSERT (!(mCpuSmmStaticPageTable &&
> (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0));
> +
> + //
> + // Static paging and SMM profile could not be enabled at the same time.
> + //
> + ASSERT (!(mCpuSmmStaticPageTable && FeaturePcdGet (PcdCpuSmmProfileEnable)));
> return ;
> }
>
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index d2965ba14c2d..36205ab63796 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -84,6 +84,7 @@ [Protocols]
> [PcdsFeatureFlag]
> ## Indicates if SMM Profile will be enabled.
> # If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged.
> + # It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable).
> # This PCD is only for validation purpose. It should be set to false in production.<BR><BR>
> # TRUE - SMM Profile will be enabled.<BR>
> # FALSE - SMM Profile will be disabled.<BR>
> @@ -225,8 +226,11 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate|0|UINT8|0x00000007
>
> ## Indicates if SMM uses static page table.
> - # If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.<BR><BR>
> + # If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.
> # This flag only impacts X64 build, because SMM alway builds static page table for IA32.
> + # It could not be enabled at the same time with SMM profile feature (PcdCpuSmmProfileEnable).
> + # It could not be enabled also at the same time with heap guard feature for SMM
> + # (PcdHeapGuardPropertyMask in MdeModulePkg).<BR><BR>
> # TRUE - SMM uses static page table for all memory.<BR>
> # FALSE - SMM uses static page table for below 4G memory and use on-demand paging for above 4G memory.<BR>
> # @Prompt Use static page table for all memory in SMM.
> diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
> index 9472b185e46e..013d2870a682 100644
> --- a/UefiCpuPkg/UefiCpuPkg.uni
> +++ b/UefiCpuPkg/UefiCpuPkg.uni
> @@ -53,7 +53,10 @@
>
> #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_PROMPT #language en-US "Enable SMM Profile"
>
> -#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_HELP #language en-US "Indicates if SMM Profile will be enabled. If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged. This PCD is only for validation purpose. It should be set to false in production.<BR><BR>\n"
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_HELP #language en-US "Indicates if SMM Profile will be enabled.\n"
> + "If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged.\n"
> + "It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable).\n"
> + "This PCD is only for validation purpose. It should be set to false in production.<BR><BR>\n"
> "TRUE - SMM Profile will be enabled.<BR>\n"
> "FALSE - SMM Profile will be disabled.<BR>"
>
> @@ -150,8 +153,11 @@
> #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmStaticPageTable_PROMPT #language en-US "Use static page table for all memory in SMM."
>
> #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmStaticPageTable_HELP #language en-US "Indicates if SMM uses static page table.\n"
> - "If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.<BR><BR>\n"
> + "If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.\n"
> "This flag only impacts X64 build, because SMM alway builds static page table for IA32.\n"
> + "It could not be enabled at the same time with SMM profile feature (PcdCpuSmmProfileEnable).\n"
> + "It could not be enabled also at the same time with heap guard feature for SMM\n"
> + "(PcdHeapGuardPropertyMask in MdeModulePkg).<BR><BR>\n"
> "TRUE - SMM uses static page table for all memory.<BR>\n"
> "FALSE - SMM uses static page table for below 4G memory and use on-demand paging for above 4G memory.<BR>"
>
>
This patch looks good to me; in particular I checked that
PcdCpuSmmProfileEnable==FALSE does not lead to any observable changes in
the code execution.
I notice two typos however:
(1) In PcdCpuSmmProfileEnable_HELP, we should not say:
"It could not be enabled at the same time with SMM profile feature
(PcdCpuSmmStaticPageTable)"
We should say:
"It could not be enabled at the same time with SMM static page table
feature (PcdCpuSmmStaticPageTable)"
(2) The same typo is present a bit higher up in the patch, in the DEC
file, in the documentation of PcdCpuSmmProfileEnable.
With those two typos fixed:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: SMM profile and static paging mutual exclusion
2017-12-07 11:32 ` Laszlo Ersek
@ 2017-12-08 1:03 ` Zeng, Star
0 siblings, 0 replies; 4+ messages in thread
From: Zeng, Star @ 2017-12-08 1:03 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org
Cc: Yao, Jiewen, Dong, Eric, Ni, Ruiyu, Wang, Jian J, Zeng, Star
Thanks and the RB to Laszlo and Eric.
And I agree and will handle them when pushing the patch. :)
Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, December 7, 2017 7:32 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
Subject: Re: [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: SMM profile and static paging mutual exclusion
Hi Star,
On 12/07/17 11:47, Star Zeng wrote:
> SMM profile and static paging could not enabled at the same time, this
> patch is add check and comments to make sure it.
>
> Similar comments are also added for the case of static paging and heap
> guard for SMM.
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 9 +++++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 13 ++++++++++---
> UefiCpuPkg/UefiCpuPkg.dec | 6 +++++-
> UefiCpuPkg/UefiCpuPkg.uni | 10 ++++++++--
> 4 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> index 6e1ffe7c6287..939ac25a506c 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> @@ -205,6 +205,15 @@ SetPageTableAttributes (
> // BIT3: SMM pool guard enabled
> //
> if ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
> + DEBUG ((DEBUG_INFO, "Don't mark page table as read-only as heap guard is enabled\n"));
> + return ;
> + }
> +
> + //
> + // Don't mark page table as read-only if SMM profile is enabled.
> + //
> + if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
> + DEBUG ((DEBUG_INFO, "Don't mark page table as read-only as SMM
> + profile is enabled\n"));
> return ;
> }
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 6478c6c3e355..0fe944fc18cc 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -919,17 +919,24 @@ SetPageTableAttributes (
> //
> // Don't do this if
> // - no static page table; or
> - // - SMM heap guard feature enabled
> + // - SMM heap guard feature enabled; or
> // BIT2: SMM page guard enabled
> // BIT3: SMM pool guard enabled
> + // - SMM profile feature enabled
> //
> if (!mCpuSmmStaticPageTable ||
> - (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
> + ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) ||
> + FeaturePcdGet (PcdCpuSmmProfileEnable)) {
> //
> - // Static paging and heap guard should not be enabled at the same time.
> + // Static paging and heap guard could not be enabled at the same time.
> //
> ASSERT (!(mCpuSmmStaticPageTable &&
> (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) !=
> 0));
> +
> + //
> + // Static paging and SMM profile could not be enabled at the same time.
> + //
> + ASSERT (!(mCpuSmmStaticPageTable && FeaturePcdGet
> + (PcdCpuSmmProfileEnable)));
> return ;
> }
>
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index d2965ba14c2d..36205ab63796 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -84,6 +84,7 @@ [Protocols]
> [PcdsFeatureFlag]
> ## Indicates if SMM Profile will be enabled.
> # If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged.
> + # It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable).
> # This PCD is only for validation purpose. It should be set to false in production.<BR><BR>
> # TRUE - SMM Profile will be enabled.<BR>
> # FALSE - SMM Profile will be disabled.<BR>
> @@ -225,8 +226,11 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate|0|UINT8|0x00000007
>
> ## Indicates if SMM uses static page table.
> - # If enabled, SMM will not use on-demand paging. SMM will build
> static page table for all memory.<BR><BR>
> + # If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.
> # This flag only impacts X64 build, because SMM alway builds static page table for IA32.
> + # It could not be enabled at the same time with SMM profile feature (PcdCpuSmmProfileEnable).
> + # It could not be enabled also at the same time with heap guard
> + feature for SMM # (PcdHeapGuardPropertyMask in
> + MdeModulePkg).<BR><BR>
> # TRUE - SMM uses static page table for all memory.<BR>
> # FALSE - SMM uses static page table for below 4G memory and use on-demand paging for above 4G memory.<BR>
> # @Prompt Use static page table for all memory in SMM.
> diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
> index 9472b185e46e..013d2870a682 100644
> --- a/UefiCpuPkg/UefiCpuPkg.uni
> +++ b/UefiCpuPkg/UefiCpuPkg.uni
> @@ -53,7 +53,10 @@
>
> #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_PROMPT #language en-US "Enable SMM Profile"
>
> -#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_HELP #language en-US "Indicates if SMM Profile will be enabled. If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged. This PCD is only for validation purpose. It should be set to false in production.<BR><BR>\n"
> +#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmProfileEnable_HELP #language en-US "Indicates if SMM Profile will be enabled.\n"
> + "If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged.\n"
> + "It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable).\n"
> + "This PCD is only for validation purpose. It should be set to false in production.<BR><BR>\n"
> "TRUE - SMM Profile will be enabled.<BR>\n"
> "FALSE - SMM Profile will be disabled.<BR>"
>
> @@ -150,8 +153,11 @@
> #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmStaticPageTable_PROMPT #language en-US "Use static page table for all memory in SMM."
>
> #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuSmmStaticPageTable_HELP #language en-US "Indicates if SMM uses static page table.\n"
> - "If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.<BR><BR>\n"
> + "If enabled, SMM will not use on-demand paging. SMM will build static page table for all memory.\n"
> "This flag only impacts X64 build, because SMM alway builds static page table for IA32.\n"
> + "It could not be enabled at the same time with SMM profile feature (PcdCpuSmmProfileEnable).\n"
> + "It could not be enabled also at the same time with heap guard feature for SMM\n"
> + "(PcdHeapGuardPropertyMask in MdeModulePkg).<BR><BR>\n"
> "TRUE - SMM uses static page table for all memory.<BR>\n"
> "FALSE - SMM uses static page table for below 4G memory and use on-demand paging for above 4G memory.<BR>"
>
>
This patch looks good to me; in particular I checked that PcdCpuSmmProfileEnable==FALSE does not lead to any observable changes in the code execution.
I notice two typos however:
(1) In PcdCpuSmmProfileEnable_HELP, we should not say:
"It could not be enabled at the same time with SMM profile feature (PcdCpuSmmStaticPageTable)"
We should say:
"It could not be enabled at the same time with SMM static page table feature (PcdCpuSmmStaticPageTable)"
(2) The same typo is present a bit higher up in the patch, in the DEC file, in the documentation of PcdCpuSmmProfileEnable.
With those two typos fixed:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-08 0:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-07 10:47 [PATCH] UefiCpuPkg PiSmmCpuDxeSmm: SMM profile and static paging mutual exclusion Star Zeng
2017-12-07 11:02 ` Dong, Eric
2017-12-07 11:32 ` Laszlo Ersek
2017-12-08 1:03 ` Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox