On 12 November 2018 at 14:09, Richard Henderson <[email protected]> wrote: > On 11/12/18 12:37 PM, Peter Maydell wrote: >> On 8 November 2018 at 17:52, Richard Henderson >> <[email protected]> wrote: >>> Signed-off-by: Richard Henderson <[email protected]> >>> --- >> >>> /* 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
