[Qemu-devel] [PATCH 1/2] target-sparc: Fix -singlestep.

[Qemu-devel] [PATCH 1/2] target-sparc: Fix -singlestep.

[Richard Henderson]

Single-stepping was not properly updating npc, resulting in some
instructions being executed twice.  In addition, we were emitting
dead code at the end of the TB.

Fix both by teaching gen_goto_tb to avoid goto_tb for single-step
and removing the special-case code in gen_intermediate_code_internal.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-sparc/translate.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index b54c520..5162b87 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -81,6 +81,7 @@ typedef struct DisasContext {
     int address_mask_32bit;
     uint32_t cc_op;  /* current CC operation */
     struct TranslationBlock *tb;
+    CPUState *env;
     sparc_def_t *def;
 } DisasContext;
 
@@ -234,7 +235,8 @@ static inline void gen_goto_tb(DisasContext *s, int tb_num,
 
     tb = s->tb;
     if ((pc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
-        (npc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK))  {
+        (npc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
+        !(s->env->singlestep_enabled || singlestep))  {
         /* jump to same page: we can use a direct jump */
         tcg_gen_goto_tb(tb_num);
         tcg_gen_movi_tl(cpu_pc, pc);
@@ -4680,6 +4682,7 @@ static inline void gen_intermediate_code_internal(TranslationBlock * tb,
 
     memset(dc, 0, sizeof(DisasContext));
     dc->tb = tb;
+    dc->env = env;
     pc_start = tb->pc;
     dc->pc = pc_start;
     last_pc = dc->pc;
@@ -4755,8 +4758,6 @@ static inline void gen_intermediate_code_internal(TranslationBlock * tb,
         /* if single step mode, we generate only one instruction and
            generate an exception */
         if (env->singlestep_enabled || singlestep) {
-            tcg_gen_movi_tl(cpu_pc, dc->pc);
-            tcg_gen_exit_tb(0);
             break;
         }
     } while ((gen_opc_ptr < gen_opc_end) &&
-- 
1.6.6.1

[Qemu-devel] [PATCH 2/2] linux-user: Fix Sparc64 syscall returns.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/main.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index b394c00..71a1b67 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -940,7 +940,8 @@ static void flush_windows(CPUSPARCState *env)
 
 void cpu_loop (CPUSPARCState *env)
 {
-    int trapnr, ret;
+    int trapnr;
+    abi_long ret;
     target_siginfo_t info;
 
     while (1) {
@@ -958,7 +959,7 @@ void cpu_loop (CPUSPARCState *env)
                               env->regwptr[0], env->regwptr[1],
                               env->regwptr[2], env->regwptr[3],
                               env->regwptr[4], env->regwptr[5]);
-            if ((unsigned int)ret >= (unsigned int)(-515)) {
+            if ((abi_ulong)ret >= (abi_ulong)(-515)) {
 #if defined(TARGET_SPARC64) && !defined(TARGET_ABI32)
                 env->xcc |= PSR_CARRY;
 #else
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH 2/2] linux-user: Fix Sparc64 syscall returns.

[Blue Swirl]

Thanks, applied.

On 4/25/10, Richard Henderson <rth@twiddle.net> wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
>  ---
>   linux-user/main.c |    5 +++--
>   1 files changed, 3 insertions(+), 2 deletions(-)
>
>  diff --git a/linux-user/main.c b/linux-user/main.c
>  index b394c00..71a1b67 100644
>  --- a/linux-user/main.c
>  +++ b/linux-user/main.c
>  @@ -940,7 +940,8 @@ static void flush_windows(CPUSPARCState *env)
>
>   void cpu_loop (CPUSPARCState *env)
>   {
>  -    int trapnr, ret;
>  +    int trapnr;
>  +    abi_long ret;
>      target_siginfo_t info;
>
>      while (1) {
>  @@ -958,7 +959,7 @@ void cpu_loop (CPUSPARCState *env)
>                                env->regwptr[0], env->regwptr[1],
>                                env->regwptr[2], env->regwptr[3],
>                                env->regwptr[4], env->regwptr[5]);
>  -            if ((unsigned int)ret >= (unsigned int)(-515)) {
>  +            if ((abi_ulong)ret >= (abi_ulong)(-515)) {
>   #if defined(TARGET_SPARC64) && !defined(TARGET_ABI32)
>                  env->xcc |= PSR_CARRY;
>   #else
>
> --
>  1.6.6.1
>
>

[Qemu-devel] Re: [PATCH 1/2] target-sparc: Fix -singlestep.

[Blue Swirl]

On 4/25/10, Richard Henderson <rth@twiddle.net> wrote:
> Single-stepping was not properly updating npc, resulting in some
>  instructions being executed twice.  In addition, we were emitting
>  dead code at the end of the TB.
>
>  Fix both by teaching gen_goto_tb to avoid goto_tb for single-step
>  and removing the special-case code in gen_intermediate_code_internal.

Nice work!

>  Signed-off-by: Richard Henderson <rth@twiddle.net>
>  ---
>   target-sparc/translate.c |    7 ++++---
>   1 files changed, 4 insertions(+), 3 deletions(-)
>
>  diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>  index b54c520..5162b87 100644
>  --- a/target-sparc/translate.c
>  +++ b/target-sparc/translate.c
>  @@ -81,6 +81,7 @@ typedef struct DisasContext {
>      int address_mask_32bit;
>      uint32_t cc_op;  /* current CC operation */
>      struct TranslationBlock *tb;
>  +    CPUState *env;

Instead of CPUState pointer, please create a flag which combines
env->singlestep_enabled || singlestep.

>      sparc_def_t *def;
>   } DisasContext;
>
>  @@ -234,7 +235,8 @@ static inline void gen_goto_tb(DisasContext *s, int tb_num,
>
>      tb = s->tb;
>      if ((pc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
>  -        (npc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK))  {
>  +        (npc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
>  +        !(s->env->singlestep_enabled || singlestep))  {
>          /* jump to same page: we can use a direct jump */
>          tcg_gen_goto_tb(tb_num);
>          tcg_gen_movi_tl(cpu_pc, pc);
>  @@ -4680,6 +4682,7 @@ static inline void gen_intermediate_code_internal(TranslationBlock * tb,
>
>      memset(dc, 0, sizeof(DisasContext));
>      dc->tb = tb;
>  +    dc->env = env;
>      pc_start = tb->pc;
>      dc->pc = pc_start;
>      last_pc = dc->pc;
>  @@ -4755,8 +4758,6 @@ static inline void gen_intermediate_code_internal(TranslationBlock * tb,
>          /* if single step mode, we generate only one instruction and
>             generate an exception */
>          if (env->singlestep_enabled || singlestep) {
>  -            tcg_gen_movi_tl(cpu_pc, dc->pc);
>  -            tcg_gen_exit_tb(0);
>              break;
>          }
>      } while ((gen_opc_ptr < gen_opc_end) &&
>
> --
>  1.6.6.1
>
>

[Qemu-devel] [PATCH] target-sparc: Fix -singlestep.

[Richard Henderson]

Single-stepping was not properly updating npc, resulting in some
instructions being executed twice.  In addition, we were emitting
dead code at the end of the TB.

Fix both by teaching gen_goto_tb to avoid goto_tb for single-step
and removing the special-case code in gen_intermediate_code_internal.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-sparc/translate.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index b54c520..be2a116 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -79,6 +79,7 @@ typedef struct DisasContext {
     int mem_idx;
     int fpu_enabled;
     int address_mask_32bit;
+    int singlestep;
     uint32_t cc_op;  /* current CC operation */
     struct TranslationBlock *tb;
     sparc_def_t *def;
@@ -234,7 +235,8 @@ static inline void gen_goto_tb(DisasContext *s, int tb_num,
 
     tb = s->tb;
     if ((pc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
-        (npc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK))  {
+        (npc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
+        !s->singlestep)  {
         /* jump to same page: we can use a direct jump */
         tcg_gen_goto_tb(tb_num);
         tcg_gen_movi_tl(cpu_pc, pc);
@@ -4694,6 +4696,7 @@ static inline void gen_intermediate_code_internal(TranslationBlock * tb,
 #ifdef TARGET_SPARC64
     dc->address_mask_32bit = env->pstate & PS_AM;
 #endif
+    dc->singlestep = (env->singlestep_enabled || singlestep);
     gen_opc_end = gen_opc_buf + OPC_MAX_SIZE;
 
     cpu_tmp0 = tcg_temp_new();
@@ -4754,9 +4757,7 @@ static inline void gen_intermediate_code_internal(TranslationBlock * tb,
             break;
         /* if single step mode, we generate only one instruction and
            generate an exception */
-        if (env->singlestep_enabled || singlestep) {
-            tcg_gen_movi_tl(cpu_pc, dc->pc);
-            tcg_gen_exit_tb(0);
+        if (dc->singlestep) {
             break;
         }
     } while ((gen_opc_ptr < gen_opc_end) &&
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH] target-sparc: Fix -singlestep.

[Blue Swirl]

Thanks, applied.

On 4/26/10, Richard Henderson <rth@twiddle.net> wrote:
> Single-stepping was not properly updating npc, resulting in some
>  instructions being executed twice.  In addition, we were emitting
>  dead code at the end of the TB.
>
>  Fix both by teaching gen_goto_tb to avoid goto_tb for single-step
>  and removing the special-case code in gen_intermediate_code_internal.
>
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
>  ---
>
>  target-sparc/translate.c |    9 +++++----
>
>  1 files changed, 5 insertions(+), 4 deletions(-)
>
>  diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>
> index b54c520..be2a116 100644
>
> --- a/target-sparc/translate.c
>  +++ b/target-sparc/translate.c
>
> @@ -79,6 +79,7 @@ typedef struct DisasContext {
>      int mem_idx;
>      int fpu_enabled;
>      int address_mask_32bit;
>  +    int singlestep;
>
>      uint32_t cc_op;  /* current CC operation */
>      struct TranslationBlock *tb;
>
>      sparc_def_t *def;
>
> @@ -234,7 +235,8 @@ static inline void gen_goto_tb(DisasContext *s, int tb_num,
>
>      tb = s->tb;
>      if ((pc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
>  -        (npc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK))  {
>  +        (npc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
>
> +        !s->singlestep)  {
>
>          /* jump to same page: we can use a direct jump */
>          tcg_gen_goto_tb(tb_num);
>          tcg_gen_movi_tl(cpu_pc, pc);
>
> @@ -4694,6 +4696,7 @@ static inline void gen_intermediate_code_internal(TranslationBlock * tb,
>   #ifdef TARGET_SPARC64
>      dc->address_mask_32bit = env->pstate & PS_AM;
>   #endif
>  +    dc->singlestep = (env->singlestep_enabled || singlestep);
>      gen_opc_end = gen_opc_buf + OPC_MAX_SIZE;
>
>      cpu_tmp0 = tcg_temp_new();
>  @@ -4754,9 +4757,7 @@ static inline void gen_intermediate_code_internal(TranslationBlock * tb,
>              break;
>
>          /* if single step mode, we generate only one instruction and
>             generate an exception */
>  -        if (env->singlestep_enabled || singlestep) {
>  -            tcg_gen_movi_tl(cpu_pc, dc->pc);
>  -            tcg_gen_exit_tb(0);
>
> +        if (dc->singlestep) {
>
>              break;
>          }
>      } while ((gen_opc_ptr < gen_opc_end) &&
>  --
>  1.6.6.1
>
>

[Qemu-devel] [PATCH] linux-user: Fix sparc32plus stat64 syscalls.

[Richard Henderson]

Check TARGET_ABI_BITS, not TARGET_LONG_BITS, when deciding
whether or not the guest needs special 64-bit stat translation.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/syscall.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 26c0fb4..eb77ade 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4021,7 +4021,7 @@ static inline abi_long host_to_target_stat64(void *cpu_env,
     } else
 #endif
     {
-#if (TARGET_LONG_BITS == 64) && (!defined(TARGET_ALPHA))
+#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA)
         struct target_stat *target_st;
 #else
         struct target_stat64 *target_st;
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH] linux-user: Fix sparc32plus stat64 syscalls.

[Blue Swirl]

Thanks, applied.

On 4/26/10, Richard Henderson <rth@twiddle.net> wrote:
> Check TARGET_ABI_BITS, not TARGET_LONG_BITS, when deciding
>  whether or not the guest needs special 64-bit stat translation.
>
>  Signed-off-by: Richard Henderson <rth@twiddle.net>
>  ---
>   linux-user/syscall.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
>  diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>  index 26c0fb4..eb77ade 100644
>  --- a/linux-user/syscall.c
>  +++ b/linux-user/syscall.c
>  @@ -4021,7 +4021,7 @@ static inline abi_long host_to_target_stat64(void *cpu_env,
>      } else
>   #endif
>      {
>  -#if (TARGET_LONG_BITS == 64) && (!defined(TARGET_ALPHA))
>  +#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA)
>          struct target_stat *target_st;
>   #else
>          struct target_stat64 *target_st;
>
> --
>  1.6.6.1
>
>