| Message ID | 1353665892-35445-3-git-send-email-jfrei@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 23.11.2012, at 11:18, Jens Freimann wrote: > When resetting vcpus on s390/kvm we have to clear registers, psw > and prefix as described in the z/Architecture PoP, otherwise a > reboot won't work. IPL PSW and prefix are set later on by the > s390-ipl device reset code. > > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > --- > target-s390x/kvm.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c > index 94de764..b1b791e 100644 > --- a/target-s390x/kvm.c > +++ b/target-s390x/kvm.c This needs to go into generic vcpu reset code. Alex > @@ -85,7 +85,31 @@ int kvm_arch_init_vcpu(CPUS390XState *env) > > void kvm_arch_reset_vcpu(CPUS390XState *env) > { > - /* FIXME: add code to reset vcpu. */ > + int i; > + > + /* The initial reset call is needed here to reset in-kernel > + * vcpu data that we can't access directly from QEMU. Before > + * this ioctl cpu_synchronize_state() is called in common kvm > + * code (kvm-all). What remains is clearing registers and psw > + * in QEMU cpu state */ > + if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) { > + perror("Can't reset vcpu\n"); > + } > + env->halted = 1; > + env->exception_index = EXCP_HLT; > + for (i = 0; i < 16; i++) { > + env->regs[i] = 0; > + env->aregs[i] = 0; > + env->cregs[i] = 0; > + env->fregs[i].ll = 0; > + } > + /* architectured initial values for CR 0 and 14 */ > + env->cregs[0] = 0xE0UL; > + env->cregs[14] = 0xC2000000UL; > + env->fpc = 0; > + env->psw.mask = 0; > + env->psw.addr = 0; > + env->psa = 0; > } > > int kvm_arch_put_registers(CPUS390XState *env, int level) > -- > 1.7.12.4 >
On Fri, Nov 23, 2012 at 02:40:10PM +0100, Alexander Graf wrote: > > On 23.11.2012, at 11:18, Jens Freimann wrote: > > > When resetting vcpus on s390/kvm we have to clear registers, psw > > and prefix as described in the z/Architecture PoP, otherwise a > > reboot won't work. IPL PSW and prefix are set later on by the > > s390-ipl device reset code. > > > > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > > --- > > target-s390x/kvm.c | 26 +++++++++++++++++++++++++- > > 1 file changed, 25 insertions(+), 1 deletion(-) > > > > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c > > index 94de764..b1b791e 100644 > > --- a/target-s390x/kvm.c > > +++ b/target-s390x/kvm.c > > This needs to go into generic vcpu reset code. Ok. Will send a new patch. Jens
On 23/11/12 14:40, Alexander Graf wrote: > > On 23.11.2012, at 11:18, Jens Freimann wrote: > >> When resetting vcpus on s390/kvm we have to clear registers, psw >> and prefix as described in the z/Architecture PoP, otherwise a >> reboot won't work. IPL PSW and prefix are set later on by the >> s390-ipl device reset code. >> >> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> >> --- >> target-s390x/kvm.c | 26 +++++++++++++++++++++++++- >> 1 file changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c >> index 94de764..b1b791e 100644 >> --- a/target-s390x/kvm.c >> +++ b/target-s390x/kvm.c > > This needs to go into generic vcpu reset code. The kvm ioctl certainly not, no? (definitely necessary for kernels without sync regs). I guess you are talking about moving the register initialisation into s390_cpu_reset (target-s390x/cpu.c). Right? Jens can you have a look? Christian
On 23.11.2012, at 15:32, Christian Borntraeger wrote: > On 23/11/12 14:40, Alexander Graf wrote: >> >> On 23.11.2012, at 11:18, Jens Freimann wrote: >> >>> When resetting vcpus on s390/kvm we have to clear registers, psw >>> and prefix as described in the z/Architecture PoP, otherwise a >>> reboot won't work. IPL PSW and prefix are set later on by the >>> s390-ipl device reset code. >>> >>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> >>> --- >>> target-s390x/kvm.c | 26 +++++++++++++++++++++++++- >>> 1 file changed, 25 insertions(+), 1 deletion(-) >>> >>> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c >>> index 94de764..b1b791e 100644 >>> --- a/target-s390x/kvm.c >>> +++ b/target-s390x/kvm.c >> >> This needs to go into generic vcpu reset code. > > The kvm ioctl certainly not, no? (definitely necessary for kernels > without sync regs). Yes, of course :). > I guess you are talking about moving the register initialisation > into s390_cpu_reset (target-s390x/cpu.c). Right? Jens can you have > a look? Yup. This is just normal reset logic that needs to be in the normal CPU reset functions. Alex
On Fri, Nov 23, 2012 at 03:32:16PM +0100, Christian Borntraeger wrote: > On 23/11/12 14:40, Alexander Graf wrote: > > > > On 23.11.2012, at 11:18, Jens Freimann wrote: > > > >> When resetting vcpus on s390/kvm we have to clear registers, psw > >> and prefix as described in the z/Architecture PoP, otherwise a > >> reboot won't work. IPL PSW and prefix are set later on by the > >> s390-ipl device reset code. > >> > >> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> > >> --- > >> target-s390x/kvm.c | 26 +++++++++++++++++++++++++- > >> 1 file changed, 25 insertions(+), 1 deletion(-) > >> > >> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c > >> index 94de764..b1b791e 100644 > >> --- a/target-s390x/kvm.c > >> +++ b/target-s390x/kvm.c > > > > This needs to go into generic vcpu reset code. > > The kvm ioctl certainly not, no? (definitely necessary for kernels > without sync regs). > > I guess you are talking about moving the register initialisation > into s390_cpu_reset (target-s390x/cpu.c). Right? Jens can you have > a look? Yes, I'm already looking into it. s390_cpu_reset() is only called when booting the first time however because it's not registered as a reset handler. Trying to find out how if I can convert it to a qemu reset handler or if I have to do some QOM magic. Jens > Christian >
On 23 November 2012 10:18, Jens Freimann <jfrei@linux.vnet.ibm.com> wrote: > + /* The initial reset call is needed here to reset in-kernel > + * vcpu data that we can't access directly from QEMU. If there's in-kernel vcpu data we can't access from QEMU, doesn't this cause problems for migration? > Before > + * this ioctl cpu_synchronize_state() is called in common kvm > + * code (kvm-all). What remains is clearing registers and psw > + * in QEMU cpu state */ > + if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) { > + perror("Can't reset vcpu\n"); > + } Doesn't this mean we're now out-of-sync again, since the kernel state will be whatever INITIAL_RESET says it should be but the QEMU state won't have been changed? Generally, I think an arch-independent 'reset vcpu' ioctl would be useful. At the moment for ARM you have to pull everything out and back in again via the GET/SET_REG interface, which works [you can keep all the state around to write back on vcpu reset] but is ugly. -- PMM
On 23.11.2012, at 16:02, Peter Maydell wrote: > On 23 November 2012 10:18, Jens Freimann <jfrei@linux.vnet.ibm.com> wrote: >> + /* The initial reset call is needed here to reset in-kernel >> + * vcpu data that we can't access directly from QEMU. > > If there's in-kernel vcpu data we can't access from QEMU, > doesn't this cause problems for migration? This one is for older kernels. For newer kernels, we have sync regs and ONE_REG interfaces to everything we need FWIW :). > >> Before >> + * this ioctl cpu_synchronize_state() is called in common kvm >> + * code (kvm-all). What remains is clearing registers and psw >> + * in QEMU cpu state */ >> + if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) { >> + perror("Can't reset vcpu\n"); >> + } > > Doesn't this mean we're now out-of-sync again, since the > kernel state will be whatever INITIAL_RESET says it should > be but the QEMU state won't have been changed? > > Generally, I think an arch-independent 'reset vcpu' ioctl > would be useful. At the moment for ARM you have to pull > everything out and back in again via the GET/SET_REG interface, > which works [you can keep all the state around to write back > on vcpu reset] but is ugly. Actually it's the way it's supposed to work. The reset call is just a hack, because it means we need to duplicate the reset logic for KVM & TCG. But it dates back to when KVM on s390 wasn't using QEMU. Alex
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 94de764..b1b791e 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -85,7 +85,31 @@ int kvm_arch_init_vcpu(CPUS390XState *env) void kvm_arch_reset_vcpu(CPUS390XState *env) { - /* FIXME: add code to reset vcpu. */ + int i; + + /* The initial reset call is needed here to reset in-kernel + * vcpu data that we can't access directly from QEMU. Before + * this ioctl cpu_synchronize_state() is called in common kvm + * code (kvm-all). What remains is clearing registers and psw + * in QEMU cpu state */ + if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) { + perror("Can't reset vcpu\n"); + } + env->halted = 1; + env->exception_index = EXCP_HLT; + for (i = 0; i < 16; i++) { + env->regs[i] = 0; + env->aregs[i] = 0; + env->cregs[i] = 0; + env->fregs[i].ll = 0; + } + /* architectured initial values for CR 0 and 14 */ + env->cregs[0] = 0xE0UL; + env->cregs[14] = 0xC2000000UL; + env->fpc = 0; + env->psw.mask = 0; + env->psw.addr = 0; + env->psa = 0; } int kvm_arch_put_registers(CPUS390XState *env, int level)
When resetting vcpus on s390/kvm we have to clear registers, psw and prefix as described in the z/Architecture PoP, otherwise a reboot won't work. IPL PSW and prefix are set later on by the s390-ipl device reset code. Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com> --- target-s390x/kvm.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)