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...)
> > + /*
> > + * 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.
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.
thanks
-- PMM
