| Message ID | 20210525010358.152808-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement SVE2 | expand |
Hi Richard, On 2021/5/25 9:02, Richard Henderson wrote: > Will be used for SVE2 isa subset enablement. > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > v2: Do not read zfr0 from kvm unless sve is available. > v7: Move zfr0 read inside existing sve_enabled block. [...] > 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? Thanks, Zenghui
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~
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 616b393253..a6e1fa6333 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -947,6 +947,7 @@ struct ARMCPU { uint64_t id_aa64mmfr2; uint64_t id_aa64dfr0; uint64_t id_aa64dfr1; + uint64_t id_aa64zfr0; } isar; uint64_t midr; uint32_t revidr; @@ -2034,6 +2035,16 @@ FIELD(ID_AA64DFR0, DOUBLELOCK, 36, 4) FIELD(ID_AA64DFR0, TRACEFILT, 40, 4) FIELD(ID_AA64DFR0, MTPMU, 48, 4) +FIELD(ID_AA64ZFR0, SVEVER, 0, 4) +FIELD(ID_AA64ZFR0, AES, 4, 4) +FIELD(ID_AA64ZFR0, BITPERM, 16, 4) +FIELD(ID_AA64ZFR0, BFLOAT16, 20, 4) +FIELD(ID_AA64ZFR0, SHA3, 32, 4) +FIELD(ID_AA64ZFR0, SM4, 40, 4) +FIELD(ID_AA64ZFR0, I8MM, 44, 4) +FIELD(ID_AA64ZFR0, F32MM, 52, 4) +FIELD(ID_AA64ZFR0, F64MM, 56, 4) + FIELD(ID_DFR0, COPDBG, 0, 4) FIELD(ID_DFR0, COPSDBG, 4, 4) FIELD(ID_DFR0, MMAPDBG, 8, 4) @@ -4215,6 +4226,11 @@ static inline bool isar_feature_aa64_ssbs(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, SSBS) != 0; } +static inline bool isar_feature_aa64_sve2(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, SVEVER) != 0; +} + /* * Feature tests for "does this exist in either 32-bit or 64-bit?" */ diff --git a/target/arm/helper.c b/target/arm/helper.c index 3b365a78cb..696aea2250 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -7561,8 +7561,7 @@ void register_cp_regs_for_features(ARMCPU *cpu) .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 4, .access = PL1_R, .type = ARM_CP_CONST, .accessfn = access_aa64_tid3, - /* At present, only SVEver == 0 is defined anyway. */ - .resetvalue = 0 }, + .resetvalue = cpu->isar.id_aa64zfr0 }, { .name = "ID_AA64PFR5_EL1_RESERVED", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 4, .opc2 = 5, .access = PL1_R, .type = ARM_CP_CONST, 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)); + } + + kvm_arm_destroy_scratch_host_vcpu(fdarray); + + if (err < 0) { + return false; } /*