Re: [PATCH v2 2/3] target/riscv: Initialize gdb_core_xml_file only once

[Daniel Henrique Barboza]

On 10/14/23 00:35, Akihiko Odaki wrote:
> gdb_core_xml_file was assigned each time a CPU is instantiated before
> this change.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   target/riscv/cpu.c         | 5 +++++
>   target/riscv/tcg/tcg-cpu.c | 4 ----
>   2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ac4a6c7eec..a811215150 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1575,6 +1575,11 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
> *data)
>       cc->get_pc = riscv_cpu_get_pc;
>       cc->gdb_read_register = riscv_cpu_gdb_read_register;
>       cc->gdb_write_register = riscv_cpu_gdb_write_register;
> +#ifdef TARGET_RISCV64
> +    cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
> +#elif defined(TARGET_RISCV32)
> +    cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
> +#endif
>       cc->gdb_num_core_regs = 33;
>       cc->gdb_stop_before_watchpoint = true;
>       cc->disas_set_info = riscv_cpu_disas_set_info;
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index e0cbc56320..626fb2acea 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -150,8 +150,6 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState 
> *env, Error **errp)
>   
>   static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
>   {
> -    RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> -    CPUClass *cc = CPU_CLASS(mcc);
>       CPURISCVState *env = &cpu->env;
>   
>       /* Validate that MISA_MXL is set properly. */
> @@ -159,11 +157,9 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
> Error **errp)
>   #ifdef TARGET_RISCV64
>       case MXL_RV64:
>       case MXL_RV128:
> -        cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
>           break;
>   #elif defined(TARGET_RISCV32)
>       case MXL_RV32:
> -        cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
>           break;


Hmmm the issue here is that, in patch 1, you added an "elif 
defined(TARGET_RISCV32)"
based on an assumption that you changed here since there's no more gdb_core 
files being
set.

My suggestion is to use patch 1 from v1, where you removed the misa_mxl_max == 
misa_mxl
check at the end of this function. And then in this patch you can remove this 
function
altogether since you're assigning gdb_core in riscv_cpu_class_init() and the 
function will
be left doing nothing of note.


Thanks,

Daniel

>   #endif
>       default: