Re: [PATCH v7 01/92] target/arm: Add ID_AA64ZFR0 fields and isar_feature_aa64_sve2

[Richard Henderson]

On 7/25/22 00:05, Zenghui Yu wrote:
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index dff85f6db9..37ceadd9a9 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -647,17 +647,26 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures 
> > *ahcf)
> >
> >      sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 
> > 0;
> >
> > -    kvm_arm_destroy_scratch_host_vcpu(fdarray);
> > -
> > -    if (err < 0) {
> > -        return false;
> > -    }
> > -
> >      /* Add feature bits that can't appear until after VCPU init. */
> >      if (sve_supported) {
> >          t = ahcf->isar.id_aa64pfr0;
> >          t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
> >          ahcf->isar.id_aa64pfr0 = t;
> > +
> > +        /*
> > +         * Before v5.1, KVM did not support SVE and did not expose
> > +         * ID_AA64ZFR0_EL1 even as RAZ.  After v5.1, KVM still does
> > +         * not expose the register to "user" requests like this
> > +         * unless the host supports SVE.
> > +         */
> > +        err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64zfr0,
> > +                              ARM64_SYS_REG(3, 0, 0, 4, 4));
>
> If I read it correctly, we haven't yet enabled SVE for the scratch vcpu
> (using the KVM_ARM_VCPU_INIT ioctl with KVM_ARM_VCPU_SVE). KVM will
> therefore expose ID_AA64ZFR0_EL1 to userspace as RAZ at this point and
> isar.id_aa64zfr0 is reset to 0. I wonder if it was intentional?

You are correct, this is a bug.  It appears this is hidden because nothing else actually 
depends on the value within the context of --accel=kvm, e.g. migration.


r~