On 11/11/16 13:53, Yao, Jiewen wrote: > Sorry, I did not explain it clear enough before. > > Jeff is right. The NX fix is introduced in this patch series. > The reason is that when we update page table to protect the SMRAM, we would > like enable the protection as early as possible. > We moved the NX enabling code from C function to ASM function to achieve that. > > You can see my GIT message in 5/6. > ============== > The XD enabling code is moved to SmiEntry to let NX take effect. > ============== > > Unfortunately, I introduced a bug there. You help to catch it and now I fix > it. > It is not related to current code. > > What about your idea?
Right, I missed that the XD handling in SmiEntry.nasm was brand new code added by this series. So, the current structure of both series should be fine; I should be able to start testing them (hopefully) soon. Thanks! Laszlo > Thank you > Yao Jiewen > > From: Fan, Jeff > Sent: Friday, November 11, 2016 8:38 PM > To: Laszlo Ersek <ler...@redhat.com>; Yao, Jiewen <jiewen....@intel.com>; > edk2-devel@lists.01.org <edk2-de...@ml01.01.org> > Cc: Tian, Feng <feng.t...@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Paolo Bonzini <pbonz...@redhat.com>; Zeng, Star > <star.z...@intel.com> > Subject: RE: [edk2] [PATCH V3 0/6] Enable SMM page level protection. > > Laszlo, > > NX support and fix in SmiEntry.asm is new code for UefiCpuPkg. So, that means > we still have other unknown problem on S3 boot issue. > > Jeff > > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Friday, November 11, 2016 8:26 PM > To: Yao, Jiewen; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; > Fan, Jeff > Cc: Tian, Feng; Kinney, Michael D; Paolo Bonzini; Zeng, Star > Subject: Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection. > > Jiewen, Jeff, > > On 11/11/16 10:12, Yao, Jiewen wrote: >> HI Laszlo >> I fixed the IA32 boot issue in this patch > > Thank you. > > Jiewen, I'd like to request the following: > > - Please separate the fix for the incorrect parameter passing to > SmiRendezvous() out to a separate patch. It is my understanding that > the issue exists already in the master branch. Is that right? If it > is, then the fix should not be tied to the SMM page level protection > feature. > > - Please give that patch to Jeff. > > Jeff, > > can you please repost your series > > [edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path > > to edk2-devel, as v3, with Jiewen's patch from above included, as patch#4? > Because, I would like to see a patch series that addresses all known S3 > issues that we've uncovered in this investigation. > > The first three patches should fix BZ#216, yes. > > The last (4th) patch, from Jiewen, is unrelated to that BZ indeed, but it > nonetheless addresses an existent issue in PiSmmCpuDxeSmm that can be hit > during S3. > > My goal is to apply that series (the first 3 patches from Jeff, and the > fourth patch from Jiewen), and to test it as one unit. I'd like to see if > those changes fix the infrequent, but still triggerable issues with > S3+SMM, for both Ia32 and Ia32X64. If everything works fine, then that > series should be committed. > > After that, I'd like to test Jiewen's v4 series for the SMM page level > protection, separately. > > In other words, first we should fix the existent bugs that Jiewen's SMM page > level protection feature only amplifies (but doesn't introduce) on QEMU/KVM + > OVMF. Once the known bugs are fixed, I'll be glad to test the new feature. > > Would this work for you guys? > > Thank you, > Laszlo > >> with DEBUG message update you suggested. >> >> My unit test failed before. Now it can pass. >> I validated on a real IA32 and Windows OVMF with and without XD. >> >> >> For QEMU installation, it is still on progress. >> We have setup a Fedora 24 host, download QEMU, and install it. >> But we are still struggling to make QEMU boot on Fedora. >> Your step by step is great. There is still some minor place we stuck in due >> to my ignorance. >> My goal is still to setup an environment like yours for our validation or >> issue reproduce. >> It just need take some time, more than I expected. sign... >> >> Thank you >> Yao Jiewen >> >>> -----Original Message----- >>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf >>> Of Jiewen Yao >>> Sent: Friday, November 11, 2016 5:01 PM >>> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> >>> Cc: Tian, Feng <feng.t...@intel.com<mailto:feng.t...@intel.com>>; Kinney, >>> Michael D >>> <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>; Paolo >>> Bonzini <pbonz...@redhat.com<mailto:pbonz...@redhat.com>>; >>> Laszlo Ersek <ler...@redhat.com<mailto:ler...@redhat.com>>; Fan, Jeff >>> <jeff....@intel.com<mailto:jeff....@intel.com>>; >>> Zeng, Star <star.z...@intel.com<mailto:star.z...@intel.com>> >>> Subject: [edk2] [PATCH V3 0/6] Enable SMM page level protection. >>> >>> >>> ==== below is V3 description ==== >>> 1) PiSmmCpu: Fix CpuIndex corruption issue due to stack malposition. >>> (Many thanks to Laszlo Ersek <ler...@redhat.com<mailto:ler...@redhat.com>> >>> for catching it.) >>> 2) PiSmmCpu: Add ASSERT for CpuIndex check. >>> 3) PiSmmCpu: Use DEBUG_VERBOSE for page table update. >>> 4) PiSmmCpu: Do not report DEBUG message for Ap non present when >>> PcdCpuSmmSyncMode==1 (Relex mode). >>> 5) PiSmmCpu: Do not report DEBUG message for AP removed when >>> PcdCpuHotPlugSupport==TRUE. >>> >>> Tested combination: >>> 1) XD disabled >>> 2) XD enabled in SMM and disabled in non-SMM. >>> 3) XD enabled in SMM and enabled in non-SMM. >>> >>> ==== below is V2 description ==== >>> 1) PiSmmCpu: resolve OVMF multiple processors boot hang issue. >>> 2) PiSmmCpu: Add debug info on StartupAp() fails. >>> 3) PiSmmCpu: Add ASSERT for AllocatePages(). >>> 4) PiSmmCpu: Add protection detail in commit message. >>> 5) UefiCpuPkg.dsc: Add page table footprint info in commit message. >>> >>> ==== below is V1 description ==== >>> This series patch enables SMM page level protection. >>> Features are: >>> 1) PiSmmCore reports SMM PE image code/data information in >>> EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned. >>> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable and set XD for >>> data page and RO for code page. >>> 3) PiSmmCpu enables Static Paging for X64 according to >>> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G is >>> used as long as it is supported. >>> 4) PiSmmCpu sets importance data structure to be read only, such as >>> Gdt, Idt, SmmEntrypoint, and PageTable itself. >>> >>> tested platform: >>> 1) Intel internal platform (X64). >>> 2) EDKII Quark IA32 >>> 3) EDKII Vlv2 X64 >>> 4) EDKII OVMF IA32 and IA32X64. (with -smp 8) >>> >>> Cc: Jeff Fan <jeff....@intel.com<mailto:jeff....@intel.com>> >>> Cc: Feng Tian <feng.t...@intel.com<mailto:feng.t...@intel.com>> >>> Cc: Star Zeng <star.z...@intel.com<mailto:star.z...@intel.com>> >>> Cc: Michael D Kinney >>> <michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>> >>> Cc: Laszlo Ersek <ler...@redhat.com<mailto:ler...@redhat.com>> >>> Cc: Paolo Bonzini <pbonz...@redhat.com<mailto:pbonz...@redhat.com>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Jiewen Yao >>> <jiewen....@intel.com<mailto:jiewen....@intel.com>> >>> >>> Jiewen Yao (6): >>> MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h >>> MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid. >>> MdeModulePkg/PiSmmCore: Add MemoryAttributes support. >>> UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable. >>> UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection. >>> QuarkPlatformPkg/dsc: enable Smm paging protection. >>> >>> MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 66 + >>> MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 1509 >>> ++++++++++++++++++++ >>> MdeModulePkg/Core/PiSmmCore/Page.c | 775 >>> +++++++++- >>> MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 40 >>> + >>> MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 91 >>> ++ >>> MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 2 >>> + >>> MdeModulePkg/Core/PiSmmCore/Pool.c | 16 >>> + >>> MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h | 51 + >>> MdeModulePkg/MdeModulePkg.dec | >>> 3 + >>> QuarkPlatformPkg/Quark.dsc | 6 + >>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 71 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S | 75 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm | 75 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 79 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S | 226 >>> +-- >>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm | 36 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm | 36 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 37 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c | 4 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 135 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | >>> 144 +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | >>> 156 +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | >>> 5 +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | >>> 871 +++++++++++ >>> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 39 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 15 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 274 >>> +++- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S | 59 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm | 62 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 69 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.S | 250 >>> +--- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.asm | 35 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.nasm | 31 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 30 >>> +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 7 >>> +- >>> UefiCpuPkg/UefiCpuPkg.dec | 8 + >>> 36 files changed, 4585 insertions(+), 803 deletions(-) create mode >>> 100644 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c >>> create mode 100644 >>> MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h >>> create mode 100644 >>> UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c >>> >>> -- >>> 2.7.4.windows.1 >>> >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> >>> https://lists.01.org/mailman/listinfo/edk2-devel >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org<mailto: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 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
