From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51880) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T99wk-00065D-PA for qemu-devel@nongnu.org; Wed, 05 Sep 2012 03:22:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T99wg-0000Xj-TC for qemu-devel@nongnu.org; Wed, 05 Sep 2012 03:22:46 -0400 Received: from e06smtp10.uk.ibm.com ([195.75.94.106]:46969) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T99wg-0000XW-Jo for qemu-devel@nongnu.org; Wed, 05 Sep 2012 03:22:42 -0400 Received: from /spool/local by e06smtp10.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 5 Sep 2012 08:22:38 +0100 Received: from d06av05.portsmouth.uk.ibm.com (d06av05.portsmouth.uk.ibm.com [9.149.37.229]) by b06cxnps4075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q857MEdA16777464 for ; Wed, 5 Sep 2012 07:22:14 GMT Received: from d06av05.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av05.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q857MJa0028174 for ; Wed, 5 Sep 2012 01:22:20 -0600 Date: Wed, 5 Sep 2012 09:22:32 +0200 From: Heiko Carstens Message-ID: <20120905072232.GA5852@osiris.de.ibm.com> References: <1346771610-52423-1-git-send-email-cornelia.huck@de.ibm.com> <1346771610-52423-3-git-send-email-cornelia.huck@de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1346771610-52423-3-git-send-email-cornelia.huck@de.ibm.com> Subject: Re: [Qemu-devel] [PATCH v2 2/7] s390/kvm: Add support for machine checks. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: linux-s390 , Anthony Liguori , Rusty Russell , KVM , Carsten Otte , Marcelo Tosatti , Sebastian Ott , qemu-devel , Alexander Graf , Christian Borntraeger , Avi Kivity , Martin Schwidefsky On Tue, Sep 04, 2012 at 05:13:25PM +0200, Cornelia Huck wrote: Just some quick comments: [...] > int kvm_s390_inject_program_int(struct kvm_vcpu *vcpu, u16 code) > { > struct kvm_s390_local_interrupt *li = &vcpu->arch.local_int; > @@ -648,6 +747,12 @@ int kvm_s390_inject_vm(struct kvm *kvm, > case KVM_S390_INT_EMERGENCY: > kfree(inti); > return -EINVAL; > + case KVM_S390_MCHK: > + VM_EVENT(kvm, 5, "inject: machine check parm64:%llx", > + s390int->parm64); > + inti->type = s390int->type; > + inti->mchk.mcic = s390int->parm64; > + break; The kvm_s390_interrupt struct seems to be inappropriate to pass machine check data around. E.g. if you want to inject an uncorrectable storage error, because the host failed to swap in a page, you must also pass a failing storage address which doesn't fit into this structure. Just something you should consider. ;) > +static int handle_lpswe(struct kvm_vcpu *vcpu) > +{ > + int base2 = vcpu->arch.sie_block->ipb >> 28; > + int disp2 = ((vcpu->arch.sie_block->ipb & 0x0fff0000) >> 16); Sooner or later we need helper functions which extract the significant parts of an instruction. Maybay something like insn_[type]_get_base2(...) or simply structures like struct insn_[type], which allow to easily access parts of an instruction. > + u64 addr; > + u64 new_psw[2]; psw_t? > + > + addr = disp2; > + if (base2) > + addr += vcpu->run->s.regs.gprs[base2]; > + > + if (addr & 7) { > + kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION); > + goto out; > + } > + > + if (copy_from_guest(vcpu, new_psw, addr, sizeof(*new_psw))) { I assume that should be sizeof(new_psw). Did that ever work?! > + if ((vcpu->arch.sie_block->gpsw.mask & 0xb80800fe7fffffff) || > + (((vcpu->arch.sie_block->gpsw.mask & 0x0000000110000000) == > + 0x0000000010000000) && > + (vcpu->arch.sie_block->gpsw.addr & 0xffffffff80000000)) || > + (!(vcpu->arch.sie_block->gpsw.mask & 0x0000000180000000) && > + (vcpu->arch.sie_block->gpsw.addr & 0xfffffffffff00000)) || > + ((vcpu->arch.sie_block->gpsw.mask & 0x0000000110000000) == > + 0x0000000100000000)) { This is not very readable... Please make use of the PSW defines in ptrace.h and add new ones if needed. Also please make use of (move) the PSW32 defines in compat.h.