[PATCH 0/2] target/sparc: Loosen decode of STBAR/RDY for v7/v8

[PATCH 0/2] target/sparc: Loosen decode of STBAR/RDY for v7/v8

[Richard Henderson]

Hi Mark,

Here's an alternative to your recent STBAR patch.  It goes further
and adjusts RDY as well for really old machines.


r~


Richard Henderson (2):
  target/sparc: Loosen decode of STBAR for v8
  target/sparc: Loosen decode of RDY for v7

 target/sparc/translate.c  | 36 ++++++++++++++++++++++++------------
 target/sparc/insns.decode | 25 ++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH 1/2] 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>
---
 target/sparc/translate.c  | 12 +++++++++++-
 target/sparc/insns.decode | 13 ++++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index b922e53bf1..c2ffd965d8 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -2823,12 +2823,22 @@ 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);
 }
 
+static bool trans_STBAR_v8(DisasContext *dc, arg_STBAR_v8 *a)
+{
+    return avail_32(dc) && do_stbar(dc);
+}
+
+static bool trans_STBAR_v9(DisasContext *dc, arg_STBAR_v9 *a)
+{
+    return avail_64(dc) && do_stbar(dc);
+}
+
 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 2/2] 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.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/translate.c  | 24 +++++++++++++-----------
 target/sparc/insns.decode | 12 ++++++++++--
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index c2ffd965d8..69d5883dec 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -2865,22 +2865,24 @@ static bool do_rd_special(DisasContext *dc, bool priv, int rd,
     return advance_pc(dc);
 }
 
-static TCGv do_rdy(DisasContext *dc, TCGv dst)
+static TCGv do_rdy_1(DisasContext *dc, TCGv dst)
 {
     return cpu_y;
 }
 
-static bool trans_RDY(DisasContext *dc, arg_RDY *a)
+static bool do_rdy(DisasContext *dc, int rd)
 {
-    /*
-     * 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);
+    return do_rd_special(dc, true, rd, do_rdy_1);
+}
+
+static bool trans_RDY_v7(DisasContext *dc, arg_RDY_v7 *a)
+{
+    return avail_32(dc) && do_rdy(dc, a->rd);
+}
+
+static bool trans_RDY_v9(DisasContext *dc, arg_RDY_v9 *a)
+{
+    return avail_64(dc) && do_rdy(dc, a->rd);
 }
 
 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

Re: [PATCH 0/2] target/sparc: Loosen decode of STBAR/RDY for v7/v8

[Mark Cave-Ayland]

On 05/09/2025 06:19, Richard Henderson wrote:

> Hi Mark,
> 
> Here's an alternative to your recent STBAR patch.  It goes further
> and adjusts RDY as well for really old machines.
> 
> r~
> 
> 
> Richard Henderson (2):
>    target/sparc: Loosen decode of STBAR for v8
>    target/sparc: Loosen decode of RDY for v7
> 
>   target/sparc/translate.c  | 36 ++++++++++++++++++++++++------------
>   target/sparc/insns.decode | 25 ++++++++++++++++++++++---
>   2 files changed, 46 insertions(+), 15 deletions(-)

Thanks! I've booted some test images (along with the original test case) and it looks 
good to me, so:

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


ATB,

Mark.