[v1] disas/riscv: Add vendor extension support

[PATCH 7/9] disas/riscv: Provide infrastructure for vendor extensions

Posted by Christoph Muellner 2 years, 8 months ago

From: Christoph Müllner <christoph.muellner@vrull.eu>

A previous patch provides a pointer to the RISCVCPUConfig data.
Let's use this to add the necessary code for vendor extensions.
This patch does not change the current behaviour, but clearly
defines how vendor extension support can be added to the disassembler.

Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
---
 disas/riscv.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 086edee6a2..db98e3ea6a 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "disas/dis-asm.h"
 #include "disas/riscv.h"
+#include "target/riscv/cpu-config.h"
 
 typedef enum {
     /* 0 is reserved for rv_op_illegal. */
@@ -4599,13 +4600,38 @@ static void decode_inst_decompress(rv_decode *dec, rv_isa isa)
 /* disassemble instruction */
 
 static void
-disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst)
+disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
+            struct disassemble_info *info)
 {
+    RISCVCPUConfig *cfg = info->private_data;
     rv_decode dec = { 0 };
     dec.pc = pc;
     dec.inst = inst;
-    dec.opcode_data = rvi_opcode_data;
-    decode_inst_opcode(&dec, isa);
+
+    static const struct {
+        bool (*guard_func)(const RISCVCPUConfig *);
+        const rv_opcode_data *opcode_data;
+        void (*decode_func)(rv_decode *, rv_isa);
+    } decoders[] = {
+        { always_true_p, rvi_opcode_data, decode_inst_opcode },
+    };
+
+    for (size_t i = 0; i < ARRAY_SIZE(decoders); i++) {
+        bool (*guard_func)(const RISCVCPUConfig *) = decoders[i].guard_func;
+        const rv_opcode_data *opcode_data = decoders[i].opcode_data;
+        void (*decode_func)(rv_decode *, rv_isa) = decoders[i].decode_func;
+
+        if (guard_func(cfg)) {
+            dec.opcode_data = opcode_data;
+            decode_func(&dec, isa);
+            if (dec.op != rv_op_illegal)
+                break;
+        }
+    }
+
+    if (dec.op == rv_op_illegal)
+        dec.opcode_data = rvi_opcode_data;
+
     decode_inst_operands(&dec, isa);
     decode_inst_decompress(&dec, isa);
     decode_inst_lift_pseudo(&dec);
@@ -4659,7 +4685,7 @@ print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
         break;
     }
 
-    disasm_inst(buf, sizeof(buf), isa, memaddr, inst);
+    disasm_inst(buf, sizeof(buf), isa, memaddr, inst, info);
     (*info->fprintf_func)(info->stream, "%s", buf);
 
     return len;
-- 
2.40.1

Re: [PATCH 7/9] disas/riscv: Provide infrastructure for vendor extensions

Posted by Alistair Francis 2 years, 8 months ago

