Re: [PATCH 2/3] disas/riscv.c: Correct disasm of lq
Daniel Henrique Barboza <[email protected]>
| Newsgroups | org.nongnu.qemu-riscv,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
On 7/15/2026 4:07 PM, [email protected] wrote: > From: Frédéric Pétrot <[email protected]> > > 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 <[email protected]> > Signed-off-by: Frédéric Pétrot <[email protected]> > --- 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 <[email protected]> > 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