On Fri, 2013-11-29 at 16:14 +0000, David Woodhouse wrote: > Again. > > Properly this time. > > In r12889 we attempted to clean this up, but we missed a vital part of > it, so it was reverted in r12898. > > What we missed was the fact that much of this is 16-bit code, and the > assembler didn't *know* that
Ping?
Peter points out that this code is not following the requirements of the
SDM. According to Vol 3A ยง9.9.2, the first instruction after clearing
CR0.PE is supposed to be a far JMP instruction. And we're doing a
*bunch* of stuff followed by a far RET. Which is not permitted.
While prodding at it, I note we jump through a *lot* of hoops to cope
with the possibility that the RealModeBuffer may not be aligned to a
multiple of 0x10 bytes. Which is weird, because it comes from
AllocateLegacyMemory() which allocates *pages*. Surely this buffer is
always going to be aligned to a *page*, and definitely to 0x10 bytes?
We could simplify this code a *lot* if we could assume that m16Start can
always be the start of a 16-bit segment; i.e. aligned to 0x10.
Patch (preserving the pointless position-independence) below; I need to
fix up the MASM version to match, and will do a bit more testing and
submit it for real. Soliciting feedback before I undertake the painful
process of trying to test with a Windows box though. Anyone requesting
changes *after* I've done that is going to get extremely grumpy
responses :)
There are other SDM violations here too which I may address in
subsequent patches, but this is the one that's actually *hurting* me at
the moment, it's best to keep things bisectable by changing one thing at
a time...
--- a/MdePkg/Library/BaseLib/Ia32/Thunk16.S 2013-12-03 23:17:03.947547523
+0000
+++ b/MdePkg/Library/BaseLib/Ia32/Thunk16.S 2013-12-04 11:39:03.589651791
+0000
@@ -113,22 +113,19 @@ 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 +186,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
--
David Woodhouse Open Source Technology Centre
david.woodho...@intel.com Intel Corporation
--
dwmw2
smime.p7s
Description: S/MIME cryptographic signature
------------------------------------------------------------------------------ Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk
_______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
