[PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Trolle Selander]

[-- Attachment #1.1: Type: text/plain, Size: 1091 bytes --]

This patch adds a number of fp instructions needed for OS/2 to boot as a HVM
guest on Intel/VT hardware. It appears to work fine, and OS/2 is now finally
working on Intel/VT as well as AMD/SVM.

I'm a little concerned about the "correctness" of the FSTSW emulation and
the use of inline assembly directly using the corresponding ops for
emulation. Wrt FSTSW, it is really two ops FNSTSW immediately preceeded by
an FWAIT. I've left FWAIT defined, but a no-op because after looking at
realmode.c it seems in the emulation, exceptions are always processed if a
call to x86_emulate returns an exception, so there should be no way for
there to be "pending" exceptions for FWAIT to wait for. I may have missed
something, though, hence the RFC.

Secondly, about the inline assembly - i saw that this was how many of the
other FP instructions were done, so I did it that way as well. However, none
of the previous instructions done that way referenced the fp registers. Does
this make a difference, or is it ok the way i've done it here?

Signed-off-by: Trolle Selander <trolle.selander@gmail.com>

[-- Attachment #1.2: Type: text/html, Size: 1303 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xen-x86_emulate-more-fp-insns.patch --]
[-- Type: text/x-patch; name=xen-x86_emulate-more-fp-insns.patch, Size: 3355 bytes --]

diff -uNr xen-unstable.hg/xen/arch/x86/x86_emulate.c xen-unstable.hg-patched/xen/arch/x86/x86_emulate.c
--- xen-unstable.hg/xen/arch/x86/x86_emulate.c	2008-02-27 09:53:48.219043994 +0100
+++ xen-unstable.hg-patched/xen/arch/x86/x86_emulate.c	2008-02-29 23:09:48.858727693 +0100
@@ -127,7 +127,7 @@
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
     /* 0x98 - 0x9F */
-    ImplicitOps, ImplicitOps, ImplicitOps, 0,
+    ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
     /* 0xA0 - 0xA7 */
     ByteOp|ImplicitOps|Mov, ImplicitOps|Mov,
@@ -160,7 +160,7 @@
     ByteOp|DstMem|SrcImplicit|ModRM, DstMem|SrcImplicit|ModRM, 
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
     /* 0xD8 - 0xDF */
-    0, ImplicitOps|ModRM, 0, ImplicitOps|ModRM, 0, ImplicitOps|ModRM, 0, 0,
+    0, ImplicitOps|ModRM, 0, ImplicitOps|ModRM, 0, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM,
     /* 0xE0 - 0xE7 */
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
@@ -2355,6 +2355,10 @@
         break;
     }
 
+    case 0x9b:  /* wait/fwait */
+        /* No-op */
+        break;
+
     case 0x9c: /* pushf */
         src.val = _regs.eflags;
         goto push;
@@ -2670,12 +2674,28 @@
     case 0xd9: /* FPU 0xd9 */
         fail_if(ops->load_fpu_ctxt == NULL);
         ops->load_fpu_ctxt(ctxt);
-        fail_if((modrm_reg & 7) != 7);
-        fail_if(modrm >= 0xc0);
-        /* fnstcw m2byte */
-        ea.bytes = 2;
-        dst = ea;
-        asm volatile ( "fnstcw %0" : "=m" (dst.val) );
+        switch ( modrm )
+        {
+        case 0xc0:
+             asm volatile ( "fld %st(0)" );
+             break;
+        case 0xe0:
+             asm volatile ( "fchs" );
+             break;
+        case 0xe8:
+             asm volatile ( "fld1" );
+             break;
+        case 0xee:
+             asm volatile ( "fldz" );
+             break;
+        default:
+            fail_if((modrm_reg & 7) != 7);
+            fail_if(modrm >= 0xc0);
+            /* fnstcw m2byte */
+            ea.bytes = 2;
+            dst = ea;
+            asm volatile ( "fnstcw %0" : "=m" (dst.val) );
+        }
         break;
 
     case 0xdb: /* FPU 0xdb */
@@ -2697,6 +2717,34 @@
         asm volatile ( "fnstsw %0" : "=m" (dst.val) );
         break;
 
+    case 0xde: /* FPU 0xde */
+        fail_if(ops->load_fpu_ctxt == NULL);
+        ops->load_fpu_ctxt(ctxt);
+        switch ( modrm )
+        {
+        case 0xd9:
+             asm volatile ( "fcompp" );
+             break;
+        case 0xf9:
+             asm volatile ( "fdivp %st(1),%st" );
+             break;
+        default:
+            goto cannot_emulate;
+        }
+        break;
+
+
+    case 0xdf: /* FPU 0xdf */
+        fail_if(ops->load_fpu_ctxt == NULL);
+        ops->load_fpu_ctxt(ctxt);
+        fail_if(modrm != 0xe0);
+        /* fnstsw %ax */
+        dst.bytes = 2;
+        dst.type = OP_REG;
+        dst.reg = (unsigned long *)&_regs.eax;
+        asm volatile ( "fnstsw %0" : "=m" (dst.val) );
+        break;
+
     case 0xe0 ... 0xe2: /* loop{,z,nz} */ {
         int rel = insn_fetch_type(int8_t);
         int do_jmp = !(_regs.eflags & EFLG_ZF); /* loopnz */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Keir Fraser]

[-- Attachment #1.1: Type: text/plain, Size: 1637 bytes --]

Regarding point 1: Don¹t FPU instructions that raise an exception only log
that in the status word, to be picked up by the next FPU instruction which
checks for exceptions? It strikes me that we can have previously loaded up a
status word that requires an exception to occur on FWAIT, in which case
semantics are broken. More worryingly, if any of the instructions you have
added can cause an exception, we will crash Xen because you have not added
fixup handling. Unfortunately I¹m not really up on exactly how FPU exception
handling works: are you?

Regarding point 2: Yes, direct use of the FPU instructions is fine as we
already loaded guest FPU context.

 -- Keir

On 29/2/08 22:14, "Trolle Selander" <trolle.selander@gmail.com> wrote:

> I'm a little concerned about the "correctness" of the FSTSW emulation and the
> use of inline assembly directly using the corresponding ops for emulation. Wrt
> FSTSW, it is really two ops FNSTSW immediately preceeded by an FWAIT. I've
> left FWAIT defined, but a no-op because after looking at realmode.c it seems
> in the emulation, exceptions are always processed if a call to x86_emulate
> returns an exception, so there should be no way for there to be "pending"
> exceptions for FWAIT to wait for. I may have missed something, though, hence
> the RFC.
> 
> Secondly, about the inline assembly - i saw that this was how many of the
> other FP instructions were done, so I did it that way as well. However, none
> of the previous instructions done that way referenced the fp registers. Does
> this make a difference, or is it ok the way i've done it here?



[-- Attachment #1.2: Type: text/html, Size: 2180 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Keir Fraser]

[-- Attachment #1.1: Type: text/plain, Size: 1140 bytes --]

Intel SDM Volume 1 Section 8.6 (³X87 FPU Exception Synchronization²) is very
helpful in this regard. It confirms my suspicions, that we will need to wrap
the FPU inline asms in some fixup glue. The only exceptions to this rule are
the FN* (no-wait) instructions. Properly speaking we should also check for
unmasked exceptions becoming pending after any FPU instruction and set the
faulting CS:IP in the FPU context block appropriately. But life is perhaps
too short. :-)

 -- Keir

On 1/3/08 08:46, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:

> Regarding point 1: Don¹t FPU instructions that raise an exception only log
> that in the status word, to be picked up by the next FPU instruction which
> checks for exceptions? It strikes me that we can have previously loaded up a
> status word that requires an exception to occur on FWAIT, in which case
> semantics are broken. More worryingly, if any of the instructions you have
> added can cause an exception, we will crash Xen because you have not added
> fixup handling. Unfortunately I¹m not really up on exactly how FPU exception
> handling works: are you?



[-- Attachment #1.2: Type: text/html, Size: 1674 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Trolle Selander]

[-- Attachment #1.1: Type: text/plain, Size: 1657 bytes --]

That's what I was worried about. I honestly had zilch experience working
with assembler-level x87 stuff before Friday afternoon when I whipped up
this patch, mainly to see if the fp instructions were all that was missing
to get OS/2 on VT to work. I'll see if I can find time to make an
exception-safe version of this sometime this week. I'm not worrying about
life being too short, but rather that March 2008 is too dense... ;)

On Sat, Mar 1, 2008 at 10:18 AM, Keir Fraser <keir.fraser@eu.citrix.com>
wrote:

>  Intel SDM Volume 1 Section 8.6 ("X87 FPU Exception Synchronization") is
> very helpful in this regard. It confirms my suspicions, that we will need to
> wrap the FPU inline asms in some fixup glue. The only exceptions to this
> rule are the FN* (no-wait) instructions. Properly speaking we should also
> check for unmasked exceptions becoming pending after any FPU instruction and
> set the faulting CS:IP in the FPU context block appropriately. But life is
> perhaps too short. :-)
>
>  -- Keir
>
> On 1/3/08 08:46, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:
>
> Regarding point 1: Don't FPU instructions that raise an exception only log
> that in the status word, to be picked up by the next FPU instruction which
> checks for exceptions? It strikes me that we can have previously loaded up a
> status word that requires an exception to occur on FWAIT, in which case
> semantics are broken. More worryingly, if *any* of the instructions you
> have added can cause an exception, we will crash Xen because you have not
> added fixup handling. Unfortunately I'm not really up on exactly how FPU
> exception handling works: are you?
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 2380 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Keir Fraser]

[-- Attachment #1.1: Type: text/plain, Size: 1880 bytes --]

I¹ll have some time to do it this afternoon.

 -- Keir

On 3/3/08 12:18, "Trolle Selander" <trolle.selander@gmail.com> wrote:

> That's what I was worried about. I honestly had zilch experience working with
> assembler-level x87 stuff before Friday afternoon when I whipped up this
> patch, mainly to see if the fp instructions were all that was missing to get
> OS/2 on VT to work. I'll see if I can find time to make an exception-safe
> version of this sometime this week. I'm not worrying about life being too
> short, but rather that March 2008 is too dense... ;)
> 
> On Sat, Mar 1, 2008 at 10:18 AM, Keir Fraser <keir.fraser@eu.citrix.com>
> wrote:
>> Intel SDM Volume 1 Section 8.6 ("X87 FPU Exception Synchronization") is very
>> helpful in this regard. It confirms my suspicions, that we will need to wrap
>> the FPU inline asms in some fixup glue. The only exceptions to this rule are
>> the FN* (no-wait) instructions. Properly speaking we should also check for
>> unmasked exceptions becoming pending after any FPU instruction and set the
>> faulting CS:IP in the FPU context block appropriately. But life is perhaps
>> too short. :-)
>> 
>>  -- Keir
>> 
>> 
>> On 1/3/08 08:46, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:
>> 
>>> Regarding point 1: Don't FPU instructions that raise an exception only log
>>> that in the status word, to be picked up by the next FPU instruction which
>>> checks for exceptions? It strikes me that we can have previously loaded up a
>>> status word that requires an exception to occur on FWAIT, in which case
>>> semantics are broken. More worryingly, if any of the instructions you have
>>> added can cause an exception, we will crash Xen because you have not added
>>> fixup handling. Unfortunately I'm not really up on exactly how FPU exception
>>> handling works: are you?
>> 
> 
> 



[-- Attachment #1.2: Type: text/html, Size: 2814 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Andi Kleen]

Keir Fraser <keir.fraser@eu.citrix.com> writes:

> Intel SDM Volume 1 Section 8.6 (?X87 FPU Exception Synchronization?) is very
> helpful in this regard. It confirms my suspicions, that we will need to wrap
> the FPU inline asms in some fixup glue. The only exceptions to this rule are

Yes you definitely need to. That was a kernel crash bug on native Linux
some time ago...

Also you need to do the same for any FXSAVEs/FXRSTORs if you haven't already.

-Andi

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Keir Fraser]

On 3/3/08 14:52, "Andi Kleen" <andi@firstfloor.org> wrote:

>> Intel SDM Volume 1 Section 8.6 (?X87 FPU Exception Synchronization?) is very
>> helpful in this regard. It confirms my suspicions, that we will need to wrap
>> the FPU inline asms in some fixup glue. The only exceptions to this rule are
> 
> Yes you definitely need to. That was a kernel crash bug on native Linux
> some time ago...
> 
> Also you need to do the same for any FXSAVEs/FXRSTORs if you haven't already.

FXSAVE/FXRSTOR do not flush pending FPU exceptions, right?

 -- Keir

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Andi Kleen]

