diff mbox series

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

Message ID 20230530131843.1186637-8-christoph.muellner@vrull.eu (mailing list archive)
State New, archived
Headers show
Series disas/riscv: Add vendor extension support | expand

Commit Message

Christoph Müllner May 30, 2023, 1:18 p.m. UTC 
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(-)

Comments

LIU Zhiwei June 8, 2023, 1:04 p.m. UTC | #1 
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;
Alistair Francis June 12, 2023, 3:37 a.m. UTC | #2 
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
>
>
Christoph Müllner June 12, 2023, 11:11 a.m. UTC | #3 
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;
diff mbox series

Patch

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;