[Qemu-devel] [PATCH 0/5] target/sh4: system emulation improvement

[Qemu-devel] [PATCH 0/5] target/sh4: system emulation improvement

[Aurelien Jarno]

This patch series fix two issues with the SH4 system emulation:
- reboot does not work when using -kernel and -initrd
- the RTE instruction is not correctly emulated in some very rare
  cases, causing userland processes to receive a segmentation fault.

Aurelien Jarno (5):
  target/sh4: log unauthorized accesses using qemu_log_mask
  target/sh4: fix reset when using a kernel and an initrd
  target/sh4: introduce DELAY_SLOT_MASK
  target/sh4: ignore interrupts in a delay slot
  target/sh4: fix RTE instruction delay slot

 target/sh4/cpu.h       | 12 ++++++++++--
 target/sh4/helper.c    | 28 ++++++++++++++++++++++------
 target/sh4/translate.c | 25 ++++++++++++++-----------
 3 files changed, 46 insertions(+), 19 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH 1/5] target/sh4: log unauthorized accesses using qemu_log_mask

[Aurelien Jarno]

qemu_log_mask() is preferred over fprintf() for logging errors.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/sh4/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index 8f8ce81401..4c024f9529 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -420,7 +420,7 @@ static int get_physical_address(CPUSH4State * env, target_ulong * physical,
         if (!(env->sr & (1u << SR_MD))
 	    && (address < 0xe0000000 || address >= 0xe4000000)) {
 	    /* Unauthorized access in user mode (only store queues are available) */
-	    fprintf(stderr, "Unauthorized access\n");
+            qemu_log_mask(LOG_GUEST_ERROR, "Unauthorized access\n");
 	    if (rw == 0)
 		return MMU_DADDR_ERROR_READ;
 	    else if (rw == 1)
-- 
2.11.0

Re: [Qemu-devel] [PATCH 1/5] target/sh4: log unauthorized accesses using qemu_log_mask

[Philippe Mathieu-Daudé]

On 05/16/2017 07:47 PM, Aurelien Jarno wrote:
> qemu_log_mask() is preferred over fprintf() for logging errors.
>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/sh4/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/sh4/helper.c b/target/sh4/helper.c
> index 8f8ce81401..4c024f9529 100644
> --- a/target/sh4/helper.c
> +++ b/target/sh4/helper.c
> @@ -420,7 +420,7 @@ static int get_physical_address(CPUSH4State * env, target_ulong * physical,
>          if (!(env->sr & (1u << SR_MD))
>  	    && (address < 0xe0000000 || address >= 0xe4000000)) {
>  	    /* Unauthorized access in user mode (only store queues are available) */
> -	    fprintf(stderr, "Unauthorized access\n");
> +            qemu_log_mask(LOG_GUEST_ERROR, "Unauthorized access\n");
>  	    if (rw == 0)
>  		return MMU_DADDR_ERROR_READ;
>  	    else if (rw == 1)
>

Re: [Qemu-devel] [PATCH 1/5] target/sh4: log unauthorized accesses using qemu_log_mask

[Richard Henderson]

On 05/16/2017 03:47 PM, Aurelien Jarno wrote:
> qemu_log_mask() is preferred over fprintf() for logging errors.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/sh4/helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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


r~

[Qemu-devel] [PATCH 2/5] target/sh4: fix reset when using a kernel and an initrd

[Aurelien Jarno]

When a masked exception happens, the SH4 CPU generates a non-masked
reset exception, which then jumps to the reset vector at address
0xA0000000. While this is emulated correctly in QEMU, this does not
work when using a kernel and initrd as this address then contain an
illegal instruction (and there is no guarantee the kernel and initrd
haven't been overwritten).

Therefore call qemu_system_reset_request to reload the kernel and initrd
and load the program counter to the kernel entry point.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/sh4/helper.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index 4c024f9529..5296e7cf4e 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -21,6 +21,7 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/log.h"
+#include "sysemu/sysemu.h"
 
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/sh4/sh_intc.h"
@@ -92,7 +93,14 @@ void superh_cpu_do_interrupt(CPUState *cs)
 
     if (env->sr & (1u << SR_BL)) {
         if (do_exp && cs->exception_index != 0x1e0) {
-            cs->exception_index = 0x000; /* masked exception -> reset */
+            /* In theory a masked exception generates a reset exception,
+               which in turn jumps to the reset vector. However this only
+               works when using a bootloader. When using a kernel and an
+               initrd, they need to be reloaded and the program counter
+               should be loaded with the kernel entry point.
+               qemu_system_reset_request takes care of that.  */
+            qemu_system_reset_request();
+            return;
         }
         if (do_irq && !env->in_sleep) {
             return; /* masked */
-- 
2.11.0

Re: [Qemu-devel] [PATCH 2/5] target/sh4: fix reset when using a kernel and an initrd

[Richard Henderson]

On 05/16/2017 03:47 PM, Aurelien Jarno wrote:
> When a masked exception happens, the SH4 CPU generates a non-masked
> reset exception, which then jumps to the reset vector at address
> 0xA0000000. While this is emulated correctly in QEMU, this does not
> work when using a kernel and initrd as this address then contain an
> illegal instruction (and there is no guarantee the kernel and initrd
> haven't been overwritten).
> 
> Therefore call qemu_system_reset_request to reload the kernel and initrd
> and load the program counter to the kernel entry point.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/sh4/helper.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)

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


r~

[Qemu-devel] [PATCH 3/5] target/sh4: introduce DELAY_SLOT_MASK

[Aurelien Jarno]

This will make easier the introduction of a new flag in the next
patches.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/sh4/cpu.h       |  3 ++-
 target/sh4/helper.c    |  4 ++--
 target/sh4/translate.c | 17 ++++++++---------
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index 6c07c6b24b..7969c9af98 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -91,6 +91,7 @@
 #define FPSCR_RM_NEAREST       (0 << 0)
 #define FPSCR_RM_ZERO          (1 << 0)
 
+#define DELAY_SLOT_MASK        0x3
 #define DELAY_SLOT             (1 << 0)
 #define DELAY_SLOT_CONDITIONAL (1 << 1)
 
@@ -380,7 +381,7 @@ static inline void cpu_get_tb_cpu_state(CPUSH4State *env, target_ulong *pc,
 {
     *pc = env->pc;
     *cs_base = 0;
-    *flags = (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) /* Bits 0-1 */
+    *flags = (env->flags & DELAY_SLOT_MASK)                    /* Bits  0- 1 */
             | (env->fpscr & (FPSCR_FR | FPSCR_SZ | FPSCR_PR))  /* Bits 19-21 */
             | (env->sr & ((1u << SR_MD) | (1u << SR_RB)))      /* Bits 29-30 */
             | (env->sr & (1u << SR_FD))                        /* Bit 15 */
diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index 5296e7cf4e..d420931530 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -172,11 +172,11 @@ void superh_cpu_do_interrupt(CPUState *cs)
     env->sgr = env->gregs[15];
     env->sr |= (1u << SR_BL) | (1u << SR_MD) | (1u << SR_RB);
 
-    if (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {
+    if (env->flags & DELAY_SLOT_MASK) {
         /* Branch instruction should be executed again before delay slot. */
 	env->spc -= 2;
 	/* Clear flags for exception/interrupt routine. */
-        env->flags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
+        env->flags &= ~DELAY_SLOT_MASK;
     }
 
     if (do_exp) {
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 0bc2f9ff19..aba316f593 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -217,8 +217,7 @@ static inline void gen_save_cpu_state(DisasContext *ctx, bool save_pc)
     if (ctx->delayed_pc != (uint32_t) -1) {
         tcg_gen_movi_i32(cpu_delayed_pc, ctx->delayed_pc);
     }
-    if ((ctx->tbflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL))
-        != ctx->envflags) {
+    if ((ctx->tbflags & DELAY_SLOT_MASK) != ctx->envflags) {
         tcg_gen_movi_i32(cpu_flags, ctx->envflags);
     }
 }
@@ -329,7 +328,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 #define DREG(x) FREG(x) /* Assumes lsb of (x) is always 0 */
 
 #define CHECK_NOT_DELAY_SLOT \
-    if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {     \
+    if (ctx->envflags & DELAY_SLOT_MASK) {                           \
         gen_save_cpu_state(ctx, true);                               \
         gen_helper_raise_slot_illegal_instruction(cpu_env);          \
         ctx->bstate = BS_EXCP;                                       \
@@ -339,7 +338,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 #define CHECK_PRIVILEGED                                             \
     if (IS_USER(ctx)) {                                              \
         gen_save_cpu_state(ctx, true);                               \
-        if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
+        if (ctx->envflags & DELAY_SLOT_MASK) {                       \
             gen_helper_raise_slot_illegal_instruction(cpu_env);      \
         } else {                                                     \
             gen_helper_raise_illegal_instruction(cpu_env);           \
@@ -351,7 +350,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
 #define CHECK_FPU_ENABLED                                            \
     if (ctx->tbflags & (1u << SR_FD)) {                              \
         gen_save_cpu_state(ctx, true);                               \
-        if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
+        if (ctx->envflags & DELAY_SLOT_MASK) {                       \
             gen_helper_raise_slot_fpu_disable(cpu_env);              \
         } else {                                                     \
             gen_helper_raise_fpu_disable(cpu_env);                   \
@@ -1784,7 +1783,7 @@ static void _decode_opc(DisasContext * ctx)
     fflush(stderr);
 #endif
     gen_save_cpu_state(ctx, true);
-    if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {
+    if (ctx->envflags & DELAY_SLOT_MASK) {
         gen_helper_raise_slot_illegal_instruction(cpu_env);
     } else {
         gen_helper_raise_illegal_instruction(cpu_env);
@@ -1798,9 +1797,9 @@ static void decode_opc(DisasContext * ctx)
 
     _decode_opc(ctx);
 
-    if (old_flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {
+    if (old_flags & DELAY_SLOT_MASK) {
         /* go out of the delay slot */
-        ctx->envflags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
+        ctx->envflags &= ~DELAY_SLOT_MASK;
         tcg_gen_movi_i32(cpu_flags, ctx->envflags);
         ctx->bstate = BS_BRANCH;
         if (old_flags & DELAY_SLOT_CONDITIONAL) {
@@ -1824,7 +1823,7 @@ void gen_intermediate_code(CPUSH4State * env, struct TranslationBlock *tb)
     pc_start = tb->pc;
     ctx.pc = pc_start;
     ctx.tbflags = (uint32_t)tb->flags;
-    ctx.envflags = tb->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
+    ctx.envflags = tb->flags & DELAY_SLOT_MASK;
     ctx.bstate = BS_NONE;
     ctx.memidx = (ctx.tbflags & (1u << SR_MD)) == 0 ? 1 : 0;
     /* We don't know if the delayed pc came from a dynamic or static branch,
-- 
2.11.0

Re: [Qemu-devel] [PATCH 3/5] target/sh4: introduce DELAY_SLOT_MASK

[Philippe Mathieu-Daudé]

On 05/16/2017 07:47 PM, Aurelien Jarno wrote:
> This will make easier the introduction of a new flag in the next
> patches.

This makes code cleaner / easier to read, no need further explanation ;)

>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/sh4/cpu.h       |  3 ++-
>  target/sh4/helper.c    |  4 ++--
>  target/sh4/translate.c | 17 ++++++++---------
>  3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
> index 6c07c6b24b..7969c9af98 100644
> --- a/target/sh4/cpu.h
> +++ b/target/sh4/cpu.h
> @@ -91,6 +91,7 @@
>  #define FPSCR_RM_NEAREST       (0 << 0)
>  #define FPSCR_RM_ZERO          (1 << 0)
>
> +#define DELAY_SLOT_MASK        0x3
>  #define DELAY_SLOT             (1 << 0)
>  #define DELAY_SLOT_CONDITIONAL (1 << 1)
>
> @@ -380,7 +381,7 @@ static inline void cpu_get_tb_cpu_state(CPUSH4State *env, target_ulong *pc,
>  {
>      *pc = env->pc;
>      *cs_base = 0;
> -    *flags = (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) /* Bits 0-1 */
> +    *flags = (env->flags & DELAY_SLOT_MASK)                    /* Bits  0- 1 */
>              | (env->fpscr & (FPSCR_FR | FPSCR_SZ | FPSCR_PR))  /* Bits 19-21 */
>              | (env->sr & ((1u << SR_MD) | (1u << SR_RB)))      /* Bits 29-30 */
>              | (env->sr & (1u << SR_FD))                        /* Bit 15 */
> diff --git a/target/sh4/helper.c b/target/sh4/helper.c
> index 5296e7cf4e..d420931530 100644
> --- a/target/sh4/helper.c
> +++ b/target/sh4/helper.c
> @@ -172,11 +172,11 @@ void superh_cpu_do_interrupt(CPUState *cs)
>      env->sgr = env->gregs[15];
>      env->sr |= (1u << SR_BL) | (1u << SR_MD) | (1u << SR_RB);
>
> -    if (env->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {
> +    if (env->flags & DELAY_SLOT_MASK) {
>          /* Branch instruction should be executed again before delay slot. */
>  	env->spc -= 2;
>  	/* Clear flags for exception/interrupt routine. */
> -        env->flags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
> +        env->flags &= ~DELAY_SLOT_MASK;
>      }
>
>      if (do_exp) {
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 0bc2f9ff19..aba316f593 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -217,8 +217,7 @@ static inline void gen_save_cpu_state(DisasContext *ctx, bool save_pc)
>      if (ctx->delayed_pc != (uint32_t) -1) {
>          tcg_gen_movi_i32(cpu_delayed_pc, ctx->delayed_pc);
>      }
> -    if ((ctx->tbflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL))
> -        != ctx->envflags) {
> +    if ((ctx->tbflags & DELAY_SLOT_MASK) != ctx->envflags) {
>          tcg_gen_movi_i32(cpu_flags, ctx->envflags);
>      }
>  }
> @@ -329,7 +328,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
>  #define DREG(x) FREG(x) /* Assumes lsb of (x) is always 0 */
>
>  #define CHECK_NOT_DELAY_SLOT \
> -    if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {     \
> +    if (ctx->envflags & DELAY_SLOT_MASK) {                           \
>          gen_save_cpu_state(ctx, true);                               \
>          gen_helper_raise_slot_illegal_instruction(cpu_env);          \
>          ctx->bstate = BS_EXCP;                                       \
> @@ -339,7 +338,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
>  #define CHECK_PRIVILEGED                                             \
>      if (IS_USER(ctx)) {                                              \
>          gen_save_cpu_state(ctx, true);                               \
> -        if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
> +        if (ctx->envflags & DELAY_SLOT_MASK) {                       \
>              gen_helper_raise_slot_illegal_instruction(cpu_env);      \
>          } else {                                                     \
>              gen_helper_raise_illegal_instruction(cpu_env);           \
> @@ -351,7 +350,7 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
>  #define CHECK_FPU_ENABLED                                            \
>      if (ctx->tbflags & (1u << SR_FD)) {                              \
>          gen_save_cpu_state(ctx, true);                               \
> -        if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) { \
> +        if (ctx->envflags & DELAY_SLOT_MASK) {                       \
>              gen_helper_raise_slot_fpu_disable(cpu_env);              \
>          } else {                                                     \
>              gen_helper_raise_fpu_disable(cpu_env);                   \
> @@ -1784,7 +1783,7 @@ static void _decode_opc(DisasContext * ctx)
>      fflush(stderr);
>  #endif
>      gen_save_cpu_state(ctx, true);
> -    if (ctx->envflags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {
> +    if (ctx->envflags & DELAY_SLOT_MASK) {
>          gen_helper_raise_slot_illegal_instruction(cpu_env);
>      } else {
>          gen_helper_raise_illegal_instruction(cpu_env);
> @@ -1798,9 +1797,9 @@ static void decode_opc(DisasContext * ctx)
>
>      _decode_opc(ctx);
>
> -    if (old_flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL)) {
> +    if (old_flags & DELAY_SLOT_MASK) {
>          /* go out of the delay slot */
> -        ctx->envflags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
> +        ctx->envflags &= ~DELAY_SLOT_MASK;
>          tcg_gen_movi_i32(cpu_flags, ctx->envflags);
>          ctx->bstate = BS_BRANCH;
>          if (old_flags & DELAY_SLOT_CONDITIONAL) {
> @@ -1824,7 +1823,7 @@ void gen_intermediate_code(CPUSH4State * env, struct TranslationBlock *tb)
>      pc_start = tb->pc;
>      ctx.pc = pc_start;
>      ctx.tbflags = (uint32_t)tb->flags;
> -    ctx.envflags = tb->flags & (DELAY_SLOT | DELAY_SLOT_CONDITIONAL);
> +    ctx.envflags = tb->flags & DELAY_SLOT_MASK;
>      ctx.bstate = BS_NONE;
>      ctx.memidx = (ctx.tbflags & (1u << SR_MD)) == 0 ? 1 : 0;
>      /* We don't know if the delayed pc came from a dynamic or static branch,
>

Re: [Qemu-devel] [PATCH 3/5] target/sh4: introduce DELAY_SLOT_MASK

[Richard Henderson]

On 05/16/2017 03:47 PM, Aurelien Jarno wrote:
> This will make easier the introduction of a new flag in the next
> patches.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/sh4/cpu.h       |  3 ++-
>   target/sh4/helper.c    |  4 ++--
>   target/sh4/translate.c | 17 ++++++++---------
>   3 files changed, 12 insertions(+), 12 deletions(-)

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


r~

[Qemu-devel] [PATCH 4/5] target/sh4: ignore interrupts in a delay slot

[Aurelien Jarno]

Delay slots are indivisible, therefore avoid scheduling an interrupt in
the delay slot. However exceptions are possible.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/sh4/helper.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target/sh4/helper.c b/target/sh4/helper.c
index d420931530..19d4ec5fb5 100644
--- a/target/sh4/helper.c
+++ b/target/sh4/helper.c
@@ -871,8 +871,16 @@ int cpu_sh4_is_cached(CPUSH4State * env, target_ulong addr)
 bool superh_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 {
     if (interrupt_request & CPU_INTERRUPT_HARD) {
-        superh_cpu_do_interrupt(cs);
-        return true;
+        SuperHCPU *cpu = SUPERH_CPU(cs);
+        CPUSH4State *env = &cpu->env;
+
+        /* Delay slots are indivisible, ignore interrupts */
+        if (env->flags & DELAY_SLOT_MASK) {
+            return false;
+        } else {
+            superh_cpu_do_interrupt(cs);
+            return true;
+        }
     }
     return false;
 }
-- 
2.11.0

Re: [Qemu-devel] [PATCH 4/5] target/sh4: ignore interrupts in a delay slot

[Philippe Mathieu-Daudé]

On 05/16/2017 07:47 PM, Aurelien Jarno wrote:
> Delay slots are indivisible, therefore avoid scheduling an interrupt in
> the delay slot. However exceptions are possible.
>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/sh4/helper.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/target/sh4/helper.c b/target/sh4/helper.c
> index d420931530..19d4ec5fb5 100644
> --- a/target/sh4/helper.c
> +++ b/target/sh4/helper.c
> @@ -871,8 +871,16 @@ int cpu_sh4_is_cached(CPUSH4State * env, target_ulong addr)
>  bool superh_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
>      if (interrupt_request & CPU_INTERRUPT_HARD) {
> -        superh_cpu_do_interrupt(cs);
> -        return true;
> +        SuperHCPU *cpu = SUPERH_CPU(cs);
> +        CPUSH4State *env = &cpu->env;
> +
> +        /* Delay slots are indivisible, ignore interrupts */
> +        if (env->flags & DELAY_SLOT_MASK) {
> +            return false;
> +        } else {
> +            superh_cpu_do_interrupt(cs);
> +            return true;
> +        }
>      }
>      return false;
>  }
>

Re: [Qemu-devel] [PATCH 4/5] target/sh4: ignore interrupts in a delay slot

[Richard Henderson]

On 05/16/2017 03:47 PM, Aurelien Jarno wrote:
> Delay slots are indivisible, therefore avoid scheduling an interrupt in
> the delay slot. However exceptions are possible.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/sh4/helper.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)

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


r~

[Qemu-devel] [PATCH 5/5] target/sh4: fix RTE instruction delay slot

[Aurelien Jarno]

The ReTurn from Exception (RTE) instruction loads the system register
(SR) with the saved system register (SSR). It has a delay slot, and
behaves specially according to the SH4 manual:

  The SR value accessed by the instruction in the RTE delay slot is the
  value restored from SSR by the RTE instruction. The SR and MD values
  defined prior to RTE execution are used to fetch the instruction in
  the RTE delay slot.

The instruction in the delay slot being often a NOP, it doesn't cause
any issue most of the time except in some rare cases where the NOP is
being splitted in a different TB (for example when the TCG op buffer
is full). In that case the NOP is fetched with the user permissions
and causes an instruction TLB protection violation exception.

This patches fixes that by introducing a new delay slot flag for the
RTE instruction. Given it's a privileged instruction, the RTE delay
slot instruction is always fetched in privileged mode. It is therefore
enough to to check for this flag in cpu_mmu_index.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/sh4/cpu.h       | 13 ++++++++++---
 target/sh4/translate.c |  8 ++++++--
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index 7969c9af98..ffb91687b8 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -91,9 +91,10 @@
 #define FPSCR_RM_NEAREST       (0 << 0)
 #define FPSCR_RM_ZERO          (1 << 0)
 
-#define DELAY_SLOT_MASK        0x3
+#define DELAY_SLOT_MASK        0x7
 #define DELAY_SLOT             (1 << 0)
 #define DELAY_SLOT_CONDITIONAL (1 << 1)
+#define DELAY_SLOT_RTE         (1 << 2)
 
 typedef struct tlb_t {
     uint32_t vpn;		/* virtual page number */
@@ -264,7 +265,13 @@ void cpu_load_tlb(CPUSH4State * env);
 #define MMU_USER_IDX 1
 static inline int cpu_mmu_index (CPUSH4State *env, bool ifetch)
 {
-    return (env->sr & (1u << SR_MD)) == 0 ? 1 : 0;
+    /* The instruction in a RTE delay slot is fetched in privileged
+       mode, but executed in user mode.  */
+    if (ifetch && (env->flags & DELAY_SLOT_RTE)) {
+        return 0;
+    } else {
+        return (env->sr & (1u << SR_MD)) == 0 ? 1 : 0;
+    }
 }
 
 #include "exec/cpu-all.h"
@@ -381,7 +388,7 @@ static inline void cpu_get_tb_cpu_state(CPUSH4State *env, target_ulong *pc,
 {
     *pc = env->pc;
     *cs_base = 0;
-    *flags = (env->flags & DELAY_SLOT_MASK)                    /* Bits  0- 1 */
+    *flags = (env->flags & DELAY_SLOT_MASK)                    /* Bits  0- 2 */
             | (env->fpscr & (FPSCR_FR | FPSCR_SZ | FPSCR_PR))  /* Bits 19-21 */
             | (env->sr & ((1u << SR_MD) | (1u << SR_RB)))      /* Bits 29-30 */
             | (env->sr & (1u << SR_FD))                        /* Bit 15 */
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index aba316f593..8bc132b27b 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -185,6 +185,9 @@ void superh_cpu_dump_state(CPUState *cs, FILE *f,
     } else if (env->flags & DELAY_SLOT_CONDITIONAL) {
 	cpu_fprintf(f, "in conditional delay slot (delayed_pc=0x%08x)\n",
 		    env->delayed_pc);
+    } else if (env->flags & DELAY_SLOT_RTE) {
+        cpu_fprintf(f, "in rte delay slot (delayed_pc=0x%08x)\n",
+                    env->delayed_pc);
     }
 }
 
@@ -427,8 +430,9 @@ static void _decode_opc(DisasContext * ctx)
 	CHECK_NOT_DELAY_SLOT
         gen_write_sr(cpu_ssr);
 	tcg_gen_mov_i32(cpu_delayed_pc, cpu_spc);
-        ctx->envflags |= DELAY_SLOT;
+        ctx->envflags |= DELAY_SLOT_RTE;
 	ctx->delayed_pc = (uint32_t) - 1;
+        ctx->bstate = BS_STOP;
 	return;
     case 0x0058:		/* sets */
         tcg_gen_ori_i32(cpu_sr, cpu_sr, (1u << SR_S));
@@ -1804,7 +1808,7 @@ static void decode_opc(DisasContext * ctx)
         ctx->bstate = BS_BRANCH;
         if (old_flags & DELAY_SLOT_CONDITIONAL) {
 	    gen_delayed_conditional_jump(ctx);
-        } else if (old_flags & DELAY_SLOT) {
+        } else {
             gen_jump(ctx);
 	}
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH 5/5] target/sh4: fix RTE instruction delay slot

[Richard Henderson]

On 05/16/2017 03:47 PM, Aurelien Jarno wrote:
> The ReTurn from Exception (RTE) instruction loads the system register
> (SR) with the saved system register (SSR). It has a delay slot, and
> behaves specially according to the SH4 manual:
> 
>    The SR value accessed by the instruction in the RTE delay slot is the
>    value restored from SSR by the RTE instruction. The SR and MD values
>    defined prior to RTE execution are used to fetch the instruction in
>    the RTE delay slot.
> 
> The instruction in the delay slot being often a NOP, it doesn't cause
> any issue most of the time except in some rare cases where the NOP is
> being splitted in a different TB (for example when the TCG op buffer
> is full). In that case the NOP is fetched with the user permissions
> and causes an instruction TLB protection violation exception.
> 
> This patches fixes that by introducing a new delay slot flag for the
> RTE instruction. Given it's a privileged instruction, the RTE delay
> slot instruction is always fetched in privileged mode. It is therefore
> enough to to check for this flag in cpu_mmu_index.
> 
> Signed-off-by: Aurelien Jarno<aurelien@aurel32.net>
> ---
>   target/sh4/cpu.h       | 13 ++++++++++---
>   target/sh4/translate.c |  8 ++++++--
>   2 files changed, 16 insertions(+), 5 deletions(-)

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


r~