Re: [PATCH v3] target/arm: Add Neoverse-N1 registers

[Richard Henderson]

On 3/6/23 18:14, Chen Baozi wrote:
> Add implementation defined registers for neoverse-n1 which
> would be accessed by TF-A. Since there is no DSU in Qemu,
> CPUCFR_EL1.SCU bit is set to 1 to avoid DSU registers definition.
>
> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
> Tested-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>   target/arm/cpu64.c     |  2 ++
>   target/arm/cpu_tcg.c   | 66 ++++++++++++++++++++++++++++++++++++++++++
>   target/arm/internals.h |  2 ++
>   3 files changed, 70 insertions(+)

You really need to base on upstream master, as these files have moved.

> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 4066950da1..a6ae7cafac 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -1094,6 +1094,8 @@ static void aarch64_neoverse_n1_initfn(Object *obj)
>   
>       /* From D5.1 AArch64 PMU register summary */
>       cpu->isar.reset_pmcr_el0 = 0x410c3000;
> +
> +    define_neoverse_n1_cp_reginfo(cpu);
>   }
>   
>   static void aarch64_host_initfn(Object *obj)
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index df0c45e523..40ec120eb2 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -150,6 +150,72 @@ void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu)
>   {
>       define_arm_cp_regs(cpu, cortex_a72_a57_a53_cp_reginfo);
>   }
> +
> +static const ARMCPRegInfo neoverse_n1_cp_reginfo[] = {

This belongs in target/arm/tcg/cpu64.c, adjacent to or inside 
aarch64_neoverse_n1_initfn.

You do not want to match the placement of define_cortex_a72_a57_a53_cp_reginfo, because 
that has a more complex usage across aarch64 kvm and tcg, along with arm32 "-cpu max".

> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 3c7341e774..0c393e971a 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1356,8 +1356,10 @@ void arm_cpu_lpa2_finalize(ARMCPU *cpu, Error **errp);
>   
>   #ifdef CONFIG_USER_ONLY
>   static inline void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu) { }
> +static inline void define_neoverse_n1_cp_reginfo(ARMCPU *cpu) {}
>   #else
>   void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu);
> +void define_neoverse_n1_cp_reginfo(ARMCPU *cpu);

No need for a public declaration at all.


r~