On Tue, May 30, 2023 at 11:21 PM Christoph Muellner
<christoph.muellner@vrull.eu> wrote:
>
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> A previous patch provides a pointer to the RISCVCPUConfig data.
> Let's use this to add the necessary code for vendor extensions.
> This patch does not change the current behaviour, but clearly
> defines how vendor extension support can be added to the disassembler.
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  disas/riscv.c | 34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 086edee6a2..db98e3ea6a 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -20,6 +20,7 @@
>  #include "qemu/osdep.h"
>  #include "disas/dis-asm.h"
>  #include "disas/riscv.h"
> +#include "target/riscv/cpu-config.h"
>
>  typedef enum {
>      /* 0 is reserved for rv_op_illegal. */
> @@ -4599,13 +4600,38 @@ static void decode_inst_decompress(rv_decode *dec, rv_isa isa)
>  /* disassemble instruction */
>
>  static void
> -disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst)
> +disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
> +            struct disassemble_info *info)
>  {
> +    RISCVCPUConfig *cfg = info->private_data;
>      rv_decode dec = { 0 };
>      dec.pc = pc;
>      dec.inst = inst;
> -    dec.opcode_data = rvi_opcode_data;
> -    decode_inst_opcode(&dec, isa);
> +
> +    static const struct {
> +        bool (*guard_func)(const RISCVCPUConfig *);
> +        const rv_opcode_data *opcode_data;
> +        void (*decode_func)(rv_decode *, rv_isa);
> +    } decoders[] = {
> +        { always_true_p, rvi_opcode_data, decode_inst_opcode },
> +    };
> +
> +    for (size_t i = 0; i < ARRAY_SIZE(decoders); i++) {
> +        bool (*guard_func)(const RISCVCPUConfig *) = decoders[i].guard_func;
> +        const rv_opcode_data *opcode_data = decoders[i].opcode_data;
> +        void (*decode_func)(rv_decode *, rv_isa) = decoders[i].decode_func;
> +
> +        if (guard_func(cfg)) {
> +            dec.opcode_data = opcode_data;
> +            decode_func(&dec, isa);
> +            if (dec.op != rv_op_illegal)
> +                break;
> +        }
> +    }
> +
> +    if (dec.op == rv_op_illegal)
> +        dec.opcode_data = rvi_opcode_data;
> +
>      decode_inst_operands(&dec, isa);
>      decode_inst_decompress(&dec, isa);
>      decode_inst_lift_pseudo(&dec);
> @@ -4659,7 +4685,7 @@ print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
>          break;
>      }
>
> -    disasm_inst(buf, sizeof(buf), isa, memaddr, inst);
> +    disasm_inst(buf, sizeof(buf), isa, memaddr, inst, info);
>      (*info->fprintf_func)(info->stream, "%s", buf);
>
>      return len;
> --
> 2.40.1
>
>

Re: [PATCH 7/9] disas/riscv: Provide infrastructure for vendor extensions

Posted by LIU Zhiwei 2 years, 8 months ago

On 2023/5/30 21:18, Christoph Muellner wrote:
> From: Christoph Müllner <christoph.muellner@vrull.eu>
>
> A previous patch provides a pointer to the RISCVCPUConfig data.
> Let's use this to add the necessary code for vendor extensions.
> This patch does not change the current behaviour, but clearly
> defines how vendor extension support can be added to the disassembler.
>
> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> ---
>   disas/riscv.c | 34 ++++++++++++++++++++++++++++++----
>   1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/disas/riscv.c b/disas/riscv.c
> index 086edee6a2..db98e3ea6a 100644
> --- a/disas/riscv.c
> +++ b/disas/riscv.c
> @@ -20,6 +20,7 @@
>   #include "qemu/osdep.h"
>   #include "disas/dis-asm.h"
>   #include "disas/riscv.h"
> +#include "target/riscv/cpu-config.h"
>   
>   typedef enum {
>       /* 0 is reserved for rv_op_illegal. */
> @@ -4599,13 +4600,38 @@ static void decode_inst_decompress(rv_decode *dec, rv_isa isa)
>   /* disassemble instruction */
>   
>   static void
> -disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst)
> +disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
> +            struct disassemble_info *info)
>   {
> +    RISCVCPUConfig *cfg = info->private_data;
>       rv_decode dec = { 0 };
>       dec.pc = pc;
>       dec.inst = inst;
> -    dec.opcode_data = rvi_opcode_data;
> -    decode_inst_opcode(&dec, isa);
> +
> +    static const struct {
> +        bool (*guard_func)(const RISCVCPUConfig *);
> +        const rv_opcode_data *opcode_data;
> +        void (*decode_func)(rv_decode *, rv_isa);
> +    } decoders[] = {
> +        { always_true_p, rvi_opcode_data, decode_inst_opcode },
> +    };
> +
> +    for (size_t i = 0; i < ARRAY_SIZE(decoders); i++) {
> +        bool (*guard_func)(const RISCVCPUConfig *) = decoders[i].guard_func;
> +        const rv_opcode_data *opcode_data = decoders[i].opcode_data;
> +        void (*decode_func)(rv_decode *, rv_isa) = decoders[i].decode_func;
> +
> +        if (guard_func(cfg)) {
> +            dec.opcode_data = opcode_data;
> +            decode_func(&dec, isa);
> +            if (dec.op != rv_op_illegal)
> +                break;
> +        }
> +    }
> +
> +    if (dec.op == rv_op_illegal)
> +        dec.opcode_data = rvi_opcode_data;

Always enclose the if sentence.

Otherwise,

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

> +
>       decode_inst_operands(&dec, isa);
>       decode_inst_decompress(&dec, isa);
>       decode_inst_lift_pseudo(&dec);
> @@ -4659,7 +4685,7 @@ print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
>           break;
>       }
>   
> -    disasm_inst(buf, sizeof(buf), isa, memaddr, inst);
> +    disasm_inst(buf, sizeof(buf), isa, memaddr, inst, info);
>       (*info->fprintf_func)(info->stream, "%s", buf);
>   
>       return len;

