From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33357) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEgCN-0002O9-99 for qemu-devel@nongnu.org; Thu, 20 Sep 2012 08:49:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TEgCM-0003z3-05 for qemu-devel@nongnu.org; Thu, 20 Sep 2012 08:49:43 -0400 Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:47874) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEgCL-0003yX-P1 for qemu-devel@nongnu.org; Thu, 20 Sep 2012 08:49:41 -0400 Received: from /spool/local by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 20 Sep 2012 13:49:39 +0100 Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by b06cxnps3074.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q8KCnNJv39387214 for ; Thu, 20 Sep 2012 12:49:23 GMT Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q8KCnRsv010015 for ; Thu, 20 Sep 2012 06:49:29 -0600 Message-ID: <505B10D6.3060606@de.ibm.com> Date: Thu, 20 Sep 2012 14:49:26 +0200 From: Christian Borntraeger MIME-Version: 1.0 References: <1345636483-6581-1-git-send-email-jfrei@linux.vnet.ibm.com> <24857C68-EAED-4C11-B931-4AEC31DB43D4@suse.de> In-Reply-To: <24857C68-EAED-4C11-B931-4AEC31DB43D4@suse.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] s390: use sync regs for register transfer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Cornelia Huck , Jens Freimann , Heinz Graalfs , qemu-devel , Einar Lueck [...] >> --- a/target-s390x/kvm.c >> +++ b/target-s390x/kvm.c >> @@ -88,50 +88,77 @@ void kvm_arch_reset_vcpu(CPUS390XState *env) >> /* FIXME: add code to reset vcpu. */ >> } >> >> +/* we want to have the prefix, the GPRS, the ACRS and the CRS up to date */ >> +#define QEMU_NEEDED_REGS (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | \ >> + KVM_SYNC_ACRS | KVM_SYNC_CRS) >> + >> +/* But qemu only changes the GPRS */ >> +#define QEMU_DIRTY_REGS (KVM_SYNC_GPRS) >> + >> int kvm_arch_put_registers(CPUS390XState *env, int level) >> { >> struct kvm_regs regs; >> int ret; >> int i; >> >> - ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s); >> - if (ret < 0) { >> - return ret; >> - } >> - >> - for (i = 0; i < 16; i++) { >> - regs.gprs[i] = env->regs[i]; >> - } >> - >> - ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s); >> - if (ret < 0) { >> - return ret; >> - } >> - >> env->kvm_run->psw_addr = env->psw.addr; >> env->kvm_run->psw_mask = env->psw.mask; >> >> - return ret; >> + if ((env->kvm_run->kvm_valid_regs & QEMU_NEEDED_REGS) == QEMU_NEEDED_REGS) { > > Isn't this also missing a check for KVM_CAP_SYNC_REGS? I ommitted the check for two reasons: - since kvm_check_extension is an ioctl by itself, this would counter the original idea of sync regs - the kvm_run structure contains zeroes at the sync reg location in kernels that dont support that, (the area was unused and the page was allocated with GFP_ZERO) So by having any of the kvm_valid_regs != 0 we know that SYNC REGS must be supported > Also, on reset we probably also want to write the other registers back, right? Well, on reset we call the initial cpu reset ioctl, which does reset the other registers mandated by a cpu reset. Furthermore, it is not different to the current implementation.... What we might want to do is to also consider a clear reset (which also zeroes the FPRs and ARs as well as memory etc) as an additional option. But this should not be the default, e.g. to make kdump or standalone dump possible. I think this would then be an addon-patch by itself. > Alex Christian