From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DE3AA81D5E for ; Fri, 4 Nov 2016 16:08:04 -0700 (PDT) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A509EC05AA43; Fri, 4 Nov 2016 23:08:06 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-156.phx2.redhat.com [10.3.116.156]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uA4N84a7027317; Fri, 4 Nov 2016 19:08:04 -0400 To: "Yao, Jiewen" , "edk2-devel@ml01.01.org" References: <1478251854-14660-1-git-send-email-jiewen.yao@intel.com> <74D8A39837DF1E4DA445A8C0B3885C50386BEA9C@shsmsx102.ccr.corp.intel.com> Cc: "Kinney, Michael D" , "Tian, Feng" , "Fan, Jeff" , "Zeng, Star" From: Laszlo Ersek Message-ID: <11079388-0a41-a445-a4ab-051425fb8df4@redhat.com> Date: Sat, 5 Nov 2016 00:08:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C50386BEA9C@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 04 Nov 2016 23:08:06 +0000 (UTC) Subject: Re: [PATCH V2 0/6] Enable SMM page level protection. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Nov 2016 23:08:05 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 11/04/16 23:46, Yao, Jiewen wrote: > Ah, yes. Laszlo. You are right. > > I forget to push the last update yesterday. Thank you to remind me. > Now it is synced. Thanks! The commit message updates and the v1->v2 differences look good/reasonable to me (I diffed the code-level end results of the two versions, plus I compared the commit messages pairwise). I hope to test v2 sometime next week, and I intend to look into the S3 instability too (I took note of Paolo's advice with the "info tlb" QEMU monitor command). Going through the (now documented) SMRAM impact again, I realize the platform can elect to set PcdCpuSmmStaticPageTable dynamically as well. I'm sort of guessing that we might want to set the PCD in OVMF's PlatformPei, based on the guest-phys address width (which we also calculate in PlatformPei), in combination with availability of 1G paging. The case we should likely avoid is > A) If the system only supports 2M paging, > When the whole memory/MMIO is 48bit, we need 1+256+256*256 pages > (~ 257M) Anyway, I don't want to be too clever about this until we see a problem (out-of-SMRAM) in practice. Thanks! Laszlo > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Saturday, November 5, 2016 6:40 AM > To: Yao, Jiewen ; edk2-devel@ml01.01.org > Cc: Kinney, Michael D ; Tian, Feng ; Fan, Jeff ; Zeng, Star > Subject: Re: [edk2] [PATCH V2 0/6] Enable SMM page level protection. > > On 11/04/16 10:30, Jiewen Yao wrote: >> ==== 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. > > Jiewen, can you please push this series to a new branch in your repo? > > I see a branch called "SmmProtection_V2", but it seems to end with an > incomplete patch (26f482d8b611d0fcb07d3ffbf3f4468fd249767b, subject > "pismmcpu"), so I figured I'd ask explicitly. > > Thanks > Laszlo > >> ==== 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 > >> Cc: Feng Tian > >> Cc: Star Zeng > >> Cc: Michael D Kinney > >> Cc: Laszlo Ersek > >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Jiewen Yao > >> >> 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 | 67 +- >> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm | 68 +- >> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 70 +- >> 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 | 127 +- >> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 142 +- >> 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 | 51 +- >> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm | 54 +- >> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 61 +- >> 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, 4529 insertions(+), 801 deletions(-) >> create mode 100644 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c >> create mode 100644 MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h >> create mode 100644 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c >> > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >