qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Define Impl. Def. registers for Neoverse-N1
@ 2023-03-03 16:15 Chen Baozi
  2023-03-03 16:15 ` [PATCH 1/2] target/arm: Add Neoverse-N1 registers Chen Baozi
  2023-03-03 16:15 ` [PATCH 2/2] target/arm: Add DynamIQ Shared Unit control registers Chen Baozi
  0 siblings, 2 replies; 7+ messages in thread
From: Chen Baozi @ 2023-03-03 16:15 UTC (permalink / raw)
  To: qemu-devel

When booting sbsa-ref board with Neoverse-N1, TF-A's would try to access
implementation defined registers in it as well as in DSU for errata.
Add the definitions for these system registers to make sbsa-ref boot
with Neoverse-N1.

I have noticed that there is a patch series under review which move TCG
CPUs into tcg/. Therefore this patch should be rework once that patch
has been merged.

Chen Baozi (2):
  target/arm: Add Neoverse-N1 registers
  target/arm: Add DynamIQ Shared Unit control registers

 target/arm/cpu64.c     |   2 +
 target/arm/cpu_tcg.c   | 114 +++++++++++++++++++++++++++++++++++++++++
 target/arm/internals.h |   2 +
 3 files changed, 118 insertions(+)

-- 
2.37.3



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] target/arm: Add Neoverse-N1 registers
  2023-03-03 16:15 [PATCH 0/2] Define Impl. Def. registers for Neoverse-N1 Chen Baozi
@ 2023-03-03 16:15 ` Chen Baozi
  2023-03-06  8:52   ` Marcin Juszkiewicz
  2023-03-06 11:33   ` Peter Maydell
  2023-03-03 16:15 ` [PATCH 2/2] target/arm: Add DynamIQ Shared Unit control registers Chen Baozi
  1 sibling, 2 replies; 7+ messages in thread
From: Chen Baozi @ 2023-03-03 16:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, open list:ARM TCG CPUs

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 },
+    { .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);
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] target/arm: Add DynamIQ Shared Unit control registers
  2023-03-03 16:15 [PATCH 0/2] Define Impl. Def. registers for Neoverse-N1 Chen Baozi
  2023-03-03 16:15 ` [PATCH 1/2] target/arm: Add Neoverse-N1 registers Chen Baozi
@ 2023-03-03 16:15 ` Chen Baozi
  2023-03-06 12:32   ` Peter Maydell
  1 sibling, 1 reply; 7+ messages in thread
From: Chen Baozi @ 2023-03-03 16:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, open list:ARM TCG CPUs

DynamIQ Shared Unit (DSU) contains system control registers in the SCU
and L3 logic which are implemented as the system registers for the cores
in the cluster. Add DSU control registers and enable it to the supported
cores.

Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
---
 target/arm/cpu_tcg.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 6a1bb56b25..db6163ede2 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -208,9 +208,61 @@ static const ARMCPRegInfo neoverse_n1_cp_reginfo[] = {
       .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
 };
 
+static const ARMCPRegInfo dsu_cp_reginfo[] = {
+    { .name = "CLUSTERCFR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 3, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERIDR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 3, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERREVIDR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 3, .opc2 = 2,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERACTLR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 3, .opc2 = 3,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERECTLR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 3, .opc2 = 4,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERPWRCTLR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 3, .opc2 = 5,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERPWRDN_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 3, .opc2 = 6,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERPWRSTAT_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 3, .opc2 = 7,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERTHREADSID_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 4, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERACPSID_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 4, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERSTASHSID_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 4, .opc2 = 2,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERPARTCR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 4, .opc2 = 3,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERBUSQOS_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 4, .opc2 = 4,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERL3HIT_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 4, .opc2 = 5,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERL3MISS_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 4, .opc2 = 6,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CLUSTERTHREADSIDOVR_EL1", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 0, .crn = 15, .crm = 4, .opc2 = 7,
+      .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);
+    define_arm_cp_regs(cpu, dsu_cp_reginfo);
 }
 #endif /* !CONFIG_USER_ONLY */
 
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] target/arm: Add Neoverse-N1 registers
  2023-03-03 16:15 ` [PATCH 1/2] target/arm: Add Neoverse-N1 registers Chen Baozi
@ 2023-03-06  8:52   ` Marcin Juszkiewicz
  2023-03-06 11:33   ` Peter Maydell
  1 sibling, 0 replies; 7+ messages in thread
From: Marcin Juszkiewicz @ 2023-03-06  8:52 UTC (permalink / raw)
  To: Chen Baozi, qemu-devel; +Cc: Peter Maydell, open list:ARM TCG CPUs

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] target/arm: Add Neoverse-N1 registers
  2023-03-03 16:15 ` [PATCH 1/2] target/arm: Add Neoverse-N1 registers Chen Baozi
  2023-03-06  8:52   ` Marcin Juszkiewicz
@ 2023-03-06 11:33   ` Peter Maydell
  2023-03-06 14:29     ` Chen Baozi
  1 sibling, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2023-03-06 11:33 UTC (permalink / raw)
  To: Chen Baozi; +Cc: qemu-devel, open list:ARM TCG CPUs

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] target/arm: Add DynamIQ Shared Unit control registers
  2023-03-03 16:15 ` [PATCH 2/2] target/arm: Add DynamIQ Shared Unit control registers Chen Baozi
@ 2023-03-06 12:32   ` Peter Maydell
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2023-03-06 12:32 UTC (permalink / raw)
  To: Chen Baozi; +Cc: qemu-devel, open list:ARM TCG CPUs

On Fri, 3 Mar 2023 at 16:16, Chen Baozi <chenbaozi@phytium.com.cn> wrote:
>
> DynamIQ Shared Unit (DSU) contains system control registers in the SCU
> and L3 logic which are implemented as the system registers for the cores
> in the cluster. Add DSU control registers and enable it to the supported
> cores.
>
> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>

This looks OK if we need it, but as I mentioned in my comments
on patch 1, I'd like to see if we can avoid having to have it...

thanks
-- PMM


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] target/arm: Add Neoverse-N1 registers
  2023-03-06 11:33   ` Peter Maydell
@ 2023-03-06 14:29     ` Chen Baozi
  0 siblings, 0 replies; 7+ messages in thread
From: Chen Baozi @ 2023-03-06 14:29 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, open list:ARM TCG CPUs

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




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-03-06 14:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-03 16:15 [PATCH 0/2] Define Impl. Def. registers for Neoverse-N1 Chen Baozi
2023-03-03 16:15 ` [PATCH 1/2] target/arm: Add Neoverse-N1 registers Chen Baozi
2023-03-06  8:52   ` Marcin Juszkiewicz
2023-03-06 11:33   ` Peter Maydell
2023-03-06 14:29     ` Chen Baozi
2023-03-03 16:15 ` [PATCH 2/2] target/arm: Add DynamIQ Shared Unit control registers Chen Baozi
2023-03-06 12:32   ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).