On Mon, Mar 03, 2008 at 03:00:52PM +0000, Keir Fraser wrote:
> On 3/3/08 14:52, "Andi Kleen" <andi@firstfloor.org> wrote:
> 
> >> Intel SDM Volume 1 Section 8.6 (?X87 FPU Exception Synchronization?) is very
> >> helpful in this regard. It confirms my suspicions, that we will need to wrap
> >> the FPU inline asms in some fixup glue. The only exceptions to this rule are
> > 
> > Yes you definitely need to. That was a kernel crash bug on native Linux
> > some time ago...
> > 
> > Also you need to do the same for any FXSAVEs/FXRSTORs if you haven't already.
> 
> FXSAVE/FXRSTOR do not flush pending FPU exceptions, right?

Yes they do not, but there are other ways to make them #GP

-Andi

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Keir Fraser]

On 3/3/08 16:59, "Andi Kleen" <andi@firstfloor.org> wrote:

>>> Yes you definitely need to. That was a kernel crash bug on native Linux
>>> some time ago...
>>> 
>>> Also you need to do the same for any FXSAVEs/FXRSTORs if you haven't
>>> already.
>> 
>> FXSAVE/FXRSTOR do not flush pending FPU exceptions, right?
> 
> Yes they do not, but there are other ways to make them #GP