Re: [PATCH 7/9] disas/riscv: Provide infrastructure for vendor extensions

Posted by Christoph Müllner 2 years, 8 months ago

On Thu, Jun 8, 2023 at 3:05 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
>
> On 2023/5/30 21:18, Christoph Muellner wrote:
> > From: Christoph Müllner <christoph.muellner@vrull.eu>
> >
> > A previous patch provides a pointer to the RISCVCPUConfig data.
> > Let's use this to add the necessary code for vendor extensions.
> > This patch does not change the current behaviour, but clearly
> > defines how vendor extension support can be added to the disassembler.
> >
> > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> > ---
> >   disas/riscv.c | 34 ++++++++++++++++++++++++++++++----
> >   1 file changed, 30 insertions(+), 4 deletions(-)
> >
> > diff --git a/disas/riscv.c b/disas/riscv.c
> > index 086edee6a2..db98e3ea6a 100644
> > --- a/disas/riscv.c
> > +++ b/disas/riscv.c
> > @@ -20,6 +20,7 @@
> >   #include "qemu/osdep.h"
> >   #include "disas/dis-asm.h"
> >   #include "disas/riscv.h"
> > +#include "target/riscv/cpu-config.h"
> >
> >   typedef enum {
> >       /* 0 is reserved for rv_op_illegal. */
> > @@ -4599,13 +4600,38 @@ static void decode_inst_decompress(rv_decode *dec, rv_isa isa)
> >   /* disassemble instruction */
> >
> >   static void
> > -disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst)
> > +disasm_inst(char *buf, size_t buflen, rv_isa isa, uint64_t pc, rv_inst inst,
> > +            struct disassemble_info *info)
> >   {
> > +    RISCVCPUConfig *cfg = info->private_data;
> >       rv_decode dec = { 0 };
> >       dec.pc = pc;
> >       dec.inst = inst;
> > -    dec.opcode_data = rvi_opcode_data;
> > -    decode_inst_opcode(&dec, isa);
> > +
> > +    static const struct {
> > +        bool (*guard_func)(const RISCVCPUConfig *);
> > +        const rv_opcode_data *opcode_data;
> > +        void (*decode_func)(rv_decode *, rv_isa);
> > +    } decoders[] = {
> > +        { always_true_p, rvi_opcode_data, decode_inst_opcode },
> > +    };
> > +
> > +    for (size_t i = 0; i < ARRAY_SIZE(decoders); i++) {
> > +        bool (*guard_func)(const RISCVCPUConfig *) = decoders[i].guard_func;
> > +        const rv_opcode_data *opcode_data = decoders[i].opcode_data;
> > +        void (*decode_func)(rv_decode *, rv_isa) = decoders[i].decode_func;
> > +
> > +        if (guard_func(cfg)) {
> > +            dec.opcode_data = opcode_data;
> > +            decode_func(&dec, isa);
> > +            if (dec.op != rv_op_illegal)
> > +                break;
> > +        }
> > +    }
> > +
> > +    if (dec.op == rv_op_illegal)
> > +        dec.opcode_data = rvi_opcode_data;
>
> Always enclose the if sentence.

Done.

Thanks,
Christoph

>
> Otherwise,
>
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>
> > +
> >       decode_inst_operands(&dec, isa);
> >       decode_inst_decompress(&dec, isa);
> >       decode_inst_lift_pseudo(&dec);
> > @@ -4659,7 +4685,7 @@ print_insn_riscv(bfd_vma memaddr, struct disassemble_info *info, rv_isa isa)
> >           break;
> >       }
> >
> > -    disasm_inst(buf, sizeof(buf), isa, memaddr, inst);
> > +    disasm_inst(buf, sizeof(buf), isa, memaddr, inst, info);
> >       (*info->fprintf_func)(info->stream, "%s", buf);
> >
> >       return len;