The V3 patch is posted. I add NASM.INC files. Thank you Yao Jiewen
> -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Yao, Jiewen > Sent: Friday, February 22, 2019 8:11 PM > To: Laszlo Ersek <ler...@redhat.com>; edk2-devel@lists.01.org > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Dong, Eric > <eric.d...@intel.com>; Gao, Liming <liming....@intel.com> > Subject: Re: [edk2] [PATCH 0/3] Add SMM CET support > > Thanks Laszlo. > > 2) I have checked NASM instruction list at > https://www.nasm.us/xdoc/2.14.02/html/nasmdocb.html > SSP related instruction is not there. > > I believe using DB maybe the only choice at this moment. > > I will create include file. > > 3) I will fix comment. Thanks to catch that. > > > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > > Laszlo Ersek > > Sent: Friday, February 22, 2019 8:01 PM > > To: Yao, Jiewen <jiewen....@intel.com>; edk2-devel@lists.01.org > > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Dong, Eric > > <eric.d...@intel.com>; Gao, Liming <liming....@intel.com> > > Subject: Re: [edk2] [PATCH 0/3] Add SMM CET support > > > > On 02/22/19 05:15, Jiewen Yao wrote: > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1521 > > > > > > This patch series implement add CET ShadowStack support for SMM. > > > > > > The CET document can be found at: > > > > > > https://software.intel.com/sites/default/files/managed/4d/2a/control-flow > > -enforcement-technology-preview.pdf > > > > > > Patch 1 adds SSP (ShadowStackPointer) to JUMP_BUFFER. > > > Patch 2 adds Control Protection exception (CP#) dump info. > > > Patch 3 adds CET ShadowStack support in SMM. > > > > > > For more detail please refer to each patch. > > > > > > I also post all update to https://github.com/jyao1/edk2/tree/CET > > > > > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > > Cc: Liming Gao <liming....@intel.com> > > > Cc: Eric Dong <eric.d...@intel.com> > > > Cc: Ray Ni <ray...@intel.com> > > > Cc: Laszlo Ersek <ler...@redhat.com> > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Yao Jiewen <jiewen....@intel.com> > > > > > > Jiewen Yao (3): > > > MdePkg/BaseLib: Add Shadow Stack Support for X86. > > > UefiCpuPkg/ExceptionLib: Add CET support. > > > UefiCpuPkg/PiSmmCpu: Add Shadow Stack Support for X86 SMM. > > > > > > (1) For the series, in my usual environment: > > > > Regression-tested-by: Laszlo Ersek <ler...@redhat.com> > > > > > > (2) I notice that the NASM code receives a bunch of DB encodings for > > various instructions. I think that's a bad idea. It was pretty difficult > > to eliminate DBs; please refer to > > <https://bugzilla.tianocore.org/show_bug.cgi?id=866>, and the commit > > range aae02dccf5b0..d22c995a4814. > > > > As far as I can see, the DBs are added to encode three instructions, > > namely READSSP, INCSSP, and SETSSBSY. Can you please confirm that the > > only reason we use DBs for these instructions is that they are related > > to the CET extension, and they are not yet supported by NASM? (Or at > > least not by the NASM that that we require?) > > > > In other words, I'd like to be sure that the DBs are not used for > > runtime instruction patching. > > > > Even that way, I think it would be better to use NASM macros for these > > instructions. The code doesn't use many forms: > > > > * SETSSBSY: DB 0xF3, 0x0F, 0x01, 0xE8 > > * READSSP EAX: DB 0xF3, 0x0F, 0x1E, 0xC8 > > * INCSSP EAX: DB 0xF3, 0x0F, 0xAE, 0xE8 > > * READSSP RAX: DB 0xF3, 0x48, 0x0F, 0x1E, 0xC8 > > * INCSSP RAX: DB 0xF3, 0x48, 0x0F, 0xAE, 0xE8 > > > > (It seems that the EAX <-> RAX encodings, for READSSP and INCSSP, are > > differentiated through the 0x48 REX.W prefix (64-bit operand size).) > > > > I think we should add the macros in a NASM include file under > > "MdePkg/Include". Later, only those macros would have to be updated, > > once NASM starts supporting these instructions directly. > > > > We've supported shared NASM include files since > > <https://bugzilla.tianocore.org/show_bug.cgi?id=1085>. Therefore, both > > UefiCpuPkg and MdePkg modules could consume the macros, from under > > MdePkg/Include. > > > > > > (3) In fact, looking at the DB encodings, I think some of the comments > > are incorrect. Namely, in patch #3, in file > > "UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Cet.nasm", function DisableCet, we > > have > > > > + DB 0xF3, 0x0F, 0xAE, 0xE8 ; INCSSP RAX > > > > but that's INCSSP EAX, not RAX, in reality. (The code is correct, the > > comment is wrong.) Using NASM macros would help us avoid such typos. > > > > Thanks > > Laszlo > > > > _______________________________________________ > > 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
