[PATCH 2/3] disas/riscv.c: Correct disasm of lq

frederic.petrot@univ-grenoble-alpes.fr posted 3 patches 2 months ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <Alistair.Francis@wdc.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Weiwei Li <liwei1518@gmail.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu@processmission.com>
There is a newer version of this series
[PATCH 2/3] disas/riscv.c: Correct disasm of lq
Posted by frederic.petrot@univ-grenoble-alpes.fr 2 months ago
From: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>

Lq shares the cmo opcode space, but the cbos must have rd = 0.
Now ensure that they are matched only in that case.
This implies that lq can be recognized as such only when rd != 0.
Update the decoder file accordingly.

Reported-by: Julien Thillard <julien.thillard@univ-grenoble-alpes.fr>
Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
---
 disas/riscv.c              | 19 ++++++++++---------
 target/riscv/insn32.decode |  2 ++
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 490bda8e56..831ff526ad 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -2889,21 +2889,22 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
             case 2:
                /*
                 * 'lq' shares the "(...) 010 ..... 0001111" opcode space
-                * with 'cbo' insns.
+                * with 'cbo' insns, but assumes rd != 0.
                 *
                 * cbo_inval  0000000 00000 ..... 010 00000 0001111
                 * cbo_clean  0000000 00001 ..... 010 00000 0001111
                 * cbo_flush  0000000 00010 ..... 010 00000 0001111
                 * cbo_zero   0000000 00100 ..... 010 00000 0001111
-                *
-                * Anything that doesn't match these will default to 'lq'.
                 */
-               switch (inst >> 20) {
-               case 0: op = rv_op_cbo_inval; break;
-               case 1: op = rv_op_cbo_clean; break;
-               case 2: op = rv_op_cbo_flush; break;
-               case 4: op = rv_op_cbo_zero; break;
-               default: op = rv_op_lq; break;
+               if ((inst >> 7) & 0b11111) {
+                  op = rv_op_lq;
+               } else {
+                  switch (inst >> 20) {
+                  case 0: op = rv_op_cbo_inval; break;
+                  case 1: op = rv_op_cbo_clean; break;
+                  case 2: op = rv_op_cbo_flush; break;
+                  case 4: op = rv_op_cbo_zero; break;
+                  }
                }
             }
             break;
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 21272fdb50..aa02dae3c9 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -216,6 +216,8 @@ ldu      ............   ..... 111 ..... 0000011 @i
   ]
 
   # *** RVI128 lq ***
+  # *** Catches an lq with rd = 0, which we disallow
+  illegal  ------------   ----- 010 00000 0001111
   lq       ............   ..... 010 ..... 0001111 @i
 }
 sq       ............   ..... 100 ..... 0100011 @s
-- 
2.43.0


Re: [PATCH 2/3] disas/riscv.c: Correct disasm of lq
Posted by Daniel Henrique Barboza 2 months ago

On 7/15/2026 4:07 PM, frederic.petrot@univ-grenoble-alpes.fr wrote:
> From: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
> 
> Lq shares the cmo opcode space, but the cbos must have rd = 0.
> Now ensure that they are matched only in that case.
> This implies that lq can be recognized as such only when rd != 0.
> Update the decoder file accordingly.
> 

Fixes: 9273cda722 ("disas/riscv.c: add 'cbo' insns to disassembler")

> Reported-by: Julien Thillard <julien.thillard@univ-grenoble-alpes.fr>
> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
> ---

This patch adds style issues that you're fixing with patch 3:

$ ./scripts/checkpatch.pl 0002-disas-riscv.c-Correct-disasm-of-lq.patch
ERROR: suspect code indent for conditional statements (15, 18)
#46: FILE: disas/riscv.c:2899:
+               if ((inst >> 7) & 0b11111) {
+                  op = rv_op_lq;

ERROR: trailing statements should be on next line
#50: FILE: disas/riscv.c:2903:
+                  case 0: op = rv_op_cbo_inval; break;

ERROR: trailing statements should be on next line
#51: FILE: disas/riscv.c:2904:
+                  case 1: op = rv_op_cbo_clean; break;

ERROR: trailing statements should be on next line
#52: FILE: disas/riscv.c:2905:
+                  case 2: op = rv_op_cbo_flush; break;

ERROR: trailing statements should be on next line
#53: FILE: disas/riscv.c:2906:
+                  case 4: op = rv_op_cbo_zero; break;

total: 5 errors, 0 warnings, 39 lines checked


What we want to do is to squash patch 3 into this patch to not have the style
issues being added in the first place.


Assuming patch 3 is squashed into this one:


Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>



>   disas/riscv.c              | 19 ++++++++++---------
>   target/riscv/insn32.decode |  2 ++
>   2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 490bda8e56..831ff526ad 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -2889,21 +2889,22 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
>               case 2:
>                  /*
>                   * 'lq' shares the "(...) 010 ..... 0001111" opcode space
> -                * with 'cbo' insns.
> +                * with 'cbo' insns, but assumes rd != 0.
>                   *
>                   * cbo_inval  0000000 00000 ..... 010 00000 0001111
>                   * cbo_clean  0000000 00001 ..... 010 00000 0001111
>                   * cbo_flush  0000000 00010 ..... 010 00000 0001111
>                   * cbo_zero   0000000 00100 ..... 010 00000 0001111
> -                *
> -                * Anything that doesn't match these will default to 'lq'.
>                   */
> -               switch (inst >> 20) {
> -               case 0: op = rv_op_cbo_inval; break;
> -               case 1: op = rv_op_cbo_clean; break;
> -               case 2: op = rv_op_cbo_flush; break;
> -               case 4: op = rv_op_cbo_zero; break;
> -               default: op = rv_op_lq; break;
> +               if ((inst >> 7) & 0b11111) {
> +                  op = rv_op_lq;
> +               } else {
> +                  switch (inst >> 20) {
> +                  case 0: op = rv_op_cbo_inval; break;
> +                  case 1: op = rv_op_cbo_clean; break;
> +                  case 2: op = rv_op_cbo_flush; break;
> +                  case 4: op = rv_op_cbo_zero; break;
> +                  }
>                  }
>               }
>               break;
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 21272fdb50..aa02dae3c9 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -216,6 +216,8 @@ ldu      ............   ..... 111 ..... 0000011 @i
>     ]
>   
>     # *** RVI128 lq ***
> +  # *** Catches an lq with rd = 0, which we disallow
> +  illegal  ------------   ----- 010 00000 0001111
>     lq       ............   ..... 010 ..... 0001111 @i
>   }
>   sq       ............   ..... 100 ..... 0100011 @s