Re: [PATCH 11/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml

[LIU Zhiwei]

On 2023/2/14 2:02, Bin Meng wrote:
> It's worth noting that the vector CSR predicate() has a similar
> run-time check logic to the FPU CSR. With the previous patch our
> gdbstub can correctly report these vector CSRs via the CSR xml.
>
> Commit 719d3561b269 ("target/riscv: gdb: support vector registers for rv64 & 
> rv32")
> inserted these vector CSRs in an ad-hoc, non-standard way in the
> riscv-vector.xml. Now we can treat these CSRs no different from
> other CSRs.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
>   target/riscv/gdbstub.c | 75 ------------------------------------------
>   1 file changed, 75 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index ef52f41460..6048541606 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -127,40 +127,6 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t 
> *mem_buf, int n)
>       return 0;
>   }
>   
> -/*
> - * Convert register index number passed by GDB to the correspond
> - * vector CSR number. Vector CSRs are defined after vector registers
> - * in dynamic generated riscv-vector.xml, thus the starting register index
> - * of vector CSRs is 32.
> - * Return 0 if register index number is out of range.
> - */
> -static int riscv_gdb_vector_csrno(int num_regs)
> -{
> -    /*
> -     * The order of vector CSRs in the switch case
> -     * should match with the order defined in csr_ops[].
> -     */
> -    switch (num_regs) {
> -    case 32:
> -        return CSR_VSTART;
> -    case 33:
> -        return CSR_VXSAT;
> -    case 34:
> -        return CSR_VXRM;
> -    case 35:
> -        return CSR_VCSR;
> -    case 36:
> -        return CSR_VL;
> -    case 37:
> -        return CSR_VTYPE;
> -    case 38:
> -        return CSR_VLENB;
> -    default:
> -        /* Unknown register. */
> -        return 0;
> -    }
> -}
> -
>   static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
>   {
>       uint16_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
> @@ -174,19 +140,6 @@ static int riscv_gdb_get_vector(CPURISCVState *env, 
> GByteArray *buf, int n)
>           return cnt;
>       }
>   
> -    int csrno = riscv_gdb_vector_csrno(n);
> -
> -    if (!csrno) {
> -        return 0;
> -    }
> -
> -    target_ulong val = 0;
> -    int result = riscv_csrrw_debug(env, csrno, &val, 0, 0);
> -
> -    if (result == RISCV_EXCP_NONE) {
> -        return gdb_get_regl(buf, val);
> -    }
> -
>       return 0;
>   }
>   
> @@ -201,19 +154,6 @@ static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n)
>           return vlenb;
>       }
>   
> -    int csrno = riscv_gdb_vector_csrno(n);
> -
> -    if (!csrno) {
> -        return 0;
> -    }
> -
> -    target_ulong val = ldtul_p(mem_buf);
> -    int result = riscv_csrrw_debug(env, csrno, NULL, val, -1);
> -
> -    if (result == RISCV_EXCP_NONE) {
> -        return sizeof(target_ulong);
> -    }
> -
>       return 0;
>   }
>   
> @@ -361,21 +301,6 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
>           num_regs++;
>       }
>   
> -    /* Define vector CSRs */
> -    const char *vector_csrs[7] = {
> -        "vstart", "vxsat", "vxrm", "vcsr",
> -        "vl", "vtype", "vlenb"
> -    };
> -
> -    for (i = 0; i < 7; i++) {
> -        g_string_append_printf(s,
> -                               "<reg name=\"%s\" bitsize=\"%d\""
> -                               " regnum=\"%d\" group=\"vector\""
> -                               " type=\"int\"/>",
> -                               vector_csrs[i], TARGET_LONG_BITS, base_reg++);
> -        num_regs++;
> -    }
> -

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

Zhiwei

>       g_string_append_printf(s, "</feature>");
>   
>       cpu->dyn_vreg_xml = g_string_free(s, false);