[PATCH] x86 emulation: fix bswap

[PATCH] x86 emulation: fix bswap

[Jan Beulich]

REX.R needs to be decoded and since bswap on a 16-bit operand is undefined,
it is best to have hardware execute this so the emulation result matches
hardware behavior. Since it is simple to do, faster, and smaller, also let hardware
do 32- and 64-bit ones.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

Index: 2007-02-07/xen/arch/x86/x86_emulate.c
===================================================================
--- 2007-02-07.orig/xen/arch/x86/x86_emulate.c	2007-02-08 08:46:39.000000000 +0100
+++ 2007-02-07/xen/arch/x86/x86_emulate.c	2007-02-08 09:32:00.000000000 +0100
@@ -2326,32 +2326,22 @@ x86_emulate(
 
     case 0xc8 ... 0xcf: /* bswap */
         dst.type  = OP_REG;
-        dst.reg   = decode_register(b & 7, &_regs, 0);
+        dst.reg   = decode_register(
+            (b & 7) | ((rex_prefix & 1) << 3), &_regs, 0);
         dst.val = *dst.reg;
         switch ( dst.bytes = op_bytes )
         {
         case 2:
-            dst.val = (((dst.val & 0x00FFUL) << 8) |
-                       ((dst.val & 0xFF00UL) >> 8));
+            __asm__("data16 bswap %k0" : "+r" (dst.val));
             break;
         case 4:
-            dst.val = (((dst.val & 0x000000FFUL) << 24) |
-                       ((dst.val & 0x0000FF00UL) <<  8) |
-                       ((dst.val & 0x00FF0000UL) >>  8) |
-                       ((dst.val & 0xFF000000UL) >> 24));
-            break;
 #ifdef __x86_64__
-        case 8:
-            dst.val = (((dst.val & 0x00000000000000FFUL) << 56) |
-                       ((dst.val & 0x000000000000FF00UL) << 40) |
-                       ((dst.val & 0x0000000000FF0000UL) << 24) |
-                       ((dst.val & 0x00000000FF000000UL) <<  8) |
-                       ((dst.val & 0x000000FF00000000UL) >>  8) |
-                       ((dst.val & 0x0000FF0000000000UL) >> 24) |
-                       ((dst.val & 0x00FF000000000000UL) >> 40) |
-                       ((dst.val & 0xFF00000000000000UL) >> 56));
+            __asm__("bswap %k0" : "+r" (dst.val));
             break;
+        case 8:
 #endif
+            __asm__("bswap %0" : "+r" (dst.val));
+            break;
         }
         break;
     }

Re: [PATCH] x86 emulation: fix bswap

[Keir Fraser]

On 15/2/07 10:22, "Jan Beulich" <jbeulich@novell.com> wrote:

> REX.R needs to be decoded and since bswap on a 16-bit operand is undefined,
> it is best to have hardware execute this so the emulation result matches
> hardware behavior. Since it is simple to do, faster, and smaller, also let
> hardware do 32- and 64-bit ones.

Can 'undefined' behaviour include e.g. #UD?

 -- Keir

Re: [PATCH] x86 emulation: fix bswap

[Jan Beulich]

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 15.02.07 11:33 >>>
>On 15/2/07 10:22, "Jan Beulich" <jbeulich@novell.com> wrote:
>
>> REX.R needs to be decoded and since bswap on a 16-bit operand is undefined,
>> it is best to have hardware execute this so the emulation result matches
>> hardware behavior. Since it is simple to do, faster, and smaller, also let
>> hardware do 32- and 64-bit ones.
>
>Can 'undefined' behaviour include e.g. #UD?

I don't think so.

Jan

RE: [PATCH] x86 emulation: fix bswap

[Petersson, Mats]

 

> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com 
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of 
> Keir Fraser
> Sent: 15 February 2007 10:33
> To: Jan Beulich; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] x86 emulation: fix bswap
> 
> On 15/2/07 10:22, "Jan Beulich" <jbeulich@novell.com> wrote:
> 
> > REX.R needs to be decoded and since bswap on a 16-bit 
> operand is undefined,
> > it is best to have hardware execute this so the emulation 
> result matches
> > hardware behavior. Since it is simple to do, faster, and 
> smaller, also let
> > hardware do 32- and 64-bit ones.
> 
> Can 'undefined' behaviour include e.g. #UD?

BSWAP isn't allowed to #UD ("Exceptions: None") according to Vol 3 of
AMD64 Programmers Manual. A 16-bit prefix to that instruction is just
not defined - it will do SOMETHING, but it won't crash the system, just
"randomly store data in your target register" [I haven't tried it, and
it may well vary depending on model of processor as well as of course
differences between different companies processors]. I would expect the
usage of 66 0F C8 <reg> to be extremely unusual - there is the "correct"
way to do it with XCHG AL,AH and related instructions. 

If anything is going to trap, it will be on the next instruction, when
the data is garbled. 

I just wrote a little bit of code to test it: 
int x = 0x12345678;
__asm__(".byte 0x66; bswap %0": "=r"(x): "0"(x))
printf("x=%x", x); 

Prints 12340000, so the data is "zerod". (and it looks like the code
generated by gcc is correct!). 


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

Re: [PATCH] x86 emulation: fix bswap

[Keir Fraser]

On 15/2/07 11:46, "Petersson, Mats" <Mats.Petersson@amd.com> wrote:

> I just wrote a little bit of code to test it:
> int x = 0x12345678;
> __asm__(".byte 0x66; bswap %0": "=r"(x): "0"(x))
> printf("x=%x", x);
> 
> Prints 12340000, so the data is "zerod". (and it looks like the code
> generated by gcc is correct!).

Same behaviour as on an Intel CPU. I am quite inclined to do that directly
in the emulator, with a comment explaining why, rather than do an undefined
operation. That just seems unnecessarily scary. Direct 32- and 64-bit BSWAP
is okay though and does reduce the code size.

 -- Keir

RE: [PATCH] x86 emulation: fix bswap

[Petersson, Mats]

 

> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com 
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of 
> Keir Fraser
> Sent: 15 February 2007 11:54
> To: Petersson, Mats; Jan Beulich; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] x86 emulation: fix bswap
> 
> 
> 
> 
> On 15/2/07 11:46, "Petersson, Mats" <Mats.Petersson@amd.com> wrote:
> 
> > I just wrote a little bit of code to test it:
> > int x = 0x12345678;
> > __asm__(".byte 0x66; bswap %0": "=r"(x): "0"(x))
> > printf("x=%x", x);
> > 
> > Prints 12340000, so the data is "zerod". (and it looks like the code
> > generated by gcc is correct!).
> 
> Same behaviour as on an Intel CPU. I am quite inclined to do 
> that directly
> in the emulator, with a comment explaining why, rather than 
> do an undefined
> operation. That just seems unnecessarily scary. Direct 32- 
> and 64-bit BSWAP
> is okay though and does reduce the code size.

Whilst this is following the (sampled) architecture, wouldn't it be just
as easy to just IGNORE that case? 

It's not going to occur unless:
1. Someone likes to play games with code (it's easier to do "mov $0,
%ax" or "xor %ax,%ax" if that's what you actually want to do). And it's
not impossible that for example a 486, 386 or the latest quad-core [or
processors of other manufacturers (via, national->amd->discontinued)]
does something completely different on the same instruction, so it's not
a reliable way to clear that register.
2. Some code has gone astray and is executing random data - well, that's
going to crash pretty soon anyways!
3. Bugs in compiler/assembler. 

[Those are listed in order of importance/likelyhood to be needed, in my
personal opinion]. 

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

Error with vfb Xen 3.0.4

[Kaushik Barde]

	
We get this while booting our DOM-0.
Somehow /dev/fb is not configured.

>>Vfb "Trying to show message with id 'ERROR_Video_Default'
>>Vfb Using image /home/user/prism/errors/video/default.png'
>>Vfb Execing

Is this a vfb problem or we have missed something?

-Kaushik