David: The patch is good. Reviewed-by: Liming Gao <liming....@intel.com>
Thanks Liming -----Original Message----- From: David Woodhouse [mailto:dw...@infradead.org] Sent: Thursday, December 12, 2013 1:27 AM To: Gao, Liming Cc: edk2-devel@lists.sourceforge.net Subject: [PATCH 2/2] MdePkg: First instruction after clearing CR0.PE must be a far jmp. The IA Software Developer's Manual Vol 3A ยง9.9.2 ("Switching Back to Real-Address Mode") sets out the procedure for switching back to 16-bit real mode. We violate this in numerous ways, not least of which is that the first instruction immediately after clearing the PE bit in the CR0 register must be a far JMP. We were doing a bunch of other stuff first, and then a far *RET* instead. That isn't good enough, and was actually causing issues on certain hardware. The other violations also want fixing, but in small incremental commits so that the sequence is fully bisectable. And this is the only one which was actually causing a *problem*. Tested on OVMF and Quark with GCC build; MASM version by manual inspection and comparison with the GCC-built version. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: David Woodhouse <david.woodho...@intel.com> --- MdePkg/Library/BaseLib/Ia32/Thunk16.S | 24 +++++++++----------- MdePkg/Library/BaseLib/Ia32/Thunk16.asm | 40 ++++++++++++++++++++------------- 2 files changed, 35 insertions(+), 29 deletions(-) diff --git a/MdePkg/Library/BaseLib/Ia32/Thunk16.S b/MdePkg/Library/BaseLib/Ia32/Thunk16.S index 331ea16..185655e 100644 --- a/MdePkg/Library/BaseLib/Ia32/Thunk16.S +++ b/MdePkg/Library/BaseLib/Ia32/Thunk16.S @@ -113,22 +113,18 @@ ASM_PFX(ToUserCode): movw %cx, %es movw %cx, %fs movw %cx, %gs - movl %eax, %cr0 - movl %ebp, %cr4 # real mode starts at next instruction + movl %eax, %cr0 # real mode starts at next instruction + # which (per SDM) *must* be a far JMP. + ljmpw $0,$0 # will be filled in by InternalAsmThunk16 +L_Base: # to point here. + movl %ebp, %cr4 movw %si, %ss # set up 16-bit stack segment xchgl %ebx, %esp # set up 16-bit stack pointer - calll L_Base # push eip -L_Base: - popl %ebp # ebp <- offset L_Base - push (IA32_REGS_SIZE + 2)(%esp) - lea (L_RealMode - L_Base)(%bp), %ax - push %ax - lret -L_RealMode: - mov %dx, %cs:(SavedSs - L_Base)(%bp) - mov %ebx, %cs:(SavedEsp - L_Base)(%bp) - lidtl %cs:(_16Idtr - L_Base)(%bp) + movw IA32_REGS_SIZE(%esp), %bp # get BackToUserCode address from stack + mov %dx, %cs:(SavedSs - ASM_PFX(BackFromUserCode))(%bp) + mov %ebx, %cs:(SavedEsp - ASM_PFX(BackFromUserCode))(%bp) + lidtl %cs:(_16Idtr - ASM_PFX(BackFromUserCode))(%bp) popal pop %ds pop %es @@ -189,6 +185,8 @@ ASM_PFX(InternalAsmThunk16): lea (ASM_PFX(BackFromUserCode) - ASM_PFX(m16Start))(%ecx), %ecx movw %cx, %ax stosl # [edi] <- return address of user code + addl $(L_Base - ASM_PFX(BackFromUserCode)), %eax + movl %eax, (L_Base - SavedCr0 - 4)(%edx) sgdtl (SavedGdt - SavedCr0)(%edx) sidtl 0x24(%esp) movl %cr0, %eax diff --git a/MdePkg/Library/BaseLib/Ia32/Thunk16.asm b/MdePkg/Library/BaseLib/Ia32/Thunk16.asm index 3e84aed..08955d4 100644 --- a/MdePkg/Library/BaseLib/Ia32/Thunk16.asm +++ b/MdePkg/Library/BaseLib/Ia32/Thunk16.asm @@ -157,24 +157,30 @@ _ToUserCode PROC mov es, ecx mov fs, ecx mov gs, ecx - mov cr0, eax - mov cr4, ebp ; real mode starts at next instruction + mov cr0, eax ; real mode starts at next instruction + ; which (per SDM) *must* be a far JMP. + DB 0eah +_RealAddr DW 0,0 ; filled in by InternalAsmThunk16 + + mov cr4, ebp mov ss, esi ; set up 16-bit stack segment xchg sp, bx ; set up 16-bit stack pointer - DB 66h - call @Base ; push eip -@Base: - pop bp ; ebp <- address of @Base - DB 67h ; address size override - push [esp + sizeof (IA32_REGS) + 2] - lea eax, [esi + (@RealMode - @Base)] - push eax - retf -@RealMode: - mov cs:[esi + (SavedSs - @Base)], edx - mov cs:[esi + (SavedEsp - @Base)], bx - DB 66h - lidt fword ptr cs:[esi + (_16Idtr - @Base)] + +; mov bp, [esp + sizeof(IA32_REGS) + DB 67h + mov ebp, [esp + sizeof(IA32_REGS)] ; BackFromUserCode address from stack + +; mov cs:[bp + (SavedSs - _BackFromUserCode)], dx + mov cs:[esi + (SavedSs - _BackFromUserCode)], edx + +; mov cs:[bp + (SavedEsp - _BackFromUserCode)], ebx + DB 2eh, 66h, 89h, 9eh + DW SavedEsp - _BackFromUserCode + +; lidt cs:[bp + (_16Idtr - _BackFromUserCode)] + DB 2eh, 66h, 0fh, 01h, 9eh + DW _16Idtr - _BackFromUserCode + popaw ; popad actually pop ds pop es @@ -230,6 +236,8 @@ InternalAsmThunk16 PROC USES ebp ebx esi edi ds es fs gs lea ecx, [ecx + (_BackFromUserCode - m16Start)] mov ax, cx stosd ; [edi] <- return address of user code + add eax, _RealAddr + 4 - _BackFromUserCode + mov dword ptr [edx + (_RealAddr - SavedCr0)], eax sgdt fword ptr [edx + (SavedGdt - SavedCr0)] sidt fword ptr [esp + 36] ; save IDT stack in argument space mov eax, cr0 -- 1.8.3.1 -- David Woodhouse Open Source Technology Centre david.woodho...@intel.com Intel Corporation ------------------------------------------------------------------------------ Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
