diff mbox series

target/arm: Allow users to set the number of VFP registers

Message ID 20230102175245.1895037-1-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series target/arm: Allow users to set the number of VFP registers | expand

Commit Message

Cédric Le Goater Jan. 2, 2023, 5:52 p.m. UTC
Cortex A7 CPUs with an FPU implementing VFPv4 without NEON support
have 16 64-bit FPU registers and not 32 registers. Let users set the
number of VFP registers with a CPU property.

The primary use case of this property is for the Cortex A7 of the
Aspeed AST2600 SoC.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/arm/cpu.h        |  2 ++
 hw/arm/aspeed_ast2600.c |  2 ++
 target/arm/cpu.c        | 14 ++++++++++++++
 3 files changed, 18 insertions(+)

Comments

Peter Maydell Jan. 6, 2023, 3:57 p.m. UTC | #1
On Mon, 2 Jan 2023 at 17:52, Cédric Le Goater <clg@kaod.org> wrote:
>
> Cortex A7 CPUs with an FPU implementing VFPv4 without NEON support
> have 16 64-bit FPU registers and not 32 registers. Let users set the
> number of VFP registers with a CPU property.
>
> The primary use case of this property is for the Cortex A7 of the
> Aspeed AST2600 SoC.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 2fa022f62b..27af57ea9a 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1258,6 +1258,9 @@ static Property arm_cpu_cfgend_property =
>  static Property arm_cpu_has_vfp_property =
>              DEFINE_PROP_BOOL("vfp", ARMCPU, has_vfp, true);
>
> +static Property arm_cpu_has_vfp_d32_property =
> +            DEFINE_PROP_BOOL("vfp-d32", ARMCPU, has_vfp_d32, true);
> +
>  static Property arm_cpu_has_neon_property =
>              DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true);
>
> @@ -1384,8 +1387,11 @@ void arm_cpu_post_init(Object *obj)
>          ? cpu_isar_feature(aa64_fp_simd, cpu)
>          : cpu_isar_feature(aa32_vfp, cpu)) {
>          cpu->has_vfp = true;
> +        cpu->has_vfp_d32 = true;

We shouldn't default to true if the CPU default is D16 already.
You should be able to use cpu_isar_feature(aa32_simd_r32, cpu)
to check this.

The setup of the property should probably be in its own if(),
rather than tucked into the has_vfp if(), so

   if (cpu->has_vfp && cpu_isar_feature(aa32_simd_r32, cpu)) {
       cpu->has_vfp_d32 = true;
       add property;
   }

This would mean that if the CPU is D16-only then the user can't
make it D32, which I think is OK. You could write it to
allow that if you wanted I guess, but then you'd need to
special-case M-profile, which doesn't permit D32 at all.

>          if (!kvm_enabled()) {
>              qdev_property_add_static(DEVICE(obj), &arm_cpu_has_vfp_property);
> +            qdev_property_add_static(DEVICE(obj),
> +                                     &arm_cpu_has_vfp_d32_property);
>          }
>      }
>
> @@ -1650,6 +1656,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>          return;
>      }
>
> +    if (!cpu->has_vfp_d32) {
> +        uint32_t u;
> +
> +        u = cpu->isar.mvfr0;
> +        u = FIELD_DP32(u, MVFR0, SIMDREG, 1); /* 16 registers */
> +        cpu->isar.mvfr0 = u;
> +    }
> +
>      if (!cpu->has_vfp) {
>          uint64_t t;
>          uint32_t u;

There should be a check so the user can't both disable D32 and enable Neon
(Neon always has 32 dregs).

Armv8A doesn't permit D16, so we shouldn't allow that combination either
(but note that v8R, support for which just landed, *does* permit it).

thanks
-- PMM
Cédric Le Goater Jan. 19, 2023, 8:44 a.m. UTC | #2
>> @@ -1650,6 +1656,14 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>>           return;
>>       }
>>
>> +    if (!cpu->has_vfp_d32) {
>> +        uint32_t u;
>> +
>> +        u = cpu->isar.mvfr0;
>> +        u = FIELD_DP32(u, MVFR0, SIMDREG, 1); /* 16 registers */
>> +        cpu->isar.mvfr0 = u;
>> +    }
>> +
>>       if (!cpu->has_vfp) {
>>           uint64_t t;
>>           uint32_t u;
> 
> There should be a check so the user can't both disable D32 and enable Neon
> (Neon always has 32 dregs).
> 
> Armv8A doesn't permit D16, so we shouldn't allow that combination either
> (but note that v8R, support for which just landed, *does* permit it).

Like below ?

Thanks,

C.


@@ -1661,6 +1672,26 @@ static void arm_cpu_realizefn(DeviceStat
          return;
      }
  
+    if (!cpu->has_vfp_d32 &&
+        arm_feature(env, ARM_FEATURE_V8) &&
+        !arm_feature(env, ARM_FEATURE_M)) {
+        error_setg(errp, "ARMv8A CPUs must have VFP32");
+        return;
+    }
+
+    if (cpu->has_vfp_d32 != cpu->has_neon) {
+        error_setg(errp, "ARM CPUs must have both VFP32 and Neon or neither");
+        return;
+    }
+
+   if (!cpu->has_vfp_d32) {
+        uint32_t u;
+
+        u = cpu->isar.mvfr0;
+        u = FIELD_DP32(u, MVFR0, SIMDREG, 1); /* 16 registers */
+        cpu->isar.mvfr0 = u;
+    }
+
      if (!cpu->has_vfp) {
          uint64_t t;
          uint32_t u;
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 2b4bd20f9d..55e2e9a584 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -897,6 +897,8 @@  struct ArchCPU {
     bool has_pmu;
     /* CPU has VFP */
     bool has_vfp;
+    /* CPU has 32 VFP registers */
+    bool has_vfp_d32;
     /* CPU has Neon */
     bool has_neon;
     /* CPU has M-profile DSP extension */
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index cd75465c2b..37f43b4165 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -309,6 +309,8 @@  static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
                                 &error_abort);
         object_property_set_bool(OBJECT(&s->cpu[i]), "neon", false,
                                 &error_abort);
+        object_property_set_bool(OBJECT(&s->cpu[i]), "vfp-d32", false,
+                                &error_abort);
         object_property_set_link(OBJECT(&s->cpu[i]), "memory",
                                  OBJECT(s->memory), &error_abort);
 
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 2fa022f62b..27af57ea9a 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1258,6 +1258,9 @@  static Property arm_cpu_cfgend_property =
 static Property arm_cpu_has_vfp_property =
             DEFINE_PROP_BOOL("vfp", ARMCPU, has_vfp, true);
 
+static Property arm_cpu_has_vfp_d32_property =
+            DEFINE_PROP_BOOL("vfp-d32", ARMCPU, has_vfp_d32, true);
+
 static Property arm_cpu_has_neon_property =
             DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true);
 
@@ -1384,8 +1387,11 @@  void arm_cpu_post_init(Object *obj)
         ? cpu_isar_feature(aa64_fp_simd, cpu)
         : cpu_isar_feature(aa32_vfp, cpu)) {
         cpu->has_vfp = true;
+        cpu->has_vfp_d32 = true;
         if (!kvm_enabled()) {
             qdev_property_add_static(DEVICE(obj), &arm_cpu_has_vfp_property);
+            qdev_property_add_static(DEVICE(obj),
+                                     &arm_cpu_has_vfp_d32_property);
         }
     }
 
@@ -1650,6 +1656,14 @@  static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (!cpu->has_vfp_d32) {
+        uint32_t u;
+
+        u = cpu->isar.mvfr0;
+        u = FIELD_DP32(u, MVFR0, SIMDREG, 1); /* 16 registers */
+        cpu->isar.mvfr0 = u;
+    }
+
     if (!cpu->has_vfp) {
         uint64_t t;
         uint32_t u;