[Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Steve D. Perkins]

Hello all -

    I wrote to the list a few days ago to gauge interest in adding 
support for GCC in the Win32 environment to the QEMU codebase.  I 
received no response to that inquiry, but decided to update my patch for 
the 0.7.1 release anyway in hopes that it will be of use to people.  The 
patchfile text is included below... simply cut-and-paste it to a file 
named "patchfile" in the location where you've decompressed the 
qemu-0.7.1 tarball, and run the command "patch -p0 < patchfile".

    This patch affects only four files, and almost all of the changes 
are confined to "dyngen.c".  Still, it will only become more and more 
difficult to maintain this patch going forward as the difference between 
it and the main QEMU codebase widens.  I cannot force anyone to move any 
of these changes into the main codebase, but I do feel strongly that GCC 
(the most widely-used open source compiler in the world) and Win32 (the 
most widely used PC platform in the world) is a combination worthy of 
official support out-of-the-box.  I believe that these code changes 
should have minimal (if any) impact on the builds for other platforms, 
and I am more than willing to work with Fabrice or others to eliminate 
any issues that are found (wrapping things in GCC/WIN32-specific 
preprocessor directives, etc). 

    I'm offering support full support for GCC and Win32, in the patch 
below and in an offer of future assistance.  I hope that I convince 
others that incorporate this support is a good thing for QEMU, but even 
partial inclusion of the changes below would put the codebase in a 
better place than it is currently.

Steve







diff -Naur qemu-orig/dyngen-exec.h qemu-0.7.1/dyngen-exec.h
--- qemu-orig/dyngen-exec.h    Sun Jul 24 14:52:08 2005
+++ qemu-0.7.1/dyngen-exec.h    Fri Jul 29 15:28:51 2005
@@ -155,7 +155,13 @@
 #endif
 
 /* force GCC to generate only one epilog at the end of the function */
+#if defined(__i386__) || defined(__x86_64__)
+/* Also add 4 bytes of padding so that we can replace the ret with a 
jmp.  */
+#define FORCE_RET() asm volatile ("nop;nop;nop;nop");
+#else
 #define FORCE_RET() asm volatile ("");
+#endif
+
 
 #ifndef OPPROTO
 #define OPPROTO
@@ -205,12 +211,19 @@
 #endif
 
 #ifdef __i386__