We don't trust FXRSTOR but we do trust FXSAVE. The AMD manual says that
FXRSTOR will #GP if 1s are written to MXCSR reserved bits, but I take this
to be a cut-and-paste typo since FXSAVE does not write to MXCSR.

If there's real evidence of FXSAVE causing #GP dependent on current FPU
state, I'd be very happy to find out about it!

 -- Keir

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Andi Kleen]

> We don't trust FXRSTOR but we do trust FXSAVE. The AMD manual says that
> FXRSTOR will #GP if 1s are written to MXCSR reserved bits, but I take this
> to be a cut-and-paste typo since FXSAVE does not write to MXCSR.
> 
> If there's real evidence of FXSAVE causing #GP dependent on current FPU
> state, I'd be very happy to find out about it!

Sorry, double checked now the Intel docs. First I admit I don't 
quite remember why it was originally done in Linux, but they
both use exception handlers.

The Intel docs say that #MF can happen for a pending FPU exception
in 64bit mode.

-Andi

Re: [PATCH] [RFC] More fp instructions for realmode emulation (Enables booting OS/2 as a HVM guest on Intel/VT hardware)

[Keir Fraser]

[-- Attachment #1.1: Type: text/plain, Size: 1881 bytes --]

Annoying fact #2 about the FPU instructions is that the GNU assembler
reverses the operand order of many two-operand FPU instructions. So, for
example, FDIVP becomes DIVRP, and vice versa. Super annoying. So I¹m going
to change our emulated opcodes for most FPU instructions to be ³.byte
xx,yy². Personally I think this is as clear as using the mnemonic opcode, or
perhaps clearer since it is now obvious that we are executing the exact
instruction that we have just decoded.

 -- Keir

On 29/2/08 22:14, "Trolle Selander" <trolle.selander@gmail.com> wrote:

> This patch adds a number of fp instructions needed for OS/2 to boot as a HVM
> guest on Intel/VT hardware. It appears to work fine, and OS/2 is now finally
> working on Intel/VT as well as AMD/SVM.
> 
> I'm a little concerned about the "correctness" of the FSTSW emulation and the
> use of inline assembly directly using the corresponding ops for emulation. Wrt
> FSTSW, it is really two ops FNSTSW immediately preceeded by an FWAIT. I've
> left FWAIT defined, but a no-op because after looking at realmode.c it seems
> in the emulation, exceptions are always processed if a call to x86_emulate
> returns an exception, so there should be no way for there to be "pending"
> exceptions for FWAIT to wait for. I may have missed something, though, hence
> the RFC.
> 
> Secondly, about the inline assembly - i saw that this was how many of the
> other FP instructions were done, so I did it that way as well. However, none
> of the previous instructions done that way referenced the fp registers. Does
> this make a difference, or is it ok the way i've done it here?
> 
> Signed-off-by: Trolle Selander <trolle.selander@gmail.com>
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



[-- Attachment #1.2: Type: text/html, Size: 2636 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel