* [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory executable @ 2018-08-08 5:27 Ruiyu Ni 2018-08-08 7:04 ` Wang, Jian J 0 siblings, 1 reply; 4+ messages in thread From: Ruiyu Ni @ 2018-08-08 5:27 UTC (permalink / raw) To: edk2-devel; +Cc: Star Zeng, Laszlo Ersek, Jian Wang Commit b22a62be5cdc8fd19d87ec1ecfa5b28fb9be50ad * IntelFrameworkModule/LegacyBios:Use reserved memory for legacy data allocates reserved memory for holding legacy code/data. But with PcdDxeNxMemoryProtectionPolicy set to certain value to forbid execution when code is in certain type of memory, it's possible that a platform forbids execution when code is in reserved memory. The patch calls GCD service to allow such case otherwise CPU exception may occur. Code execution in BSCode area should be enabled by platform by default. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Jian Wang <jian.j.wang@intel.com> --- .../Csm/LegacyBiosDxe/LegacyBios.c | 28 ++++++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c index 8f14687b28..80efe40489 100644 --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c @@ -64,8 +64,9 @@ AllocateLegacyMemory ( OUT EFI_PHYSICAL_ADDRESS *Result ) { - EFI_STATUS Status; - EFI_PHYSICAL_ADDRESS MemPage; + EFI_STATUS Status; + EFI_PHYSICAL_ADDRESS MemPage; + EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc; // // Allocate Pages of memory less <= StartPageAddress @@ -81,12 +82,29 @@ AllocateLegacyMemory ( // Do not ASSERT on Status error but let caller decide since some cases // memory is already taken but that is ok. // + if (!EFI_ERROR (Status)) { + if (MemoryType != EfiBootServicesCode) { + // + // Make sure that the buffer can be used to store code. + // + Status = gDS->GetMemorySpaceDescriptor (MemPage, &MemDesc); + if (!EFI_ERROR (Status) && (MemDesc.Attributes & EFI_MEMORY_XP) != 0) { + Status = gDS->SetMemorySpaceAttributes ( + MemPage, + EFI_PAGES_TO_SIZE (Pages), + MemDesc.Attributes & (~EFI_MEMORY_XP) + ); + } + if (EFI_ERROR (Status)) { + gBS->FreePages (MemPage, Pages); + } + } + } + if (!EFI_ERROR (Status)) { *Result = (EFI_PHYSICAL_ADDRESS) (UINTN) MemPage; } - // - // If reach here the status = EFI_SUCCESS - // + return Status; } -- 2.16.1.windows.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory executable 2018-08-08 5:27 [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory executable Ruiyu Ni @ 2018-08-08 7:04 ` Wang, Jian J 2018-08-08 11:06 ` Laszlo Ersek 0 siblings, 1 reply; 4+ messages in thread From: Wang, Jian J @ 2018-08-08 7:04 UTC (permalink / raw) To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Zeng, Star, Laszlo Ersek Hi Ruiyu, In function description comments, the @retval doesn't match the changes in this patch. With those changes, Reviewed-by: Jian J Wang <jian.j.wang@intel.com> Regards, Jian > -----Original Message----- > From: Ni, Ruiyu > Sent: Wednesday, August 08, 2018 1:28 PM > To: edk2-devel@lists.01.org > Cc: Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; > Wang, Jian J <jian.j.wang@intel.com> > Subject: [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory > executable > > Commit b22a62be5cdc8fd19d87ec1ecfa5b28fb9be50ad > * IntelFrameworkModule/LegacyBios:Use reserved memory for legacy data > allocates reserved memory for holding legacy code/data. > > But with PcdDxeNxMemoryProtectionPolicy set to certain value to > forbid execution when code is in certain type of memory, it's > possible that a platform forbids execution when code is in reserved > memory. The patch calls GCD service to allow such case otherwise > CPU exception may occur. > > Code execution in BSCode area should be enabled by platform by > default. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> > Cc: Star Zeng <star.zeng@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Jian Wang <jian.j.wang@intel.com> > --- > .../Csm/LegacyBiosDxe/LegacyBios.c | 28 ++++++++++++++++++---- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c > b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c > index 8f14687b28..80efe40489 100644 > --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c > +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c > @@ -64,8 +64,9 @@ AllocateLegacyMemory ( > OUT EFI_PHYSICAL_ADDRESS *Result > ) > { > - EFI_STATUS Status; > - EFI_PHYSICAL_ADDRESS MemPage; > + EFI_STATUS Status; > + EFI_PHYSICAL_ADDRESS MemPage; > + EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc; > > // > // Allocate Pages of memory less <= StartPageAddress > @@ -81,12 +82,29 @@ AllocateLegacyMemory ( > // Do not ASSERT on Status error but let caller decide since some cases > // memory is already taken but that is ok. > // > + if (!EFI_ERROR (Status)) { > + if (MemoryType != EfiBootServicesCode) { > + // > + // Make sure that the buffer can be used to store code. > + // > + Status = gDS->GetMemorySpaceDescriptor (MemPage, &MemDesc); > + if (!EFI_ERROR (Status) && (MemDesc.Attributes & EFI_MEMORY_XP) != 0) { > + Status = gDS->SetMemorySpaceAttributes ( > + MemPage, > + EFI_PAGES_TO_SIZE (Pages), > + MemDesc.Attributes & (~EFI_MEMORY_XP) > + ); > + } > + if (EFI_ERROR (Status)) { > + gBS->FreePages (MemPage, Pages); > + } > + } > + } > + > if (!EFI_ERROR (Status)) { > *Result = (EFI_PHYSICAL_ADDRESS) (UINTN) MemPage; > } > - // > - // If reach here the status = EFI_SUCCESS > - // > + > return Status; > } > > -- > 2.16.1.windows.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory executable 2018-08-08 7:04 ` Wang, Jian J @ 2018-08-08 11:06 ` Laszlo Ersek 2018-08-08 11:37 ` Zeng, Star 0 siblings, 1 reply; 4+ messages in thread From: Laszlo Ersek @ 2018-08-08 11:06 UTC (permalink / raw) To: Wang, Jian J, Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Zeng, Star On 08/08/18 09:04, Wang, Jian J wrote: > Hi Ruiyu, > > In function description comments, the @retval doesn't match the changes in this patch. > > With those changes, > Reviewed-by: Jian J Wang <jian.j.wang@intel.com> Same from my side, Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo >> -----Original Message----- >> From: Ni, Ruiyu >> Sent: Wednesday, August 08, 2018 1:28 PM >> To: edk2-devel@lists.01.org >> Cc: Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; >> Wang, Jian J <jian.j.wang@intel.com> >> Subject: [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory >> executable >> >> Commit b22a62be5cdc8fd19d87ec1ecfa5b28fb9be50ad >> * IntelFrameworkModule/LegacyBios:Use reserved memory for legacy data >> allocates reserved memory for holding legacy code/data. >> >> But with PcdDxeNxMemoryProtectionPolicy set to certain value to >> forbid execution when code is in certain type of memory, it's >> possible that a platform forbids execution when code is in reserved >> memory. The patch calls GCD service to allow such case otherwise >> CPU exception may occur. >> >> Code execution in BSCode area should be enabled by platform by >> default. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> >> Cc: Star Zeng <star.zeng@intel.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Jian Wang <jian.j.wang@intel.com> >> --- >> .../Csm/LegacyBiosDxe/LegacyBios.c | 28 ++++++++++++++++++---- >> 1 file changed, 23 insertions(+), 5 deletions(-) >> >> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c >> b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c >> index 8f14687b28..80efe40489 100644 >> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c >> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c >> @@ -64,8 +64,9 @@ AllocateLegacyMemory ( >> OUT EFI_PHYSICAL_ADDRESS *Result >> ) >> { >> - EFI_STATUS Status; >> - EFI_PHYSICAL_ADDRESS MemPage; >> + EFI_STATUS Status; >> + EFI_PHYSICAL_ADDRESS MemPage; >> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc; >> >> // >> // Allocate Pages of memory less <= StartPageAddress >> @@ -81,12 +82,29 @@ AllocateLegacyMemory ( >> // Do not ASSERT on Status error but let caller decide since some cases >> // memory is already taken but that is ok. >> // >> + if (!EFI_ERROR (Status)) { >> + if (MemoryType != EfiBootServicesCode) { >> + // >> + // Make sure that the buffer can be used to store code. >> + // >> + Status = gDS->GetMemorySpaceDescriptor (MemPage, &MemDesc); >> + if (!EFI_ERROR (Status) && (MemDesc.Attributes & EFI_MEMORY_XP) != 0) { >> + Status = gDS->SetMemorySpaceAttributes ( >> + MemPage, >> + EFI_PAGES_TO_SIZE (Pages), >> + MemDesc.Attributes & (~EFI_MEMORY_XP) >> + ); >> + } >> + if (EFI_ERROR (Status)) { >> + gBS->FreePages (MemPage, Pages); >> + } >> + } >> + } >> + >> if (!EFI_ERROR (Status)) { >> *Result = (EFI_PHYSICAL_ADDRESS) (UINTN) MemPage; >> } >> - // >> - // If reach here the status = EFI_SUCCESS >> - // >> + >> return Status; >> } >> >> -- >> 2.16.1.windows.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory executable 2018-08-08 11:06 ` Laszlo Ersek @ 2018-08-08 11:37 ` Zeng, Star 0 siblings, 0 replies; 4+ messages in thread From: Zeng, Star @ 2018-08-08 11:37 UTC (permalink / raw) To: Laszlo Ersek, Wang, Jian J, Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Zeng, Star The function description comments should be not related to this patch. I am ok with or without updating the comments in this patch. Reviewed-by: Star Zeng <star.zeng@intel.com>. Thanks, Star -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Wednesday, August 8, 2018 7:06 PM To: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org Cc: Zeng, Star <star.zeng@intel.com> Subject: Re: [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory executable On 08/08/18 09:04, Wang, Jian J wrote: > Hi Ruiyu, > > In function description comments, the @retval doesn't match the changes in this patch. > > With those changes, > Reviewed-by: Jian J Wang <jian.j.wang@intel.com> Same from my side, Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo >> -----Original Message----- >> From: Ni, Ruiyu >> Sent: Wednesday, August 08, 2018 1:28 PM >> To: edk2-devel@lists.01.org >> Cc: Zeng, Star <star.zeng@intel.com>; Laszlo Ersek >> <lersek@redhat.com>; Wang, Jian J <jian.j.wang@intel.com> >> Subject: [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory >> executable >> >> Commit b22a62be5cdc8fd19d87ec1ecfa5b28fb9be50ad >> * IntelFrameworkModule/LegacyBios:Use reserved memory for legacy data >> allocates reserved memory for holding legacy code/data. >> >> But with PcdDxeNxMemoryProtectionPolicy set to certain value to >> forbid execution when code is in certain type of memory, it's >> possible that a platform forbids execution when code is in reserved >> memory. The patch calls GCD service to allow such case otherwise CPU >> exception may occur. >> >> Code execution in BSCode area should be enabled by platform by >> default. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> >> Cc: Star Zeng <star.zeng@intel.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Jian Wang <jian.j.wang@intel.com> >> --- >> .../Csm/LegacyBiosDxe/LegacyBios.c | 28 ++++++++++++++++++---- >> 1 file changed, 23 insertions(+), 5 deletions(-) >> >> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c >> b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c >> index 8f14687b28..80efe40489 100644 >> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c >> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c >> @@ -64,8 +64,9 @@ AllocateLegacyMemory ( >> OUT EFI_PHYSICAL_ADDRESS *Result >> ) >> { >> - EFI_STATUS Status; >> - EFI_PHYSICAL_ADDRESS MemPage; >> + EFI_STATUS Status; >> + EFI_PHYSICAL_ADDRESS MemPage; >> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc; >> >> // >> // Allocate Pages of memory less <= StartPageAddress @@ -81,12 >> +82,29 @@ AllocateLegacyMemory ( >> // Do not ASSERT on Status error but let caller decide since some cases >> // memory is already taken but that is ok. >> // >> + if (!EFI_ERROR (Status)) { >> + if (MemoryType != EfiBootServicesCode) { >> + // >> + // Make sure that the buffer can be used to store code. >> + // >> + Status = gDS->GetMemorySpaceDescriptor (MemPage, &MemDesc); >> + if (!EFI_ERROR (Status) && (MemDesc.Attributes & EFI_MEMORY_XP) != 0) { >> + Status = gDS->SetMemorySpaceAttributes ( >> + MemPage, >> + EFI_PAGES_TO_SIZE (Pages), >> + MemDesc.Attributes & (~EFI_MEMORY_XP) >> + ); >> + } >> + if (EFI_ERROR (Status)) { >> + gBS->FreePages (MemPage, Pages); >> + } >> + } >> + } >> + >> if (!EFI_ERROR (Status)) { >> *Result = (EFI_PHYSICAL_ADDRESS) (UINTN) MemPage; >> } >> - // >> - // If reach here the status = EFI_SUCCESS >> - // >> + >> return Status; >> } >> >> -- >> 2.16.1.windows.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-08-08 11:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-08 5:27 [PATCH v3] IntelFrameworkModulePkg/Csm: Set CSM memory executable Ruiyu Ni 2018-08-08 7:04 ` Wang, Jian J 2018-08-08 11:06 ` Laszlo Ersek 2018-08-08 11:37 ` Zeng, Star
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox