Re: [PATCH] target/sh4: Fix TB_FLAG_UNALIGN

[Yoshinori Sato <ysato@users.sourceforge.jp>]

On Tue, 30 Aug 2022 01:10:29 +0900,
Richard Henderson wrote:
> 
> On 8/29/22 02:05, BALATON Zoltan wrote:
> > On Sun, 28 Aug 2022, Richard Henderson wrote:
> >> The value previously chosen overlaps GUSA_MASK.
> >> 
> >> Cc: qemu-stable@nongnu.org
> >> Fixes: 4da06fb3062 ("target/sh4: Implement prctl_unalign_sigbus")
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/856
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >> target/sh4/cpu.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
> >> index 9f15ef913c..e79cbc59e2 100644
> >> --- a/target/sh4/cpu.h
> >> +++ b/target/sh4/cpu.h
> >> @@ -84,7 +84,7 @@
> >> #define DELAY_SLOT_RTE         (1 << 2)
> >> 
> >> #define TB_FLAG_PENDING_MOVCA  (1 << 3)
> >> -#define TB_FLAG_UNALIGN        (1 << 4)
> >> +#define TB_FLAG_UNALIGN        (1 << 13)
> > 
> > Is it worth a comment to note why that value to avoid the same
> > problem if another flag is added in the future?
> 
> Hmm, or perhaps move it down below, so that we see bit 3 used, then bits 4-12, then bit 13.
> 
> 
> r~

How about this fix?

From 69fc46c0e439026cabedc8ddfa0a880d0df09a6b Mon Sep 17 00:00:00 2001
From: Yoshinori Sato <ysato@users.sourceforge.jp>
Date: Wed, 31 Aug 2022 17:12:59 +0900
Subject: [PATCH] sh4: cleanup for flags definition.

Fix conflict TB_FLAG_UNALIGN and GUSA field.
Add comment for gUSA operations.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 target/sh4/cpu.h       | 9 +++++++--
 target/sh4/translate.c | 5 ++++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
index 9f15ef913c..91810fda9b 100644
--- a/target/sh4/cpu.h
+++ b/target/sh4/cpu.h
@@ -86,9 +86,14 @@
 #define TB_FLAG_PENDING_MOVCA  (1 << 3)
 #define TB_FLAG_UNALIGN        (1 << 4)
 
-#define GUSA_SHIFT             4
 #ifdef CONFIG_USER_ONLY
-#define GUSA_EXCLUSIVE         (1 << 12)
+/* gUSA information field in CPUArchState.flags */
+/*
+   b16 - b23: Exclusive region range (negative)
+   b24: pc in exclusive region flag (use normal decode)
+*/
+#define GUSA_SHIFT             16
+#define GUSA_EXCLUSIVE         (1 << 24)
 #define GUSA_MASK              ((0xff << GUSA_SHIFT) | GUSA_EXCLUSIVE)
 #else
 /* Provide dummy versions of the above to allow tests against tbflags
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index f1b190e7cf..1d79a0721b 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -516,7 +516,7 @@ static void _decode_opc(DisasContext * ctx)
         /* Detect the start of a gUSA region.  If so, update envflags
            and end the TB.  This will allow us to see the end of the
            region (stored in R0) in the next TB.  */
-        if (B11_8 == 15 && B7_0s < 0 &&
+        if (B11_8 == 15 && B7_0s < 0 &&		/* mov #-xxx, r15 */
             (tb_cflags(ctx->base.tb) & CF_PARALLEL)) {
             ctx->envflags = deposit32(ctx->envflags, GUSA_SHIFT, 8, B7_0s);
             ctx->base.is_jmp = DISAS_STOP;
@@ -2267,7 +2267,9 @@ static void sh4_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
                   (tbflags & (1 << SR_RB))) * 0x10;
     ctx->fbank = tbflags & FPSCR_FR ? 0x10 : 0;
 
+#ifdef CONFIG_USER_ONLY
     if (tbflags & GUSA_MASK) {
+        /* In gUSA exclusive region */
         uint32_t pc = ctx->base.pc_next;
         uint32_t pc_end = ctx->base.tb->cs_base;
         int backup = sextract32(ctx->tbflags, GUSA_SHIFT, 8);
@@ -2285,6 +2287,7 @@ static void sh4_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
             return;
         }
     }
+#endif
 
     /* Since the ISA is fixed-width, we can bound by the number
        of instructions remaining on the page.  */
-- 
2.30.2

-- 
Yosinori Sato