On 10/23/17 08:50, Jian J Wang wrote:
> More than one entry of RT_CODE memory might cause boot problem for some
> old OSs. This patch will fix this issue to keep OS compatibility as much
> as possible.
>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.w...@intel.com>
> ---
> UefiCpuPkg/CpuDxe/CpuPageTable.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
Can you please explain in the commit message; what OSes are affected by
this issue, to your knowledge?
Also, the code being patched seems to originate from commit c1cab54ce57c
("UefiCpuPkg/CpuDxe: Fix out-of-sync issue in page attributes",
2017-09-16). I vaguely recall that this commit was related to your "page
0 protection" work.
Can you please explain, in the commit message, under what circumstances
(PCD settings etc) the issue arises, and how we end up with multiple
RT_CODE entries in the memory map?
(BTW, multiple RT_CODE entries in the memmap should be perfectly
normal... So I'm extra curious about the OSes that are not compatible
with that.)
Thanks,
Laszlo
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index d312eb66f8..0802464b9d 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -829,6 +829,15 @@ RefreshGcdMemoryAttributesFromPaging (
> // Sync real page attributes to GCD
> BaseAddress = MemorySpaceMap[Index].BaseAddress;
> MemorySpaceLength = MemorySpaceMap[Index].Length;
> + Capabilities = MemorySpaceMap[Index].Capabilities |
> + EFI_MEMORY_PAGETYPE_MASK;
> + Status = gDS->SetMemorySpaceCapabilities (
> + BaseAddress,
> + MemorySpaceLength,
> + Capabilities
> + );
> + ASSERT_EFI_ERROR (Status);
> +
> while (MemorySpaceLength > 0) {
> if (PageLength == 0) {
> PageEntry = GetPageTableEntry (&PagingContext, BaseAddress,
> &PageAttribute);
> @@ -846,7 +855,6 @@ RefreshGcdMemoryAttributesFromPaging (
> if (Attributes != (MemorySpaceMap[Index].Attributes &
> EFI_MEMORY_PAGETYPE_MASK)) {
> DoUpdate = TRUE;
> Attributes |= (MemorySpaceMap[Index].Attributes &
> ~EFI_MEMORY_PAGETYPE_MASK);
> - Capabilities = Attributes | MemorySpaceMap[Index].Capabilities;
> } else {
> DoUpdate = FALSE;
> }
> @@ -854,8 +862,8 @@ RefreshGcdMemoryAttributesFromPaging (
>
> Length = MIN (PageLength, MemorySpaceLength);
> if (DoUpdate) {
> - gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Capabilities);
> - gDS->SetMemorySpaceAttributes (BaseAddress, Length, Attributes);
> + Status = gDS->SetMemorySpaceAttributes (BaseAddress, Length,
> Attributes);
> + ASSERT_EFI_ERROR (Status);
> DEBUG ((DEBUG_INFO, "Update memory space attribute: [%02d] %016lx -
> %016lx (%08lx -> %08lx)\r\n",
> Index, BaseAddress, BaseAddress + Length - 1,
> MemorySpaceMap[Index].Attributes, Attributes));
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
