| Message ID | 1421706621-23731-5-git-send-email-greg.bellows@linaro.org |
|---|---|
| State | New |
| Headers | show |
Greg Bellows <greg.bellows@linaro.org> writes: > Add 32-bit to/from 64-bit register synchronization on register gets and puts. > Set EL1_32BIT feature flag passed to KVM > > Signed-off-by: Greg Bellows <greg.bellows@linaro.org> > --- > target-arm/kvm64.c | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c > index ba16821..0061099 100644 > --- a/target-arm/kvm64.c > +++ b/target-arm/kvm64.c > @@ -81,8 +81,7 @@ int kvm_arch_init_vcpu(CPUState *cs) > int ret; > ARMCPU *cpu = ARM_CPU(cs); > > - if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE || > - !arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > + if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) { > fprintf(stderr, "KVM is not supported for this guest CPU type\n"); > return -EINVAL; > } > @@ -96,6 +95,9 @@ int kvm_arch_init_vcpu(CPUState *cs) > cpu->psci_version = 2; > cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2; > } > + if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > + cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT; > + } > > /* Do KVM_ARM_VCPU_INIT ioctl */ > ret = kvm_arm_vcpu_init(cs); > @@ -133,6 +135,7 @@ int kvm_arch_put_registers(CPUState *cs, int level) > ARMCPU *cpu = ARM_CPU(cs); > CPUARMState *env = &cpu->env; > > + aarch64_sync_32_to_64(env); > for (i = 0; i < 31; i++) { > reg.id = AARCH64_CORE_REG(regs.regs[i]); > reg.addr = (uintptr_t) &env->xregs[i]; > @@ -162,7 +165,11 @@ int kvm_arch_put_registers(CPUState *cs, int level) > } > > /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */ > - val = pstate_read(env); > + if (is_a64(env)) { > + val = pstate_read(env); > + } else { > + val = cpsr_read(env); > + } I know why we do this (especially given where my attempt ended up) but perhaps we could at list have a single state aware accessor so we don't end up duplicating this test all over the place?
On Tue, Jan 20, 2015 at 10:57 AM, Alex Bennée <alex.bennee@linaro.org> wrote: > > Greg Bellows <greg.bellows@linaro.org> writes: > >> Add 32-bit to/from 64-bit register synchronization on register gets and puts. >> Set EL1_32BIT feature flag passed to KVM >> >> Signed-off-by: Greg Bellows <greg.bellows@linaro.org> >> --- >> target-arm/kvm64.c | 21 +++++++++++++++++---- >> 1 file changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c >> index ba16821..0061099 100644 >> --- a/target-arm/kvm64.c >> +++ b/target-arm/kvm64.c >> @@ -81,8 +81,7 @@ int kvm_arch_init_vcpu(CPUState *cs) >> int ret; >> ARMCPU *cpu = ARM_CPU(cs); >> >> - if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE || >> - !arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { >> + if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) { >> fprintf(stderr, "KVM is not supported for this guest CPU type\n"); >> return -EINVAL; >> } >> @@ -96,6 +95,9 @@ int kvm_arch_init_vcpu(CPUState *cs) >> cpu->psci_version = 2; >> cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2; >> } >> + if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { >> + cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT; >> + } >> >> /* Do KVM_ARM_VCPU_INIT ioctl */ >> ret = kvm_arm_vcpu_init(cs); >> @@ -133,6 +135,7 @@ int kvm_arch_put_registers(CPUState *cs, int level) >> ARMCPU *cpu = ARM_CPU(cs); >> CPUARMState *env = &cpu->env; >> >> + aarch64_sync_32_to_64(env); >> for (i = 0; i < 31; i++) { >> reg.id = AARCH64_CORE_REG(regs.regs[i]); >> reg.addr = (uintptr_t) &env->xregs[i]; >> @@ -162,7 +165,11 @@ int kvm_arch_put_registers(CPUState *cs, int level) >> } >> >> /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */ >> - val = pstate_read(env); >> + if (is_a64(env)) { >> + val = pstate_read(env); >> + } else { >> + val = cpsr_read(env); >> + } > > I know why we do this (especially given where my attempt ended up) but > perhaps we could at list have a single state aware accessor so we don't > end up duplicating this test all over the place? I'd happily add an accessor function, but I only found 1 other location that does this conditional so I'm not sure it is warranted. > > -- > Alex Bennée
Greg Bellows <greg.bellows@linaro.org> writes: > On Tue, Jan 20, 2015 at 10:57 AM, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> Greg Bellows <greg.bellows@linaro.org> writes: >> >>> Add 32-bit to/from 64-bit register synchronization on register gets and puts. >>> Set EL1_32BIT feature flag passed to KVM >>> >>> Signed-off-by: Greg Bellows <greg.bellows@linaro.org> <snip> >>> } >>> >>> /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */ >>> - val = pstate_read(env); >>> + if (is_a64(env)) { >>> + val = pstate_read(env); >>> + } else { >>> + val = cpsr_read(env); >>> + } >> >> I know why we do this (especially given where my attempt ended up) but >> perhaps we could at list have a single state aware accessor so we don't >> end up duplicating this test all over the place? > > I'd happily add an accessor function, but I only found 1 other > location that does this conditional so I'm not sure it is warranted. The migration/serialisation code? Today one other, tomorrow just one more?
On 21 January 2015 at 10:54, Alex Bennée <alex.bennee@linaro.org> wrote: > > Greg Bellows <greg.bellows@linaro.org> writes: > >> On Tue, Jan 20, 2015 at 10:57 AM, Alex Bennée <alex.bennee@linaro.org> wrote: >>> I know why we do this (especially given where my attempt ended up) but >>> perhaps we could at list have a single state aware accessor so we don't >>> end up duplicating this test all over the place? >> >> I'd happily add an accessor function, but I only found 1 other >> location that does this conditional so I'm not sure it is warranted. > > The migration/serialisation code? Today one other, tomorrow just one more? Meh. I suggested to Greg that just inlining this at point of use was the simplest approach. Maybe one day we'll clean this stuff up but yet another accessor function doesn't seem too great either. -- PMM
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index ba16821..0061099 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -81,8 +81,7 @@ int kvm_arch_init_vcpu(CPUState *cs) int ret; ARMCPU *cpu = ARM_CPU(cs); - if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE || - !arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { + if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE) { fprintf(stderr, "KVM is not supported for this guest CPU type\n"); return -EINVAL; } @@ -96,6 +95,9 @@ int kvm_arch_init_vcpu(CPUState *cs) cpu->psci_version = 2; cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2; } + if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { + cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT; + } /* Do KVM_ARM_VCPU_INIT ioctl */ ret = kvm_arm_vcpu_init(cs); @@ -133,6 +135,7 @@ int kvm_arch_put_registers(CPUState *cs, int level) ARMCPU *cpu = ARM_CPU(cs); CPUARMState *env = &cpu->env; + aarch64_sync_32_to_64(env); for (i = 0; i < 31; i++) { reg.id = AARCH64_CORE_REG(regs.regs[i]); reg.addr = (uintptr_t) &env->xregs[i]; @@ -162,7 +165,11 @@ int kvm_arch_put_registers(CPUState *cs, int level) } /* Note that KVM thinks pstate is 64 bit but we use a uint32_t */ - val = pstate_read(env); + if (is_a64(env)) { + val = pstate_read(env); + } else { + val = cpsr_read(env); + } reg.id = AARCH64_CORE_REG(regs.pstate); reg.addr = (uintptr_t) &val; ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); @@ -218,6 +225,7 @@ int kvm_arch_get_registers(CPUState *cs) return ret; } } + aarch64_sync_64_to_32(env); reg.id = AARCH64_CORE_REG(regs.sp); reg.addr = (uintptr_t) &env->sp_el[0]; @@ -239,7 +247,12 @@ int kvm_arch_get_registers(CPUState *cs) if (ret) { return ret; } - pstate_write(env, val); + if (is_a64(env)) { + pstate_write(env, val); + } else { + env->uncached_cpsr = val & CPSR_M; + cpsr_write(env, val, 0xffffffff); + } /* KVM puts SP_EL0 in regs.sp and SP_EL1 in regs.sp_el1. On the * QEMU side we keep the current SP in xregs[31] as well.
Add 32-bit to/from 64-bit register synchronization on register gets and puts. Set EL1_32BIT feature flag passed to KVM Signed-off-by: Greg Bellows <greg.bellows@linaro.org> --- target-arm/kvm64.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)