Hi, Do you have further concern to this patch and have a RB? If no, I am happy to help push this patch with my AB.
Thanks, Star -----Original Message----- From: Wang, Jian J Sent: Tuesday, November 7, 2017 8:55 AM To: Zeng, Star <star.z...@intel.com>; edk2-devel@lists.01.org Cc: Laszlo Ersek <ler...@redhat.com>; Yao, Jiewen <jiewen....@intel.com>; Dong, Eric <eric.d...@intel.com> Subject: RE: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map Thanks for the review. And I agree that GCD.SetMemoryAttributes should be used all the time in DxeCore. Let's fix it in another patch. > -----Original Message----- > From: Zeng, Star > Sent: Monday, November 06, 2017 5:16 PM > To: Wang, Jian J <jian.j.w...@intel.com>; edk2-devel@lists.01.org > Cc: Laszlo Ersek <ler...@redhat.com>; Yao, Jiewen > <jiewen....@intel.com>; Dong, Eric <eric.d...@intel.com>; Zeng, Star > <star.z...@intel.com> > Subject: RE: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries > of RT_CODE in memory map > > I am ok to this code approach. > > Acknowledged-by: Star Zeng <star.z...@intel.com> > > Besides, I think except GCD services, DxeCore should not call gCpu- > >SetMemoryAttributes() directly, for example > >ApplyMemoryProtectionPolicy(), it > should have no assumption of the CPU code (to set capabilities), and > call GCD setcapabilities first, and then call GCD setattributes since > 14dde9e903bb9a719ebb8f3381da72b19509bc36 "MdeModulePkg/Core: Fix out- > of-sync issue in GCD", otherwise there may be mismatch of page > attributes between GCD and gCPU after > RefreshGcdMemoryAttributesFromPaging() by the calling > gCpu->SetMemoryAttributes() in ApplyMemoryProtectionPolicy(). > > Anyway, that could be in a separated patch. :) > > Thanks, > Star > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Jian J Wang > Sent: Friday, November 3, 2017 8:57 AM > To: edk2-devel@lists.01.org > Cc: Laszlo Ersek <ler...@redhat.com>; Yao, Jiewen > <jiewen....@intel.com>; Dong, Eric <eric.d...@intel.com> > Subject: [edk2] [PATCH v2] UefiCpuPkg/CpuDxe: Fix multiple entries of > RT_CODE in memory map > > > v2 > > a. Fix an issue which will cause setting capability failure if size is > > smaller > > than a page. > > 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. > > More detailed information, please refer to > https://bugzilla.tianocore.org/show_bug.cgi?id=753 > > Cc: Eric Dong <eric.d...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang <jian.j.w...@intel.com> > --- > UefiCpuPkg/CpuDxe/CpuPageTable.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c > b/UefiCpuPkg/CpuDxe/CpuPageTable.c > index d312eb66f8..4a7827ebc9 100644 > --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c > +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c > @@ -809,7 +809,9 @@ RefreshGcdMemoryAttributesFromPaging ( > PageLength = 0; > > for (Index = 0; Index < NumberOfDescriptors; Index++) { > - if (MemorySpaceMap[Index].GcdMemoryType == > EfiGcdMemoryTypeNonExistent) { > + if (MemorySpaceMap[Index].GcdMemoryType == > EfiGcdMemoryTypeNonExistent > + || (MemorySpaceMap[Index].BaseAddress & EFI_PAGE_MASK) != 0 > + || (MemorySpaceMap[Index].Length & EFI_PAGE_MASK) != 0) { > continue; > } > > @@ -829,6 +831,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 +857,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 +864,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)); > -- > 2.14.1.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
