[PATCH v2 0/2] Implement Cortex-A5 and add to virt device

[PATCH v2 0/2] Implement Cortex-A5 and add to virt device

[Byron Lathi]

Patch 1 adds the Cortex-A5 to the tcg
Patch 2 adds the Cortex-A5 as a supported cpu for the virt machine, and updates the documentation accordingly.

Byron Lathi (2):
  target/arm: Implement Cortex-A5
  hw/arm: Add Cortex-A5 to virt device

 docs/system/arm/virt.rst |  1 +
 hw/arm/virt.c            |  1 +
 target/arm/cpu_tcg.c     | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+)

-- 
2.30.2

[PATCH v2 1/2] target/arm: Implement Cortex-A5

[Byron Lathi]

Add support for the Cortex-A5. These changes are based off of the A7 and
A9 Init functions, using the appropriate values from the technical
reference manual for the A5.

Signed-off-by: Byron Lathi <bslathi19@gmail.com>
---
 target/arm/cpu_tcg.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 13d0e9b195..2b54fb618b 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -304,6 +304,41 @@ static void cortex_a8_initfn(Object *obj)
     define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
 }
 
+static void cortex_a5_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    cpu->dtb_compatible = "arm,cortex-a5";
+    set_feature(&cpu->env, ARM_FEATURE_V7);
+    set_feature(&cpu->env, ARM_FEATURE_NEON);
+    set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
+    set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
+    set_feature(&cpu->env, ARM_FEATURE_PMU);
+    cpu->midr = 0x410fc0f1;
+    cpu->reset_fpsid = 0x41023051;
+    cpu->isar.mvfr0 = 0x10110221;
+    cpu->isar.mvfr1 = 0x11000011;
+    cpu->ctr = 0x83338003;
+    cpu->reset_sctlr = 0x00c50078;
+    cpu->isar.id_pfr0 = 0x00001231;
+    cpu->isar.id_pfr1 = 0x00000011;
+    cpu->isar.id_dfr0 = 0x02010444;
+    cpu->id_afr0 = 0x00000000;
+    cpu->isar.id_mmfr0 = 0x00100103;
+    cpu->isar.id_mmfr1 = 0x40000000;
+    cpu->isar.id_mmfr2 = 0x01230000;
+    cpu->isar.id_mmfr3 = 0x00102211;
+    cpu->isar.id_isar0 = 0x00101111;
+    cpu->isar.id_isar1 = 0x13112111;
+    cpu->isar.id_isar2 = 0x21232041;
+    cpu->isar.id_isar3 = 0x11112131;
+    cpu->isar.id_isar4 = 0x00011142;
+    cpu->isar.dbgdidr = 0x1203f001;
+    cpu->clidr = 0x09200003;
+    cpu->ccsidr[0] = 0x701fe00a;
+    cpu->ccsidr[1] = 0x203fe00a;
+}
+
 static const ARMCPRegInfo cortexa9_cp_reginfo[] = {
     /*
      * power_control should be set to maximum latency. Again,
@@ -1019,6 +1054,7 @@ static const ARMCPUInfo arm_tcg_cpus[] = {
     { .name = "arm1136",     .initfn = arm1136_initfn },
     { .name = "arm1176",     .initfn = arm1176_initfn },
     { .name = "arm11mpcore", .initfn = arm11mpcore_initfn },
+    { .name = "cortex-a5",  .initfn = cortex_a5_initfn },
     { .name = "cortex-a7",   .initfn = cortex_a7_initfn },
     { .name = "cortex-a8",   .initfn = cortex_a8_initfn },
     { .name = "cortex-a9",   .initfn = cortex_a9_initfn },
-- 
2.30.2

[PATCH v2 2/2] hw/arm: Add Cortex-A5 to virt device

[Byron Lathi]

Add the Cortex-A5 to the list of supported CPUs by the virt platform.

Signed-off-by: Byron Lathi <bslathi19@gmail.com>
---
 docs/system/arm/virt.rst | 1 +
 hw/arm/virt.c            | 1 +
 2 files changed, 2 insertions(+)

diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 850787495b..2384606ae7 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -50,6 +50,7 @@ The virt board supports:
 
 Supported guest CPU types:
 
+- ``cortex-a5`` (32-bit)
 - ``cortex-a7`` (32-bit)
 - ``cortex-a15`` (32-bit; the default)
 - ``cortex-a53`` (64-bit)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 6bce595aba..545bd2f958 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -195,6 +195,7 @@ static const int a15irqmap[] = {
 };
 
 static const char *valid_cpus[] = {
+    ARM_CPU_TYPE_NAME("cortex-a5"),
     ARM_CPU_TYPE_NAME("cortex-a7"),
     ARM_CPU_TYPE_NAME("cortex-a15"),
     ARM_CPU_TYPE_NAME("cortex-a53"),
-- 
2.30.2

Re: [PATCH v2 1/2] target/arm: Implement Cortex-A5

[Byron Lathi]

[-- Attachment #1: Type: text/plain, Size: 2754 bytes --]

..and I've just realized that I left the processor id as C0F instead of C05
again.
I also removed the generic timer as I don't think the A5 has one.

On Thu, Dec 16, 2021 at 12:48 AM Byron Lathi <bslathi19@gmail.com> wrote:

> Add support for the Cortex-A5. These changes are based off of the A7 and
> A9 Init functions, using the appropriate values from the technical
> reference manual for the A5.
>
> Signed-off-by: Byron Lathi <bslathi19@gmail.com>
> ---
>  target/arm/cpu_tcg.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index 13d0e9b195..2b54fb618b 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -304,6 +304,41 @@ static void cortex_a8_initfn(Object *obj)
>      define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
>  }
>
> +static void cortex_a5_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    cpu->dtb_compatible = "arm,cortex-a5";
> +    set_feature(&cpu->env, ARM_FEATURE_V7);
> +    set_feature(&cpu->env, ARM_FEATURE_NEON);
> +    set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
> +    set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
> +    set_feature(&cpu->env, ARM_FEATURE_PMU);
> +    cpu->midr = 0x410fc0f1;
> +    cpu->reset_fpsid = 0x41023051;
> +    cpu->isar.mvfr0 = 0x10110221;
> +    cpu->isar.mvfr1 = 0x11000011;
> +    cpu->ctr = 0x83338003;
> +    cpu->reset_sctlr = 0x00c50078;
> +    cpu->isar.id_pfr0 = 0x00001231;
> +    cpu->isar.id_pfr1 = 0x00000011;
> +    cpu->isar.id_dfr0 = 0x02010444;
> +    cpu->id_afr0 = 0x00000000;
> +    cpu->isar.id_mmfr0 = 0x00100103;
> +    cpu->isar.id_mmfr1 = 0x40000000;
> +    cpu->isar.id_mmfr2 = 0x01230000;
> +    cpu->isar.id_mmfr3 = 0x00102211;
> +    cpu->isar.id_isar0 = 0x00101111;
> +    cpu->isar.id_isar1 = 0x13112111;
> +    cpu->isar.id_isar2 = 0x21232041;
> +    cpu->isar.id_isar3 = 0x11112131;
> +    cpu->isar.id_isar4 = 0x00011142;
> +    cpu->isar.dbgdidr = 0x1203f001;
> +    cpu->clidr = 0x09200003;
> +    cpu->ccsidr[0] = 0x701fe00a;
> +    cpu->ccsidr[1] = 0x203fe00a;
> +}
> +
>  static const ARMCPRegInfo cortexa9_cp_reginfo[] = {
>      /*
>       * power_control should be set to maximum latency. Again,
> @@ -1019,6 +1054,7 @@ static const ARMCPUInfo arm_tcg_cpus[] = {
>      { .name = "arm1136",     .initfn = arm1136_initfn },
>      { .name = "arm1176",     .initfn = arm1176_initfn },
>      { .name = "arm11mpcore", .initfn = arm11mpcore_initfn },
> +    { .name = "cortex-a5",  .initfn = cortex_a5_initfn },
>      { .name = "cortex-a7",   .initfn = cortex_a7_initfn },
>      { .name = "cortex-a8",   .initfn = cortex_a8_initfn },
>      { .name = "cortex-a9",   .initfn = cortex_a9_initfn },
> --
> 2.30.2
>
>

[-- Attachment #2: Type: text/html, Size: 3623 bytes --]

Re: [PATCH v2 2/2] hw/arm: Add Cortex-A5 to virt device

[Philippe Mathieu-Daudé]

On 12/16/21 07:48, Byron Lathi wrote:
> Add the Cortex-A5 to the list of supported CPUs by the virt platform.
> 
> Signed-off-by: Byron Lathi <bslathi19@gmail.com>
> ---
>  docs/system/arm/virt.rst | 1 +
>  hw/arm/virt.c            | 1 +
>  2 files changed, 2 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v2 1/2] target/arm: Implement Cortex-A5

[Peter Maydell]

On Thu, 16 Dec 2021 at 06:48, Byron Lathi <bslathi19@gmail.com> wrote:
>
> Add support for the Cortex-A5. These changes are based off of the A7 and
> A9 Init functions, using the appropriate values from the technical
> reference manual for the A5.
>
> Signed-off-by: Byron Lathi <bslathi19@gmail.com>
> ---
>  target/arm/cpu_tcg.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
> index 13d0e9b195..2b54fb618b 100644
> --- a/target/arm/cpu_tcg.c
> +++ b/target/arm/cpu_tcg.c
> @@ -304,6 +304,41 @@ static void cortex_a8_initfn(Object *obj)
>      define_arm_cp_regs(cpu, cortexa8_cp_reginfo);
>  }
>
> +static void cortex_a5_initfn(Object *obj)
> +{
> +    ARMCPU *cpu = ARM_CPU(obj);
> +
> +    cpu->dtb_compatible = "arm,cortex-a5";
> +    set_feature(&cpu->env, ARM_FEATURE_V7);
> +    set_feature(&cpu->env, ARM_FEATURE_NEON);
> +    set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
> +    set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);

We should avoid using the DUMMY_C15_REGS feature where we
can for new cores. It's a legacy thing where we just RAZ/WI
the whole cp15 crn=15 space. For new CPU definitions we
should define the actual coprocessor registers they have in
this space, typically just as NOP/RAZ registers. You can see
how that's done for the Cortex-A9 with the
cortexa9_cp_reginfo[] array. The A5's registers are slightly
different but they are documented in the TRM:
https://developer.arm.com/documentation/ddi0433/c/system-control/register-summary/c15-summary-table?lang=en

Of those, the CBAR we handle with a feature bit (see below);
the others you can make ARM_CP_NOP for the WO ones and
ARM_CP_CONST with value 0 for the RO and RW ones. Make
sure you get the .access field right: these are all
available only frome secure mode, so should use
.access = PL3_RW / PL3_R / PL3_W for RW/RO/WO.

> +    set_feature(&cpu->env, ARM_FEATURE_PMU);

Missing ARM_FEATURE_EL3.
Missing ARM_FEATURE_CBAR_RO.
Missing ARM_FEATURE_V7MP.

> +    cpu->midr = 0x410fc0f1;

As you note, this is the A9 MIDR. A5 is 0x410fc051.

> +    cpu->reset_fpsid = 0x41023051;
> +    cpu->isar.mvfr0 = 0x10110221;

0x10110222

> +    cpu->isar.mvfr1 = 0x11000011;

0x11111111

These numbers are in the Cortex-A5 NEON Media Processing
Engine TRM:
https://developer.arm.com/documentation/ddi0450/b/Programmers-Model/Register-summary

and indicate the A5's VFPv4 support, among other things.
(QEMU drives its insn enablement off these ID register
values, so if we get them wrong then some guest code that
works on a real A5 will UNDEF on QEMU.)

> +    cpu->ctr = 0x83338003;
> +    cpu->reset_sctlr = 0x00c50078;
> +    cpu->isar.id_pfr0 = 0x00001231;
> +    cpu->isar.id_pfr1 = 0x00000011;
> +    cpu->isar.id_dfr0 = 0x02010444;
> +    cpu->id_afr0 = 0x00000000;
> +    cpu->isar.id_mmfr0 = 0x00100103;

0x00100003

> +    cpu->isar.id_mmfr1 = 0x40000000;
> +    cpu->isar.id_mmfr2 = 0x01230000;
> +    cpu->isar.id_mmfr3 = 0x00102211;
> +    cpu->isar.id_isar0 = 0x00101111;
> +    cpu->isar.id_isar1 = 0x13112111;
> +    cpu->isar.id_isar2 = 0x21232041;
> +    cpu->isar.id_isar3 = 0x11112131;
> +    cpu->isar.id_isar4 = 0x00011142;
> +    cpu->isar.dbgdidr = 0x1203f001;
> +    cpu->clidr = 0x09200003;
> +    cpu->ccsidr[0] = 0x701fe00a;
> +    cpu->ccsidr[1] = 0x203fe00a;

These don't match what the A5 TRM says are valid values for the
CCSIDR fields: specifically the associativity and linesize
fields don't match what the TRM says are the mandated values.
Where did you get your values from?

Following the TRM fields and using 32KB for each would be:

  cpu->ccsidr[0] = 0x701fe019; /* dcache: 32KB 4-way, 8 words/line */
  cpu->ccsidr[1] = 0x203fe009; /* icache: 32KB 2-way, 8 words/line */

> +}
> +
>  static const ARMCPRegInfo cortexa9_cp_reginfo[] = {
>      /*
>       * power_control should be set to maximum latency. Again,
> @@ -1019,6 +1054,7 @@ static const ARMCPUInfo arm_tcg_cpus[] = {
>      { .name = "arm1136",     .initfn = arm1136_initfn },
>      { .name = "arm1176",     .initfn = arm1176_initfn },
>      { .name = "arm11mpcore", .initfn = arm11mpcore_initfn },
> +    { .name = "cortex-a5",  .initfn = cortex_a5_initfn },
>      { .name = "cortex-a7",   .initfn = cortex_a7_initfn },
>      { .name = "cortex-a8",   .initfn = cortex_a8_initfn },
>      { .name = "cortex-a9",   .initfn = cortex_a9_initfn },
> --
> 2.30.2
>

thanks
-- PMM