Message ID | 20181108175246.13416-3-richard.henderson@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | target/arm: KVM vs ARMISARegisters | expand |
On 8 November 2018 at 17:52, Richard Henderson <richard.henderson@linaro.org> wrote: > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > /* Old kernels may not know about the PREFERRED_TARGET ioctl: however > * we know these will only support creating one kind of guest CPU, > * which is its preferred CPU type. Fortunately these old kernels > @@ -474,8 +497,71 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > ahcf->target = init.target; > ahcf->dtb_compatible = "arm,arm-v8"; > > + err = read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64pfr0, > + ARM64_SYS_REG(3, 0, 0, 4, 0)); > + if (unlikely(err < 0)) { > + /* > + * Before v4.15, the kernel only exposed a limited number of system > + * registers, not including any of the interesting AArch64 ID regs. > + * For the most part we could leave these fields as zero with minimal > + * effect, since this does not affect the values seen by the guest. These older kernels do implement reading of id_isar0 through id_isar5, though -- we could read and use those values rather than leaving them zero. > + * > + * However, it could cause problems down the line for QEMU, > + * so provide a minimal v8.0 default. > + * > + * ??? Could read MIDR and use knowledge from cpu64.c. > + * ??? Could map a page of memory into our temp guest and > + * run the tiniest of hand-crafted kernels to extract > + * the values seen by the guest. > + * ??? Either of these sounds like too much effort just > + * to work around running a modern host kernel. > + */ > + ahcf->isar.id_aa64pfr0 = 0x00000011; /* EL1&0, AArch64 only */ > + err = 0; Doesn't this code path leave everything except id_aa64pfr0 as zero, thus leaving us with the "could cause problems down the line" situation ? thanks -- PMM
On 11/12/18 12:37 PM, Peter Maydell wrote: > On 8 November 2018 at 17:52, Richard Henderson > <richard.henderson@linaro.org> wrote: >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- > >> /* Old kernels may not know about the PREFERRED_TARGET ioctl: however >> * we know these will only support creating one kind of guest CPU, >> * which is its preferred CPU type. Fortunately these old kernels >> @@ -474,8 +497,71 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >> ahcf->target = init.target; >> ahcf->dtb_compatible = "arm,arm-v8"; >> >> + err = read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64pfr0, >> + ARM64_SYS_REG(3, 0, 0, 4, 0)); >> + if (unlikely(err < 0)) { >> + /* >> + * Before v4.15, the kernel only exposed a limited number of system >> + * registers, not including any of the interesting AArch64 ID regs. >> + * For the most part we could leave these fields as zero with minimal >> + * effect, since this does not affect the values seen by the guest. > > These older kernels do implement reading of id_isar0 through > id_isar5, though -- we could read and use those values rather than > leaving them zero. But without id_aa64pfr0, we don't know if id_isar0 et al have UNKNOWN values due to aa32 mode not being present. E.g. run a 4.8 kernel on a thunderx1 cpu. >> + ahcf->isar.id_aa64pfr0 = 0x00000011; /* EL1&0, AArch64 only */ >> + err = 0; > > Doesn't this code path leave everything except id_aa64pfr0 as > zero, thus leaving us with the "could cause problems down the > line" situation ? The other id_aa64* register fields are all extensions to v8.0, so they should be zero. I am of course assuming that AdvSIMD is present, otherwise qemu itself probably have failed before now. r~
On 12 November 2018 at 14:09, Richard Henderson <richard.henderson@linaro.org> wrote: > On 11/12/18 12:37 PM, Peter Maydell wrote: >> On 8 November 2018 at 17:52, Richard Henderson >> <richard.henderson@linaro.org> wrote: >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >> >>> /* Old kernels may not know about the PREFERRED_TARGET ioctl: however >>> * we know these will only support creating one kind of guest CPU, >>> * which is its preferred CPU type. Fortunately these old kernels >>> @@ -474,8 +497,71 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >>> ahcf->target = init.target; >>> ahcf->dtb_compatible = "arm,arm-v8"; >>> >>> + err = read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64pfr0, >>> + ARM64_SYS_REG(3, 0, 0, 4, 0)); >>> + if (unlikely(err < 0)) { >>> + /* >>> + * Before v4.15, the kernel only exposed a limited number of system >>> + * registers, not including any of the interesting AArch64 ID regs. >>> + * For the most part we could leave these fields as zero with minimal >>> + * effect, since this does not affect the values seen by the guest. >> >> These older kernels do implement reading of id_isar0 through >> id_isar5, though -- we could read and use those values rather than >> leaving them zero. > > But without id_aa64pfr0, we don't know if id_isar0 et al have UNKNOWN values > due to aa32 mode not being present. E.g. run a 4.8 kernel on a thunderx1 cpu. Good point, which makes things kind of awkward. >>> + ahcf->isar.id_aa64pfr0 = 0x00000011; /* EL1&0, AArch64 only */ >>> + err = 0; >> >> Doesn't this code path leave everything except id_aa64pfr0 as >> zero, thus leaving us with the "could cause problems down the >> line" situation ? > > The other id_aa64* register fields are all extensions to v8.0, so they should > be zero. I am of course assuming that AdvSIMD is present, otherwise qemu > itself probably have failed before now. You don't set the id_isar* to say "AdvSIMD present" (or "VFP present"), though. So if we in a later commit change target/arm/machine.c:vfp_needed() to check an id_isar* field instead of ARM_FEATURE_VFP then we'll break migration for these old kernel versions. That was the kind of thing I was hoping we could avoid having to remember for the future... thanks -- PMM
On 11/12/18 3:34 PM, Peter Maydell wrote: >> The other id_aa64* register fields are all extensions to v8.0, so they should >> be zero. I am of course assuming that AdvSIMD is present, otherwise qemu >> itself probably have failed before now. > > You don't set the id_isar* to say "AdvSIMD present" (or "VFP present"), though. Those two are almost unique in that they are signed fields. So 0 indicates present and -1 indicates not present (and 1 indicates fp16 support). r~
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 5de8ff0ac5..e1128b94b2 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -443,17 +443,40 @@ static inline void unset_feature(uint64_t *features, int feature) *features &= ~(1ULL << feature); } +static int read_sys_reg32(int fd, uint32_t *pret, uint64_t id) +{ + uint64_t ret; + struct kvm_one_reg idreg = { .id = id, .addr = (uintptr_t)&ret }; + int err; + + assert((id & KVM_REG_SIZE_MASK) == KVM_REG_SIZE_U64); + err = ioctl(fd, KVM_GET_ONE_REG, &idreg); + if (err < 0) { + return -1; + } + *pret = ret; + return 0; +} + +static int read_sys_reg64(int fd, uint64_t *pret, uint64_t id) +{ + struct kvm_one_reg idreg = { .id = id, .addr = (uintptr_t)pret }; + + assert((id & KVM_REG_SIZE_MASK) == KVM_REG_SIZE_U64); + return ioctl(fd, KVM_GET_ONE_REG, &idreg); +} + bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) { /* Identify the feature bits corresponding to the host CPU, and * fill out the ARMHostCPUClass fields accordingly. To do this * we have to create a scratch VM, create a single CPU inside it, * and then query that CPU for the relevant ID registers. - * For AArch64 we currently don't care about ID registers at - * all; we just want to know the CPU type. */ int fdarray[3]; uint64_t features = 0; + int err; + /* Old kernels may not know about the PREFERRED_TARGET ioctl: however * we know these will only support creating one kind of guest CPU, * which is its preferred CPU type. Fortunately these old kernels @@ -474,8 +497,71 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) ahcf->target = init.target; ahcf->dtb_compatible = "arm,arm-v8"; + err = read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64pfr0, + ARM64_SYS_REG(3, 0, 0, 4, 0)); + if (unlikely(err < 0)) { + /* + * Before v4.15, the kernel only exposed a limited number of system + * registers, not including any of the interesting AArch64 ID regs. + * For the most part we could leave these fields as zero with minimal + * effect, since this does not affect the values seen by the guest. + * + * However, it could cause problems down the line for QEMU, + * so provide a minimal v8.0 default. + * + * ??? Could read MIDR and use knowledge from cpu64.c. + * ??? Could map a page of memory into our temp guest and + * run the tiniest of hand-crafted kernels to extract + * the values seen by the guest. + * ??? Either of these sounds like too much effort just + * to work around running a modern host kernel. + */ + ahcf->isar.id_aa64pfr0 = 0x00000011; /* EL1&0, AArch64 only */ + err = 0; + } else { + err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64pfr1, + ARM64_SYS_REG(3, 0, 0, 4, 1)); + err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64isar0, + ARM64_SYS_REG(3, 0, 0, 6, 0)); + err |= read_sys_reg64(fdarray[2], &ahcf->isar.id_aa64isar1, + ARM64_SYS_REG(3, 0, 0, 6, 1)); + + /* + * Note that if AArch32 support is not present in the host, + * the AArch32 sysregs are present to be read, but will + * return UNKNOWN values. This is neither better nor worse + * than skipping the reads and leaving 0, as we must avoid + * considering the values in every case. + */ + err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_isar0, + ARM64_SYS_REG(3, 0, 0, 2, 0)); + err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_isar1, + ARM64_SYS_REG(3, 0, 0, 2, 1)); + err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_isar2, + ARM64_SYS_REG(3, 0, 0, 2, 2)); + err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_isar3, + ARM64_SYS_REG(3, 0, 0, 2, 3)); + err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_isar4, + ARM64_SYS_REG(3, 0, 0, 2, 4)); + err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_isar5, + ARM64_SYS_REG(3, 0, 0, 2, 5)); + err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_isar6, + ARM64_SYS_REG(3, 0, 0, 2, 7)); + + err |= read_sys_reg32(fdarray[2], &ahcf->isar.mvfr0, + ARM64_SYS_REG(3, 0, 0, 3, 0)); + err |= read_sys_reg32(fdarray[2], &ahcf->isar.mvfr1, + ARM64_SYS_REG(3, 0, 0, 3, 1)); + err |= read_sys_reg32(fdarray[2], &ahcf->isar.mvfr2, + ARM64_SYS_REG(3, 0, 0, 3, 2)); + } + kvm_arm_destroy_scratch_host_vcpu(fdarray); + if (err < 0) { + return false; + } + /* We can assume any KVM supporting CPU is at least a v8 * with VFPv4+Neon; this in turn implies most of the other * feature bits.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/kvm64.c | 90 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 88 insertions(+), 2 deletions(-) -- 2.17.2