| Message ID | 20230303161518.3411149-2-chenbaozi@phytium.com.cn |
|---|---|
| State | New |
| Headers | show |
| Series | Define Impl. Def. registers for Neoverse-N1 | expand |
W dniu 3.03.2023 o 17:15, Chen Baozi pisze: > Add implementation defined registers for neoverse-n1 which > would be accessed by TF-A. > > Signed-off-by: Chen Baozi<chenbaozi@phytium.com.cn> Tested-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> ~ # cat /proc/cpuinfo processor : 0 BogoMIPS : 125.00 Features : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm lrcpc dcpop asimddp ssbs CPU implementer : 0x41 CPU architecture: 8 CPU variant : 0x4 CPU part : 0xd0c CPU revision : 1 I used TF-A with Chen's N1 patches, EDK2 upstream and Debian 6.1.12 kernel.
On Fri, 3 Mar 2023 at 16:15, Chen Baozi <chenbaozi@phytium.com.cn> wrote: > > Add implementation defined registers for neoverse-n1 which > would be accessed by TF-A. > > Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn> > --- > target/arm/cpu64.c | 2 ++ > target/arm/cpu_tcg.c | 62 ++++++++++++++++++++++++++++++++++++++++++ > target/arm/internals.h | 2 ++ > 3 files changed, 66 insertions(+) > > 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..6a1bb56b25 100644 > --- a/target/arm/cpu_tcg.c > +++ b/target/arm/cpu_tcg.c > @@ -150,6 +150,68 @@ 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[] = { > + { .name = "ATCR_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 7, .opc2 = 0, > + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "ATCR_EL2", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 4, .crn = 15, .crm = 7, .opc2 = 0, > + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "ATCR_EL3", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 7, .opc2 = 0, > + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "ATCR_EL12", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 5, .crn = 15, .crm = 7, .opc2 = 0, > + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "AVTCR_EL2", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 4, .crn = 15, .crm = 7, .opc2 = 1, > + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "CPUACTLR_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 0, > + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "CPUACTLR2_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 1, > + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "CPUACTLR3_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 2, > + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "CPUCFR_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 0, .opc2 = 0, > + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, .PL1_R -- this register is read-only. If we report bit 2 as 1 ("the DSU SCU is not present"), does TF-A pay attention to that and not try to access the DSU related registers you define in patch 2 ? If so, it would probably be nicer to say "no DSU" and not have to define dummy registers for it... > + { .name = "CPUECTLR_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 4, > + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0x961563010 }, > + { .name = "CPUPCR_EL3", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 1, > + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "CPUPMR_EL3", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 3, > + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "CPUPOR_EL3", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 2, > + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "CPUPSELR_EL3", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 0, > + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "CPUPWRCTLR_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 7, > + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + { .name = "ERXPFGCDN_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 2, > + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, Shouldn't this be PL1_RW ? > + { .name = "ERXPFGCTL_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 1, > + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, Also PL1_RW. > + { .name = "ERXPFGF_EL1", .state = ARM_CP_STATE_AA64, > + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 0, > + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > +}; > + > +void define_neoverse_n1_cp_reginfo(ARMCPU *cpu) > +{ > + define_arm_cp_regs(cpu, neoverse_n1_cp_reginfo); > +} > #endif /* !CONFIG_USER_ONLY */ thanks -- PMM
Hi Peter, > On Mar 6, 2023, at 19:33, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 3 Mar 2023 at 16:15, Chen Baozi <chenbaozi@phytium.com.cn> wrote: >> >> Add implementation defined registers for neoverse-n1 which >> would be accessed by TF-A. >> >> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn> >> --- >> target/arm/cpu64.c | 2 ++ >> target/arm/cpu_tcg.c | 62 ++++++++++++++++++++++++++++++++++++++++++ >> target/arm/internals.h | 2 ++ >> 3 files changed, 66 insertions(+) >> >> 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..6a1bb56b25 100644 >> --- a/target/arm/cpu_tcg.c >> +++ b/target/arm/cpu_tcg.c >> @@ -150,6 +150,68 @@ 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[] = { >> + { .name = "ATCR_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 7, .opc2 = 0, >> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "ATCR_EL2", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 4, .crn = 15, .crm = 7, .opc2 = 0, >> + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "ATCR_EL3", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 7, .opc2 = 0, >> + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "ATCR_EL12", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 5, .crn = 15, .crm = 7, .opc2 = 0, >> + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "AVTCR_EL2", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 4, .crn = 15, .crm = 7, .opc2 = 1, >> + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "CPUACTLR_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 0, >> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "CPUACTLR2_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 1, >> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "CPUACTLR3_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 2, >> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "CPUCFR_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 0, .opc2 = 0, >> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > > .PL1_R -- this register is read-only. > > If we report bit 2 as 1 ("the DSU SCU is not present"), does > TF-A pay attention to that and not try to access the DSU > related registers you define in patch 2 ? If so, it would > probably be nicer to say "no DSU" and not have to define > dummy registers for it... Aha! Yes, TF-A does have a function “is_scu_present_in_dsu" to check this bit. I’ll work another version to fix them all (as well as the following issues). Thanks. Baozi. > >> + { .name = "CPUECTLR_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 4, >> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0x961563010 }, >> + { .name = "CPUPCR_EL3", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 1, >> + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "CPUPMR_EL3", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 3, >> + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "CPUPOR_EL3", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 2, >> + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "CPUPSELR_EL3", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 0, >> + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "CPUPWRCTLR_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 7, >> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + { .name = "ERXPFGCDN_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 2, >> + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > > Shouldn't this be PL1_RW ? > >> + { .name = "ERXPFGCTL_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 1, >> + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > > Also PL1_RW. > >> + { .name = "ERXPFGF_EL1", .state = ARM_CP_STATE_AA64, >> + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 0, >> + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> +}; >> + >> +void define_neoverse_n1_cp_reginfo(ARMCPU *cpu) >> +{ >> + define_arm_cp_regs(cpu, neoverse_n1_cp_reginfo); >> +} >> #endif /* !CONFIG_USER_ONLY */ > > thanks > -- PMM
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..6a1bb56b25 100644 --- a/target/arm/cpu_tcg.c +++ b/target/arm/cpu_tcg.c @@ -150,6 +150,68 @@ 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[] = { + { .name = "ATCR_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 7, .opc2 = 0, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "ATCR_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 15, .crm = 7, .opc2 = 0, + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "ATCR_EL3", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 7, .opc2 = 0, + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "ATCR_EL12", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 5, .crn = 15, .crm = 7, .opc2 = 0, + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "AVTCR_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 15, .crm = 7, .opc2 = 1, + .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "CPUACTLR_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 0, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "CPUACTLR2_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 1, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "CPUACTLR3_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 2, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "CPUCFR_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 0, .opc2 = 0, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "CPUECTLR_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 1, .opc2 = 4, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0x961563010 }, + { .name = "CPUPCR_EL3", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 1, + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "CPUPMR_EL3", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 3, + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "CPUPOR_EL3", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 2, + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "CPUPSELR_EL3", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 6, .crn = 15, .crm = 8, .opc2 = 0, + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "CPUPWRCTLR_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 7, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "ERXPFGCDN_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 2, + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "ERXPFGCTL_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 1, + .access = PL3_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "ERXPFGF_EL1", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 0, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, +}; + +void define_neoverse_n1_cp_reginfo(ARMCPU *cpu) +{ + define_arm_cp_regs(cpu, neoverse_n1_cp_reginfo); +} #endif /* !CONFIG_USER_ONLY */ /* CPU models. These are not needed for the AArch64 linux-user build. */ 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); #endif bool el_is_in_host(CPUARMState *env, int el);
Add implementation defined registers for neoverse-n1 which would be accessed by TF-A. Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn> --- target/arm/cpu64.c | 2 ++ target/arm/cpu_tcg.c | 62 ++++++++++++++++++++++++++++++++++++++++++ target/arm/internals.h | 2 ++ 3 files changed, 66 insertions(+)