Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver

[Alex Bennée]

Konrad Schwarz <konrad.schwarz@siemens.com> writes:

> GDB target descriptions support typed registers;
> such that `info register X' displays not only the hex value of
> register `X', but also the individual bitfields the register
> comprises (if any), using textual labels if possible.
>
> This patch includes type information for GDB for
> a large subset of the RISC-V Control and Status Registers (CSRs).
>
> Signed-off-by: Konrad Schwarz <konrad.schwarz@siemens.com>
<snip>

Not withstanding my general comments (wish) to eventually get rid of
per-arch XML generation:

>  static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
> @@ -163,21 +167,33 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int 
> base_reg)
>      riscv_csr_predicate_fn predicate;
>      int bitsize = 16 << env->misa_mxl_max;
>      int i;
> +    riscv_csr_operations *csr_op;
> +    struct riscv_gdb_csr_tg const *csr_tg;
>  
>      g_string_printf(s, "<?xml version=\"1.0\"?>");
>      g_string_append_printf(s, "<!DOCTYPE feature SYSTEM 
> \"gdb-target.dtd\">");
>      g_string_append_printf(s, "<feature>      
> name=\"org.gnu.gdb.riscv.csr\">");

With these changes does it still match the org.gnu.gdb.riscv.csr
register description in gdb? Previously for custom XML I've used the
org.qemu.ARCH.REGS form to distinguish between something GDB expects and
something we invented (changed since 797920b952ea).

>  
> -    for (i = 0; i < CSR_TABLE_SIZE; i++) {
> -        predicate = csr_ops[i].predicate;
> +    g_string_append(s, riscv_gdb_csr_types);
> +
> +    for (i = 0, csr_op = csr_ops, csr_tg = riscv_gdb_csr_type_group;
> +            i < CSR_TABLE_SIZE; ++csr_op, ++csr_tg, ++i) {
> +        predicate = csr_op->predicate;
>          if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
> -            if (csr_ops[i].name) {
> -                g_string_append_printf(s, "<reg name=\"%s\"", 
> csr_ops[i].name);
> +            if (csr_op->name) {
> +                g_string_append_printf(s, "<reg name=\"%s\"", csr_op->name);
>              } else {
>                  g_string_append_printf(s, "<reg name=\"csr%03x\"", i);
>              }
>              g_string_append_printf(s, " bitsize=\"%d\"", bitsize);
> -            g_string_append_printf(s, " regnum=\"%d\"/>", base_reg + i);
> +            g_string_append_printf(s, " regnum=\"%d\"", base_reg + i);
> +            if (csr_tg->gdb_type) {
> +                g_string_append_printf(s, " type=\"%s\"", csr_tg->gdb_type);
> +            }
> +            if (csr_tg->gdb_group) {
> +                g_string_append_printf(s, " group=\"%s\"", 
> csr_tg->gdb_group);
> +            }
> +            g_string_append(s, " />\n");
>          }
>      }
<snip>

-- 
Alex Bennée