-#define EXIT_TB() asm volatile ("ret")
-#define GOTO_LABEL_PARAM(n) asm volatile ("jmp " 
ASM_NAME(__op_gen_label) #n)
+/* Dyngen will replace hlt instructions with a ret instruction.  
Inserting a
+   ret directly would confuse dyngen.  */
+#define EXIT_TB() asm volatile ("hlt")
+/* Dyngen will replace cli with 0x9e (jmp).
+   We generate the offset manually.  */
+#define GOTO_LABEL_PARAM(n) \
+  asm volatile ("cli;.long " ASM_NAME(__op_gen_label) #n " - 1f;1:")
 #endif
 #ifdef __x86_64__
-#define EXIT_TB() asm volatile ("ret")
-#define GOTO_LABEL_PARAM(n) asm volatile ("jmp " 
ASM_NAME(__op_gen_label) #n)
+/* The same as i386.  */
+#define EXIT_TB() asm volatile ("hlt")
+#define GOTO_LABEL_PARAM(n) \
+  asm volatile ("cli;.long " ASM_NAME(__op_gen_label) #n " - 1f;1:")
 #endif
 #ifdef __powerpc__
 #define EXIT_TB() asm volatile ("blr")
diff -Naur qemu-orig/dyngen.c qemu-0.7.1/dyngen.c
--- qemu-orig/dyngen.c    Sun Jul 24 14:52:08 2005
+++ qemu-0.7.1/dyngen.c    Fri Jul 29 15:29:28 2005
@@ -32,6 +32,649 @@
 
 #include "config-host.h"
 
+#if defined(HOST_I386) || defined(HOST_X86_64)
+
+/* This byte is the first byte of an instruction.  */
+#define FLAG_INSN     (1 << 0)
+/* This byte has been processed as part of an instruction.  */
+#define FLAG_SCANNED  (1 << 1)
+/* This instruction is a return instruction.  Gcc cometimes generates 
prefix
+   bytes, so may be more than one byte long.  */
+#define FLAG_RET      (1 << 2)
+/* This is either the target of a jump, or the preceeding instruction uses
+   a pc-relative offset.  */
+#define FLAG_TARGET   (1 << 3)
+/* This is a magic instruction that needs fixing up.  */
+#define FLAG_EXIT     (1 << 4)
+#define MAX_EXITS     5
+
+static void
+bad_opcode(const char *name, uint32_t op)
+{
+    //error("Unsupported opcode %0*x in %s", (op > 0xff) ? 4 : 2, op, 
name);
+    exit(1);
+}
+
+/* Mark len bytes as scanned,  Returns insn_size + len.  Reports an error
+   if these bytes have already been scanned.  */
+static int
+eat_bytes(const char *name, char *flags, int insn, int insn_size, int len)
+{
+    while (len > 0) {
+        /* This should never occur in sane code.  */
+        if (flags[insn + insn_size] & FLAG_SCANNED)
+            //error ("Overlapping instructions in %s", name);
+            exit(1);
+        flags[insn + insn_size] |= FLAG_SCANNED;
+        insn_size++;
+        len--;
+    }
+    return insn_size;
+}
+
+static void
+trace_i386_insn (const char *name, uint8_t *start_p, char *flags, int insn,
+                 int len)
+{
+    uint8_t *ptr;
+    uint8_t op;
+    int modrm;
+    int is_prefix;
+    int op_size;
+    int addr_size;
+    int insn_size;
+    int is_ret;
+    int is_condjmp;
+    int is_jmp;
+    int is_exit;
+    int is_pcrel;
+    int immed;
+    int seen_rexw;
+    int32_t disp;
+
+    ptr = start_p + insn;
+    /* nonzero if this insn has a ModR/M byte.  */
+    modrm = 1;
+    /* The size of the immediate value in this instruction.  */
+    immed = 0;
+    /* The operand size.  */
+    op_size = 4;
+    /* The address size */
+    addr_size = 4;
+    /* The total length of this instruction.  */
+    insn_size = 0;
+    is_prefix = 1;
+    is_ret = 0;
+    is_condjmp = 0;
+    is_jmp = 0;
+    is_exit = 0;
+    seen_rexw = 0;
+    is_pcrel = 0;
+
+    while (is_prefix) {
+        op = ptr[insn_size];
+        insn_size = eat_bytes(name, flags, insn, insn_size, 1);
+        is_prefix = 0;
+        switch (op >> 4) {
+        case 0:
+        case 1:
+        case 2:
+        case 3:
+            if (op == 0x0f) {
+                /* two-byte opcode.  */
+                op = ptr[insn_size];
+                insn_size = eat_bytes(name, flags, insn, insn_size, 1);
+                switch (op >> 4) {
+                case 0:
+                    if ((op & 0xf) > 3)
+                      modrm = 0;
+                    break;
+                case 1: /* vector move or prefetch */
+                case 2: /* various moves and vector compares.  */
+                case 4: /* cmov */
+                case 5: /* vector instructions */
+                case 6:
+                case 13:
+                case 14:
+                case 15:
+                    break;
+                case 7: /* mmx */
+                    if (op & 0x77) /* emms */
+                      modrm = 0;
+                    break;
+                case 3: /* wrmsr, rdtsc, rdmsr, rdpmc, sysenter, sysexit */
+                    modrm = 0;
+                    break;
+                case 8: /* long conditional jump */
+                    is_condjmp = 1;
+                    immed = op_size;
+                    modrm = 0;
+                    break;
+                case 9: /* setcc */
+                    break;
+                case 10:
+                    switch (op & 0x7) {
+                    case 0: /* push fs/gs */
+                    case 1: /* pop fs/gs */
+                    case 2: /* cpuid/rsm */
+                        modrm = 0;
+                        break;
+                    case 4: /* shld/shrd immediate */
+                        immed = 1;
+                        break;
+                    default: /* Normal instructions with a ModR/M byte.  */
+                        break;
+                    }
+                    break;
+                case 11:
+                    switch (op & 0xf) {
+                    case 10: /* bt, bts, btr, btc */
+                        immed = 1;
+                        break;
+                    default:
+                        /* cmpxchg, lss, btr, lfs, lgs, movzx, btc, 
bsf, bsr
+                           undefined, and movsx */
+                        break;
+                    }
+                    break;
+                case 12:
+                    if (op & 8) {
+                        /* bswap */
+                        modrm = 0;
+                    } else {
+                        switch (op & 0x7) {
+                        case 2:
+                        case 4:
+                        case 5:
+                        case 6:
+                            immed = 1;
+                            break;
+                        default:
+                            break;
+                        }
+                    }
+                    break;
+                }
+            } else if ((op & 0x07) <= 0x3) {
+                /* General arithmentic ax.  */
+            } else if ((op & 0x07) <= 0x5) {
+                /* General arithmetic ax, immediate.  */
+                if (op & 0x01)
+                    immed = op_size;
+                else
+                    immed = 1;
+                modrm = 0;
+            } else if ((op & 0x23) == 0x22) {
+                /* Segment prefix.  */
+                is_prefix = 1;
+            } else {
+                /* Segment register push/pop or DAA/AAA/DAS/AAS.  */
+                modrm = 0;
+            }
+            break;
+
+#if defined(HOST_X86_64)
+        case 4: /* rex prefix.  */
+            is_prefix = 1;
+            /* The address/operand size is actually 64-bit, but the 
immediate
+               values in the instruction are still 32-bit.  */
+            op_size = 4;
+            addr_size = 4;
+            if (op & 8)
+                seen_rexw = 1;
+            break;
+#else
+        case 4: /* inc/dec register.  */
+#endif
+        case 5: /* push/pop general register.  */
+            modrm = 0;
+            break;
+
+        case 6:
+            switch (op & 0x0f) {
+            case 0: /* pusha */
+            case 1: /* popa */
+                modrm = 0;
+                break;
+            case 2: /* bound */
+            case 3: /* arpl */
+                break;
+            case 4: /* FS */
+            case 5: /* GS */
+                is_prefix = 1;
+                break;
+            case 6: /* opcode size prefix.  */
+                op_size = 2;
+                is_prefix = 1;
+                break;
+            case 7: /* Address size prefix.  */
+                addr_size = 2;
+                is_prefix = 1;
+                break;
+            case 8: /* push immediate */
+            case 10: /* pop immediate */
+                immed = op_size;
+                modrm = 0;
+                break;
+            case 9: /* imul immediate */
+            case 11: /* imul immediate */
+                immed = op_size;
+                break;
+            case 12: /* insb */
+            case 13: /* insw */
+            case 14: /* outsb */
+            case 15: /* outsw */
+                modrm = 0;
+                break;
+            }
+            break;
+
+        case 7: /* Short conditional jump.  */
+            is_condjmp = 1;
+            immed = 1;
+            modrm = 0;
+            break;
+         
+        case 8:
+            if ((op & 0xf) <= 3) {
+                /* arithmetic immediate.  */
+                if ((op & 3) == 1)
+                    immed = op_size;
+                else
+                    immed = 1;
+            }
+            /* else test, xchg, mov, lea or pop general.  */
+            break;
+
+        case 9:
+            /* Various single-byte opcodes with no modrm byte.  */
+            modrm = 0;
+            if (op == 10) {
+                /* Call */
+                immed = 4;
+            }
+            break;
+
+        case 10:
+            switch ((op & 0xe) >> 1) {
+            case 0: /* mov absoliute immediate.  */
+            case 1:
+                if (seen_rexw)
+                    immed = 8;
+                else
+                    immed = addr_size;
+                break;
+            case 4: /* test immediate.  */
+                if (op & 1)
+                    immed = op_size;
+                else
+                    immed = 1;
+                break;
+            default: /* Various string ops.  */
+                break;
+            }
+            modrm = 0;
+            break;
+
+        case 11: /* move immediate to register */
+            if (op & 8) {
+                if (seen_rexw)
+                    immed = 8;
+                else
+                    immed = op_size;
+            } else {
+                immed = 1;
+            }
+            modrm = 0;
+            break;
+
+          case 12:
+            switch (op & 0xf) {
+            case 0: /* shift immediate */
+            case 1:
+                immed = 1;
+                break;
+            case 2: /* ret immediate */
+                immed = 2;
+                modrm = 0;
+                bad_opcode(name, op);
+                break;
+            case 3: /* ret */
+                modrm = 0;
+                is_ret = 1;
+            case 4: /* les */
+            case 5: /* lds */
+                break;
+            case 6: /* mov immediate byte */
+                immed = 1;
+                break;
+            case 7: /* mov immediate */
+                immed = op_size;
+                break;
+            case 8: /* enter */
+                /* TODO: Is this right?  */
+                immed = 3;
+                modrm = 0;
+                break;
+            case 10: /* retf immediate */
+                immed = 2;
+                modrm = 0;
+                bad_opcode(name, op);
+                break;
+            case 13: /* int */
+                immed = 1;
+                modrm = 0;
+                break;
+            case 11: /* retf */
+            case 15: /* iret */
+                modrm = 0;
+                bad_opcode(name, op);
+                break;
+            default: /* leave, int3 or into */
+                modrm = 0;
+                break;
+            }
+            break;
+
+        case 13:
+            if ((op & 0xf) >= 8) {
+                /* Coprocessor escape.  For our purposes this is just a 
normal
+                   instruction with a ModR/M byte.  */
+            } else if ((op & 0xf) >= 4) {
+                /* AAM, AAD or XLAT */
+                modrm = 0;
+            }
+            /* else shift instruction */
+            break;
+
+        case 14:
+            switch ((op & 0xc) >> 2) {
+            case 0: /* loop or jcxz */
+                is_condjmp = 1;
+                immed = 1;
+                break;
+            case 1: /* in/out immed */
+                immed = 1;
+                break;
+            case 2: /* call or jmp */
+                switch (op & 3) {
+                case 0: /* call */
+                    immed = op_size;
+                    break;
+                case 1: /* long jump */
+                    immed = 4;
+                    is_jmp = 1;
+                    break;
+                case 2: /* far jmp */
+                    bad_opcode(name, op);
+                    break;
+                case 3: /* short jmp */
+                    immed = 1;
+                    is_jmp = 1;
+                    break;
+                }
+                break;
+            case 3: /* in/out register */
+                break;
+            }
+            modrm = 0;
+            break;
+
+        case 15:
+            switch ((op & 0xe) >> 1) {
+            case 0:
+            case 1:
+                is_prefix = 1;
+                break;
+            case 2:
+            case 4:
+            case 5:
+            case 6:
+                modrm = 0;
+                /* Some privileged insns are used as markers.  */
+                switch (op) {
+                case 0xf4: /* hlt: Exit translation block.  */
+                    is_exit = 1;
+                    break;
+                case 0xfa: /* cli: Jump to label.  */
+                    is_exit = 1;
+                    immed = 4;
+                    break;
+                case 0xfb: /* sti: TB patch jump.  */
+                    /* Mark the insn for patching, but continue 
sscanning.  */
+                    flags[insn] |= FLAG_EXIT;
+                    immed = 4;
+                    break;
+                }
+                break;
+            case 3: /* unary grp3 */
+                if ((ptr[insn_size] & 0x38) == 0) {
+                    if (op == 0xf7)
+                        immed = op_size;
+                    else
+                        immed = 1; /* test immediate */
+                }
+                break;
+            case 7: /* inc/dec grp4/5 */
+                /* TODO: This includes indirect jumps.  We should fail 
if we
+                   encounter one of these. */
+                break;
+            }
+            break;
+        }
+    }
+
+    if (modrm) {
+        if (addr_size != 4)
+            //error("16-bit addressing mode used in %s", name);
+            exit(1);
+
+        disp = 0;
+        modrm = ptr[insn_size];
+        insn_size = eat_bytes(name, flags, insn, insn_size, 1);
+        modrm &= 0xc7;
+        switch ((modrm & 0xc0) >> 6) {
+        case 0:
+            if (modrm == 5)
+              disp = 4;
+            break;
+        case 1:
+            disp = 1;
+            break;
+        case 2:
+            disp = 4;
+            break;
+        }
+        if ((modrm & 0xc0) != 0xc0 && (modrm & 0x7) == 4) {
+            /* SIB byte */
+            if (modrm == 4 && (ptr[insn_size] & 0x7) == 5) {
+                disp = 4;
+                is_pcrel = 1;
+            }
+            insn_size = eat_bytes(name, flags, insn, insn_size, 1);
+        }
+        insn_size = eat_bytes(name, flags, insn, insn_size, disp);
+    }
+    insn_size = eat_bytes(name, flags, insn, insn_size, immed);
+    if (is_condjmp || is_jmp) {
+        if (immed == 1) {
+            disp = (int8_t)*(ptr + insn_size - 1);
+        } else {
+            disp = (((int32_t)*(ptr + insn_size - 1)) << 24)
+                   | (((int32_t)*(ptr + insn_size - 2)) << 16)
+                   | (((int32_t)*(ptr + insn_size - 3)) << 8)
+                   | *(ptr + insn_size - 4);
+        }
+        disp += insn_size;
+        /* Jumps to external symbols point to the address of the offset
+           before relocation.  */
+        /* ??? These are probably a tailcall.  We could fix them up by
+           replacing them with jmp to EOB + call, but it's easier to just
+           prevent the compiler generating them.  */
+        if (disp == 1)
+            //error("Unconditional jump (sibcall?) in %s", name);
+            exit(1);
+        disp += insn;
+        if (disp < 0 || disp > len)
+            //error("Jump outside instruction in %s", name);
+            exit(1);
+
+        if ((flags[disp] & (FLAG_INSN | FLAG_SCANNED)) == FLAG_SCANNED)
+            //error("Overlapping instructions in %s", name);
+            exit(1);
+
+        flags[disp] |= (FLAG_INSN | FLAG_TARGET);
+        is_pcrel = 1;
+    }
+    if (is_pcrel) {
+        /* Mark the following insn as a jump target.  This will stop
+           this instruction being moved.  */
+        flags[insn + insn_size] |= FLAG_TARGET;
+    }
+    if (is_ret)
+      flags[insn] |= FLAG_RET;
+
+    if (is_exit)
+      flags[insn] |= FLAG_EXIT;
+
+    if (!(is_jmp || is_ret || is_exit))
+      flags[insn + insn_size] |= FLAG_INSN;
+}
+
+/* Scan a function body.  Returns the position of the return sequence.
+   Sets *patch_bytes to the number of bytes that need to be copied from 
that
+   location.  If no patching is required (ie. the return is the last insn)
+   *patch_bytes will be set to -1.  *plen is the number of code bytes 
to copy.
+ */
+static int trace_i386_op(const char * name, uint8_t *start_p, int *plen,
+                         int *patch_bytes, int *exit_addrs)
+{
+    char *flags;
+    int more;
+    int insn;
+    int retpos;
+    int bytes;
+    int num_exits;
+    int len;
+    int last_insn;
+
+    len = *plen;
+    flags = malloc(len + 1);
+    memset(flags, 0, len + 1);
+    flags[0] |= FLAG_INSN;
+    more = 1;
+    while (more) {
+        more = 0;
+        for (insn = 0; insn < len; insn++) {
+            if ((flags[insn] & (FLAG_INSN | FLAG_SCANNED)) == FLAG_INSN) {
+                trace_i386_insn(name, start_p, flags, insn, len);
+                more = 1;
+            }
+        }
+    }
+
+    /* Strip any unused code at the end of the function.  */
+    while (len > 0 && flags[len - 1] == 0)
+      len--;
+
+    retpos = -1;
+    num_exits = 0;
+    last_insn = 0;
+    for (insn = 0; insn < len; insn++) {
+        if (flags[insn] & FLAG_RET) {
+            /* ??? In theory it should be possible to handle multiple 
return
+               points.  In practice it's not worth the effort.  */
+            if (retpos != -1)
+                //error("Multiple return instructions in %s", name);
+                exit(1);
+            retpos = insn;
+        }
+        if (flags[insn] & FLAG_EXIT) {
+            if (num_exits == MAX_EXITS)
+                //error("Too many block exits in %s", name);
+                exit(1);
+            exit_addrs[num_exits] = insn;
+            num_exits++;
+        }
+        if (flags[insn] & FLAG_INSN)
+            last_insn = insn;
+    }
+
+    exit_addrs[num_exits] = -1;
+    if (retpos == -1) {
+        if (num_exits == 0) {
+            //error ("No return instruction found in %s", name);
+            exit(1);
+        } else {
+            retpos = len;
+            last_insn = len;
+        }
+    }
+   
+    /* If the return instruction is the last instruction we can just
+       remove it.  */
+    if (retpos == last_insn)
+        *patch_bytes = -1;
+    else
+        *patch_bytes = 0;
+
+    /* Back up over any nop instructions.  */
+    while (retpos > 0
+           && (flags[retpos] & FLAG_TARGET) == 0
+           && (flags[retpos - 1] & FLAG_INSN) != 0
+           && start_p[retpos - 1] == 0x90) {
+        retpos--;
+    }
+
+    if (*patch_bytes == -1) {
+        *plen = retpos;
+        free (flags);
+        return retpos;
+    }
+    *plen = len;
+
+    /* The ret is in the middle of the function.  Find four more bytes that
+       so the ret can be replaced by a jmp. */
+    /* ??? Use a short jump where possible. */
+    bytes = 4;
+    insn = retpos + 1;
+    /* We can clobber everything up to the next jump target.  */
+    while (insn < len && bytes > 0 && (flags[insn] & FLAG_TARGET) == 0) {
+        insn++;
+        bytes--;
+    }
+    if (bytes > 0) {
+        /* ???: Strip out nop blocks.  */
+        /* We can't do the replacement without clobbering anything 
important.
+           Copy preceeding instructions(s) to give us some space.  */
+        while (retpos > 0) {
+            /* If this byte is the target of a jmp we can't move it.  */
+            if (flags[retpos] & FLAG_TARGET)
+                break;
+
+            (*patch_bytes)++;
+            bytes--;
+            retpos--;
+
+            /* Break out of the loop if we have enough space and this 
is either
+               the first byte of an instruction or a pad byte.  */
+            if ((flags[retpos] & (FLAG_INSN | FLAG_SCANNED)) != 
FLAG_SCANNED
+                && bytes <= 0) {
+                break;
+            }
+        }
+    }
+
+    if (bytes > 0)
+        //error("Unable to replace ret with jmp in %s\n", name);
+        exit(1);
+
+    free(flags);
+    return retpos;
+}
+
+#endif
+
 /* NOTE: we test CONFIG_WIN32 instead of _WIN32 to enabled cross
    compilation */
 #if defined(CONFIG_WIN32)
@@ -1356,6 +1999,12 @@
     uint8_t args_present[MAX_ARGS];
     const char *sym_name, *p;
     EXE_RELOC *rel;
+   
+#if defined(HOST_I386) || defined(HOST_X86_64)
+    int patch_bytes;
+    int retpos;
+    int exit_addrs[MAX_EXITS];
+#endif   
 
     /* Compute exact size excluding prologue and epilogue instructions.
      * Increment start_offset to skip epilogue instructions, then compute
@@ -1366,33 +2015,12 @@
     p_end = p_start + size;
     start_offset = offset;
 #if defined(HOST_I386) || defined(HOST_X86_64)
-#ifdef CONFIG_FORMAT_COFF
-    {
-        uint8_t *p;
-        p = p_end - 1;
-        if (p == p_start)
-            error("empty code for %s", name);
-        while (*p != 0xc3) {
-            p--;
-            if (p <= p_start)
-                error("ret or jmp expected at the end of %s", name);
-        }
-        copy_size = p - p_start;
-    }
-#else
     {
         int len;
         len = p_end - p_start;
-        if (len == 0)
-            error("empty code for %s", name);
-        if (p_end[-1] == 0xc3) {
-            len--;
-        } else {
-            error("ret or jmp expected at the end of %s", name);
-        }
+        retpos = trace_i386_op(name, p_start, &len, &patch_bytes, 
exit_addrs);
         copy_size = len;
     }
-#endif   
 #elif defined(HOST_PPC)
     {
         uint8_t *p;
@@ -1559,6 +2187,13 @@
     }
 
     if (gen_switch == 2) {
+#if defined(HOST_I386) || defined(HOST_X86_64)
+        if (patch_bytes != -1)
+            copy_size += patch_bytes;
+#ifdef DEBUG_OP
+        copy_size += 2;
+#endif
+#endif
         fprintf(outfile, "DEF(%s, %d, %d)\n", name + 3, nb_args, 
copy_size);
     } else if (gen_switch == 1) {
 
@@ -1629,7 +2264,7 @@
 
             for(i = 0, sym = symtab; i < nb_syms; i++, sym++) {
                 sym_name = get_sym_name(sym);
-                if (strstart(sym_name, "__op_label", &p)) {
+                if (strstart(sym_name, "__op_label", &p) || 
strstart(sym_name, "_op_label", &p)) {
                     uint8_t *ptr;
                     unsigned long offset;
                    
@@ -1760,9 +2395,45 @@
 #else
 #error unsupport object format
 #endif
-                }
-                }
             }
+        }
+        /* Replace the marker instructions with the actual opcodes.  */
+        for (i = 0; exit_addrs[i] != -1; i++) {
+            int op;
+            switch (p_start[exit_addrs[i]])
+              {
+              case 0xf4: op = 0xc3; break; /* hlt -> ret */
+              case 0xfa: op = 0xe9; break; /* cli -> jmp */
+              case 0xfb: op = 0xe9; break; /* sti -> jmp */
+              default: error("Internal error");
+              }
+            fprintf(outfile,
+                    "    *(uint8_t *)(gen_code_ptr + %d) = 0x%x;\n",
+                    exit_addrs[i], op);
+        }
+        /* Fix up the return instruction.  */
+        if (patch_bytes != -1) {
+            if (patch_bytes) {
+                fprintf(outfile, "    memcpy(gen_code_ptr + %d,"
+                        "gen_code_ptr + %d, %d);\n",
+                        copy_size, retpos, patch_bytes);
+            }
+            fprintf(outfile,
+                    "    *(uint8_t *)(gen_code_ptr + %d) = 0xe9;\n",
+                    retpos);
+            fprintf(outfile,
+                    "    *(uint32_t *)(gen_code_ptr + %d) = 0x%x;\n",
+                    retpos + 1, copy_size - (retpos + 5));
+           
+            copy_size += patch_bytes;           
+        }
+        }
+#ifdef DEBUG_OP
+                fprintf(outfile,
+                        "    *(uint16_t *)(gen_code_ptr + %d) = 0x9090;\n",
+                        copy_size);
+                copy_size += 2;
+#endif
 #elif defined(HOST_X86_64)
             {
                 char name[256];
@@ -1792,7 +2463,46 @@
                         error("unsupported X86_64 relocation (%d)", type);
                     }
                 }
