| Message ID | 20230119123449.531826-16-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | aspeed: Various extensions, fixes and cleanups | expand |
On 1/19/23 13:34, Cédric Le Goater 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> > --- > target/arm/cpu.h | 2 ++ > hw/arm/aspeed_ast2600.c | 2 ++ > target/arm/cpu.c | 31 +++++++++++++++++++++++++++++++ > 3 files changed, 35 insertions(+) > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index bf2bce046d..5f2fefef4e 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -901,6 +901,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 5f63316dbf..ad90de75fb 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -1269,6 +1269,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); > > @@ -1400,6 +1403,14 @@ void arm_cpu_post_init(Object *obj) > } > } > > + if (cpu->has_vfp && cpu_isar_feature(aa32_simd_r32, cpu)) { > + cpu->has_vfp_d32 = true; > + if (!kvm_enabled()) { > + qdev_property_add_static(DEVICE(obj), > + &arm_cpu_has_vfp_d32_property); > + } > + } > + > if (arm_feature(&cpu->env, ARM_FEATURE_NEON)) { > cpu->has_neon = true; > if (!kvm_enabled()) { > @@ -1661,6 +1672,26 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) > 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; > + } The specs say that the permitted values of the SIMDReg bits [3:0] in Armv8-A are 0b0000 and 0b0010. In Armv8-M, the possible values of the field are b0001. All other values are reserved. This should be better : if (arm_feature(env, ARM_FEATURE_V8)) { if (arm_feature(env, ARM_FEATURE_M)) { if (cpu->has_vfp_d32) { error_setg(errp, "ARMv8-M CPUs do not support VFP-D32"); return; } } else if (!cpu->has_vfp_d32) { error_setg(errp, "ARMv8-A CPUs must have VFP-D32"); return; } } ? Thanks, C. > + 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;
On Fri, 20 Jan 2023 at 08:40, Cédric Le Goater <clg@kaod.org> wrote: > > On 1/19/23 13:34, Cédric Le Goater 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> > > --- > > target/arm/cpu.h | 2 ++ > > hw/arm/aspeed_ast2600.c | 2 ++ > > target/arm/cpu.c | 31 +++++++++++++++++++++++++++++++ > > 3 files changed, 35 insertions(+) > > > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > > index bf2bce046d..5f2fefef4e 100644 > > --- a/target/arm/cpu.h > > +++ b/target/arm/cpu.h > > @@ -901,6 +901,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 5f63316dbf..ad90de75fb 100644 > > --- a/target/arm/cpu.c > > +++ b/target/arm/cpu.c > > @@ -1269,6 +1269,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); > > > > @@ -1400,6 +1403,14 @@ void arm_cpu_post_init(Object *obj) > > } > > } > > > > + if (cpu->has_vfp && cpu_isar_feature(aa32_simd_r32, cpu)) { > > + cpu->has_vfp_d32 = true; > > + if (!kvm_enabled()) { > > + qdev_property_add_static(DEVICE(obj), > > + &arm_cpu_has_vfp_d32_property); > > + } > > + } > > + > > if (arm_feature(&cpu->env, ARM_FEATURE_NEON)) { > > cpu->has_neon = true; > > if (!kvm_enabled()) { > > @@ -1661,6 +1672,26 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) > > 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"); I wonder if we should instead not create the property for v8A CPUs? That way there's no way for has_vfp_d32 to get set to false. But checking after the fact is fine too. (One day we might move away from the current setup where properties get set up per-object for Arm CPUs and towards a "the class of the object determines the properties it has" setup; the way we do it today is a bit odd for historical reasons.) > > + return; > > + } > > The specs say that the permitted values of the SIMDReg bits [3:0] > in Armv8-A are 0b0000 and 0b0010. In Armv8-M, the possible values > of the field are b0001. All other values are reserved. > > This should be better : > > if (arm_feature(env, ARM_FEATURE_V8)) { > if (arm_feature(env, ARM_FEATURE_M)) { > if (cpu->has_vfp_d32) { This is unreachable code, because we only set has_vfp_d32 to true and set up the property if the CPU has the aa32_simd_r32 feature; and v8M CPUs will never have that feature, so has_vfp_d32 must be false for them here. (Put another way, we set up the property so we can only "downgrade" from 32 to 16 dregs, so there is no need to check whether a CPU that can't have 32 dregs still doesn't have 32 dregs.) > error_setg(errp, "ARMv8-M CPUs do not support VFP-D32"); > return; > } > } else if (!cpu->has_vfp_d32) { > error_setg(errp, "ARMv8-A CPUs must have VFP-D32"); > return; > } > } > > ? thanks -- PMM
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index bf2bce046d..5f2fefef4e 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -901,6 +901,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 5f63316dbf..ad90de75fb 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1269,6 +1269,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); @@ -1400,6 +1403,14 @@ void arm_cpu_post_init(Object *obj) } } + if (cpu->has_vfp && cpu_isar_feature(aa32_simd_r32, cpu)) { + cpu->has_vfp_d32 = true; + if (!kvm_enabled()) { + qdev_property_add_static(DEVICE(obj), + &arm_cpu_has_vfp_d32_property); + } + } + if (arm_feature(&cpu->env, ARM_FEATURE_NEON)) { cpu->has_neon = true; if (!kvm_enabled()) { @@ -1661,6 +1672,26 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) 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;
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 | 31 +++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+)