[PATCH 0/2] update the property of PcdCpuStackGuard

[PATCH 0/2] update the property of PcdCpuStackGuard

[Chen, Gang C]

This serial patch is to change the property of PcdCpuStackGuard from
PcdsFixedAtBuild to PcdsDynamicEx.

The purpose of this change is for the requirement to set the different
value in FSP API mode and FSP Dispatch mode.

And there is duplicated check for this PCD in both
InitializeCpuExceptionHandlersEx and InitializeMpExceptionStackSwitchHandlers.
The patch is to remove the duplicted check in
InitializeCpuExceptionHandlersEx.

Gang Chen (2):
  MdeModulePkg/dec: update the PcdCpuStackGuard property
  UefiCpuPkg/CpuExceptionHandlerLib: remove duplicated pcd check

 MdeModulePkg/MdeModulePkg.dec                      | 14 +++++++-------
 .../CpuExceptionHandlerLib/PeiCpuException.c       |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.35.1

[PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property

[Chen, Gang C]

Update the PcdCpuStackGuard from PcdsFixedAtBuild to PcdsDynamicEx
for the requirement to set different value in FSP API mode and FSP
Dispatch mode.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3897

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: devel@edk2.groups.io

Signed-off-by: Gang Chen <gang.c.chen@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 463e889e9a..72e7e2eced 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1070,13 +1070,6 @@
   # @Prompt The Heap Guard feature mask
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0|UINT8|0x30001054
 
-  ## Indicates if UEFI Stack Guard will be enabled.
-  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
-  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
-  #   FALSE - UEFI Stack Guard will be disabled.<BR>
-  # @Prompt Enable UEFI Stack Guard.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30001055
-
 [PcdsFixedAtBuild, PcdsPatchableInModule]
   ## Dynamic type PCD can be registered callback function for Pcd setting action.
   #  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function
@@ -2079,6 +2072,13 @@
   # @Prompt Enable PCIe Resizable BAR Capability support.
   gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport|FALSE|BOOLEAN|0x10000024
 
+  ## Indicates if UEFI Stack Guard will be enabled.
+  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
+  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
+  #   FALSE - UEFI Stack Guard will be disabled.<BR>
+  # @Prompt Enable UEFI Stack Guard.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x00010025
+
 [PcdsPatchableInModule]
   ## Specify memory size with page number for PEI code when
   #  Loading Module at Fixed Address feature is enabled.
-- 
2.35.1

Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property

[Ni, Ray]

FixedAtBuild PCD can be accessed at any time from any CPU.
But dynamic PCD can only be accessed from BSP and after PCD database is initialized.

This impact of this change is not as simple as what the patch does.

Why cannot FSP API and Dispatch mode use the same setting? (either enabled or disabled)

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chen, Gang C
> Sent: Monday, April 11, 2022 6:14 PM
> To: devel@edk2.groups.io
> Cc: Jiang, Guomin <guomin.jiang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Zhang, Di <di.zhang@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property
> 
> Update the PcdCpuStackGuard from PcdsFixedAtBuild to PcdsDynamicEx
> for the requirement to set different value in FSP API mode and FSP
> Dispatch mode.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3897
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: devel@edk2.groups.io
> 
> Signed-off-by: Gang Chen <gang.c.chen@intel.com>
> ---
>  MdeModulePkg/MdeModulePkg.dec | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index 463e889e9a..72e7e2eced 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1070,13 +1070,6 @@
>    # @Prompt The Heap Guard feature mask
>    gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0|UINT8|0x30001054
> 
> -  ## Indicates if UEFI Stack Guard will be enabled.
> -  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
> -  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
> -  #   FALSE - UEFI Stack Guard will be disabled.<BR>
> -  # @Prompt Enable UEFI Stack Guard.
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30001055
> -
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
>    ## Dynamic type PCD can be registered callback function for Pcd setting action.
>    #  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function
> @@ -2079,6 +2072,13 @@
>    # @Prompt Enable PCIe Resizable BAR Capability support.
>    gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport|FALSE|BOOLEAN|0x10000024
> 
> +  ## Indicates if UEFI Stack Guard will be enabled.
> +  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
> +  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
> +  #   FALSE - UEFI Stack Guard will be disabled.<BR>
> +  # @Prompt Enable UEFI Stack Guard.
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x00010025
> +
>  [PcdsPatchableInModule]
>    ## Specify memory size with page number for PEI code when
>    #  Loading Module at Fixed Address feature is enabled.
> --
> 2.35.1
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property

[Chen, Gang C]

This impact of this change is not as simple as what the patch does.
-->You're correct, that's why the PATCH 2/2 removed the check in AP.

Why cannot FSP API and Dispatch mode use the same setting? (either enabled or disabled)
-->For example, in coreboot, it's 32-bit, so there is no Paging needed. In this case, it needs to disable it for FSP API mode. But this feature is needed for Dispatch mode. 

@Liu, Shuo, if any information is wrong or missed, please correct me. Thanks.

Best Regards
Gang

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Monday, April 11, 2022 9:55 PM
To: devel@edk2.groups.io; Chen, Gang C <gang.c.chen@intel.com>
Cc: Jiang, Guomin <guomin.jiang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Zhang, Di <di.zhang@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: RE: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property

FixedAtBuild PCD can be accessed at any time from any CPU.
But dynamic PCD can only be accessed from BSP and after PCD database is initialized.

This impact of this change is not as simple as what the patch does.

Why cannot FSP API and Dispatch mode use the same setting? (either enabled or disabled)

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chen, 
> Gang C
> Sent: Monday, April 11, 2022 6:14 PM
> To: devel@edk2.groups.io
> Cc: Jiang, Guomin <guomin.jiang@intel.com>; Bi, Dandan 
> <dandan.bi@intel.com>; Zhang, Di <di.zhang@intel.com>; Wang, Jian J 
> <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the 
> PcdCpuStackGuard property
> 
> Update the PcdCpuStackGuard from PcdsFixedAtBuild to PcdsDynamicEx for 
> the requirement to set different value in FSP API mode and FSP 
> Dispatch mode.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3897
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: devel@edk2.groups.io
> 
> Signed-off-by: Gang Chen <gang.c.chen@intel.com>
> ---
>  MdeModulePkg/MdeModulePkg.dec | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec 
> b/MdeModulePkg/MdeModulePkg.dec index 463e889e9a..72e7e2eced 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1070,13 +1070,6 @@
>    # @Prompt The Heap Guard feature mask
>    
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0|UINT8|0x30
> 001054
> 
> -  ## Indicates if UEFI Stack Guard will be enabled.
> -  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
> -  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
> -  #   FALSE - UEFI Stack Guard will be disabled.<BR>
> -  # @Prompt Enable UEFI Stack Guard.
> -  
> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x300010
> 55
> -
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
>    ## Dynamic type PCD can be registered callback function for Pcd setting action.
>    #  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum 
> number of callback function @@ -2079,6 +2072,13 @@
>    # @Prompt Enable PCIe Resizable BAR Capability support.
>    
> gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport|FALSE|BOOLEA
> N|0x10000024
> 
> +  ## Indicates if UEFI Stack Guard will be enabled.
> +  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
> +  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
> +  #   FALSE - UEFI Stack Guard will be disabled.<BR>
> +  # @Prompt Enable UEFI Stack Guard.
> +  
> + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x0001
> + 0025
> +
>  [PcdsPatchableInModule]
>    ## Specify memory size with page number for PEI code when
>    #  Loading Module at Fixed Address feature is enabled.
> --
> 2.35.1
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property

[Jeff Fan]

[-- Attachment #1: Type: text/plain, Size: 4533 bytes --]

On 32bit protectd mode, is there any impact if PcdCpuStackGuard is set to TRUE?  Even thers is no page table enabling at that time.

Best Regards,
Jeff



fanjianfeng@byosoft.com.cn
 
From: Chen, Gang C
Date: 2022-04-11 22:51
To: Ni, Ray; devel@edk2.groups.io; Liu, Shuo
CC: Jiang, Guomin; Bi, Dandan; Zhang, Di; Wang, Jian J; Gao, Liming
Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property
This impact of this change is not as simple as what the patch does.
-->You're correct, that's why the PATCH 2/2 removed the check in AP.
 
Why cannot FSP API and Dispatch mode use the same setting? (either enabled or disabled)
-->For example, in coreboot, it's 32-bit, so there is no Paging needed. In this case, it needs to disable it for FSP API mode. But this feature is needed for Dispatch mode. 
 
@Liu, Shuo, if any information is wrong or missed, please correct me. Thanks.
 
Best Regards
Gang
 
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Monday, April 11, 2022 9:55 PM
To: devel@edk2.groups.io; Chen, Gang C <gang.c.chen@intel.com>
Cc: Jiang, Guomin <guomin.jiang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Zhang, Di <di.zhang@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: RE: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property
 
FixedAtBuild PCD can be accessed at any time from any CPU.
But dynamic PCD can only be accessed from BSP and after PCD database is initialized.
 
This impact of this change is not as simple as what the patch does.
 
Why cannot FSP API and Dispatch mode use the same setting? (either enabled or disabled)
 
Thanks,
Ray
 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chen, 
> Gang C
> Sent: Monday, April 11, 2022 6:14 PM
> To: devel@edk2.groups.io
> Cc: Jiang, Guomin <guomin.jiang@intel.com>; Bi, Dandan 
> <dandan.bi@intel.com>; Zhang, Di <di.zhang@intel.com>; Wang, Jian J 
> <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the 
> PcdCpuStackGuard property
> 
> Update the PcdCpuStackGuard from PcdsFixedAtBuild to PcdsDynamicEx for 
> the requirement to set different value in FSP API mode and FSP 
> Dispatch mode.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3897
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: devel@edk2.groups.io
> 
> Signed-off-by: Gang Chen <gang.c.chen@intel.com>
> ---
>  MdeModulePkg/MdeModulePkg.dec | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dec 
> b/MdeModulePkg/MdeModulePkg.dec index 463e889e9a..72e7e2eced 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1070,13 +1070,6 @@
>    # @Prompt The Heap Guard feature mask
>    
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0|UINT8|0x30
> 001054
> 
> -  ## Indicates if UEFI Stack Guard will be enabled.
> -  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
> -  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
> -  #   FALSE - UEFI Stack Guard will be disabled.<BR>
> -  # @Prompt Enable UEFI Stack Guard.
> -  
> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x300010
> 55
> -
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
>    ## Dynamic type PCD can be registered callback function for Pcd setting action.
>    #  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum 
> number of callback function @@ -2079,6 +2072,13 @@
>    # @Prompt Enable PCIe Resizable BAR Capability support.
>    
> gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport|FALSE|BOOLEA
> N|0x10000024
> 
> +  ## Indicates if UEFI Stack Guard will be enabled.
> +  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
> +  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
> +  #   FALSE - UEFI Stack Guard will be disabled.<BR>
> +  # @Prompt Enable UEFI Stack Guard.
> +  
> + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x0001
> + 0025
> +
>  [PcdsPatchableInModule]
>    ## Specify memory size with page number for PEI code when
>    #  Loading Module at Fixed Address feature is enabled.
> --
> 2.35.1
> 
> 
> 
> 
> 
 
 
 

 
 
 

[-- Attachment #2: Type: text/html, Size: 7948 bytes --]

Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property

[Jeff Fan]

[-- Attachment #1: Type: text/plain, Size: 2586 bytes --]

Please correct the commit message which confused me.

It should be "Update the PcdCpuStackGuard to support PcdsPatchableInModule, PcdsDynamci and PcdsDynamciEx ... ..."

Thanks!
Jeff


fanjianfeng@byosoft.com.cn
 
From: Chen, Gang C
Date: 2022-04-11 18:13
To: devel
CC: guomin.jiang; dandan.bi; di.zhang; Jian J Wang; Liming Gao
Subject: [edk2-devel] [PATCH 1/2] MdeModulePkg/dec: update the PcdCpuStackGuard property
Update the PcdCpuStackGuard from PcdsFixedAtBuild to PcdsDynamicEx
for the requirement to set different value in FSP API mode and FSP
Dispatch mode.
 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3897
 
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: devel@edk2.groups.io
 
Signed-off-by: Gang Chen <gang.c.chen@intel.com>
---
MdeModulePkg/MdeModulePkg.dec | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
 
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 463e889e9a..72e7e2eced 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1070,13 +1070,6 @@
   # @Prompt The Heap Guard feature mask
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0|UINT8|0x30001054
-  ## Indicates if UEFI Stack Guard will be enabled.
-  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
-  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
-  #   FALSE - UEFI Stack Guard will be disabled.<BR>
-  # @Prompt Enable UEFI Stack Guard.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30001055
-
[PcdsFixedAtBuild, PcdsPatchableInModule]
   ## Dynamic type PCD can be registered callback function for Pcd setting action.
   #  PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function
@@ -2079,6 +2072,13 @@
   # @Prompt Enable PCIe Resizable BAR Capability support.
   gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport|FALSE|BOOLEAN|0x10000024
+  ## Indicates if UEFI Stack Guard will be enabled.
+  #  If enabled, stack overflow in UEFI can be caught, preventing chaotic consequences.<BR><BR>
+  #   TRUE  - UEFI Stack Guard will be enabled.<BR>
+  #   FALSE - UEFI Stack Guard will be disabled.<BR>
+  # @Prompt Enable UEFI Stack Guard.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x00010025
+
[PcdsPatchableInModule]
   ## Specify memory size with page number for PEI code when
   #  Loading Module at Fixed Address feature is enabled.
-- 
2.35.1
 
 
 

 
 
 

[-- Attachment #2: Type: text/html, Size: 5094 bytes --]

[PATCH 2/2] UefiCpuPkg/CpuExceptionHandlerLib: remove duplicated pcd check

[Chen, Gang C]

Remove the duplicated PcdCpuStackGuard check for it has been checked at
the entry of InitializeMpExceptionStackSwitchHandlers, which will invoke
the current function.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3897
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: devel@edk2.groups.io

Signed-off-by: Gang Chen <gang.c.chen@intel.com>
---
 UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
index 687fc4177f..bcd4175ffa 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
@@ -254,7 +254,7 @@ InitializeCpuExceptionHandlersEx (
     //
     // Initializing stack switch is only necessary for Stack Guard functionality.
     //
-    if (PcdGetBool (PcdCpuStackGuard) && (InitData != NULL)) {
+    if (InitData != NULL) {
       Status = ArchSetupExceptionStack (InitData);
     }
   }
-- 
2.35.1