diff mbox

[v11,09/29] target/i386: [tcg] Refactor breakpoint_check

Message ID 149865437871.17063.9119703949695421203.stgit@frigg.lan (mailing list archive)
State New, archived
Headers show

Commit Message

Lluís Vilanova June 28, 2017, 12:52 p.m. UTC
Incrementally paves the way towards using the generic instruction translation
loop.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 target/i386/translate.c |   59 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 11 deletions(-)

Comments

Emilio Cota June 30, 2017, 12:24 a.m. UTC | #1
On Wed, Jun 28, 2017 at 15:52:58 +0300, Lluís Vilanova wrote:
> Incrementally paves the way towards using the generic instruction translation
> loop.
> 
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  target/i386/translate.c |   59 ++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 48 insertions(+), 11 deletions(-)
> 
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index ad57be2928..3eee348de7 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -18,6 +18,7 @@
>   */
>  #include "qemu/osdep.h"
>  
> +#include "qemu/error-report.h"

I can't find why this one is included here.

>  #include "qemu/host-utils.h"
>  #include "cpu.h"
>  #include "disas/disas.h"
> @@ -8458,6 +8459,25 @@ static void i386_trblock_insn_start(DisasContextBase *dcbase, CPUState *cpu)
>      tcg_gen_insn_start(dc->base.pc_next, dc->cc_op);
>  }
>  
> +static BreakpointCheckType i386_trblock_breakpoint_check(
> +    DisasContextBase *dcbase, CPUState *cpu, const CPUBreakpoint *bp)
> +{
> +    DisasContext *dc = container_of(dcbase, DisasContext, base);
> +    /* If RF is set, suppress an internally generated breakpoint.  */
> +    int flags = dc->base.tb->flags & HF_RF_MASK ? BP_GDB : BP_ANY;
> +    if (bp->flags & flags) {
> +        gen_debug(dc, dc->base.pc_next - dc->cs_base);
> +        /* The address covered by the breakpoint must be included in
> +           [tb->pc, tb->pc + tb->size) in order to for it to be
> +           properly cleared -- thus we increment the PC here so that
> +           the logic setting tb->size below does the right thing.  */
> +        dc->base.pc_next += 1;
> +        return BC_HIT_TB;
> +    } else {
> +        return BC_MISS;
> +    }
> +}
> +
>  /* generate intermediate code for basic block 'tb'.  */
>  void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb)
>  {
> @@ -8490,18 +8510,35 @@ void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb)
>          i386_trblock_insn_start(&dc->base, cpu);
>          num_insns++;
>  
> -        /* If RF is set, suppress an internally generated breakpoint.  */
> -        if (unlikely(cpu_breakpoint_test(cpu, dc->base.pc_next,
> -                                         tb->flags & HF_RF_MASK
> -                                         ? BP_GDB : BP_ANY))) {
> -            gen_debug(dc, dc->base.pc_next - dc->cs_base);
> -            /* The address covered by the breakpoint must be included in
> -               [tb->pc, tb->pc + tb->size) in order to for it to be
> -               properly cleared -- thus we increment the PC here so that
> -               the logic setting tb->size below does the right thing.  */
> -            dc->base.pc_next += 1;
> -            goto done_generating;
> +        if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
> +            CPUBreakpoint *bp;
> +            QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
> +                if (bp->pc == dc->base.pc_next) {
> +                    BreakpointCheckType bp_check =
> +                        i386_trblock_breakpoint_check(&dc->base, cpu, bp);
> +                    switch (bp_check) {
> +                    case BC_MISS:
> +                        /* Target ignored this breakpoint, go to next */
> +                        break;
> +                    case BC_HIT_INSN:
> +                        /* Hit, keep translating */
> +                        /*
> +                         * TODO: if we're never going to have more than one
> +                         *       BP in a single address, we can simply use a
> +                         *       bool here.
> +                         */
> +                        goto done_breakpoints;

BC_HIT_INSN is not needed here, but I see what you're doing.

Reviewed-by: Emilio G. Cota <cota@braap.org>

		E.
Richard Henderson July 2, 2017, 12:28 a.m. UTC | #2
On 06/28/2017 05:52 AM, Lluís Vilanova wrote:
> Incrementally paves the way towards using the generic instruction translation
> loop.
> 
> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>
> ---
>   target/i386/translate.c |   59 ++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 48 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~
diff mbox

Patch

diff --git a/target/i386/translate.c b/target/i386/translate.c
index ad57be2928..3eee348de7 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -18,6 +18,7 @@ 
  */
 #include "qemu/osdep.h"
 
+#include "qemu/error-report.h"
 #include "qemu/host-utils.h"
 #include "cpu.h"
 #include "disas/disas.h"
@@ -8458,6 +8459,25 @@  static void i386_trblock_insn_start(DisasContextBase *dcbase, CPUState *cpu)
     tcg_gen_insn_start(dc->base.pc_next, dc->cc_op);
 }
 
+static BreakpointCheckType i386_trblock_breakpoint_check(
+    DisasContextBase *dcbase, CPUState *cpu, const CPUBreakpoint *bp)
+{
+    DisasContext *dc = container_of(dcbase, DisasContext, base);
+    /* If RF is set, suppress an internally generated breakpoint.  */
+    int flags = dc->base.tb->flags & HF_RF_MASK ? BP_GDB : BP_ANY;
+    if (bp->flags & flags) {
+        gen_debug(dc, dc->base.pc_next - dc->cs_base);
+        /* The address covered by the breakpoint must be included in
+           [tb->pc, tb->pc + tb->size) in order to for it to be
+           properly cleared -- thus we increment the PC here so that
+           the logic setting tb->size below does the right thing.  */
+        dc->base.pc_next += 1;
+        return BC_HIT_TB;
+    } else {
+        return BC_MISS;
+    }
+}
+
 /* generate intermediate code for basic block 'tb'.  */
 void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb)
 {
@@ -8490,18 +8510,35 @@  void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb)
         i386_trblock_insn_start(&dc->base, cpu);
         num_insns++;
 
-        /* If RF is set, suppress an internally generated breakpoint.  */
-        if (unlikely(cpu_breakpoint_test(cpu, dc->base.pc_next,
-                                         tb->flags & HF_RF_MASK
-                                         ? BP_GDB : BP_ANY))) {
-            gen_debug(dc, dc->base.pc_next - dc->cs_base);
-            /* The address covered by the breakpoint must be included in
-               [tb->pc, tb->pc + tb->size) in order to for it to be
-               properly cleared -- thus we increment the PC here so that
-               the logic setting tb->size below does the right thing.  */
-            dc->base.pc_next += 1;
-            goto done_generating;
+        if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
+            CPUBreakpoint *bp;
+            QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
+                if (bp->pc == dc->base.pc_next) {
+                    BreakpointCheckType bp_check =
+                        i386_trblock_breakpoint_check(&dc->base, cpu, bp);
+                    switch (bp_check) {
+                    case BC_MISS:
+                        /* Target ignored this breakpoint, go to next */
+                        break;
+                    case BC_HIT_INSN:
+                        /* Hit, keep translating */
+                        /*
+                         * TODO: if we're never going to have more than one
+                         *       BP in a single address, we can simply use a
+                         *       bool here.
+                         */
+                        goto done_breakpoints;
+                    case BC_HIT_TB:
+                        /* Hit, end TB */
+                        goto done_generating;
+                    default:
+                        g_assert_not_reached();
+                    }
+                }
+            }
         }
+    done_breakpoints:
+
         if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
             gen_io_start();
         }