Re: [PATCH] target/riscv: Add RVV registers to log

[Richard Henderson <richard.henderson@linaro.org>]

On 2/1/23 04:24, Ivan Klokov wrote:
> + * @CPU_DUMP_RVV: dump RISC-V RVV registers
>    */
>   enum CPUDumpFlags {
>       CPU_DUMP_CODE = 0x00010000,
>       CPU_DUMP_FPU  = 0x00020000,
>       CPU_DUMP_CCOP = 0x00040000,
> +    CPU_DUMP_RVV  = 0x00080000,

We're certainly not going to call this "RVV", as that is not generic.  But we certainly 
can add something named "VPU" or "VEC" or "VECTOR".

> +++ b/accel/tcg/cpu-exec.c
> @@ -296,6 +296,11 @@ static void log_cpu_exec(target_ulong pc, CPUState *cpu,
>                  }
>  #if defined(TARGET_I386)
>                  flags |= CPU_DUMP_CCOP;
> +#endif
> +#if defined(TARGET_RISCV)
> +                if (qemu_loglevel_mask(CPU_LOG_RISCV_RVV)) {
> +                    flags |= CPU_DUMP_RVV;
> +                }
>  #endif

Hard no here.  Most of the time while debugging we don't care about vector state.
What you should add is a new -d vec flag to match -d fpu, so that one can select vector 
state explicitly.

> @@ -459,6 +468,44 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
>               }
>           }
>       }
> +    if (riscv_has_ext(env, RVV)) {
> +        if (flags & CPU_DUMP_RVV) {
> +
> +            static const int dump_rvv_csrs[] = {
> +                        CSR_VSTART,
> +                        CSR_VXSAT,
> +                        CSR_VXRM,
> +                        CSR_VCSR,
> +                        CSR_VL,
> +                        CSR_VTYPE,
> +                        CSR_VLENB,
> +                    };
> +            for (int i = 0; i < ARRAY_SIZE(dump_rvv_csrs); ++i) {
> +                int csrno = dump_rvv_csrs[i];
> +                target_ulong val = 0;
> +                RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
> +
> +                /*
> +                 * Rely on the smode, hmode, etc, predicates within csr.c
> +                 * to do the filtering of the registers that are present.
> +                 */
> +                if (res == RISCV_EXCP_NONE) {
> +                    qemu_fprintf(f, " %-8s " TARGET_FMT_lx "\n",
> +                                 csr_ops[csrno].name, val);
> +                }
> +            }
> +            uint16_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
> +
> +            for (i = 0; i < 32; i++) {
> +                qemu_fprintf(f, " %-8s ", riscv_rvv_regnames[i]);
> +                p = (uint8_t *)env->vreg;
> +                for (j = 0; j < vlenb; j++) {
> +                    qemu_fprintf(f, "%02x", *(p + i * vlenb + j));
> +                }
> +                qemu_fprintf(f, "\n");
> +            }

This doesn't take host byte ordering into account.
See HOST_BIG_ENDIAN in vector_helper.c.

If you read and print in units of uint64_t, you'll avoid this problem.


r~