| Message ID | 20221110090444.550362-1-tkutergin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: added cortex-a55 CPU support for qemu-virt | expand |
Patch looks good to me. Maybe you might add some references to chapters from RM where this numbers come from. Similar with aarch64_a76_initfn function for example.
On Thu, 10 Nov 2022 at 09:04, Timofey Kutergin <tkutergin@gmail.com> wrote: > > cortex-a55 is one of newer armv8.2+ CPUs supporting native > Privileged Access Never (PAN) feature. Hi; thanks for this patch. There are a few missing ID register values below, but otherwise it looks good. > Using this CPU > provides access to this feature without using fictious "max" "fictitious" > CPU. > > Signed-off-by: Timofey Kutergin <tkutergin@gmail.com> > --- > hw/arm/virt.c | 1 + > target/arm/cpu64.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index b871350856..fc0c9baba6 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -201,6 +201,7 @@ static const char *valid_cpus[] = { > ARM_CPU_TYPE_NAME("cortex-a15"), > ARM_CPU_TYPE_NAME("cortex-a35"), > ARM_CPU_TYPE_NAME("cortex-a53"), > + ARM_CPU_TYPE_NAME("cortex-a55"), > ARM_CPU_TYPE_NAME("cortex-a57"), > ARM_CPU_TYPE_NAME("cortex-a72"), > ARM_CPU_TYPE_NAME("cortex-a76"), There's a corresponding list of supported CPUs in docs/system/arm/virt.rst that also needs the new CPU type adding. > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 3d74f134f5..e1ade1b2a3 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -792,6 +792,60 @@ static void aarch64_a53_initfn(Object *obj) > define_cortex_a72_a57_a53_cp_reginfo(cpu); > } > > +static void aarch64_a55_initfn(Object *obj) > +{ > + ARMCPU *cpu = ARM_CPU(obj); > + > + cpu->dtb_compatible = "arm,cortex-a55"; > + set_feature(&cpu->env, ARM_FEATURE_V8); > + set_feature(&cpu->env, ARM_FEATURE_NEON); > + set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER); > + set_feature(&cpu->env, ARM_FEATURE_AARCH64); > + set_feature(&cpu->env, ARM_FEATURE_CBAR_RO); > + set_feature(&cpu->env, ARM_FEATURE_EL2); > + set_feature(&cpu->env, ARM_FEATURE_EL3); > + set_feature(&cpu->env, ARM_FEATURE_PMU); It's helpful for review to order the ID register values in the same layout they are in the TRM, in the "AArch64 registers by functional group" table, which is how we've ordered other new CPUs we've added recently (eg see the Neoverse N1 or the Cortex-A76). > + cpu->midr = 0x411fd050; There's an r2p0 TRM out, so we might as well advertise ourselves as r2p0: 0x412FD050 > + cpu->revidr = 0x00000000; > + cpu->reset_fpsid = 0x41034070; > + cpu->isar.mvfr0 = 0x10110222; > + cpu->isar.mvfr1 = 0x13211111; > + cpu->isar.mvfr2 = 0x00000043; > + cpu->ctr = 0x84448004; /* L1Ip = VIPT */ > + cpu->reset_sctlr = 0x00c50838; Should be 0x30c50838 : bits [29:28] are RES1 > + cpu->isar.id_pfr0 = 0x10000131; > + cpu->isar.id_pfr1 = 0x00011011; Forgotten id_pfr2: 0x00000011 > + cpu->isar.id_dfr0 = 0x04010088; > + cpu->id_afr0 = 0x00000000; > + cpu->isar.id_mmfr0 = 0x10201105; > + cpu->isar.id_mmfr1 = 0x40000000; > + cpu->isar.id_mmfr2 = 0x01260000; > + cpu->isar.id_mmfr3 = 0x02122211; You've forgotten id_mmfr4 : 0x00021110 > + cpu->isar.id_isar0 = 0x02101110; > + cpu->isar.id_isar1 = 0x13112111; > + cpu->isar.id_isar2 = 0x21232042; > + cpu->isar.id_isar3 = 0x01112131; > + cpu->isar.id_isar4 = 0x00011142; > + cpu->isar.id_isar5 = 0x01011121; > + cpu->isar.id_isar6 = 0x00000010; > + cpu->isar.id_aa64pfr0 = 0x10112222; You've missed out id_aa64pfr1 : should be 0x0000000000000010ull > + cpu->isar.id_aa64dfr0 = 0x10305408; > + cpu->isar.id_aa64isar0 = 0x10211120; You've missed out the DP field: this should be 0x0000100010211120ull > + cpu->isar.id_aa64isar1 = 0x00100001; > + cpu->isar.id_aa64mmfr0 = 0x00101122; > + cpu->isar.id_aa64mmfr1 = 0x10212122; > + cpu->isar.id_aa64mmfr2 = 0x00001011; > + cpu->isar.dbgdidr = 0x3516d000; > + cpu->clidr = 0x82000023; > + cpu->ccsidr[0] = 0x700fe01a; /* 32KB L1 dcache */ > + cpu->ccsidr[1] = 0x200fe01a; /* 32KB L1 icache */ > + cpu->ccsidr[2] = 0x703fe07a; /* 512KB L2 cache */ The A55 TRM says that it has an L3 cache, so I think we also need to fill in cpu->ccsidr[4] here (NB not [3], which is a reserved index since the L2 is not split I&D). > + cpu->dcz_blocksize = 4; /* 64 bytes */ > + cpu->gic_num_lrs = 4; > + cpu->gic_vpribits = 5; > + cpu->gic_vprebits = 5; You need to set gic_pribits also. Missing reset_pmcr_el0: 0x41453000 The impdef sysregs are different from A53/A57/A72 etc, so it's correct that we don't call define_cortex_a72_a57_a53_cp_reginfo(). We can add definitions of the impdef sysregs later if guest software runs into them I guess... thanks -- PMM
Hi all, added some references to TRM but honestly many fields were dumped from hardware - odroid c4/amlogic s905x3/quad cortex-a55 On Thu, Nov 17, 2022 at 6:03 PM Daniel Baluta <daniel.baluta@nxp.com> wrote: > Patch looks good to me. > > Maybe you might add some references to chapters from RM where this numbers > > come from. Similar with aarch64_a76_initfn function for example. > >
Thank you very much for your comments, I have tried to fix them. Regarding L3 cache - cortex-a55 may have or not have it. CLIDR register value shows that this specific instance (dumped from odroid c4) does not have it. But if you think that having L3 cache may be useful - it may be added too... BR Timofey On Thu, Nov 17, 2022 at 6:54 PM Peter Maydell <peter.maydell@linaro.org> wrote: > On Thu, 10 Nov 2022 at 09:04, Timofey Kutergin <tkutergin@gmail.com> > wrote: > > > > cortex-a55 is one of newer armv8.2+ CPUs supporting native > > Privileged Access Never (PAN) feature. > > Hi; thanks for this patch. There are a few missing ID register > values below, but otherwise it looks good. > > > Using this CPU > > provides access to this feature without using fictious "max" > > "fictitious" > > > CPU. > > > > Signed-off-by: Timofey Kutergin <tkutergin@gmail.com> > > --- > > hw/arm/virt.c | 1 + > > target/arm/cpu64.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 56 insertions(+) > > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index b871350856..fc0c9baba6 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -201,6 +201,7 @@ static const char *valid_cpus[] = { > > ARM_CPU_TYPE_NAME("cortex-a15"), > > ARM_CPU_TYPE_NAME("cortex-a35"), > > ARM_CPU_TYPE_NAME("cortex-a53"), > > + ARM_CPU_TYPE_NAME("cortex-a55"), > > ARM_CPU_TYPE_NAME("cortex-a57"), > > ARM_CPU_TYPE_NAME("cortex-a72"), > > ARM_CPU_TYPE_NAME("cortex-a76"), > > There's a corresponding list of supported CPUs in > docs/system/arm/virt.rst that also needs the new CPU type adding. > > > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > > index 3d74f134f5..e1ade1b2a3 100644 > > --- a/target/arm/cpu64.c > > +++ b/target/arm/cpu64.c > > @@ -792,6 +792,60 @@ static void aarch64_a53_initfn(Object *obj) > > define_cortex_a72_a57_a53_cp_reginfo(cpu); > > } > > > > +static void aarch64_a55_initfn(Object *obj) > > +{ > > + ARMCPU *cpu = ARM_CPU(obj); > > + > > + cpu->dtb_compatible = "arm,cortex-a55"; > > + set_feature(&cpu->env, ARM_FEATURE_V8); > > + set_feature(&cpu->env, ARM_FEATURE_NEON); > > + set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER); > > + set_feature(&cpu->env, ARM_FEATURE_AARCH64); > > + set_feature(&cpu->env, ARM_FEATURE_CBAR_RO); > > + set_feature(&cpu->env, ARM_FEATURE_EL2); > > + set_feature(&cpu->env, ARM_FEATURE_EL3); > > + set_feature(&cpu->env, ARM_FEATURE_PMU); > > It's helpful for review to order the ID register values in > the same layout they are in the TRM, in the "AArch64 registers > by functional group" table, which is how we've ordered other > new CPUs we've added recently (eg see the Neoverse N1 or > the Cortex-A76). > > > + cpu->midr = 0x411fd050; > > There's an r2p0 TRM out, so we might as well advertise > ourselves as r2p0: 0x412FD050 > > > + cpu->revidr = 0x00000000; > > + cpu->reset_fpsid = 0x41034070; > > + cpu->isar.mvfr0 = 0x10110222; > > + cpu->isar.mvfr1 = 0x13211111; > > + cpu->isar.mvfr2 = 0x00000043; > > + cpu->ctr = 0x84448004; /* L1Ip = VIPT */ > > + cpu->reset_sctlr = 0x00c50838; > > Should be 0x30c50838 : bits [29:28] are RES1 > > > + cpu->isar.id_pfr0 = 0x10000131; > > + cpu->isar.id_pfr1 = 0x00011011; > > Forgotten id_pfr2: 0x00000011 > > > + cpu->isar.id_dfr0 = 0x04010088; > > + cpu->id_afr0 = 0x00000000; > > + cpu->isar.id_mmfr0 = 0x10201105; > > + cpu->isar.id_mmfr1 = 0x40000000; > > + cpu->isar.id_mmfr2 = 0x01260000; > > + cpu->isar.id_mmfr3 = 0x02122211; > > You've forgotten id_mmfr4 : 0x00021110 > > > + cpu->isar.id_isar0 = 0x02101110; > > + cpu->isar.id_isar1 = 0x13112111; > > + cpu->isar.id_isar2 = 0x21232042; > > + cpu->isar.id_isar3 = 0x01112131; > > + cpu->isar.id_isar4 = 0x00011142; > > + cpu->isar.id_isar5 = 0x01011121; > > + cpu->isar.id_isar6 = 0x00000010; > > + cpu->isar.id_aa64pfr0 = 0x10112222; > > You've missed out id_aa64pfr1 : should be 0x0000000000000010ull > > > + cpu->isar.id_aa64dfr0 = 0x10305408; > > + cpu->isar.id_aa64isar0 = 0x10211120; > > You've missed out the DP field: this should be > 0x0000100010211120ull > > > + cpu->isar.id_aa64isar1 = 0x00100001; > > + cpu->isar.id_aa64mmfr0 = 0x00101122; > > + cpu->isar.id_aa64mmfr1 = 0x10212122; > > + cpu->isar.id_aa64mmfr2 = 0x00001011; > > + cpu->isar.dbgdidr = 0x3516d000; > > + cpu->clidr = 0x82000023; > > + cpu->ccsidr[0] = 0x700fe01a; /* 32KB L1 dcache */ > > + cpu->ccsidr[1] = 0x200fe01a; /* 32KB L1 icache */ > > + cpu->ccsidr[2] = 0x703fe07a; /* 512KB L2 cache */ > > The A55 TRM says that it has an L3 cache, so I think we > also need to fill in cpu->ccsidr[4] here (NB not [3], which > is a reserved index since the L2 is not split I&D). > > > + cpu->dcz_blocksize = 4; /* 64 bytes */ > > + cpu->gic_num_lrs = 4; > > + cpu->gic_vpribits = 5; > > + cpu->gic_vprebits = 5; > > You need to set gic_pribits also. > > Missing reset_pmcr_el0: 0x41453000 > > The impdef sysregs are different from A53/A57/A72 etc, > so it's correct that we don't call > define_cortex_a72_a57_a53_cp_reginfo(). We can add definitions > of the impdef sysregs later if guest software runs into them > I guess... > > thanks > -- PMM >
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index b871350856..fc0c9baba6 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -201,6 +201,7 @@ static const char *valid_cpus[] = { ARM_CPU_TYPE_NAME("cortex-a15"), ARM_CPU_TYPE_NAME("cortex-a35"), ARM_CPU_TYPE_NAME("cortex-a53"), + ARM_CPU_TYPE_NAME("cortex-a55"), ARM_CPU_TYPE_NAME("cortex-a57"), ARM_CPU_TYPE_NAME("cortex-a72"), ARM_CPU_TYPE_NAME("cortex-a76"), diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 3d74f134f5..e1ade1b2a3 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -792,6 +792,60 @@ static void aarch64_a53_initfn(Object *obj) define_cortex_a72_a57_a53_cp_reginfo(cpu); } +static void aarch64_a55_initfn(Object *obj) +{ + ARMCPU *cpu = ARM_CPU(obj); + + cpu->dtb_compatible = "arm,cortex-a55"; + set_feature(&cpu->env, ARM_FEATURE_V8); + set_feature(&cpu->env, ARM_FEATURE_NEON); + set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER); + set_feature(&cpu->env, ARM_FEATURE_AARCH64); + set_feature(&cpu->env, ARM_FEATURE_CBAR_RO); + set_feature(&cpu->env, ARM_FEATURE_EL2); + set_feature(&cpu->env, ARM_FEATURE_EL3); + set_feature(&cpu->env, ARM_FEATURE_PMU); + cpu->midr = 0x411fd050; + cpu->revidr = 0x00000000; + cpu->reset_fpsid = 0x41034070; + cpu->isar.mvfr0 = 0x10110222; + cpu->isar.mvfr1 = 0x13211111; + cpu->isar.mvfr2 = 0x00000043; + cpu->ctr = 0x84448004; /* L1Ip = VIPT */ + cpu->reset_sctlr = 0x00c50838; + cpu->isar.id_pfr0 = 0x10000131; + cpu->isar.id_pfr1 = 0x00011011; + cpu->isar.id_dfr0 = 0x04010088; + cpu->id_afr0 = 0x00000000; + cpu->isar.id_mmfr0 = 0x10201105; + cpu->isar.id_mmfr1 = 0x40000000; + cpu->isar.id_mmfr2 = 0x01260000; + cpu->isar.id_mmfr3 = 0x02122211; + cpu->isar.id_isar0 = 0x02101110; + cpu->isar.id_isar1 = 0x13112111; + cpu->isar.id_isar2 = 0x21232042; + cpu->isar.id_isar3 = 0x01112131; + cpu->isar.id_isar4 = 0x00011142; + cpu->isar.id_isar5 = 0x01011121; + cpu->isar.id_isar6 = 0x00000010; + cpu->isar.id_aa64pfr0 = 0x10112222; + cpu->isar.id_aa64dfr0 = 0x10305408; + cpu->isar.id_aa64isar0 = 0x10211120; + cpu->isar.id_aa64isar1 = 0x00100001; + cpu->isar.id_aa64mmfr0 = 0x00101122; + cpu->isar.id_aa64mmfr1 = 0x10212122; + cpu->isar.id_aa64mmfr2 = 0x00001011; + cpu->isar.dbgdidr = 0x3516d000; + cpu->clidr = 0x82000023; + cpu->ccsidr[0] = 0x700fe01a; /* 32KB L1 dcache */ + cpu->ccsidr[1] = 0x200fe01a; /* 32KB L1 icache */ + cpu->ccsidr[2] = 0x703fe07a; /* 512KB L2 cache */ + cpu->dcz_blocksize = 4; /* 64 bytes */ + cpu->gic_num_lrs = 4; + cpu->gic_vpribits = 5; + cpu->gic_vprebits = 5; +} + static void aarch64_a72_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); @@ -1243,6 +1297,7 @@ static const ARMCPUInfo aarch64_cpus[] = { { .name = "cortex-a35", .initfn = aarch64_a35_initfn }, { .name = "cortex-a57", .initfn = aarch64_a57_initfn }, { .name = "cortex-a53", .initfn = aarch64_a53_initfn }, + { .name = "cortex-a55", .initfn = aarch64_a55_initfn }, { .name = "cortex-a72", .initfn = aarch64_a72_initfn }, { .name = "cortex-a76", .initfn = aarch64_a76_initfn }, { .name = "a64fx", .initfn = aarch64_a64fx_initfn },
cortex-a55 is one of newer armv8.2+ CPUs supporting native Privileged Access Never (PAN) feature. Using this CPU provides access to this feature without using fictious "max" CPU. Signed-off-by: Timofey Kutergin <tkutergin@gmail.com> --- hw/arm/virt.c | 1 + target/arm/cpu64.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+)