+            }
+
+            /* Replace the marker instructions with the actual opcodes.  */
+            for (i = 0; exit_addrs[i] != -1; i++) {
+                int op;
+                switch (p_start[exit_addrs[i]])
+                  {
+                  case 0xf4: op = 0xc3; break; /* hlt -> ret */
+                  case 0xfa: op = 0xe9; break; /* cli -> jmp */
+                  case 0xfb: op = 0xe9; break; /* sti -> jmp */
+                  default: error("Internal error");
+                  }
+                fprintf(outfile,
+                        "    *(uint8_t *)(gen_code_ptr + %d) = 0x%x;\n",
+                        exit_addrs[i], op);
+            }
+            /* Fix up the return instruction.  */
+            if (patch_bytes != -1) {
+                if (patch_bytes) {
+                    fprintf(outfile, "    memcpy(gen_code_ptr + %d,"
+                            "gen_code_ptr + %d, %d);\n",
+                            copy_size, retpos, patch_bytes);
                 }
+                fprintf(outfile,
+                        "    *(uint8_t *)(gen_code_ptr + %d) = 0xe9;\n",
+                        retpos);
+                fprintf(outfile,
+                        "    *(uint32_t *)(gen_code_ptr + %d) = 0x%x;\n",
+                        retpos + 1, copy_size - (retpos + 5));
+               
+                copy_size += patch_bytes;
+            }
+#ifdef DEBUG_OP
+                fprintf(outfile,
+                        "    *(uint16_t *)(gen_code_ptr + %d) = 0x9090;\n",
+                        copy_size);
+                copy_size += 2;
+#endif       
+           
+           
             }
 #elif defined(HOST_PPC)
             {
diff -Naur qemu-orig/exec-all.h qemu-0.7.1/exec-all.h
--- qemu-orig/exec-all.h    Sun Jul 24 14:52:08 2005
+++ qemu-0.7.1/exec-all.h    Fri Jul 29 15:28:52 2005
@@ -342,7 +342,7 @@
           ASM_NAME(__op_label) #n "." ASM_NAME(opname) ":\n"\
           ".long 1f\n"\
           ASM_PREVIOUS_SECTION \
-                  "jmp " ASM_NAME(__op_jmp) #n "\n"\
+                  "sti;.long " ASM_NAME(__op_jmp) #n " - 1f\n"\
           "1:\n");\
 } while (0)
 
