[Qemu-devel] [PATCH] handle multibyte NOPs

[Qemu-devel] [PATCH] handle multibyte NOPs

[Igor Kovalenko]

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

Hi!

This patch adds handling of multi-byte NOPs, recent gcc/gas uses them for
x86 code padding.
Patch checked with current opensuse-factory x86 guest installation which is
built with new gcc/gas tools.

--
Kind Regards,
Igor V. Kovalenko

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

[-- Attachment #2: x86-multibyte-noop.diff --]
[-- Type: text/plain, Size: 1529 bytes --]

Index: target-i386/translate.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/translate.c,v
retrieving revision 1.59
diff -u -r1.59 translate.c
--- target-i386/translate.c	10 Jul 2006 19:53:04 -0000	1.59
+++ target-i386/translate.c	18 Aug 2006 23:23:40 -0000
@@ -5795,6 +5795,49 @@
             goto illegal_op;
         }
         break;
+    case 0x11f:
+        /* multi-byte noop */
+
+        modrm = ldub_code(s->pc++);
+        rm  = modrm & 7;
+        mod = (modrm >> 6) & 3;
+        reg = (modrm >> 3) & 7;
+
+        if (reg != 0)
+        {
+            goto illegal_op;
+        }
+
+        if (rm == 0x04)
+        {
+            /* SIB byte follows */
+            s->pc += 1;
+        }
+
+        switch (mod)
+        {
+            case 0x00:
+                if (rm == 0x05)
+                {
+                    /* 32bit data follows */
+                    s->pc += 4;
+                }
+                /* else register is specified */
+                break;
+            case 0x01:
+                /* 8bit data follows */
+                s->pc += 1;
+                break;
+            case 0x10:
+                /* 32bit data follows */
+                s->pc += 4;
+                break;
+            case 0x11:
+            default:
+                /* register is specified */
+                break;
+        }
+        break;
     case 0x120: /* mov reg, crN */
     case 0x122: /* mov crN, reg */
         if (s->cpl != 0) {

Re: [Qemu-devel] [PATCH] handle multibyte NOPs

[Paul Brook]

On Saturday 19 August 2006 00:40, Igor Kovalenko wrote:
> Hi!
>
> This patch adds handling of multi-byte NOPs, recent gcc/gas uses them for
> x86 code padding.
> Patch checked with current opensuse-factory x86 guest installation which is
> built with new gcc/gas tools.

According to my amd64 documentation all of the opcodes 0f 19 through 0f 1f 
should be implemented as a NOP. Also, I suspect the "invalid" prefetch ops 
(0f 18) should also be implemented as nops.
The same implementation should be useable to all these opcodes.

Paul

Re: [Qemu-devel] [PATCH] handle multibyte NOPs

[Igor Kovalenko]

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

On 8/19/06, Paul Brook <paul@codesourcery.com> wrote:
>
> On Saturday 19 August 2006 00:40, Igor Kovalenko wrote:
> > Hi!
> >
> > This patch adds handling of multi-byte NOPs, recent gcc/gas uses them
> for
> > x86 code padding.
> > Patch checked with current opensuse-factory x86 guest installation which
> is
> > built with new gcc/gas tools.
>
> According to my amd64 documentation all of the opcodes 0f 19 through 0f 1f
> should be implemented as a NOP. Also, I suspect the "invalid" prefetch ops
> (0f 18) should also be implemented as nops.
> The same implementation should be useable to all these opcodes.
>
>
Updated patch, extracted noop modrm processing into separate function and
used it for 0f 18 case as well. I also removed reg sense from noop
processing.

--
Kind Regards,
Igor V. Kovalenko

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

[-- Attachment #2: x86-multibyte-noop-4.diff --]
[-- Type: text/plain, Size: 1696 bytes --]

Index: target-i386/translate.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/translate.c,v
retrieving revision 1.59
diff -u -r1.59 translate.c
--- target-i386/translate.c	10 Jul 2006 19:53:04 -0000	1.59
+++ target-i386/translate.c	20 Aug 2006 20:54:37 -0000
@@ -1615,6 +1615,43 @@
     *offset_ptr = disp;
 }
 
+static void gen_nop_modrm(DisasContext *s, int modrm)
+{
+    int rm, mod;
+    rm  = modrm & 7;
+    mod = (modrm >> 6) & 3;
+
+    if (rm == 0x04)
+    {
+        /* SIB byte follows */
+        s->pc += 1;
+    }
+
+    switch (mod)
+    {
+        case 0x00:
+            if (rm == 0x05)
+            {
+                /* 32bit data follows */
+                s->pc += 4;
+            }
+            /* else register is specified */
+            break;
+        case 0x01:
+            /* 8bit data follows */
+            s->pc += 1;
+            break;
+        case 0x10:
+            /* 32bit data follows */
+            s->pc += 4;
+            break;
+        case 0x11:
+        default:
+            /* register is specified */
+            break;
+    }
+}
+
 /* used for LEA and MOV AX, mem */
 static void gen_add_A0_ds_seg(DisasContext *s)
 {
@@ -5792,9 +5829,15 @@
             /* nothing more to do */
             break;
         default:
-            goto illegal_op;
+            gen_nop_modrm(s, modrm);
+            break;
         }
         break;
+    case 0x119 ... 0x11f:
+        /* multi-byte noop */
+        modrm = ldub_code(s->pc++);
+        gen_nop_modrm(s, modrm);
+        break;
     case 0x120: /* mov reg, crN */
     case 0x122: /* mov crN, reg */
         if (s->cpl != 0) {

Re: [Qemu-devel] [PATCH] handle multibyte NOPs

[malc]

On Mon, 21 Aug 2006, Igor Kovalenko wrote:

> On 8/19/06, Paul Brook <paul@codesourcery.com> wrote:
>> 
>> On Saturday 19 August 2006 00:40, Igor Kovalenko wrote:
>> > Hi!
>> >
>> > This patch adds handling of multi-byte NOPs, recent gcc/gas uses them
>> for
>> > x86 code padding.
>> > Patch checked with current opensuse-factory x86 guest installation which
>> is
>> > built with new gcc/gas tools.
>> 
>> According to my amd64 documentation all of the opcodes 0f 19 through 0f 1f
>> should be implemented as a NOP. Also, I suspect the "invalid" prefetch ops
>> (0f 18) should also be implemented as nops.
>> The same implementation should be useable to all these opcodes.
>> 
>> 
> Updated patch, extracted noop modrm processing into separate function and
> used it for 0f 18 case as well. I also removed reg sense from noop
> processing.

Case labels in your `switch (mod)' statement are incorrect. I guess you
wanted 0x to behave like 0b, while it works for 0 and 1, for everything
else it's way of.

--
mailto:malc@pulsesoft.com

Re: [Qemu-devel] [PATCH] handle multibyte NOPs

[Igor Kovalenko]

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

On 8/21/06, malc <malc@pulsesoft.com> wrote:
>
> On Mon, 21 Aug 2006, Igor Kovalenko wrote:
>
> > On 8/19/06, Paul Brook <paul@codesourcery.com> wrote:
> >>
> >> On Saturday 19 August 2006 00:40, Igor Kovalenko wrote:
> >> > Hi!
> >> >
> >> > This patch adds handling of multi-byte NOPs, recent gcc/gas uses them
> >> for
> >> > x86 code padding.
> >> > Patch checked with current opensuse-factory x86 guest installation
> which
> >> is
> >> > built with new gcc/gas tools.
> >>
> >> According to my amd64 documentation all of the opcodes 0f 19 through 0f
> 1f
> >> should be implemented as a NOP. Also, I suspect the "invalid" prefetch
> ops
> >> (0f 18) should also be implemented as nops.
> >> The same implementation should be useable to all these opcodes.
> >>
> >>
> > Updated patch, extracted noop modrm processing into separate function
> and
> > used it for 0f 18 case as well. I also removed reg sense from noop
> > processing.
>
> Case labels in your `switch (mod)' statement are incorrect. I guess you
> wanted 0x to behave like 0b, while it works for 0 and 1, for everything
> else it's way of.
>

Right. Here is the real one with correct case labels.

-- 
Kind Regards,
Igor V. Kovalenko

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

[-- Attachment #2: x86-multibyte-noop-5.diff --]
[-- Type: text/plain, Size: 1684 bytes --]

Index: target-i386/translate.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/translate.c,v
retrieving revision 1.59
diff -u -r1.59 translate.c
--- target-i386/translate.c	10 Jul 2006 19:53:04 -0000	1.59
+++ target-i386/translate.c	20 Aug 2006 22:27:32 -0000
@@ -1615,6 +1615,43 @@
     *offset_ptr = disp;
 }
 
+static void gen_nop_modrm(DisasContext *s, int modrm)
+{
+    int rm, mod;
+    rm  = modrm & 7;
+    mod = (modrm >> 6) & 3;
+
+    if (rm == 0x04)
+    {
+        /* SIB byte follows */
+        s->pc += 1;
+    }
+
+    switch (mod)
+    {
+        case 0:
+            if (rm == 0x05)
+            {
+                /* 32bit data follows */
+                s->pc += 4;
+            }
+            /* else register is specified */
+            break;
+        case 1:
+            /* 8bit data follows */
+            s->pc += 1;
+            break;
+        case 2:
+            /* 32bit data follows */
+            s->pc += 4;
+            break;
+        case 3:
+        default:
+            /* register is specified */
+            break;
+    }
+}
+
 /* used for LEA and MOV AX, mem */
 static void gen_add_A0_ds_seg(DisasContext *s)
 {
@@ -5792,9 +5829,15 @@
             /* nothing more to do */
             break;
         default:
-            goto illegal_op;
+            gen_nop_modrm(s, modrm);
+            break;
         }
         break;
+    case 0x119 ... 0x11f:
+        /* multi-byte noop */
+        modrm = ldub_code(s->pc++);
+        gen_nop_modrm(s, modrm);
+        break;
     case 0x120: /* mov reg, crN */
     case 0x122: /* mov crN, reg */
         if (s->cpl != 0) {

Re: [Qemu-devel] [PATCH] handle multibyte NOPs

[malc]

On Mon, 21 Aug 2006, Igor Kovalenko wrote:

<snip>

> Right. Here is the real one with correct case labels.

Sorry, forgot to mention that atop of the hex/bin problem the code also
suffers from lack of proper 16/64 bit modrm decoding.

--
mailto:malc@pulsesoft.com

Re: [Qemu-devel] [PATCH] handle multibyte NOPs

[Igor Kovalenko]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On 8/21/06, malc <malc@pulsesoft.com> wrote:
>
> On Mon, 21 Aug 2006, Igor Kovalenko wrote:
>
> <snip>
>
> > Right. Here is the real one with correct case labels.
>
> Sorry, forgot to mention that atop of the hex/bin problem the code also
> suffers from lack of proper 16/64 bit modrm decoding.


seems to me that 16 and 64 bit cases does not apply here

-- 
Kind Regards,
Igor V. Kovalenko

[-- Attachment #2: Type: text/html, Size: 696 bytes --]

Re: [Qemu-devel] [PATCH] handle multibyte NOPs

[malc]

On Mon, 21 Aug 2006, Igor Kovalenko wrote:

> On 8/21/06, malc <malc@pulsesoft.com> wrote:
>> 
>> On Mon, 21 Aug 2006, Igor Kovalenko wrote:
>> 
>> <snip>
>> 
>> > Right. Here is the real one with correct case labels.
>> 
>> Sorry, forgot to mention that atop of the hex/bin problem the code also
>> suffers from lack of proper 16/64 bit modrm decoding.
>
>
> seems to me that 16 and 64 bit cases does not apply here

Well, quick experiment in vm86 environment suggests that i'm right at
least for 16bit case.

<moo.asm>
         xor ax, ax
         int 16h
         db 0xf, 0x19, 5, 0xde, 0xad
         db 0xf, 0x19, 5, 0xbe, 0xef
         mov ah, 9
         mov dx, erm
         add dx, 0x100
         int 21h
         xor ax, ax
         int 16h
         ret
erm     db     "moo$"
</moo.asm>

nasm -o moo.com moo.asm
dosemu moo.com

64bit case should be investigated by someone who possesses 64bit OS.

--
mailto:malc@pulsesoft.com

Re: [Qemu-devel] [PATCH] handle multibyte NOPs

[Igor Kovalenko]

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

On 8/21/06, malc <malc@pulsesoft.com> wrote:
>
> On Mon, 21 Aug 2006, Igor Kovalenko wrote:
>
> > On 8/21/06, malc <malc@pulsesoft.com> wrote:
> >>
> >> On Mon, 21 Aug 2006, Igor Kovalenko wrote:
> >>
> >> <snip>
> >>
> >> > Right. Here is the real one with correct case labels.
> >>
> >> Sorry, forgot to mention that atop of the hex/bin problem the code also
> >> suffers from lack of proper 16/64 bit modrm decoding.
> >
> >
> > seems to me that 16 and 64 bit cases does not apply here
>
> Well, quick experiment in vm86 environment suggests that i'm right at
> least for 16bit case.
>
> <moo.asm>
>          xor ax, ax
>          int 16h
>          db 0xf, 0x19, 5, 0xde, 0xad
>          db 0xf, 0x19, 5, 0xbe, 0xef
>          mov ah, 9
>          mov dx, erm
>          add dx, 0x100
>          int 21h
>          xor ax, ax
>          int 16h
>          ret
> erm     db     "moo$"
> </moo.asm>
>
> nasm -o moo.com moo.asm
> dosemu moo.com
>
> 64bit case should be investigated by someone who possesses 64bit OS.
>

Here is the updated patch, should implement 16/32/64 modes according to
public intel docs.
Operand size is taken from DisassContext->dflag which is set beforehand; I
assume it is decoded correctly wrt appropriate instruction prefixes.

-- 
Kind Regards,
Igor V. Kovalenko

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

[-- Attachment #2: x86-multibyte-noop-6.diff --]
[-- Type: text/plain, Size: 2302 bytes --]

Index: target-i386/translate.c
===================================================================
RCS file: /cvsroot/qemu/qemu/target-i386/translate.c,v
retrieving revision 1.59
diff -u -r1.59 translate.c
--- target-i386/translate.c	10 Jul 2006 19:53:04 -0000	1.59
+++ target-i386/translate.c	30 Aug 2006 19:19:45 -0000
@@ -1615,6 +1615,63 @@
     *offset_ptr = disp;
 }
 
+static void gen_nop_modrm(DisasContext *s, int modrm)
+{
+    int rm, mod;
+    rm  = modrm & 7;
+    mod = (modrm >> 6) & 3;
+
+    switch (s->dflag)
+    {
+        case 0:
+            /* 16 bit */
+            if (mod == 0 && rm == 6)
+            {
+                /* 16 bit data follows */
+                s->pc += 2;
+            }
+            else if (mod < 3)
+            {
+                /* 0, 8 or 16 bit data follows */
+                s->pc += mod;
+            }
+            break;
+
+        case 2:
+            /* 64 bit, modr/m size does not change, use 32 bit case */
+        case 1:
+            /*32 bit*/
+            if (mod == 0 && rm == 5)
+            {
+               /* 32 bit data follows */
+               s->pc += 4;
+            }
+            else
+            {
+                if (mod < 3 && rm == 4)
+                {
+                    /* SIB byte follows */
+                    s->pc += 1;
+                }
+
+                if (mod == 1)
+                {
+                    /* 8 bit data follows */
+                    s->pc += 1;
+                }
+                else if (mod == 1)
+                {
+                    /* 32 bit data follows */
+                    s->pc += 4;
+                }
+            }
+            break;
+        default:
+            /* undefined */
+            break;
+    }
+}
+
 /* used for LEA and MOV AX, mem */
 static void gen_add_A0_ds_seg(DisasContext *s)
 {
@@ -5792,9 +5849,15 @@
             /* nothing more to do */
             break;
         default:
-            goto illegal_op;
+            gen_nop_modrm(s, modrm);
+            break;
         }
         break;
+    case 0x119 ... 0x11f:
+        /* multi-byte noop */
+        modrm = ldub_code(s->pc++);
+        gen_nop_modrm(s, modrm);
+        break;
     case 0x120: /* mov reg, crN */
     case 0x122: /* mov crN, reg */
         if (s->cpl != 0) {