| Message ID | 20181024113709.16599-4-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: KVM vs ARMISARegisters | expand |
On 10/24/18 12:37 PM, Richard Henderson wrote: > - int i, ret, fdarray[3]; > + int i, err = 0, fdarray[3]; Bah. The bit about having compile-tested for arm32 was fake news -- "i" is now unused. Ho hum. > @@ -77,16 +69,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > */ > ahcf->dtb_compatible = "arm,arm-v7"; > > - for (i = 0; i < ARRAY_SIZE(idregs); i++) { > - ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]); > - if (ret) { > - break; > - } > - } > + err |= read_sys_reg32(fdarray[2], &midr, ARM_CP15_REG32(0, 0, 0, 0)); > + err |= read_sys_reg32(fdarray[2], &id_pfr0, ARM_CP15_REG32(0, 0, 1, 0)); > + err |= read_sys_reg32(fdarray[2], &mvfr1, > + KVM_REG_ARM | KVM_REG_SIZE_U32 | > + KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1); r~
On 24 October 2018 at 12:37, Richard Henderson <richard.henderson@linaro.org> wrote: > Assert that the value to be written is the correct size. > No change in functionality here, just mirroring the same > function from kvm64. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/kvm32.c | 41 ++++++++++++++++------------------------- > 1 file changed, 16 insertions(+), 25 deletions(-) > - struct kvm_one_reg idregs[] = { > - { > - .id = KVM_REG_ARM | KVM_REG_SIZE_U32 > - | ENCODE_CP_REG(15, 0, 0, 0, 0, 0, 0), > - .addr = (uintptr_t)&midr, > - }, > - { > - .id = KVM_REG_ARM | KVM_REG_SIZE_U32 > - | ENCODE_CP_REG(15, 0, 0, 0, 1, 0, 0), > - .addr = (uintptr_t)&id_pfr0, > - }, > - { > - .id = KVM_REG_ARM | KVM_REG_SIZE_U32 > - | KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1, > - .addr = (uintptr_t)&mvfr1, > - }, > - }; > > if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) { > return false; > @@ -77,16 +69,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > */ > ahcf->dtb_compatible = "arm,arm-v7"; > > - for (i = 0; i < ARRAY_SIZE(idregs); i++) { > - ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]); > - if (ret) { > - break; > - } > - } > + err |= read_sys_reg32(fdarray[2], &midr, ARM_CP15_REG32(0, 0, 0, 0)); > + err |= read_sys_reg32(fdarray[2], &id_pfr0, ARM_CP15_REG32(0, 0, 1, 0)); Reviewed-by: Peter Maydell <peter.maydell@linaro.org> with the declaration of 'i' removed. (Confusingly, ENCODE_CP_REG and ARM_CP15_REG32 take the op1/crn/crm/op2 arguments in different orders.) thanks -- PMM
On 11/2/18 2:30 PM, Peter Maydell wrote: > (Confusingly, ENCODE_CP_REG and ARM_CP15_REG32 take the > op1/crn/crm/op2 arguments in different orders.) As Shaggy said, "It wasn't me". ;-) r~
diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c index 0f1e94c7b5..da08f7aab8 100644 --- a/target/arm/kvm32.c +++ b/target/arm/kvm32.c @@ -28,6 +28,14 @@ static inline void set_feature(uint64_t *features, int feature) *features |= 1ULL << feature; } +static int read_sys_reg32(int fd, uint32_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_U32); + 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 @@ -35,9 +43,10 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) * we have to create a scratch VM, create a single CPU inside it, * and then query that CPU for the relevant ID registers. */ - int i, ret, fdarray[3]; + int i, err = 0, fdarray[3]; uint32_t midr, id_pfr0, mvfr1; uint64_t features = 0; + /* 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. @@ -47,23 +56,6 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) QEMU_KVM_ARM_TARGET_NONE }; struct kvm_vcpu_init init; - struct kvm_one_reg idregs[] = { - { - .id = KVM_REG_ARM | KVM_REG_SIZE_U32 - | ENCODE_CP_REG(15, 0, 0, 0, 0, 0, 0), - .addr = (uintptr_t)&midr, - }, - { - .id = KVM_REG_ARM | KVM_REG_SIZE_U32 - | ENCODE_CP_REG(15, 0, 0, 0, 1, 0, 0), - .addr = (uintptr_t)&id_pfr0, - }, - { - .id = KVM_REG_ARM | KVM_REG_SIZE_U32 - | KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1, - .addr = (uintptr_t)&mvfr1, - }, - }; if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) { return false; @@ -77,16 +69,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) */ ahcf->dtb_compatible = "arm,arm-v7"; - for (i = 0; i < ARRAY_SIZE(idregs); i++) { - ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &idregs[i]); - if (ret) { - break; - } - } + err |= read_sys_reg32(fdarray[2], &midr, ARM_CP15_REG32(0, 0, 0, 0)); + err |= read_sys_reg32(fdarray[2], &id_pfr0, ARM_CP15_REG32(0, 0, 1, 0)); + err |= read_sys_reg32(fdarray[2], &mvfr1, + KVM_REG_ARM | KVM_REG_SIZE_U32 | + KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1); kvm_arm_destroy_scratch_host_vcpu(fdarray); - if (ret) { + if (err < 0) { return false; }
Assert that the value to be written is the correct size. No change in functionality here, just mirroring the same function from kvm64. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/kvm32.c | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) -- 2.17.2