@@ -350,10 +350,17 @@
 
 /* jump to next block operations (more portable code, does not need
    cache flushing, but slower because of indirect jump) */
+#if __GNUC__ == 3 && __GNUC_MINOR__ < 4
+#define UNUSED __attribute__((unused))
+#elif defined(__GNUC__)
+#define UNUSED __attribute__((used))
+#else
+#define UNUSED
+#endif  
 #define GOTO_TB(opname, tbparam, n)\
 do {\
-    static void __attribute__((unused)) *dummy ## n = &&dummy_label ## n;\
-    static void __attribute__((unused)) *__op_label ## n = &&label ## n;\
+    static void UNUSED *dummy ## n = &&dummy_label ## n;\
+    static void UNUSED *__op_label ## n = &&label ## n;\
     goto *(void *)(((TranslationBlock *)tbparam)->tb_next[n]);\
 label ## n: ;\
 dummy_label ## n: ;\
diff -Naur qemu-orig/target-ppc/exec.h qemu-0.7.1/target-ppc/exec.h
--- qemu-orig/target-ppc/exec.h    Sun Jul 24 14:52:08 2005
+++ qemu-0.7.1/target-ppc/exec.h    Fri Jul 29 15:28:52 2005
@@ -37,11 +37,7 @@
 #define FT1 (env->ft1)
 #define FT2 (env->ft2)
 
-#if defined (DEBUG_OP)
-#define RETURN() __asm__ __volatile__("nop");
-#else
-#define RETURN() __asm__ __volatile__("");
-#endif
+#define RETURN() FORCE_RET()
 
 #include "cpu.h"
 #include "exec-all.h"

Re: [Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Paul Brook]

On Saturday 30 July 2005 19:42, Steve D. Perkins wrote:
> Hello all -
>
>     I wrote to the list a few days ago to gauge interest in adding
> support for GCC in the Win32 environment to the QEMU codebase.  I
> received no response to that inquiry, but decided to update my patch for
> the 0.7.1 release anyway in hopes that it will be of use to people.  The
> patchfile text is included below... simply cut-and-paste it to a file
> named "patchfile" in the location where you've decompressed the
> qemu-0.7.1 tarball, and run the command "patch -p0 < patchfile".

Are you sure you included the correct patch?

I don't see anything win32 specific about this patch. AFAICS it's just the 
gcc4 patch *that I wrote* and posed some time ago.
Both cygwin and mingw is still using gcc3.4.x, so it shouldn't be necessary.

Paul

Re: [Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Steve D. Perkins]

Paul Brook wrote:

>Are you sure you included the correct patch?
>
>I don't see anything win32 specific about this patch. AFAICS it's just the 
>gcc4 patch *that I wrote* and posed some time ago.
>Both cygwin and mingw is still using gcc3.4.x, so it shouldn't be necessary.
>
>  
>
Paul -

    Please don't think I'm trying to "plagiarize" or steal anyone else's 
credit.  I've been talking about this patch for some time, in posts to 
the QEMU forum at "http://qemu.dad-answers.com/" and in the "Applying 
windows source patch to CVS?" post to this mailing list on 7/27.  When I 
was trying to build QEMU 0.7.0, I found a patch at 
"http://www.h7.dion.ne.jp/~qemu-win/PortingTips-en.html" for the 0.6 
codebase... and I made a few modifications to it for 0.7.0.  Yesterday, 
I went through the same exercise to update it for QEMU 0.7.1.  I've 
changed some of the calls to the "error()" function in "dyngen.c", but 
otherwise the main patch content remains the same.

    I've been very up-front about the origins of the patch, and haven't 
tried to pass myself off as the original author.  However, the original 
author hasn't posted an updated patch since the 0.6 generation of QEMU, 
so you could say I've "adopted" and kept it maintained for the past two 
QEMU releases.  My INITIAL desire, before I started, was to contact the 
original patch author and ask him/her to update it... but the website 
listed above includes no names or contact information at all.  Believe 
me, I am NOT looking to be "The Official GCC/Win32 Guy" and continue 
maintaining this patch forever... my ideal wish is for the changes to go 
into the main codebase so future patching isn't necessary for anybody!

    I'm not sure what you mean about the patch "not being necessary".  I 
haven't tried building with Cygwin, so I can't speak for that platform.  
My build environment is MinGW with the MSYS shell (to avoid onerous 
licensing issues and DLL dependencies), and I can assure that the patch 
very much is necessary for the current release of QEMU with the current 
release of MinGW.  QEMU does compile fine on Win32 with GCC 3.4, but the 
executable crashes immediately when you try to run it.  By applying the 
patch I just posted, you get an executable that compiles and runs 
properly (I've been running a FreeBSD environment just fine with a build 
from the 0.7.1 patch I just posted). 

    I get the impression that QEMU is not really being tested on the 
Win32/GCC platform, so the team (is there a "team" or is it really just 
Fabrice?) may not be aware that support is still lacking for that 
platform.  Unfortunately, that is the case... but the patch I just 
posted corrects the issue, and that's why I'm encouraging its inclusion 
in the main codebase.

Steve

Re: [Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Filip Navara]

Hi!

Steve D. Perkins wrote:

> Paul Brook wrote:
>
>> Are you sure you included the correct patch?
>>
>> I don't see anything win32 specific about this patch. AFAICS it's 
>> just the gcc4 patch *that I wrote* and posed some time ago.
>> Both cygwin and mingw is still using gcc3.4.x, so it shouldn't be 
>> necessary.
>
It is needed for gcc 3.4.x on MinGW. GCC 3.3.x works just fine.

> Paul -

[snip]

>    I get the impression that QEMU is not really being tested on the 
> Win32/GCC platform, so the team (is there a "team" or is it really 
> just Fabrice?) may not be aware that support is still lacking for that 
> platform.

[snip]

I think this is false impression. I'm testing the Win32 version 
frequently and if any compilation problem arises I try to send the 
patches back (though I've been lacking in that lately) ...

- Filip

Re: [Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Christian MICHON]

First rule of (efficient) engineering: <<if it is not broken,
do not try to fix it>>

for the record: gcc-3.3-x is *fine* and *most* stable,
not just on windows. It's still my reference compiler
even for linux kernels.

Why do you feel it's necessary to upgrade? Because
gcc people do? There's *no* speed improvement, as
far as I can see (well, it crashes faster :))
This is not a C++ based project.

Now that for the first time on win XP hosts we get
decent speed, unless patches are really a must, I do
not intend personnally to upgrade at every snapshot
my current version of qemu.

And testing does not mean using the latest only.
We're talking stability/reliability now that we have
kqemu in the picture (taking care of speed improvement).
I believe reporting each pb is a team effort. Fabrice
never reports any pb, he fixes them efficiently at his
own pace.

In short: there is a team out there. Or this mailing list
would be dead.

Christian

Re: [Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Paul Brook]

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

>     I'm not sure what you mean about the patch "not being necessary".  I
> haven't tried building with Cygwin, so I can't speak for that platform.
> My build environment is MinGW with the MSYS shell (to avoid onerous
> licensing issues and DLL dependencies), and I can assure that the patch
> very much is necessary for the current release of QEMU with the current
> release of MinGW.  QEMU does compile fine on Win32 with GCC 3.4, but the
> executable crashes immediately when you try to run it.  By applying the
> patch I just posted, you get an executable that compiles and runs
> properly (I've been running a FreeBSD environment just fine with a build
> from the 0.7.1 patch I just posted).

"sledgehammer" and "nut" spring to mind :-)

The attached patch is sufficient to get qemu working on win32 when compiled 
with gcc3.4. I've successfully booted a knoppix CD inside qemu on a windows 
host with this patch.

The problem was that gcc is choosing inconvenient names for static local 
variables.  These symbols are never supposed to be seen outside the compiler, 
so it's allowed to change these at random. The solution is to use explicitly 
specify the asm name for the variable.

Paul

[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1591 bytes --]

Index: exec-all.h
===================================================================
RCS file: /cvsroot/qemu/qemu/exec-all.h,v
retrieving revision 1.34
diff -u -p -r1.34 exec-all.h
--- exec-all.h	24 Jul 2005 14:14:53 -0000	1.34
+++ exec-all.h	30 Jul 2005 22:56:09 -0000
@@ -320,13 +320,16 @@ TranslationBlock *tb_find_pc(unsigned lo
 #define ASM_PREVIOUS_SECTION ".previous\n"
 #endif
 
+#define ASM_OP_LABEL_NAME(n, opname) \
+    ASM_NAME(__op_label) #n "." ASM_NAME(opname)
+
 #if defined(__powerpc__)
 
 /* we patch the jump instruction directly */
 #define GOTO_TB(opname, tbparam, n)\
 do {\
     asm volatile (ASM_DATA_SECTION\
-		  ASM_NAME(__op_label) #n "." ASM_NAME(opname) ":\n"\
+		  ASM_OP_LABEL_NAME(n, opname) ":\n"\
 		  ".long 1f\n"\
 		  ASM_PREVIOUS_SECTION \
                   "b " ASM_NAME(__op_jmp) #n "\n"\
@@ -339,7 +342,7 @@ do {\
 #define GOTO_TB(opname, tbparam, n)\
 do {\
     asm volatile (".section .data\n"\
-		  ASM_NAME(__op_label) #n "." ASM_NAME(opname) ":\n"\
+		  ASM_OP_LABEL_NAME(n, opname) ":\n"\
 		  ".long 1f\n"\
 		  ASM_PREVIOUS_SECTION \
                   "jmp " ASM_NAME(__op_jmp) #n "\n"\
@@ -353,7 +356,8 @@ do {\
 #define GOTO_TB(opname, tbparam, n)\
 do {\
     static void __attribute__((unused)) *dummy ## n = &&dummy_label ## n;\
-    static void __attribute__((unused)) *__op_label ## n = &&label ## n;\
+    static void __attribute__((unused)) *__op_label ## n \
+        __asm__(ASM_OP_LABEL_NAME(n, opname)) = &&label ## n;\
     goto *(void *)(((TranslationBlock *)tbparam)->tb_next[n]);\
 label ## n: ;\
 dummy_label ## n: ;\

Re: [Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Steve D. Perkins]

Christian MICHON wrote:

>First rule of (efficient) engineering: <<if it is not broken,
>do not try to fix it>>
>
>for the record: gcc-3.3-x is *fine* and *most* stable,
>not just on windows. It's still my reference compiler
>even for linux kernels.
>
>Why do you feel it's necessary to upgrade? Because
>gcc people do? 
>  
>
    Don't get me wrong, Christian.  I don't advocate using the 
bleeding-edge latest snapshot from CVS as your full-time reference 
compiler.  At the same time, you can't lag behind indefinitely without 
risking irrelevance.  The 3.4.x generation of GCC is several months old 
now (much older if you consider release candidates).  If it's not yet 
time to target 3.4.x as an officially supported compiler, it is AT LEAST 
time to start discussions on a timetable/strategy for doing so. 

    I was unaware that QEMU works properly when built with GCC 3.3.x... 
is that a true statement, or am I reading too deeply into your email?  
If it is true that QEMU builds on Win32 with GCC 3.3 but not 3.4, then 
the "qemu-doc.html" file at the very least should be updated before the 
next release.  That doc simply says "Install the current versions of 
MSYS and MinGW"... which has meant GCC 3.4.x for some time now.

>Now that for the first time on win XP hosts we get
>decent speed, unless patches are really a must, I do
>not intend personnally to upgrade at every snapshot
>my current version of qemu.
>  
>
    I feel the same way.  I don't plan to upgrade the "production" QEMU 
environment I run my virtual machines in every time there's an 
incremental upgrade.  However, I do plan to build every release for 
testing until the issues we're discussing are resolved, and I'm sure I 
will periodically upgrade my main QEMU environment.

    I don't think that chasing the bleeding-edge is a practical 
approach.  I also don't think that expecting the world to stick with GCC 
3.3 and QEMU 0.6.0 forever is feasible either.  I'm advocating what I 
believe is reasonable middle ground.  This type of discussion is EXACTLY 
why most seasoned open-source projects eventually evolve into "stable" 
and "unstable" branches... so the project can simultaneously meet your 
goal of providing a reliable current release, while providing a 
framework to meet my goal of adapting new things into the project 
incrementally.  Again, I'm just trying to start a discussion.

>In short: there is a team out there. Or this mailing list
>would be dead.
>
    No offense intended, that was a poor choice of phrasing on my part.  
What I was trying to figure out was whether multiple people are writing 
code for the project, either through direct source-control access or 
sending patches to Fabrice, or if the team's role is more supporting 
Fabrice's coding efforts through QA testing and advice/feedback. 

    Most open-source projects I've been involved with are so STARVED for 
volunteers willing-and-able to actually write code, I was surprised last 
week when I offered to write a patch and the response was dead silence.  
I'm just trying to get a feel for how the project operates, I hope that 
my enthusiasm doesn't come across the wrong way.

Steve

Re: [Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Steve D. Perkins]

Paul Brook wrote:

>"sledgehammer" and "nut" spring to mind :-)
>
>The attached patch is sufficient to get qemu working on win32 when compiled with gcc3.4. I've successfully booted a knoppix CD inside qemu on a windows host with this patch.
>
>The problem was that gcc is choosing inconvenient names for static local variables.  These symbols are never supposed to be seen outside the compiler, so it's allowed to change these at random. The solution is to use explicitly specify the asm name for the variable.
>
>Paul
>  
>
    Haha... thanks alot Paul!  I now have a big bruise from slapping my 
palm against my forehead... seeing how much more simplified the changes 
could be after I went through all that hassle editing "dyngen.c"!

    Given that QEMU can support the current release of GCC on Windows by 
changing less than 40 lines of code in one file, can we please begin 
discussing the inclusion of these changes in the main codebase?  
Explicitly specifying this one variable name in a preprocessor directive 
doesn't look as if it would affect builds on other platforms, but it 
would be easy enough to test either way.

Steve

Re: [Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Paul Brook]

>     Given that QEMU can support the current release of GCC on Windows by
> changing less than 40 lines of code in one file, can we please begin
> discussing the inclusion of these changes in the main codebase?
> Explicitly specifying this one variable name in a preprocessor directive
> doesn't look as if it would affect builds on other platforms, but it
> would be easy enough to test either way.

My 6-line gen_op_label patch should be safe, and isn't unintrusive, so IMHO 
should be applied.

The gcc4 changes are a different matter.  I discusses this with Fabrice on IRC 
shortly after submitting the patch. The patch is pretty invasive, high risk, 
and potentially hard to debug and maintain. Given this it's not clear if it's 
worth the hassle just to support gcc4.

It's generally agreed that the long term solution is to replace dyngen with a 
hand coded generator. I'm working on this, but there's still quite a bit to 
do.

Paul

Re: [Qemu-devel] [Patch Submission] QEMU with GCC/Win32

[Gwenole Beauchesne]

On Sun, 31 Jul 2005, Paul Brook wrote:

> The gcc4 changes are a different matter.  I discusses this with Fabrice on IRC 
> shortly after submitting the patch. The patch is pretty invasive, high risk, 
> and potentially hard to debug and maintain.

Talking of which, I don't know if I have sent you this patch but I see it
lying in my old home dir for submission. IIRC, I hit the imul case in real 
condition, the others are from further inspection from the manual.

--- dyngen.c.orig	2005-06-02 21:41:51.000000000 +0200
+++ dyngen.c	2005-06-02 22:19:59.000000000 +0200
@@ -1672,14 +1672,19 @@
                 is_prefix = 1;
                 break;
             case 8: /* push immediate */
-            case 10: /* pop immediate */
                 immed = op_size;
                 modrm = 0;
                 break;
+            case 10: /* push 8-bit immediate */
+                immed = 1;
+                modrm = 0;
+                break;
             case 9: /* imul immediate */
-            case 11: /* imul immediate */
                 immed = op_size;
                 break;
+            case 11: /* imul 8-bit immediate */
+                immed = 1;
+                break;
             case 12: /* insb */
             case 13: /* insw */
             case 14: /* outsb */