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.w...@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.z...@intel.com>; Laszlo Ersek <ler...@redhat.com>;
> Wang, Jian J <jian.j.w...@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...@intel.com>
> Cc: Star Zeng <star.z...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Cc: Jian Wang <jian.j.w...@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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
