[PATCH v2 0/7] target/sparc: Relax some decode for v7/v8

[PATCH v2 0/7] target/sparc: Relax some decode for v7/v8

[Richard Henderson]

Hi Mark,

I noticed a few other places where we probably want to
allow unused bits to be set and ignored for pre-v9.

Changes for v2:
  - Use TRANS() in the STBAR and RDY patches.
  - Apply the same relaxation for RDPSR, RDWIM, RDTBR.
  - Do not report illegal_instruction when unused bits
    are set in the rs2_or_imm field.

r~


Supercedes: 20250905051936.92815-1-richard.henderson@linaro.org

Richard Henderson (7):
  target/sparc: Allow TRANS macro with no extra arguments
  target/sparc: Loosen decode of STBAR for v8
  target/sparc: Loosen decode of RDY for v7
  target/sparc: Loosen decode of RDPSR for v7
  target/sparc: Loosen decode of RDWIM for v7
  target/sparc: Loosen decode of RDTBR for v7
  target/sparc: Relax decode of rs2_or_imm for v7

 target/sparc/translate.c  | 77 ++++++++++++++++++++----------------
 target/sparc/insns.decode | 83 +++++++++++++++++++++++++--------------
 2 files changed, 97 insertions(+), 63 deletions(-)

-- 
2.43.0

[PATCH v2 1/7] target/sparc: Allow TRANS macro with no extra arguments

[Richard Henderson]

Use ## to drop the preceding comma if __VA_ARGS__ is empty.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index b922e53bf1..336583beab 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -2487,7 +2487,7 @@ static int extract_qfpreg(DisasContext *dc, int x)
 
 #define TRANS(NAME, AVAIL, FUNC, ...) \
     static bool trans_##NAME(DisasContext *dc, arg_##NAME *a) \
