HI Mike Current gXdSupported is defined in C code. I am not able to refer it directly in protected mode asm directly. One possible way is to define AsmXdSupported variable in protected mode ASM, then we can let C code back fill the data value. Like what we did for gSmiStack and gSmiCr3. Do you think it is better solution?
Agree. I will use MACRO instead of hardcode 0xc0000080 and 0x800. Agree. I will remove unused ActivateXd(). -----Original Message----- From: Kinney, Michael D Sent: Thursday, November 26, 2015 1:28 AM To: Yao, Jiewen; [email protected]; Kinney, Michael D Cc: Fan, Jeff Subject: RE: [patch] UefiCpuPkg/PiSmmCpu: Move XD enable to ASM before paging enable. Jiewen, Doing 2 CPUID instructions in every SMI looks expensive. We have global mXdSupported. Can we change that to gXdSupported and detect support in driver entry and use this BOOLEAN flag in assembly code to determine if the MSR_EFER need to be updated? Also, please use MSR_EFER and MSR_EFER_XD defines instead of hard coded 0xc0000080 and 0x800 values. Also, if you are not calling C function ActivateXd() anymore, this function should be removed from SmmProfile.c and SmmProfile.h Thanks, Mike > -----Original Message----- > From: Yao, Jiewen > Sent: Wednesday, November 25, 2015 12:57 AM > To: [email protected] > Cc: Yao, Jiewen <[email protected]>; Fan, Jeff <[email protected]>; > Kinney, Michael D <[email protected]> > Subject: [patch] UefiCpuPkg/PiSmmCpu: Move XD enable to ASM before paging > enable. > > There might be page table set SMM data region be XD. > So we have to enable XD before enable paging. Or #PF might be generated. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: "Yao, Jiewen" <[email protected]> > Cc: "Fan, Jeff" <[email protected]> > Cc: "Kinney, Michael D" <[email protected]> > --- > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S | 18 ++++++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm | 18 ++++++++++++++++++ > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 7 ------- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S | 20 +++++++++++++++++++- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm | 20 +++++++++++++++++++- > 5 files changed, 74 insertions(+), 9 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S > b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S > index fbaa072..7e1787c 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S > @@ -122,6 +122,24 @@ L11: > orl $BIT10, %eax > L12: # as cr4.PGE is not set here, > refresh cr3 > movl %eax, %cr4 # in PreModifyMtrrs() to flush > TLB. > + > +# > +# Need to test for XD support > +# > + movl $0x80000000, %eax > + cpuid > + cmpl $0x80000000, %eax > + jbe L13 > + movl $0x80000001, %eax > + cpuid > + testl $BIT20, %edx > + jz L13 > + movl $0xc0000080, %ecx > + rdmsr > + orb $8,%ah # enable NXE > + wrmsr > +L13: > + > movl %cr0, %ebx > orl $0x080000000, %ebx # enable paging > movl %ebx, %cr0 > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm > b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm > index 8a12927..e6af344 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm > @@ -128,6 +128,24 @@ gSmiCr3 DD ? > or eax, BIT10 > @@: ; as cr4.PGE is not set here, > refresh cr3 > mov cr4, eax ; in PreModifyMtrrs() to flush TLB. > + > +; > +; Need to test for XD support > +; > + mov eax, 080000000h > + cpuid > + cmp eax, 080000000h > + jbe @f > + mov eax, 080000001h > + cpuid > + test edx, BIT20 > + jz @f > + mov ecx, 0c0000080h > + rdmsr > + or ah, 8 ; enable NXE > + wrmsr > +@@: > + > mov ebx, cr0 > or ebx, 080000000h ; enable paging > mov cr0, ebx > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > index 031a5fe..26c9a0f 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > @@ -1075,13 +1075,6 @@ SmiRendezvous ( > InitializeSpinLock (&mSmmMpSyncData->CpuData[CpuIndex].Busy); > } > > - // > - // Try to enable NX > - // > - if (mXdSupported) { > - ActivateXd (); > - } > - > if (FeaturePcdGet (PcdCpuSmmProfileEnable)) { > ActivateSmmProfile (CpuIndex); > } > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S > index b488b74..1d40819 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S > @@ -139,9 +139,27 @@ ASM_PFX(gSmiCr3): .space 4 > call Base # push return address for retf > later > Base: > addl $(LongMode - Base), (%rsp) # offset for far retf, seg is the > 1st arg > + > +# > +# Need to test for XD support > +# > + movl $0x80000000, %eax > + cpuid > + cmpl $0x80000000, %eax > + jbe NxeDone > + movl $0x80000001, %eax > + cpuid > + testl $BIT20, %edx > + jz NxeDone > + movl $0xc0000080, %ecx > + rdmsr > + orb $8,%ah # enable NXE > + wrmsr > +NxeDone: > + > movl $0xc0000080, %ecx > rdmsr > - orb $1,%ah > + orb $1,%ah # enable LME > wrmsr > movq %cr0, %rbx > btsl $31, %ebx > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm > index 4f5c03c..6e1d3f1 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm > @@ -135,9 +135,27 @@ gSmiCr3 DD ? > call Base ; push return address for retf later > Base: > add dword ptr [rsp], @LongMode - Base; offset for far retf, seg is > the 1st arg > + > +; > +; Need to test for XD support > +; > + mov eax, 080000000h > + cpuid > + cmp eax, 080000000h > + jbe @f > + mov eax, 080000001h > + cpuid > + test edx, BIT20 > + jz @f > + mov ecx, 0c0000080h > + rdmsr > + or ah, 8 ; enable NXE > + wrmsr > +@@: > + > mov ecx, 0c0000080h > rdmsr > - or ah, 1 > + or ah, 1 ; enable LME > wrmsr > mov rbx, cr0 > bts ebx, 31 > -- > 1.9.5.msysgit.0 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
