Re: [Qemu-devel] Problems with MIPS full system emulation and breakpoints

[Daniel Jacobowitz <drow@false.org>]

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

On Fri, Apr 20, 2007 at 02:22:09PM -0400, Daniel Jacobowitz wrote:
> I have an idea.  When I was talking to Paul about breakpoints
> recently, I noticed something very strange in the ARM port: it
> continues to disassemble the instruction under a breakpoint after
> generating the debug op.  This is a waste of CPU and memory, so I
> tried taking it out - but he told me that if I did that, things would
> go wrong because the size of the tb would be too small.  We'd try to
> flush the tb at the breakpoint location, but it wouldn't seem to cover
> there.
> 
> MIPS doesn't do that extra disassembly because it has a goto instead
> of a break from the nested loop.  What happens if you add an extra
> +1 to the translation block size if there's a breakpoint, in
> target-mips/translate.c?

It won't help because that problem related to "hardware" breakpoints
through QEMU's gdb stub.

The attached patch fixes that, and Jason's issue, and probably the
FPU emulation issue also.  The real problem was "tb->size = 0" in the
search_pc case.  Alpha, ARM, m68k, mips, ppc, sh4, and sparc all
did this.  But it can't be right - the tb passed when searching for a
pc is in the cache, and clearing its size prevents it from being
flushed properly.

I got a couple of strange oopses after this, and one unidentified
lockup.  I don't think they are related, though.

-- 
Daniel Jacobowitz
CodeSourcery

[-- Attachment #2: qemu-mips-breakpoints.patch --]
[-- Type: text/x-diff, Size: 3027 bytes --]

--- target-alpha/translate.c	(revision 181278)
+++ target-alpha/translate.c	(local)
@@ -2047,7 +2047,6 @@ int gen_intermediate_code_internal (CPUS
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = ctx.pc - pc_start;
     }
--- target-arm/translate.c	(revision 181278)
+++ target-arm/translate.c	(local)
@@ -3656,7 +3656,6 @@ static inline int gen_intermediate_code_
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = dc->pc - pc_start;
     }
--- target-m68k/translate.c	(revision 181278)
+++ target-m68k/translate.c	(local)
@@ -3260,7 +3260,6 @@ gen_intermediate_code_internal(CPUState 
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = dc->pc - pc_start;
     }
--- target-mips/translate.c	(revision 181278)
+++ target-mips/translate.c	(local)
@@ -5882,10 +5882,6 @@ static void decode_opc (CPUState *env, D
             generate_exception(ctx, EXCP_SYSCALL);
             break;
         case OPC_BREAK:
-            /* XXX: Hack to work around wrong handling of self-modifying code. */
-            ctx->pc += 4;
-            save_cpu_state(ctx, 1);
-            ctx->pc -= 4;
             generate_exception(ctx, EXCP_BREAK);
             break;
         case OPC_SPIM:
@@ -6433,6 +6429,9 @@ gen_intermediate_code_internal (CPUState
                     save_cpu_state(&ctx, 1);
                     ctx.bstate = BS_BRANCH;
                     gen_op_debug();
+                    /* Include the breakpoint location or the tb won't
+                     * be flushed when it must be.  */
+                    ctx.pc += 4;
                     goto done_generating;
                 }
             }
@@ -6493,7 +6492,6 @@ done_generating:
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = ctx.pc - pc_start;
     }
--- target-ppc/translate.c	(revision 181278)
+++ target-ppc/translate.c	(local)
@@ -5878,7 +5878,6 @@ static inline int gen_intermediate_code_
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
     } else {
         tb->size = ctx.nip - pc_start;
     }
--- target-sh4/translate.c	(revision 181278)
+++ target-sh4/translate.c	(local)
@@ -1242,7 +1242,6 @@ gen_intermediate_code_internal(CPUState 
         ii++;
         while (ii <= i)
             gen_opc_instr_start[ii++] = 0;
-        tb->size = 0;
     } else {
         tb->size = ctx.pc - pc_start;
     }
--- target-sparc/translate.c	(revision 181278)
+++ target-sparc/translate.c	(local)
@@ -3365,7 +3365,6 @@ static inline int gen_intermediate_code_
         lj++;
         while (lj <= j)
             gen_opc_instr_start[lj++] = 0;
-        tb->size = 0;
 #if 0
         if (loglevel > 0) {
             page_dump(logfile);