On 6/19/23 15:41, Peter Maydell wrote:
On Mon, 19 Jun 2023 at 13:47, Mads Ynddal <m...@ynddal.dk> wrote:
Sorry, if this has already been acknowledged, but I couldn't find it on the
mailinglist.
This commit seems to break compatibility with macOS accelerator hvf when
virtualizing ARM CPUs.
It breaks the VM on boot-up with the message "ARM CPUs must have both VFP-D32
and Neon or neither". I haven't looked into what VFP-D32 and Neon are, but the
same VM worked on earlier versions of QEMU.
It can be reproduced with the following:
qemu-system-aarch64 \
-nodefaults \
-display "none" \
-machine "virt" \
-accel "hvf" \
-cpu "host" \
-serial "mon:stdio"
qemu-system-aarch64: ARM CPUs must have both VFP-D32 and Neon or neither
If you fix/work on this issue in a separate thread/patch, you can add
reported-by, so I'll automatically follow and help test it:
Reported-by: Mads Ynddal <m...@ynddal.dk>
@@ -1406,6 +1409,22 @@ 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()) {
Probably this should be "if (!kvm_enabled() && !hvf_enabled())".
Is that sufficient to fix the regression ? (I have a feeling it
isn't, but we might as well test...)
Yes, insufficient. But I'm also changing these to tcg || qtest.
+ /*
+ * The permitted values of the SIMDReg bits [3:0] on
+ * Armv8-A are either 0b0000 and 0b0010. On such CPUs,
+ * make sure that has_vfp_d32 can not be set to false.
+ */
+ if (!(arm_feature(&cpu->env, ARM_FEATURE_V8) &&
+ !arm_feature(&cpu->env, ARM_FEATURE_M))) {
+ 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()) {
@@ -1672,6 +1691,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
return;
}
+ if (cpu->has_vfp_d32 != cpu->has_neon) {
+ error_setg(errp, "ARM CPUs must have both VFP-D32 and Neon or
neither");
+ return;
+ }
The other thing I see looking again at this code is that it
doesn't account for CPUs which don't have AArch32 support
at all. The MVFR0 register which the aa32_simd_r32 feature
test is looking at is an AArch32 register, and the test
will not return a sensible answer on an AArch64-only CPU.
This is the problem. The code needs restructuring (which I am about to test).
On the other side of this, target/arm/hvf/hvf.c always
sets ARM_FEATURE_NEON, which I think is probably not
correct given that Neon is also an AArch32-only thing.
At one time NEON also meant AdvSIMD, though we have now changed aa64 to the isar test. We
could probably get rid of NEON now too, with just a little more cleanup.
r~
