| Message ID | 20181024113709.16599-1-richard.henderson@linaro.org |
|---|---|
| Headers | show |
| Series | target/arm: KVM vs ARMISARegisters | expand |
Richard Henderson <richard.henderson@linaro.org> writes: > My previous patch set for replacing feature bits with id registers > failed to consider that these id registers are beginning to control > migration, and thus we must fill them in for KVM as well. > > Thus, we want to initialize these values within CPU from the host. > > Finally, re-send the T32EE conversion patch, fixing the build > failure on an arm32 host in kvm32.c. > > Tested on arm64; cross-build tested for arm32. I'm still seeing the following assert on qemu-test: qemu-system-aarch64: /home/alex/lsrc/qemu.git/target/arm/cpu.c:832: arm_cpu_realizefn: Assertion `cpu_isar_feature(arm_div, cpu)' failed. Which is a regression caused by: 7e0cf8b: target/arm: Convert division from feature bits to isar0 tests I think the problem is the we trip over the assert because: /* Some features automatically imply others: */ if (arm_feature(env, ARM_FEATURE_V8)) { if (arm_feature(env, ARM_FEATURE_M)) { set_feature(env, ARM_FEATURE_V7); } else { set_feature(env, ARM_FEATURE_V7VE); } } Allows: if (arm_feature(env, ARM_FEATURE_V7VE)) { assert(cpu_isar_feature(arm_div, cpu)); Which isn't strictly true on kvm guests. > > > r~ > > > Richard Henderson (5): > target/arm: Install ARMISARegisters from kvm host > target/arm: Fill in ARMISARegisters for kvm64 > target/arm: Introduce read_sys_reg32 for kvm32 > target/arm: Fill in ARMISARegisters for kvm32 > target/arm: Convert t32ee from feature bit to isar3 test > > target/arm/cpu.h | 6 +++- > target/arm/kvm_arm.h | 1 + > linux-user/elfload.c | 2 +- > target/arm/cpu.c | 4 --- > target/arm/helper.c | 2 +- > target/arm/kvm.c | 1 + > target/arm/kvm32.c | 75 +++++++++++++++++++++++++------------------- > target/arm/kvm64.c | 63 +++++++++++++++++++++++++++++++++++-- > target/arm/machine.c | 3 +- > 9 files changed, 114 insertions(+), 43 deletions(-) -- Alex Bennée
On 1 November 2018 at 17:26, Alex Bennée <alex.bennee@linaro.org> wrote: > > Richard Henderson <richard.henderson@linaro.org> writes: > >> My previous patch set for replacing feature bits with id registers >> failed to consider that these id registers are beginning to control >> migration, and thus we must fill them in for KVM as well. >> >> Thus, we want to initialize these values within CPU from the host. >> >> Finally, re-send the T32EE conversion patch, fixing the build >> failure on an arm32 host in kvm32.c. >> >> Tested on arm64; cross-build tested for arm32. > > I'm still seeing the following assert on qemu-test: > > qemu-system-aarch64: /home/alex/lsrc/qemu.git/target/arm/cpu.c:832: arm_cpu_realizefn: Assertion `cpu_isar_feature(arm_div, cpu)' failed. > > Which is a regression caused by: > > 7e0cf8b: target/arm: Convert division from feature bits to isar0 tests > > I think the problem is the we trip over the assert because: > > /* Some features automatically imply others: */ > if (arm_feature(env, ARM_FEATURE_V8)) { > if (arm_feature(env, ARM_FEATURE_M)) { > set_feature(env, ARM_FEATURE_V7); > } else { > set_feature(env, ARM_FEATURE_V7VE); > } > } > > Allows: > > if (arm_feature(env, ARM_FEATURE_V7VE)) { > assert(cpu_isar_feature(arm_div, cpu)); > > Which isn't strictly true on kvm guests. KVM guests should definitely all be v7VE and all have the arm divide instruction, if they implement AArch32 at all. I think what we're hitting here is the case where the host CPU has no AArch32 support. In that case the ID_ISAR0_EL1 sysreg (which we read from KVM and use to populate the cpu->isar struct) has an UNKNOWN value. thanks -- PMM
On 1 November 2018 at 17:30, Peter Maydell <peter.maydell@linaro.org> wrote: > On 1 November 2018 at 17:26, Alex Bennée <alex.bennee@linaro.org> wrote: >> I think the problem is the we trip over the assert because: >> >> /* Some features automatically imply others: */ >> if (arm_feature(env, ARM_FEATURE_V8)) { >> if (arm_feature(env, ARM_FEATURE_M)) { >> set_feature(env, ARM_FEATURE_V7); >> } else { >> set_feature(env, ARM_FEATURE_V7VE); >> } >> } >> >> Allows: >> >> if (arm_feature(env, ARM_FEATURE_V7VE)) { >> assert(cpu_isar_feature(arm_div, cpu)); >> >> Which isn't strictly true on kvm guests. > > KVM guests should definitely all be v7VE and all have > the arm divide instruction, if they implement AArch32 at all. > I think what we're hitting here is the case where the host > CPU has no AArch32 support. In that case the ID_ISAR0_EL1 > sysreg (which we read from KVM and use to populate the > cpu->isar struct) has an UNKNOWN value. So I think the assert should probably be "assert that if this cpu has any EL that can execute at AArch32 then it has the arm_div ISAR feature". You can determine the former by looking at ID_AA64PFR0_EL1 field EL0 (bits [3:0]), which will be >= 0b0010 if AArch32 is supported at any EL. thanks -- PMM