disas/riscv.c | 29 ++++++++++++++++++++++++++++- target/riscv/insn32.decode | 2 +- 2 files changed, 29 insertions(+), 2 deletions(-)
We forgot to add 'cbo' insns to disas/riscv.c. The result is that the
disassembler recognizes all of them as 'lq', an insn that happens to
share the same opcode space.
While we're at it reorder cbo_* entries in insn32.decode using opcode
order instead of insn name.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3480
Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
---
disas/riscv.c | 29 ++++++++++++++++++++++++++++-
target/riscv/insn32.decode | 2 +-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/disas/riscv.c b/disas/riscv.c
index d416a4d6b3..36609efdf5 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -985,6 +985,10 @@ typedef enum {
rv_op_ssamoswap_d = 953,
rv_op_c_sspush = 954,
rv_op_c_sspopchk = 955,
+ rv_op_cbo_inval = 956,
+ rv_op_cbo_clean = 957,
+ rv_op_cbo_flush = 958,
+ rv_op_cbo_zero = 959,
} rv_op;
/* register names */
@@ -2255,6 +2259,10 @@ const rv_opcode_data rvi_opcode_data[] = {
rv_op_sspush, 0 },
{ "c.sspopchk", rv_codec_cmop_ss, rv_fmt_rs1, NULL, rv_op_sspopchk,
rv_op_sspopchk, 0 },
+ { "cbo.inval", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
+ { "cbo.clean", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
+ { "cbo.flush", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
+ { "cbo.zero", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
};
/* CSR names */
@@ -2876,7 +2884,26 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
switch ((inst >> 12) & 0b111) {
case 0: op = rv_op_fence; break;
case 1: op = rv_op_fence_i; break;
- case 2: op = rv_op_lq; break;
+ case 2:
+ /*
+ * 'lq' shares the "(...) 010 ..... 0001111" opcode space
+ * with 'cbo' insns. Check the next 5 bits to select
+ * what we want:
+ *
+ * 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 >> 17) & 0b11111) {
+ 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;
+ }
}
break;
case 4:
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 6e35c4b1e6..21272fdb50 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -207,9 +207,9 @@ ldu ............ ..... 111 ..... 0000011 @i
{
[
# *** RV32 Zicbom Standard Extension ***
+ cbo_inval 0000000 00000 ..... 010 00000 0001111 @sfence_vm
cbo_clean 0000000 00001 ..... 010 00000 0001111 @sfence_vm
cbo_flush 0000000 00010 ..... 010 00000 0001111 @sfence_vm
- cbo_inval 0000000 00000 ..... 010 00000 0001111 @sfence_vm
# *** RV32 Zicboz Standard Extension ***
cbo_zero 0000000 00100 ..... 010 00000 0001111 @sfence_vm
--
2.43.0On Wed, May 20, 2026 at 6:48 AM Daniel Henrique Barboza
<daniel.barboza@oss.qualcomm.com> wrote:
>
> We forgot to add 'cbo' insns to disas/riscv.c. The result is that the
> disassembler recognizes all of them as 'lq', an insn that happens to
> share the same opcode space.
>
> While we're at it reorder cbo_* entries in insn32.decode using opcode
> order instead of insn name.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3480
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> disas/riscv.c | 29 ++++++++++++++++++++++++++++-
> target/riscv/insn32.decode | 2 +-
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index d416a4d6b3..36609efdf5 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -985,6 +985,10 @@ typedef enum {
> rv_op_ssamoswap_d = 953,
> rv_op_c_sspush = 954,
> rv_op_c_sspopchk = 955,
> + rv_op_cbo_inval = 956,
> + rv_op_cbo_clean = 957,
> + rv_op_cbo_flush = 958,
> + rv_op_cbo_zero = 959,
> } rv_op;
>
> /* register names */
> @@ -2255,6 +2259,10 @@ const rv_opcode_data rvi_opcode_data[] = {
> rv_op_sspush, 0 },
> { "c.sspopchk", rv_codec_cmop_ss, rv_fmt_rs1, NULL, rv_op_sspopchk,
> rv_op_sspopchk, 0 },
> + { "cbo.inval", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> + { "cbo.clean", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> + { "cbo.flush", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> + { "cbo.zero", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> };
>
> /* CSR names */
> @@ -2876,7 +2884,26 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
> switch ((inst >> 12) & 0b111) {
> case 0: op = rv_op_fence; break;
> case 1: op = rv_op_fence_i; break;
> - case 2: op = rv_op_lq; break;
> + case 2:
> + /*
> + * 'lq' shares the "(...) 010 ..... 0001111" opcode space
> + * with 'cbo' insns. Check the next 5 bits to select
> + * what we want:
> + *
> + * 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 >> 17) & 0b11111) {
> + 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;
> + }
> }
> break;
> case 4:
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 6e35c4b1e6..21272fdb50 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -207,9 +207,9 @@ ldu ............ ..... 111 ..... 0000011 @i
> {
> [
> # *** RV32 Zicbom Standard Extension ***
> + cbo_inval 0000000 00000 ..... 010 00000 0001111 @sfence_vm
> cbo_clean 0000000 00001 ..... 010 00000 0001111 @sfence_vm
> cbo_flush 0000000 00010 ..... 010 00000 0001111 @sfence_vm
> - cbo_inval 0000000 00000 ..... 010 00000 0001111 @sfence_vm
>
> # *** RV32 Zicboz Standard Extension ***
> cbo_zero 0000000 00100 ..... 010 00000 0001111 @sfence_vm
> --
> 2.43.0
>
>
On Tue, May 19, 2026 at 05:47:14PM +0800, Daniel Henrique Barboza wrote:
> We forgot to add 'cbo' insns to disas/riscv.c. The result is that the
> disassembler recognizes all of them as 'lq', an insn that happens to
> share the same opcode space.
>
> While we're at it reorder cbo_* entries in insn32.decode using opcode
> order instead of insn name.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3480
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
> ---
> disas/riscv.c | 29 ++++++++++++++++++++++++++++-
> target/riscv/insn32.decode | 2 +-
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index d416a4d6b3..36609efdf5 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -985,6 +985,10 @@ typedef enum {
> rv_op_ssamoswap_d = 953,
> rv_op_c_sspush = 954,
> rv_op_c_sspopchk = 955,
> + rv_op_cbo_inval = 956,
> + rv_op_cbo_clean = 957,
> + rv_op_cbo_flush = 958,
> + rv_op_cbo_zero = 959,
> } rv_op;
>
> /* register names */
> @@ -2255,6 +2259,10 @@ const rv_opcode_data rvi_opcode_data[] = {
> rv_op_sspush, 0 },
> { "c.sspopchk", rv_codec_cmop_ss, rv_fmt_rs1, NULL, rv_op_sspopchk,
> rv_op_sspopchk, 0 },
> + { "cbo.inval", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> + { "cbo.clean", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> + { "cbo.flush", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> + { "cbo.zero", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> };
>
> /* CSR names */
> @@ -2876,7 +2884,26 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
> switch ((inst >> 12) & 0b111) {
> case 0: op = rv_op_fence; break;
> case 1: op = rv_op_fence_i; break;
> - case 2: op = rv_op_lq; break;
> + case 2:
> + /*
> + * 'lq' shares the "(...) 010 ..... 0001111" opcode space
> + * with 'cbo' insns. Check the next 5 bits to select
> + * what we want:
> + *
> + * 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 >> 17) & 0b11111) {
> + 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;
> + }
> }
> break;
> case 4:
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 6e35c4b1e6..21272fdb50 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -207,9 +207,9 @@ ldu ............ ..... 111 ..... 0000011 @i
> {
> [
> # *** RV32 Zicbom Standard Extension ***
> + cbo_inval 0000000 00000 ..... 010 00000 0001111 @sfence_vm
> cbo_clean 0000000 00001 ..... 010 00000 0001111 @sfence_vm
> cbo_flush 0000000 00010 ..... 010 00000 0001111 @sfence_vm
> - cbo_inval 0000000 00000 ..... 010 00000 0001111 @sfence_vm
>
> # *** RV32 Zicboz Standard Extension ***
> cbo_zero 0000000 00100 ..... 010 00000 0001111 @sfence_vm
> --
> 2.43.0
>
On 5/19/26 13:47, Daniel Henrique Barboza wrote:
> We forgot to add 'cbo' insns to disas/riscv.c. The result is that the
> disassembler recognizes all of them as 'lq', an insn that happens to
> share the same opcode space.
>
> While we're at it reorder cbo_* entries in insn32.decode using opcode
> order instead of insn name.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3480
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> ---
> disas/riscv.c | 29 ++++++++++++++++++++++++++++-
> target/riscv/insn32.decode | 2 +-
> 2 files changed, 29 insertions(+), 2 deletions(-)
As far as it goes,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
However, I have long-standing nits:
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index d416a4d6b3..36609efdf5 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -985,6 +985,10 @@ typedef enum {
> rv_op_ssamoswap_d = 953,
> rv_op_c_sspush = 954,
> rv_op_c_sspopchk = 955,
> + rv_op_cbo_inval = 956,
> + rv_op_cbo_clean = 957,
> + rv_op_cbo_flush = 958,
> + rv_op_cbo_zero = 959,
> } rv_op;
>
> /* register names */
> @@ -2255,6 +2259,10 @@ const rv_opcode_data rvi_opcode_data[] = {
> rv_op_sspush, 0 },
> { "c.sspopchk", rv_codec_cmop_ss, rv_fmt_rs1, NULL, rv_op_sspopchk,
> rv_op_sspopchk, 0 },
> + { "cbo.inval", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> + { "cbo.clean", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> + { "cbo.flush", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> + { "cbo.zero", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0 },
> };
There's got to be a better way to structure this code so that you don't have a magic
correspondence between enumerators and array indexes.
>
> /* CSR names */
> @@ -2876,7 +2884,26 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa isa)
> switch ((inst >> 12) & 0b111) {
> case 0: op = rv_op_fence; break;
> case 1: op = rv_op_fence_i; break;
> - case 2: op = rv_op_lq; break;
> + case 2:
> + /*
> + * 'lq' shares the "(...) 010 ..... 0001111" opcode space
> + * with 'cbo' insns. Check the next 5 bits to select
> + * what we want:
> + *
> + * 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 >> 17) & 0b11111) {
> + case 0: op = rv_op_cbo_inval; break;
Surely "return foo" would be easier than arranging to fall through all the way to the
bottom of the function, just to do
dec->op = op;
with no return value.
To my mind,
void (*decode_func)(rv_decode *, rv_isa);
should return "const rv_opcode_data *", with NULL for unrecognized/illegal, so that each
opcode_data array need not be exported along with the decode function.
Indeed, that means you need not have opcode_data arrays at all, and could write
static const rv_op_cbo_inval = {
"cbo.inval", rv_codec_r, rv_fmt_rs1, NULL, 0, 0, 0
};
...
case 0: return &rv_op_cbo_inval;
r~
© 2016 - 2026 Red Hat, Inc.