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 <[email protected]>.
Thanks, Star -----Original Message----- From: Laszlo Ersek [mailto:[email protected]] Sent: Wednesday, August 8, 2018 7:06 PM To: Wang, Jian J <[email protected]>; Ni, Ruiyu <[email protected]>; [email protected] Cc: Zeng, Star <[email protected]> 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 <[email protected]> Same from my side, Acked-by: Laszlo Ersek <[email protected]> Thanks! Laszlo >> -----Original Message----- >> From: Ni, Ruiyu >> Sent: Wednesday, August 08, 2018 1:28 PM >> To: [email protected] >> Cc: Zeng, Star <[email protected]>; Laszlo Ersek >> <[email protected]>; Wang, Jian J <[email protected]> >> 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 <[email protected]> >> Cc: Star Zeng <[email protected]> >> Cc: Laszlo Ersek <[email protected]> >> Cc: Jian Wang <[email protected]> >> --- >> .../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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