-    { return avail_##AVAIL(dc) && FUNC(dc, __VA_ARGS__); }
+    { return avail_##AVAIL(dc) && FUNC(dc, ## __VA_ARGS__); }
 
 #define avail_ALL(C)      true
 #ifdef TARGET_SPARC64
-- 
2.43.0

Re: [PATCH v2 1/7] target/sparc: Allow TRANS macro with no extra arguments

[Mark Cave-Ayland]

On 05/09/2025 12:51, Richard Henderson wrote:

> Use ## to drop the preceding comma if __VA_ARGS__ is empty.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sparc/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/sparc/translate.c b/target/sparc/translate.c
> index b922e53bf1..336583beab 100644
> --- a/target/sparc/translate.c
> +++ b/target/sparc/translate.c
> @@ -2487,7 +2487,7 @@ static int extract_qfpreg(DisasContext *dc, int x)
>   
>   #define TRANS(NAME, AVAIL, FUNC, ...) \
>       static bool trans_##NAME(DisasContext *dc, arg_##NAME *a) \
> -    { return avail_##AVAIL(dc) && FUNC(dc, __VA_ARGS__); }
> +    { return avail_##AVAIL(dc) && FUNC(dc, ## __VA_ARGS__); }
>   
>   #define avail_ALL(C)      true
>   #ifdef TARGET_SPARC64

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

[PATCH v2 2/7] target/sparc: Loosen decode of STBAR for v8

[Richard Henderson]

Solaris 8 appears to have a bug whereby it executes v9 MEMBAR
instructions when booting a freshly installed image. According
to the SPARC v8 architecture manual, whilst bits 13 and bits 12-0
of the "Read State Register Instructions" are notionally zero,
they are marked as unused (i.e. ignored).

Fixes: af25071c1d ("target/sparc: Move RDASR, STBAR, MEMBAR to decodetree")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3097
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/sparc/translate.c  |  5 ++++-
 target/sparc/insns.decode | 13 ++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 336583beab..ece393fffc 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -2823,12 +2823,15 @@ static bool trans_Tcc_i_v9(DisasContext *dc, arg_Tcc_i_v9 *a)
     return do_tcc(dc, a->cond, a->cc, a->rs1, true, a->i);
 }
 
-static bool trans_STBAR(DisasContext *dc, arg_STBAR *a)
+static bool do_stbar(DisasContext *dc)
 {
     tcg_gen_mb(TCG_MO_ST_ST | TCG_BAR_SC);
     return advance_pc(dc);
 }
 
+TRANS(STBAR_v8, 32, do_stbar)
+TRANS(STBAR_v9, 64, do_stbar)
+
 static bool trans_MEMBAR(DisasContext *dc, arg_MEMBAR *a)
 {
     if (avail_32(dc)) {
diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
index 9e39d23273..1b1b85e9c2 100644
--- a/target/sparc/insns.decode
+++ b/target/sparc/insns.decode
@@ -88,7 +88,7 @@ CALL    01 i:s30
 
 {
   [
-    STBAR           10 00000 101000 01111 0 0000000000000
+    STBAR_v9        10 00000 101000 01111 0 0000000000000
     MEMBAR          10 00000 101000 01111 1 000000 cmask:3 mmask:4
 
     RDCCR           10 rd:5  101000 00010 0 0000000000000
@@ -107,6 +107,17 @@ CALL    01 i:s30
     RDSTICK_CMPR    10 rd:5  101000 11001 0 0000000000000
     RDSTRAND_STATUS 10 rd:5  101000 11010 0 0000000000000
   ]
+
+  # The v8 manual, section B.30 STBAR instruction, says
+  # bits [12:0] are ignored, but bit 13 must be 0.
+  # However, section B.28 Read State Register Instruction has a
+  # comment that RDASR with rs1 = 15, rd = 0 is STBAR.  Here,
+  # bit 13 is also ignored and rd != 0 is merely reserved.
+  #
+  # Solaris 8 executes v9 MEMBAR instruction 0x8143e008 during boot.
+  # This confirms that bit 13 is ignored, as 0x8143c000 is STBAR.
+  STBAR_v8          10 ----- 101000 01111 - -------------
+
   # Before v8, all rs1 accepted; otherwise rs1==0.
   RDY               10 rd:5  101000 rs1:5 0 0000000000000
 }
-- 
2.43.0

[PATCH v2 3/7] target/sparc: Loosen decode of RDY for v7

[Richard Henderson]

Bits [18:0] are not decoded with v7, and for v8 unused values
of rs1 simply produce undefined results.

Fixes: af25071c1d ("target/sparc: Move RDASR, STBAR, MEMBAR to decodetree")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/sparc/translate.c  | 14 ++------------
 target/sparc/insns.decode | 12 ++++++++++--
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index ece393fffc..cfdd9c1ce4 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -2863,18 +2863,8 @@ static TCGv do_rdy(DisasContext *dc, TCGv dst)
     return cpu_y;
 }
 
-static bool trans_RDY(DisasContext *dc, arg_RDY *a)
-{
-    /*
-     * TODO: Need a feature bit for sparcv8.  In the meantime, treat all
-     * 32-bit cpus like sparcv7, which ignores the rs1 field.
-     * This matches after all other ASR, so Leon3 Asr17 is handled first.
-     */
-    if (avail_64(dc) && a->rs1 != 0) {
-        return false;
-    }
-    return do_rd_special(dc, true, a->rd, do_rdy);
-}
+TRANS(RDY_v7, 32, do_rd_special, true, a->rd, do_rdy)
+TRANS(RDY_v9, 64, do_rd_special, true, a->rd, do_rdy)
 
 static TCGv do_rd_leon3_config(DisasContext *dc, TCGv dst)
 {
diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
index 1b1b85e9c2..74848996ae 100644
--- a/target/sparc/insns.decode
+++ b/target/sparc/insns.decode
@@ -91,6 +91,7 @@ CALL    01 i:s30
     STBAR_v9        10 00000 101000 01111 0 0000000000000
     MEMBAR          10 00000 101000 01111 1 000000 cmask:3 mmask:4
 
+    RDY_v9          10 rd:5  101000 00000 0 0000000000000
     RDCCR           10 rd:5  101000 00010 0 0000000000000
     RDASI           10 rd:5  101000 00011 0 0000000000000
     RDTICK          10 rd:5  101000 00100 0 0000000000000
@@ -118,8 +119,15 @@ CALL    01 i:s30
   # This confirms that bit 13 is ignored, as 0x8143c000 is STBAR.
   STBAR_v8          10 ----- 101000 01111 - -------------
 
-  # Before v8, all rs1 accepted; otherwise rs1==0.
-  RDY               10 rd:5  101000 rs1:5 0 0000000000000
+  # For v7, bits [18:0] are ignored.
+  # For v8, bits [18:14], aka rs1, are repurposed and rs1 = 0 is RDY,
+  # and other values are RDASR.  However, the v8 manual explicitly
+  # says that rs1 in 1..14 yield undefined results and do not cause
+  # an illegal instruction trap, and rs1 in 16..31 are available for
+  # implementation specific usage.
+  # Implement not causing an illegal instruction trap for v8 by
+  # continuing to interpret unused values per v7, i.e. as RDY.
+  RDY_v7            10 rd:5  101000 ----- - -------------
 }
 
 {
-- 
2.43.0

[PATCH v2 4/7] target/sparc: Loosen decode of RDPSR for v7

[Richard Henderson]

For v7, bits [18:0] are ignored.
For v8, bits [18:14] are reserved and bits [13:0] are ignored.

Fixes: 668bb9b755e ("target/sparc: Move RDPSR, RDHPR to decodetree")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/insns.decode | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
index 74848996ae..1c6403ad8a 100644
--- a/target/sparc/insns.decode
+++ b/target/sparc/insns.decode
@@ -158,14 +158,16 @@ CALL    01 i:s30
 }
 
 {
-  RDPSR             10 rd:5  101001 00000 0 0000000000000
-  RDHPR_hpstate     10 rd:5  101001 00000 0 0000000000000
+  [
+    RDHPR_hpstate       10 rd:5  101001 00000 0 0000000000000
+    RDHPR_htstate       10 rd:5  101001 00001 0 0000000000000
+    RDHPR_hintp         10 rd:5  101001 00011 0 0000000000000
+    RDHPR_htba          10 rd:5  101001 00101 0 0000000000000
+    RDHPR_hver          10 rd:5  101001 00110 0 0000000000000
+    RDHPR_hstick_cmpr   10 rd:5  101001 11111 0 0000000000000
+  ]
+  RDPSR                 10 rd:5  101001 ----- - -------------
 }
-RDHPR_htstate       10 rd:5  101001 00001 0 0000000000000
-RDHPR_hintp         10 rd:5  101001 00011 0 0000000000000
-RDHPR_htba          10 rd:5  101001 00101 0 0000000000000
-RDHPR_hver          10 rd:5  101001 00110 0 0000000000000
-RDHPR_hstick_cmpr   10 rd:5  101001 11111 0 0000000000000
 
 {
   WRPSR             10 00000 110001 ..... . .............  @n_r_ri
-- 
2.43.0

Re: [PATCH v2 4/7] target/sparc: Loosen decode of RDPSR for v7

[Mark Cave-Ayland]

On 05/09/2025 12:51, Richard Henderson wrote:

> For v7, bits [18:0] are ignored.
> For v8, bits [18:14] are reserved and bits [13:0] are ignored.
> 
> Fixes: 668bb9b755e ("target/sparc: Move RDPSR, RDHPR to decodetree")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sparc/insns.decode | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
> index 74848996ae..1c6403ad8a 100644
> --- a/target/sparc/insns.decode
> +++ b/target/sparc/insns.decode
> @@ -158,14 +158,16 @@ CALL    01 i:s30
>   }
>   
>   {
> -  RDPSR             10 rd:5  101001 00000 0 0000000000000
> -  RDHPR_hpstate     10 rd:5  101001 00000 0 0000000000000
> +  [
> +    RDHPR_hpstate       10 rd:5  101001 00000 0 0000000000000
> +    RDHPR_htstate       10 rd:5  101001 00001 0 0000000000000
> +    RDHPR_hintp         10 rd:5  101001 00011 0 0000000000000
> +    RDHPR_htba          10 rd:5  101001 00101 0 0000000000000
> +    RDHPR_hver          10 rd:5  101001 00110 0 0000000000000
> +    RDHPR_hstick_cmpr   10 rd:5  101001 11111 0 0000000000000
> +  ]
> +  RDPSR                 10 rd:5  101001 ----- - -------------
>   }
> -RDHPR_htstate       10 rd:5  101001 00001 0 0000000000000
> -RDHPR_hintp         10 rd:5  101001 00011 0 0000000000000
> -RDHPR_htba          10 rd:5  101001 00101 0 0000000000000
> -RDHPR_hver          10 rd:5  101001 00110 0 0000000000000
> -RDHPR_hstick_cmpr   10 rd:5  101001 11111 0 0000000000000
>   
>   {
>     WRPSR             10 00000 110001 ..... . .............  @n_r_ri

This looks like it matches the behaviour of the old decoder.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

[PATCH v2 5/7] target/sparc: Loosen decode of RDWIM for v7

[Richard Henderson]

For v7, bits [18:0] are ignored.
For v8, bits [18:14] are reserved and bits [13:0] are ignored.

Fixes: 5d617bfba07 ("target/sparc: Move RDWIM, RDPR to decodetree")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/insns.decode | 40 ++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
index 1c6403ad8a..77b2f54fdf 100644
--- a/target/sparc/insns.decode
+++ b/target/sparc/insns.decode
@@ -180,26 +180,28 @@ RESTORED            10 00001 110001 00000 0 0000000000000
 # UA2005 INVALW
 
 {
-  RDWIM             10 rd:5  101010 00000 0 0000000000000
-  RDPR_tpc          10 rd:5  101010 00000 0 0000000000000
+  [
+    RDPR_tpc            10 rd:5  101010 00000 0 0000000000000
+    RDPR_tnpc           10 rd:5  101010 00001 0 0000000000000
+    RDPR_tstate         10 rd:5  101010 00010 0 0000000000000
+    RDPR_tt             10 rd:5  101010 00011 0 0000000000000
+    RDPR_tick           10 rd:5  101010 00100 0 0000000000000
+    RDPR_tba            10 rd:5  101010 00101 0 0000000000000
+    RDPR_pstate         10 rd:5  101010 00110 0 0000000000000
+    RDPR_tl             10 rd:5  101010 00111 0 0000000000000
+    RDPR_pil            10 rd:5  101010 01000 0 0000000000000
+    RDPR_cwp            10 rd:5  101010 01001 0 0000000000000
+    RDPR_cansave        10 rd:5  101010 01010 0 0000000000000
+    RDPR_canrestore     10 rd:5  101010 01011 0 0000000000000
+    RDPR_cleanwin       10 rd:5  101010 01100 0 0000000000000
+    RDPR_otherwin       10 rd:5  101010 01101 0 0000000000000
+    RDPR_wstate         10 rd:5  101010 01110 0 0000000000000
+    RDPR_gl             10 rd:5  101010 10000 0 0000000000000
+    RDPR_strand_status  10 rd:5  101010 11010 0 0000000000000
+    RDPR_ver            10 rd:5  101010 11111 0 0000000000000
+  ]
+  RDWIM                 10 rd:5  101010 ----- - -------------
 }
-RDPR_tnpc           10 rd:5  101010 00001 0 0000000000000
-RDPR_tstate         10 rd:5  101010 00010 0 0000000000000
-RDPR_tt             10 rd:5  101010 00011 0 0000000000000
-RDPR_tick           10 rd:5  101010 00100 0 0000000000000
-RDPR_tba            10 rd:5  101010 00101 0 0000000000000
-RDPR_pstate         10 rd:5  101010 00110 0 0000000000000
-RDPR_tl             10 rd:5  101010 00111 0 0000000000000
-RDPR_pil            10 rd:5  101010 01000 0 0000000000000
-RDPR_cwp            10 rd:5  101010 01001 0 0000000000000
-RDPR_cansave        10 rd:5  101010 01010 0 0000000000000
-RDPR_canrestore     10 rd:5  101010 01011 0 0000000000000
-RDPR_cleanwin       10 rd:5  101010 01100 0 0000000000000
-RDPR_otherwin       10 rd:5  101010 01101 0 0000000000000
-RDPR_wstate         10 rd:5  101010 01110 0 0000000000000
-RDPR_gl             10 rd:5  101010 10000 0 0000000000000
-RDPR_strand_status  10 rd:5  101010 11010 0 0000000000000
-RDPR_ver            10 rd:5  101010 11111 0 0000000000000
 
 {
   WRWIM             10 00000 110010 ..... . .............  @n_r_ri
-- 
2.43.0

Re: [PATCH v2 5/7] target/sparc: Loosen decode of RDWIM for v7

[Mark Cave-Ayland]

On 05/09/2025 12:51, Richard Henderson wrote:

> For v7, bits [18:0] are ignored.
> For v8, bits [18:14] are reserved and bits [13:0] are ignored.
> 
> Fixes: 5d617bfba07 ("target/sparc: Move RDWIM, RDPR to decodetree")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sparc/insns.decode | 40 ++++++++++++++++++++-------------------
>   1 file changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
> index 1c6403ad8a..77b2f54fdf 100644
> --- a/target/sparc/insns.decode
> +++ b/target/sparc/insns.decode
> @@ -180,26 +180,28 @@ RESTORED            10 00001 110001 00000 0 0000000000000
>   # UA2005 INVALW
>   
>   {
> -  RDWIM             10 rd:5  101010 00000 0 0000000000000
> -  RDPR_tpc          10 rd:5  101010 00000 0 0000000000000
> +  [
> +    RDPR_tpc            10 rd:5  101010 00000 0 0000000000000
> +    RDPR_tnpc           10 rd:5  101010 00001 0 0000000000000
> +    RDPR_tstate         10 rd:5  101010 00010 0 0000000000000
> +    RDPR_tt             10 rd:5  101010 00011 0 0000000000000
> +    RDPR_tick           10 rd:5  101010 00100 0 0000000000000
> +    RDPR_tba            10 rd:5  101010 00101 0 0000000000000
> +    RDPR_pstate         10 rd:5  101010 00110 0 0000000000000
> +    RDPR_tl             10 rd:5  101010 00111 0 0000000000000
> +    RDPR_pil            10 rd:5  101010 01000 0 0000000000000
> +    RDPR_cwp            10 rd:5  101010 01001 0 0000000000000
> +    RDPR_cansave        10 rd:5  101010 01010 0 0000000000000
> +    RDPR_canrestore     10 rd:5  101010 01011 0 0000000000000
> +    RDPR_cleanwin       10 rd:5  101010 01100 0 0000000000000
> +    RDPR_otherwin       10 rd:5  101010 01101 0 0000000000000
> +    RDPR_wstate         10 rd:5  101010 01110 0 0000000000000
> +    RDPR_gl             10 rd:5  101010 10000 0 0000000000000
> +    RDPR_strand_status  10 rd:5  101010 11010 0 0000000000000
> +    RDPR_ver            10 rd:5  101010 11111 0 0000000000000
> +  ]
> +  RDWIM                 10 rd:5  101010 ----- - -------------
>   }
> -RDPR_tnpc           10 rd:5  101010 00001 0 0000000000000
> -RDPR_tstate         10 rd:5  101010 00010 0 0000000000000
> -RDPR_tt             10 rd:5  101010 00011 0 0000000000000
> -RDPR_tick           10 rd:5  101010 00100 0 0000000000000
> -RDPR_tba            10 rd:5  101010 00101 0 0000000000000
> -RDPR_pstate         10 rd:5  101010 00110 0 0000000000000
> -RDPR_tl             10 rd:5  101010 00111 0 0000000000000
> -RDPR_pil            10 rd:5  101010 01000 0 0000000000000
> -RDPR_cwp            10 rd:5  101010 01001 0 0000000000000
> -RDPR_cansave        10 rd:5  101010 01010 0 0000000000000
> -RDPR_canrestore     10 rd:5  101010 01011 0 0000000000000
> -RDPR_cleanwin       10 rd:5  101010 01100 0 0000000000000
> -RDPR_otherwin       10 rd:5  101010 01101 0 0000000000000
> -RDPR_wstate         10 rd:5  101010 01110 0 0000000000000
> -RDPR_gl             10 rd:5  101010 10000 0 0000000000000
> -RDPR_strand_status  10 rd:5  101010 11010 0 0000000000000
> -RDPR_ver            10 rd:5  101010 11111 0 0000000000000
>   
>   {
>     WRWIM             10 00000 110010 ..... . .............  @n_r_ri

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

[PATCH v2 6/7] target/sparc: Loosen decode of RDTBR for v7

[Richard Henderson]

For v7, bits [18:0] are ignored.
For v8, bits [18:14] are reserved and bits [13:0] are ignored.

Fixes: e8325dc02d0 ("target/sparc: Move RDTBR, FLUSHW to decodetree")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/insns.decode | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
index 77b2f54fdf..242ec42016 100644
--- a/target/sparc/insns.decode
+++ b/target/sparc/insns.decode
@@ -226,7 +226,7 @@ WRPR_strand_status  10 11010 110010 ..... . .............  @n_r_ri
 
 {
   FLUSHW    10 00000 101011 00000 0 0000000000000
-  RDTBR     10 rd:5  101011 00000 0 0000000000000
+  RDTBR     10 rd:5  101011 ----- - -------------
 }
 
 {
-- 
2.43.0

Re: [PATCH v2 6/7] target/sparc: Loosen decode of RDTBR for v7

[Mark Cave-Ayland]

On 05/09/2025 12:51, Richard Henderson wrote:

> For v7, bits [18:0] are ignored.
> For v8, bits [18:14] are reserved and bits [13:0] are ignored.
> 
> Fixes: e8325dc02d0 ("target/sparc: Move RDTBR, FLUSHW to decodetree")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sparc/insns.decode | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
> index 77b2f54fdf..242ec42016 100644
> --- a/target/sparc/insns.decode
> +++ b/target/sparc/insns.decode
> @@ -226,7 +226,7 @@ WRPR_strand_status  10 11010 110010 ..... . .............  @n_r_ri
>   
>   {
>     FLUSHW    10 00000 101011 00000 0 0000000000000
> -  RDTBR     10 rd:5  101011 00000 0 0000000000000
> +  RDTBR     10 rd:5  101011 ----- - -------------
>   }
>   
>   {

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

[PATCH v2 7/7] target/sparc: Relax decode of rs2_or_imm for v7

[Richard Henderson]

For v7, bits [13:5] are ignored for !imm.
For v8, those same bits are reserved, but are not trapped.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/translate.c | 56 ++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index cfdd9c1ce4..810e2491a6 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -2526,6 +2526,32 @@ static int extract_qfpreg(DisasContext *dc, int x)
 # define avail_VIS4(C)    false
 #endif
 
+/*
+ * We decoded bit 13 as imm, and bits [12:0] as rs2_or_imm.
+ * For v9, if !imm, then the unused bits [12:5] must be zero.
+ * For v7 and v8, the unused bits are ignored; clear them here.
+ */
+static bool check_rs2(DisasContext *dc, int *rs2)
+{
+    if (unlikely(*rs2 & ~0x1f)) {
+        if (avail_64(dc)) {
+            return false;
+        }
+        *rs2 &= 0x1f;
+    }
+    return true;
+}
+
+static bool check_r_r_ri(DisasContext *dc, arg_r_r_ri *a)
+{
+    return a->imm || check_rs2(dc, &a->rs2_or_imm);
+}
+
+static bool check_r_r_ri_cc(DisasContext *dc, arg_r_r_ri_cc *a)
+{
+    return a->imm || check_rs2(dc, &a->rs2_or_imm);
+}
+
 /* Default case for non jump instructions. */
 static bool advance_pc(DisasContext *dc)
 {
@@ -3249,8 +3275,7 @@ static bool do_wr_special(DisasContext *dc, arg_r_r_ri *a, bool priv,
 {
     TCGv src;
 
-    /* For simplicity, we under-decoded the rs2 form. */
-    if (!a->imm && (a->rs2_or_imm & ~0x1f)) {
+    if (!check_r_r_ri(dc, a)) {
         return false;
     }
     if (!priv) {
@@ -3693,8 +3718,7 @@ static bool do_arith_int(DisasContext *dc, arg_r_r_ri_cc *a,
 {
     TCGv dst, src1;
 
-    /* For simplicity, we under-decoded the rs2 form. */
-    if (!a->imm && a->rs2_or_imm & ~0x1f) {
+    if (!check_r_r_ri_cc(dc, a)) {
         return false;
     }
 
@@ -3778,11 +3802,11 @@ static bool trans_OR(DisasContext *dc, arg_r_r_ri_cc *a)
 {
     /* OR with %g0 is the canonical alias for MOV. */
     if (!a->cc && a->rs1 == 0) {
+        if (!check_r_r_ri_cc(dc, a)) {
+            return false;
+        }
         if (a->imm || a->rs2_or_imm == 0) {
             gen_store_gpr(dc, a->rd, tcg_constant_tl(a->rs2_or_imm));
-        } else if (a->rs2_or_imm & ~0x1f) {
-            /* For simplicity, we under-decoded the rs2 form. */
-            return false;
         } else {
             gen_store_gpr(dc, a->rd, cpu_regs[a->rs2_or_imm]);
         }
@@ -3799,8 +3823,7 @@ static bool trans_UDIV(DisasContext *dc, arg_r_r_ri *a)
     if (!avail_DIV(dc)) {
         return false;
     }
-    /* For simplicity, we under-decoded the rs2 form. */
-    if (!a->imm && a->rs2_or_imm & ~0x1f) {
+    if (!check_r_r_ri(dc, a)) {
         return false;
     }
 
@@ -3851,8 +3874,7 @@ static bool trans_UDIVX(DisasContext *dc, arg_r_r_ri *a)
     if (!avail_64(dc)) {
         return false;
     }
-    /* For simplicity, we under-decoded the rs2 form. */
-    if (!a->imm && a->rs2_or_imm & ~0x1f) {
+    if (!check_r_r_ri(dc, a)) {
         return false;
     }
 
@@ -3889,8 +3911,7 @@ static bool trans_SDIVX(DisasContext *dc, arg_r_r_ri *a)
     if (!avail_64(dc)) {
         return false;
     }
-    /* For simplicity, we under-decoded the rs2 form. */
-    if (!a->imm && a->rs2_or_imm & ~0x1f) {
+    if (!check_r_r_ri(dc, a)) {
         return false;
     }
 
@@ -4186,8 +4207,7 @@ TRANS(SRA_i, ALL, do_shift_i, a, false, false)
 
 static TCGv gen_rs2_or_imm(DisasContext *dc, bool imm, int rs2_or_imm)
 {
-    /* For simplicity, we under-decoded the rs2 form. */
-    if (!imm && rs2_or_imm & ~0x1f) {
+    if (!imm && !check_rs2(dc, &rs2_or_imm)) {
         return NULL;
     }
     if (imm || rs2_or_imm == 0) {
@@ -4250,8 +4270,7 @@ static bool do_add_special(DisasContext *dc, arg_r_r_ri *a,
 {
     TCGv src1, sum;
 
-    /* For simplicity, we under-decoded the rs2 form. */
-    if (!a->imm && a->rs2_or_imm & ~0x1f) {
+    if (!check_r_r_ri(dc, a)) {
         return false;
     }
 
@@ -4369,8 +4388,7 @@ static TCGv gen_ldst_addr(DisasContext *dc, int rs1, bool imm, int rs2_or_imm)
 {
     TCGv addr, tmp = NULL;
 
-    /* For simplicity, we under-decoded the rs2 form. */
-    if (!imm && rs2_or_imm & ~0x1f) {
+    if (!imm && !check_rs2(dc, &rs2_or_imm)) {
         return NULL;
     }
 
-- 
2.43.0

Re: [PATCH v2 7/7] target/sparc: Relax decode of rs2_or_imm for v7

[Mark Cave-Ayland]

On 05/09/2025 12:51, Richard Henderson wrote:

> For v7, bits [13:5] are ignored for !imm.

Should that be [12:5] here?

> For v8, those same bits are reserved, but are not trapped.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/sparc/translate.c | 56 ++++++++++++++++++++++++++--------------
>   1 file changed, 37 insertions(+), 19 deletions(-)
> 
> diff --git a/target/sparc/translate.c b/target/sparc/translate.c
> index cfdd9c1ce4..810e2491a6 100644
> --- a/target/sparc/translate.c
> +++ b/target/sparc/translate.c
> @@ -2526,6 +2526,32 @@ static int extract_qfpreg(DisasContext *dc, int x)
>   # define avail_VIS4(C)    false
>   #endif
>   
> +/*
> + * We decoded bit 13 as imm, and bits [12:0] as rs2_or_imm.
> + * For v9, if !imm, then the unused bits [12:5] must be zero.
> + * For v7 and v8, the unused bits are ignored; clear them here.
> + */
> +static bool check_rs2(DisasContext *dc, int *rs2)
> +{
> +    if (unlikely(*rs2 & ~0x1f)) {
> +        if (avail_64(dc)) {
> +            return false;
> +        }
> +        *rs2 &= 0x1f;
> +    }
> +    return true;
> +}
> +
> +static bool check_r_r_ri(DisasContext *dc, arg_r_r_ri *a)
> +{
> +    return a->imm || check_rs2(dc, &a->rs2_or_imm);
> +}
> +
> +static bool check_r_r_ri_cc(DisasContext *dc, arg_r_r_ri_cc *a)
> +{
> +    return a->imm || check_rs2(dc, &a->rs2_or_imm);
> +}
> +
>   /* Default case for non jump instructions. */
>   static bool advance_pc(DisasContext *dc)
>   {
> @@ -3249,8 +3275,7 @@ static bool do_wr_special(DisasContext *dc, arg_r_r_ri *a, bool priv,
>   {
>       TCGv src;
>   
> -    /* For simplicity, we under-decoded the rs2 form. */
> -    if (!a->imm && (a->rs2_or_imm & ~0x1f)) {
> +    if (!check_r_r_ri(dc, a)) {
>           return false;
>       }
>       if (!priv) {
> @@ -3693,8 +3718,7 @@ static bool do_arith_int(DisasContext *dc, arg_r_r_ri_cc *a,
>   {
>       TCGv dst, src1;
>   
> -    /* For simplicity, we under-decoded the rs2 form. */
> -    if (!a->imm && a->rs2_or_imm & ~0x1f) {
> +    if (!check_r_r_ri_cc(dc, a)) {
>           return false;
>       }
>   
> @@ -3778,11 +3802,11 @@ static bool trans_OR(DisasContext *dc, arg_r_r_ri_cc *a)
>   {
>       /* OR with %g0 is the canonical alias for MOV. */
>       if (!a->cc && a->rs1 == 0) {
> +        if (!check_r_r_ri_cc(dc, a)) {
> +            return false;
> +        }
>           if (a->imm || a->rs2_or_imm == 0) {
>               gen_store_gpr(dc, a->rd, tcg_constant_tl(a->rs2_or_imm));
> -        } else if (a->rs2_or_imm & ~0x1f) {
> -            /* For simplicity, we under-decoded the rs2 form. */
> -            return false;
>           } else {
>               gen_store_gpr(dc, a->rd, cpu_regs[a->rs2_or_imm]);
>           }
> @@ -3799,8 +3823,7 @@ static bool trans_UDIV(DisasContext *dc, arg_r_r_ri *a)
>       if (!avail_DIV(dc)) {
>           return false;
>       }
> -    /* For simplicity, we under-decoded the rs2 form. */
> -    if (!a->imm && a->rs2_or_imm & ~0x1f) {
> +    if (!check_r_r_ri(dc, a)) {
>           return false;
>       }
>   
> @@ -3851,8 +3874,7 @@ static bool trans_UDIVX(DisasContext *dc, arg_r_r_ri *a)
>       if (!avail_64(dc)) {
>           return false;
>       }
> -    /* For simplicity, we under-decoded the rs2 form. */
> -    if (!a->imm && a->rs2_or_imm & ~0x1f) {
> +    if (!check_r_r_ri(dc, a)) {
>           return false;
>       }
>   
> @@ -3889,8 +3911,7 @@ static bool trans_SDIVX(DisasContext *dc, arg_r_r_ri *a)
>       if (!avail_64(dc)) {
>           return false;
>       }
> -    /* For simplicity, we under-decoded the rs2 form. */
> -    if (!a->imm && a->rs2_or_imm & ~0x1f) {
> +    if (!check_r_r_ri(dc, a)) {
>           return false;
>       }
>   
> @@ -4186,8 +4207,7 @@ TRANS(SRA_i, ALL, do_shift_i, a, false, false)
>   
>   static TCGv gen_rs2_or_imm(DisasContext *dc, bool imm, int rs2_or_imm)
>   {
> -    /* For simplicity, we under-decoded the rs2 form. */
> -    if (!imm && rs2_or_imm & ~0x1f) {
> +    if (!imm && !check_rs2(dc, &rs2_or_imm)) {
>           return NULL;
>       }
>       if (imm || rs2_or_imm == 0) {
> @@ -4250,8 +4270,7 @@ static bool do_add_special(DisasContext *dc, arg_r_r_ri *a,
>   {
>       TCGv src1, sum;
>   
> -    /* For simplicity, we under-decoded the rs2 form. */
> -    if (!a->imm && a->rs2_or_imm & ~0x1f) {
> +    if (!check_r_r_ri(dc, a)) {
>           return false;
>       }
>   
> @@ -4369,8 +4388,7 @@ static TCGv gen_ldst_addr(DisasContext *dc, int rs1, bool imm, int rs2_or_imm)
>   {
>       TCGv addr, tmp = NULL;
>   
> -    /* For simplicity, we under-decoded the rs2 form. */
> -    if (!imm && rs2_or_imm & ~0x1f) {
> +    if (!imm && !check_rs2(dc, &rs2_or_imm)) {
>           return NULL;
>       }

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH v2 7/7] target/sparc: Relax decode of rs2_or_imm for v7

[Richard Henderson]

On 9/17/25 13:42, Mark Cave-Ayland wrote:
> On 05/09/2025 12:51, Richard Henderson wrote:
> 
>> For v7, bits [13:5] are ignored for !imm.
> 
> Should that be [12:5] here?
> 

Oops, yes.


r~

Re: [PATCH v2 0/7] target/sparc: Relax some decode for v7/v8

[Michael Tokarev]

On 05.09.2025 14:51, Richard Henderson wrote:
> Hi Mark,
> 
> I noticed a few other places where we probably want to
> allow unused bits to be set and ignored for pre-v9.
> 
> Changes for v2:
>    - Use TRANS() in the STBAR and RDY patches.
>    - Apply the same relaxation for RDPSR, RDWIM, RDTBR.
>    - Do not report illegal_instruction when unused bits
>      are set in the rs2_or_imm field.
> 
> r~
> 
> 
> Supercedes: 20250905051936.92815-1-richard.henderson@linaro.org
> 
> Richard Henderson (7):
>    target/sparc: Allow TRANS macro with no extra arguments
>    target/sparc: Loosen decode of STBAR for v8
>    target/sparc: Loosen decode of RDY for v7
>    target/sparc: Loosen decode of RDPSR for v7
>    target/sparc: Loosen decode of RDWIM for v7
>    target/sparc: Loosen decode of RDTBR for v7
>    target/sparc: Relax decode of rs2_or_imm for v7

This might also be good candidate for stable series (10.0 and 10.1
at least).  Dunno if it is worth the effort though, but the effort
is minimal so why not.

Thanks,

/mjt

Re: [PATCH v2 0/7] target/sparc: Relax some decode for v7/v8

[Richard Henderson]

On 9/24/25 13:25, Michael Tokarev wrote:
> On 05.09.2025 14:51, Richard Henderson wrote:
>> Hi Mark,
>>
>> I noticed a few other places where we probably want to
>> allow unused bits to be set and ignored for pre-v9.
>>
>> Changes for v2:
>>    - Use TRANS() in the STBAR and RDY patches.
>>    - Apply the same relaxation for RDPSR, RDWIM, RDTBR.
>>    - Do not report illegal_instruction when unused bits
>>      are set in the rs2_or_imm field.
>>
>> r~
>>
>>
>> Supercedes: 20250905051936.92815-1-richard.henderson@linaro.org
>>
>> Richard Henderson (7):
>>    target/sparc: Allow TRANS macro with no extra arguments
>>    target/sparc: Loosen decode of STBAR for v8
>>    target/sparc: Loosen decode of RDY for v7
>>    target/sparc: Loosen decode of RDPSR for v7
>>    target/sparc: Loosen decode of RDWIM for v7
>>    target/sparc: Loosen decode of RDTBR for v7
>>    target/sparc: Relax decode of rs2_or_imm for v7
> 
> This might also be good candidate for stable series (10.0 and 10.1
> at least).  Dunno if it is worth the effort though, but the effort
> is minimal so why not.

Yes, since it's easy.  Especially the second patch is needed for Solaris 8.


r~