Re: [PATCH] target/loongarch: Use physical cpu id about CSR CPUID for sysemu

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

On 10/22/24 05:42, Bibo Mao wrote:
> For user tcg, there is no physical cpu id provided and logic cpuid
> is used. For system emulation, physical cpu id is provided, initial
> value of register CSR CPUID can be set from physical cpu id.
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>   hw/intc/loongarch_ipi.c           | 3 ++-
>   target/loongarch/cpu.c            | 7 ++++++-
>   target/loongarch/tcg/csr_helper.c | 4 ----
>   3 files changed, 8 insertions(+), 6 deletions(-)

Since cpu_index is arbitrary and assigned by hw/loongarch/virt.c anyway, why do these two 
values differ?  Surely arch_id is already unique per cpu?


r~


> 
> diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
> index 2ae1a42c46..78b6fce81b 100644
> --- a/hw/intc/loongarch_ipi.c
> +++ b/hw/intc/loongarch_ipi.c
> @@ -42,7 +42,8 @@ static CPUState *loongarch_cpu_by_arch_id(int64_t arch_id)
>       CPUArchId *archid;
>   
>       archid = find_cpu_by_archid(machine, arch_id);
> -    if (archid) {
> +    /* For offlined cpus, archid->cpu may be NULL */
> +    if (archid && archid->cpu) {
>           return CPU(archid->cpu);
>       }
>   
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index 7212fb5f8f..d4659e8d45 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -510,8 +510,10 @@ static void loongarch_max_initfn(Object *obj)
>   static void loongarch_cpu_reset_hold(Object *obj, ResetType type)
>   {
>       CPUState *cs = CPU(obj);
> +    LoongArchCPU *cpu = LOONGARCH_CPU(obj);
>       LoongArchCPUClass *lacc = LOONGARCH_CPU_GET_CLASS(obj);
>       CPULoongArchState *env = cpu_env(cs);
> +    int n;
>   
>       if (lacc->parent_phases.hold) {
>           lacc->parent_phases.hold(obj, type);
> @@ -522,7 +524,6 @@ static void loongarch_cpu_reset_hold(Object *obj, ResetType type)
>   #endif
>       env->fcsr0 = 0x0;
>   
> -    int n;
>       /* Set csr registers value after reset, see the manual 6.4. */
>       env->CSR_CRMD = FIELD_DP64(env->CSR_CRMD, CSR_CRMD, PLV, 0);
>       env->CSR_CRMD = FIELD_DP64(env->CSR_CRMD, CSR_CRMD, IE, 0);
> @@ -543,7 +544,11 @@ static void loongarch_cpu_reset_hold(Object *obj, ResetType type)
>   
>       env->CSR_ESTAT = env->CSR_ESTAT & (~MAKE_64BIT_MASK(0, 2));
>       env->CSR_RVACFG = FIELD_DP64(env->CSR_RVACFG, CSR_RVACFG, RBITS, 0);
> +#ifndef CONFIG_USER_ONLY
> +    env->CSR_CPUID = cpu->phy_id;
> +#else
>       env->CSR_CPUID = cs->cpu_index;
> +#endif
>       env->CSR_TCFG = FIELD_DP64(env->CSR_TCFG, CSR_TCFG, EN, 0);
>       env->CSR_LLBCTL = FIELD_DP64(env->CSR_LLBCTL, CSR_LLBCTL, KLO, 0);
>       env->CSR_TLBRERA = FIELD_DP64(env->CSR_TLBRERA, CSR_TLBRERA, ISTLBR, 0);
> diff --git a/target/loongarch/tcg/csr_helper.c b/target/loongarch/tcg/csr_helper.c
> index 15f94caefa..2aeca2343d 100644
> --- a/target/loongarch/tcg/csr_helper.c
> +++ b/target/loongarch/tcg/csr_helper.c
> @@ -37,10 +37,6 @@ target_ulong helper_csrrd_pgd(CPULoongArchState *env)
>   
>   target_ulong helper_csrrd_cpuid(CPULoongArchState *env)
>   {
> -    LoongArchCPU *lac = env_archcpu(env);
> -
> -    env->CSR_CPUID = CPU(lac)->cpu_index;
> -
>       return env->CSR_CPUID;
>   }
>   
> 
> base-commit: cc5adbbd50d81555b8eb73602ec16fde40b55be4