* [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page setup for APs @ 2017-12-21 1:27 Jian J Wang 2017-12-22 4:26 ` Wang, Jian J 0 siblings, 1 reply; 4+ messages in thread From: Jian J Wang @ 2017-12-21 1:27 UTC (permalink / raw) To: edk2-devel; +Cc: Jiewen Yao, Eric Dong, Laszlo Ersek AP has its own stack for code execution. If PcdCpuStackGuard is enabled, the page at the bottom of stack of AP will be disabled (NOT PRESENT) to monitor the stack overflow issue. This requires PcdCpuApStackSize to be set with value more than one page of memory. Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.wang@intel.com> --- UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 34 ++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf index 805641b516..e7b9eb4462 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf @@ -73,4 +73,5 @@ gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## SOMETIMES_CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c index 479f8189f6..40c1bf407a 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c @@ -17,6 +17,7 @@ #include <Library/UefiLib.h> #include <Library/UefiBootServicesTableLib.h> #include <Library/DebugAgentLib.h> +#include <Library/DxeServicesTableLib.h> #include <Protocol/Timer.h> @@ -288,9 +289,12 @@ InitMpGlobalData ( IN CPU_MP_DATA *CpuMpData ) { - EFI_STATUS Status; - EFI_PHYSICAL_ADDRESS Address; - UINTN ApSafeBufferSize; + EFI_STATUS Status; + EFI_PHYSICAL_ADDRESS Address; + UINTN ApSafeBufferSize; + UINTN Index; + EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc; + UINTN StackBase; SaveCpuMpData (CpuMpData); @@ -301,6 +305,30 @@ InitMpGlobalData ( return; } + if (PcdGetBool (PcdCpuStackGuard)) { + // + // One extra page at the bottom of the stack is needed for Guard page. + // + if (CpuMpData->CpuApStackSize <= EFI_PAGE_SIZE) { + DEBUG ((DEBUG_ERROR, "PcdCpuApStackSize is not big enough for Stack Guard!\n")); + ASSERT (FALSE); + } + + for (Index = 0; Index < CpuMpData->CpuCount; ++Index) { + StackBase = CpuMpData->Buffer + Index * CpuMpData->CpuApStackSize; + + Status = gDS->GetMemorySpaceDescriptor (StackBase, &MemDesc); + ASSERT_EFI_ERROR (Status); + + Status = gDS->SetMemorySpaceAttributes ( + StackBase, + EFI_PAGES_TO_SIZE (1), + MemDesc.Attributes | EFI_MEMORY_RP + ); + ASSERT_EFI_ERROR (Status); + } + } + // // Avoid APs access invalid buffer data which allocated by BootServices, // so we will allocate reserved data for AP loop code. We also need to -- 2.15.1.windows.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page setup for APs 2017-12-21 1:27 [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page setup for APs Jian J Wang @ 2017-12-22 4:26 ` Wang, Jian J 2017-12-22 5:22 ` Yao, Jiewen 0 siblings, 1 reply; 4+ messages in thread From: Wang, Jian J @ 2017-12-22 4:26 UTC (permalink / raw) To: Wang, Jian J, edk2-devel@lists.01.org Cc: Laszlo Ersek, Yao, Jiewen, Dong, Eric Hi, Anyone has any comments on this? Regards, Jian > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J > Wang > Sent: Thursday, December 21, 2017 9:27 AM > To: edk2-devel@lists.01.org > Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; > Dong, Eric <eric.dong@intel.com> > Subject: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page setup > for APs > > AP has its own stack for code execution. If PcdCpuStackGuard is enabled, > the page at the bottom of stack of AP will be disabled (NOT PRESENT) to > monitor the stack overflow issue. This requires PcdCpuApStackSize to be > set with value more than one page of memory. > > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang <jian.j.wang@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 34 > ++++++++++++++++++++++++--- > 2 files changed, 32 insertions(+), 3 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > index 805641b516..e7b9eb4462 100644 > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > @@ -73,4 +73,5 @@ > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## > SOMETIMES_CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## > CONSUMES > > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > index 479f8189f6..40c1bf407a 100644 > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > @@ -17,6 +17,7 @@ > #include <Library/UefiLib.h> > #include <Library/UefiBootServicesTableLib.h> > #include <Library/DebugAgentLib.h> > +#include <Library/DxeServicesTableLib.h> > > #include <Protocol/Timer.h> > > @@ -288,9 +289,12 @@ InitMpGlobalData ( > IN CPU_MP_DATA *CpuMpData > ) > { > - EFI_STATUS Status; > - EFI_PHYSICAL_ADDRESS Address; > - UINTN ApSafeBufferSize; > + EFI_STATUS Status; > + EFI_PHYSICAL_ADDRESS Address; > + UINTN ApSafeBufferSize; > + UINTN Index; > + EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc; > + UINTN StackBase; > > SaveCpuMpData (CpuMpData); > > @@ -301,6 +305,30 @@ InitMpGlobalData ( > return; > } > > + if (PcdGetBool (PcdCpuStackGuard)) { > + // > + // One extra page at the bottom of the stack is needed for Guard page. > + // > + if (CpuMpData->CpuApStackSize <= EFI_PAGE_SIZE) { > + DEBUG ((DEBUG_ERROR, "PcdCpuApStackSize is not big enough for Stack > Guard!\n")); > + ASSERT (FALSE); > + } > + > + for (Index = 0; Index < CpuMpData->CpuCount; ++Index) { > + StackBase = CpuMpData->Buffer + Index * CpuMpData->CpuApStackSize; > + > + Status = gDS->GetMemorySpaceDescriptor (StackBase, &MemDesc); > + ASSERT_EFI_ERROR (Status); > + > + Status = gDS->SetMemorySpaceAttributes ( > + StackBase, > + EFI_PAGES_TO_SIZE (1), > + MemDesc.Attributes | EFI_MEMORY_RP > + ); > + ASSERT_EFI_ERROR (Status); > + } > + } > + > // > // Avoid APs access invalid buffer data which allocated by BootServices, > // so we will allocate reserved data for AP loop code. We also need to > -- > 2.15.1.windows.2 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page setup for APs 2017-12-22 4:26 ` Wang, Jian J @ 2017-12-22 5:22 ` Yao, Jiewen 2017-12-22 5:35 ` Wang, Jian J 0 siblings, 1 reply; 4+ messages in thread From: Yao, Jiewen @ 2017-12-22 5:22 UTC (permalink / raw) To: Wang, Jian J, edk2-devel@lists.01.org; +Cc: Laszlo Ersek, Dong, Eric I do not think we have AP guard page before, right? So I suggest we change commit message to be: Add missing Guard page setup for APs. With commit message update, reviewed-by: Jiewen.yao@intel.com > -----Original Message----- > From: Wang, Jian J > Sent: Friday, December 22, 2017 12:26 PM > To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org > Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; > Dong, Eric <eric.dong@intel.com> > Subject: RE: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page > setup for APs > > Hi, > > Anyone has any comments on this? > > Regards, > Jian > > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian > J > > Wang > > Sent: Thursday, December 21, 2017 9:27 AM > > To: edk2-devel@lists.01.org > > Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; > > Dong, Eric <eric.dong@intel.com> > > Subject: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page setup > > for APs > > > > AP has its own stack for code execution. If PcdCpuStackGuard is enabled, > > the page at the bottom of stack of AP will be disabled (NOT PRESENT) to > > monitor the stack overflow issue. This requires PcdCpuApStackSize to be > > set with value more than one page of memory. > > > > Cc: Jiewen Yao <jiewen.yao@intel.com> > > Cc: Eric Dong <eric.dong@intel.com> > > Cc: Laszlo Ersek <lersek@redhat.com> > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang <jian.j.wang@intel.com> > > --- > > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + > > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 34 > > ++++++++++++++++++++++++--- > > 2 files changed, 32 insertions(+), 3 deletions(-) > > > > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > > b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > > index 805641b516..e7b9eb4462 100644 > > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > > @@ -73,4 +73,5 @@ > > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize > ## > > CONSUMES > > gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode > ## CONSUMES > > gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate > ## > > SOMETIMES_CONSUMES > > + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard > ## > > CONSUMES > > > > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > > b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > > index 479f8189f6..40c1bf407a 100644 > > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > > @@ -17,6 +17,7 @@ > > #include <Library/UefiLib.h> > > #include <Library/UefiBootServicesTableLib.h> > > #include <Library/DebugAgentLib.h> > > +#include <Library/DxeServicesTableLib.h> > > > > #include <Protocol/Timer.h> > > > > @@ -288,9 +289,12 @@ InitMpGlobalData ( > > IN CPU_MP_DATA *CpuMpData > > ) > > { > > - EFI_STATUS Status; > > - EFI_PHYSICAL_ADDRESS Address; > > - UINTN ApSafeBufferSize; > > + EFI_STATUS Status; > > + EFI_PHYSICAL_ADDRESS Address; > > + UINTN ApSafeBufferSize; > > + UINTN Index; > > + EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc; > > + UINTN StackBase; > > > > SaveCpuMpData (CpuMpData); > > > > @@ -301,6 +305,30 @@ InitMpGlobalData ( > > return; > > } > > > > + if (PcdGetBool (PcdCpuStackGuard)) { > > + // > > + // One extra page at the bottom of the stack is needed for Guard page. > > + // > > + if (CpuMpData->CpuApStackSize <= EFI_PAGE_SIZE) { > > + DEBUG ((DEBUG_ERROR, "PcdCpuApStackSize is not big enough for > Stack > > Guard!\n")); > > + ASSERT (FALSE); > > + } > > + > > + for (Index = 0; Index < CpuMpData->CpuCount; ++Index) { > > + StackBase = CpuMpData->Buffer + Index * > CpuMpData->CpuApStackSize; > > + > > + Status = gDS->GetMemorySpaceDescriptor (StackBase, &MemDesc); > > + ASSERT_EFI_ERROR (Status); > > + > > + Status = gDS->SetMemorySpaceAttributes ( > > + StackBase, > > + EFI_PAGES_TO_SIZE (1), > > + MemDesc.Attributes | EFI_MEMORY_RP > > + ); > > + ASSERT_EFI_ERROR (Status); > > + } > > + } > > + > > // > > // Avoid APs access invalid buffer data which allocated by BootServices, > > // so we will allocate reserved data for AP loop code. We also need to > > -- > > 2.15.1.windows.2 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page setup for APs 2017-12-22 5:22 ` Yao, Jiewen @ 2017-12-22 5:35 ` Wang, Jian J 0 siblings, 0 replies; 4+ messages in thread From: Wang, Jian J @ 2017-12-22 5:35 UTC (permalink / raw) To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Laszlo Ersek, Dong, Eric You're right. It will be updated to use more accurate wording. Thanks for catching it. Regards, Jian > -----Original Message----- > From: Yao, Jiewen > Sent: Friday, December 22, 2017 1:22 PM > To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org > Cc: Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com> > Subject: RE: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page > setup for APs > > I do not think we have AP guard page before, right? > So I suggest we change commit message to be: Add missing Guard page setup > for APs. > > With commit message update, reviewed-by: Jiewen.yao@intel.com > > > > -----Original Message----- > > From: Wang, Jian J > > Sent: Friday, December 22, 2017 12:26 PM > > To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org > > Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; > > Dong, Eric <eric.dong@intel.com> > > Subject: RE: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page > > setup for APs > > > > Hi, > > > > Anyone has any comments on this? > > > > Regards, > > Jian > > > > > > > -----Original Message----- > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Jian > > J > > > Wang > > > Sent: Thursday, December 21, 2017 9:27 AM > > > To: edk2-devel@lists.01.org > > > Cc: Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen > <jiewen.yao@intel.com>; > > > Dong, Eric <eric.dong@intel.com> > > > Subject: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page > setup > > > for APs > > > > > > AP has its own stack for code execution. If PcdCpuStackGuard is enabled, > > > the page at the bottom of stack of AP will be disabled (NOT PRESENT) to > > > monitor the stack overflow issue. This requires PcdCpuApStackSize to be > > > set with value more than one page of memory. > > > > > > Cc: Jiewen Yao <jiewen.yao@intel.com> > > > Cc: Eric Dong <eric.dong@intel.com> > > > Cc: Laszlo Ersek <lersek@redhat.com> > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Jian J Wang <jian.j.wang@intel.com> > > > --- > > > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + > > > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 34 > > > ++++++++++++++++++++++++--- > > > 2 files changed, 32 insertions(+), 3 deletions(-) > > > > > > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > > > b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > > > index 805641b516..e7b9eb4462 100644 > > > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > > > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > > > @@ -73,4 +73,5 @@ > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize > > ## > > > CONSUMES > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode > > ## CONSUMES > > > gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate > > ## > > > SOMETIMES_CONSUMES > > > + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard > > ## > > > CONSUMES > > > > > > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > > > b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > > > index 479f8189f6..40c1bf407a 100644 > > > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > > > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c > > > @@ -17,6 +17,7 @@ > > > #include <Library/UefiLib.h> > > > #include <Library/UefiBootServicesTableLib.h> > > > #include <Library/DebugAgentLib.h> > > > +#include <Library/DxeServicesTableLib.h> > > > > > > #include <Protocol/Timer.h> > > > > > > @@ -288,9 +289,12 @@ InitMpGlobalData ( > > > IN CPU_MP_DATA *CpuMpData > > > ) > > > { > > > - EFI_STATUS Status; > > > - EFI_PHYSICAL_ADDRESS Address; > > > - UINTN ApSafeBufferSize; > > > + EFI_STATUS Status; > > > + EFI_PHYSICAL_ADDRESS Address; > > > + UINTN ApSafeBufferSize; > > > + UINTN Index; > > > + EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc; > > > + UINTN StackBase; > > > > > > SaveCpuMpData (CpuMpData); > > > > > > @@ -301,6 +305,30 @@ InitMpGlobalData ( > > > return; > > > } > > > > > > + if (PcdGetBool (PcdCpuStackGuard)) { > > > + // > > > + // One extra page at the bottom of the stack is needed for Guard page. > > > + // > > > + if (CpuMpData->CpuApStackSize <= EFI_PAGE_SIZE) { > > > + DEBUG ((DEBUG_ERROR, "PcdCpuApStackSize is not big enough for > > Stack > > > Guard!\n")); > > > + ASSERT (FALSE); > > > + } > > > + > > > + for (Index = 0; Index < CpuMpData->CpuCount; ++Index) { > > > + StackBase = CpuMpData->Buffer + Index * > > CpuMpData->CpuApStackSize; > > > + > > > + Status = gDS->GetMemorySpaceDescriptor (StackBase, &MemDesc); > > > + ASSERT_EFI_ERROR (Status); > > > + > > > + Status = gDS->SetMemorySpaceAttributes ( > > > + StackBase, > > > + EFI_PAGES_TO_SIZE (1), > > > + MemDesc.Attributes | EFI_MEMORY_RP > > > + ); > > > + ASSERT_EFI_ERROR (Status); > > > + } > > > + } > > > + > > > // > > > // Avoid APs access invalid buffer data which allocated by BootServices, > > > // so we will allocate reserved data for AP loop code. We also need to > > > -- > > > 2.15.1.windows.2 > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.01.org > > > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-22 5:30 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-21 1:27 [PATCH] UefiCpuPkg/MpInitLib: Fix incorrect Guard page setup for APs Jian J Wang 2017-12-22 4:26 ` Wang, Jian J 2017-12-22 5:22 ` Yao, Jiewen 2017-12-22 5:35 ` Wang, Jian J
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox