On 11/25/15 22:56, Yao, Jiewen wrote: > Hi Mike > Thanks for the suggestion. > Previously, I just want to *ADD* without touch old logic. I will use your way > to do it. > > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, November 26, 2015 2:01 AM > To: Yao, Jiewen; edk2-de...@ml01.01.org; Kinney, Michael D > Cc: Fan, Jeff > Subject: RE: [patch 2/2] UefiCpuPkg/PiSmmCpu: Always set WP in CR0. > > Jiewen, > > For IA32 assembly, can we combine into a single OR instruction that sets both > page enable and WP? > For X64, does it make sense to use single OR instruction instead of 2 BTS > instructions as well? > > With those updates, > > Reviewed-by: Michael Kinney <michael.d.kin...@intel.com> > > Thanks, > > Mike > >> -----Original Message----- >> From: Yao, Jiewen >> Sent: Wednesday, November 25, 2015 4:35 AM >> To: edk2-de...@ml01.01.org >> Cc: Yao, Jiewen <jiewen....@intel.com>; Fan, Jeff <jeff....@intel.com>; >> Kinney, Michael D <michael.d.kin...@intel.com> >> Subject: [patch 2/2] UefiCpuPkg/PiSmmCpu: Always set WP in CR0. >> >> So that we can use write-protection for code later. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: "Yao, Jiewen" <jiewen....@intel.com> >> Cc: "Fan, Jeff" <jeff....@intel.com> >> Cc: "Kinney, Michael D" <michael.d.kin...@intel.com> >> --- >> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S | 1 + >> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm | 1 + >> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S | 1 + >> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm | 1 + >> 4 files changed, 4 insertions(+) >> >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S >> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S >> index 7e1787c..8e64ce8 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S >> @@ -142,6 +142,7 @@ L13: >> >> movl %cr0, %ebx >> orl $0x080000000, %ebx # enable paging >> + orl $0x000010000, %ebx # set WP >> movl %ebx, %cr0 >> leal DSC_OFFSET(%edi),%ebx >> movw DSC_DS(%ebx),%ax >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm >> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm >> index e6af344..a955785 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm >> @@ -148,6 +148,7 @@ gSmiCr3 DD ? >> >> mov ebx, cr0 >> or ebx, 080000000h ; enable paging >> + or ebx, 000010000h ; set WP >> mov cr0, ebx >> lea ebx, [edi + DSC_OFFSET] >> mov ax, [ebx + DSC_DS] >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S >> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S >> index 1d40819..8050a00 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S >> @@ -163,6 +163,7 @@ NxeDone: >> wrmsr >> movq %cr0, %rbx >> btsl $31, %ebx >> + btsl $16, %ebx # set WP >> movq %rbx, %cr0 >> retf >> LongMode: # long mode (64-bit code) starts >> here >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm >> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm >> index 6e1d3f1..db170d6 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm >> @@ -159,6 +159,7 @@ Base: >> wrmsr >> mov rbx, cr0 >> bts ebx, 31 >> + bts ebx, 16 ; set WP >> mov cr0, rbx >> retf >> @LongMode: ; long mode (64-bit code) starts >> here >> -- >> 1.9.5.msysgit.0
This patch (= SVN r18960) breaks the 32-bit (Ia32) build of the OVMF SMM series. KVM "rejects" the guest soon after the SMBASE relocation. These are the last messages from the OVMF debug log: > Loading SMM driver at 0x0007FFD2000 EntryPoint=0x0007FFD2253 > PiSmmCpuDxeSmm.efi > SMRR Base: 0x7F800000, SMRR Size: 0x800000 > PcdCpuSmmCodeAccessCheckEnable = 1 > SMRAM TileSize = 0x00002000 (0x00001000, 0x00001000) > SMRAM SaveState Buffer (0x7FFC4000, 0x0000E000) > CPU[000] APIC ID=0000 SMBASE=7FFBC000 SaveState=7FFCBC00 Size=00000400 > CPU[001] APIC ID=0002 SMBASE=7FFBE000 SaveState=7FFCDC00 Size=00000400 > CPU[002] APIC ID=0003 SMBASE=7FFC0000 SaveState=7FFCFC00 Size=00000400 > CPU[003] APIC ID=0001 SMBASE=7FFC2000 SaveState=7FFD1C00 Size=00000400 > InstallProtocolInterface: 26EEB3DE-B689-492E-80F0-BE8BD7DA4BA7 7FFE0FD0 > SMM IPL registered SMM Entry Point address 7FFF19AE > SmmInstallProtocolInterface: EB346B97-975F-4A9F-8B22-F8E92BB3D569 7FFE0F68 > SmmInstallProtocolInterface: 1D202CAB-C8AB-4D5C-94F7-3CFCC0D3D335 7FFE0FDC > SMM S3 SMRAM Structure = 7F5816C0 > SMM S3 Structure = 7F800000 > SMM CPU Module exit from SMRAM with EFI_SUCCESS > SMM IPL closed SMRAM window QEMU logs the following: > KVM: entry failed, hardware error 0x80000021 > > If you're running a guest on an Intel machine without unrestricted mode > support, the failure can be most likely due to the guest entering an invalid > state for Intel VT. For example, the guest maybe running in big real mode > which is not supported on less recent Intel processors. > > EAX=00000668 EBX=80010033 ECX=80200001 EDX=0f89fbff > ESI=7f714158 EDI=7ffbc000 EBP=7ffb6000 ESP=7ffbdffc > EIP=7ffc40ce EFL=00000086 [--S--P-] CPL=0 II=0 A20=1 SMM=1 HLT=0 > ES =0020 00000000 ffffffff 00c09300 DPL=0 DS [-WA] > CS =0008 00000000 ffffffff 00c09b00 DPL=0 CS32 [-RA] > SS =0020 00000000 ffffffff 00c09300 DPL=0 DS [-WA] > DS =0020 00000000 ffffffff 00c09300 DPL=0 DS [-WA] > FS =0020 00000000 ffffffff 00c09300 DPL=0 DS [-WA] > GS =0020 00000000 ffffffff 00c09300 DPL=0 DS [-WA] > LDT=0000 00000000 0000ffff 00008200 DPL=0 LDT > TR =0000 00000000 0000ffff 00008b00 DPL=0 TSS32-busy > GDT= 7ffb6000 0000003f > IDT= 7ffe11b8 000000ff > CR0=80010033 CR2=00000000 CR3=7ffb7000 CR4=00000668 > DR0=00000000 DR1=00000000 DR2=00000000 DR3=00000000 > DR6=ffff0ff0 DR7=00000400 > EFER=0000000000000000 > Code=?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? <??> ?? ?? > ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? > ?? (My host does support unrestricted guest, just for clarity's sake.) The relevant snippet from the KVM trace is as follows: > qemu-system-i38-4063 [001] 21044.710895: kvm_entry: vcpu 0 > qemu-system-i38-4063 [001] 21044.710896: kvm_exit: reason > CR_ACCESS rip 0x8045 info 300 0 > qemu-system-i38-4063 [001] 21044.710896: kvm_cr: cr_write 0 = > 0x33 > qemu-system-i38-4063 [001] 21044.710897: kvm_entry: vcpu 0 > qemu-system-i38-4063 [001] 21044.710897: kvm_exit: reason > CR_ACCESS rip 0x7ffc4078 info 3 0 > qemu-system-i38-4063 [001] 21044.710897: kvm_cr: cr_write 3 = > 0x7ffb7000 > qemu-system-i38-4063 [001] 21044.710898: kvm_entry: vcpu 0 > qemu-system-i38-4063 [001] 21044.710898: kvm_exit: reason CPUID > rip 0x7ffc4080 info 0 0 > qemu-system-i38-4063 [001] 21044.710899: kvm_cpuid: func 1 rax > 6e8 rbx 800 rcx 80200001 rdx f89fbff > qemu-system-i38-4063 [001] 21044.710899: kvm_entry: vcpu 0 > qemu-system-i38-4063 [001] 21044.710899: kvm_exit: reason > CR_ACCESS rip 0x7ffc40bf info 4 0 > qemu-system-i38-4063 [001] 21044.710899: kvm_cr: cr_write 4 = > 0x668 > qemu-system-i38-4063 [001] 21044.710901: kvm_entry: vcpu 0 > qemu-system-i38-4063 [001] 21044.710901: kvm_exit: reason > CR_ACCESS rip 0x7ffc40cb info 300 0 > qemu-system-i38-4063 [001] 21044.710901: kvm_cr: cr_write 0 = > 0x80010033 <--------------- this one > qemu-system-i38-4063 [001] 21044.710904: kvm_entry: vcpu 0 > qemu-system-i38-4063 [001] 21044.710904: kvm_exit: reason > UNKNOWN rip 0x7ffc40ce info 2 0 > qemu-system-i38-4063 [001] 21044.710937: kvm_userspace_exit: reason > KVM_EXIT_FAIL_ENTRY (9) ... I now it sounds a little bit paranoid, but can we *please* suspend the churn in the SMM infrastructure in edk2 until the OVMF series goes in? Even if the above edk2 change is otherwise valid, and it turns out to be a KVM or QEMU issue? The above breakage is just one thing. Today I had to adapt the series to SVN r18958 as well (otherwise it wouldn't even build). I knew about the two new SmmCpuFeaturesLib functions introduced by that commit, yes. What I didn't expect was that they would force me to *re-sync* Paolo's patches: OvmfPkg: import SmmCpuFeaturesLib from UefiCpuPkg OvmfPkg: SmmCpuFeaturesLib: remove unnecessary bits -- which he originally contributed on October 13th -- with the *fresh* library instance in UefiCpuPkg. And *that* was utter pain. ... And now apparently Jordan wants me to delay the series even longer, until the S3Resume2Pei module is fixed to work on X64 + SMM. On one hand, I can't be thankful enough to Mike and others who have immensely helped with this. On the other hand, this half year plus long experience has been terrible. In the future, please CC me on *all* SMM patches, and give me a chance to test them with OVMF / QEMU, before committing them. Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
