From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42218) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SeQ9T-0004tx-OB for qemu-devel@nongnu.org; Tue, 12 Jun 2012 08:24:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SeQ9M-0000km-VF for qemu-devel@nongnu.org; Tue, 12 Jun 2012 08:24:51 -0400 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:47647) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SeQ9M-0000jd-BK for qemu-devel@nongnu.org; Tue, 12 Jun 2012 08:24:44 -0400 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 12 Jun 2012 13:24:41 +0100 Received: from d06av12.portsmouth.uk.ibm.com (d06av12.portsmouth.uk.ibm.com [9.149.37.247]) by d06nrmr1707.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q5CCOdYn2064570 for ; Tue, 12 Jun 2012 13:24:39 +0100 Received: from d06av12.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av12.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q5CCOcIY032390 for ; Tue, 12 Jun 2012 06:24:38 -0600 Message-ID: <4FD73507.3040204@de.ibm.com> Date: Tue, 12 Jun 2012 14:24:39 +0200 From: Christian Borntraeger MIME-Version: 1.0 References: <1338984323-21914-1-git-send-email-jfrei@de.ibm.com> <1338984323-21914-6-git-send-email-jfrei@de.ibm.com> <4FD712D4.3040401@suse.de> In-Reply-To: <4FD712D4.3040401@suse.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/8] s390: Cleanup sclp functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Jens Freimann , Heinz Graalfs , qemu-devel , Jens Freimann , Cornelia Huck , =?ISO-8859-1?Q?Andreas_F=E4r?= =?ISO-8859-1?Q?ber?= Yes we will re-split the sclp patches. besides that, some comments: On 12/06/12 11:58, Alexander Graf wrote: >> +#include "hw/s390-sclp.h" >> > > No need for hw/. will fix. >> +void sclp_service_interrupt(CPUS390XState *env, uint32_t sccb) >> +{ >> + if (!sccb) { >> + return; >> + } >> + >> + if (kvm_enabled()) { >> +#ifdef CONFIG_KVM >> > > You shouldn't know about CONFIG_KVM in hw/. So we have to generalize > this code. Ok, Maybe an exported interface for sending interrupts to the guest under target-s390/ that hides the kvm/tcg thing. ice_call(CPUS390XState *env, struct kvm_run *run, >> r = sclp_service_call(env, sccb, code); >> if (r) { >> setcc(env, 3); >> + } else { >> + setcc(env, 0); >> > > This one looks like an actual fix that is not part of the cleanup? Yes it is. Separate patch? > >> } >> >> return 0; >> diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c >> index 7b72473..74bd9ad 100644 >> --- a/target-s390x/op_helper.c >> +++ b/target-s390x/op_helper.c >> @@ -31,6 +31,7 @@ >> >> #if !defined (CONFIG_USER_ONLY) >> #include "sysemu.h" >> +#include "hw/s390-sclp.h" >> > > #include in hw/ from target-XXX is a no-go. It means our abstraction > layer is broken. Disagree here. The sclp is a processor that helps the CPU and there is a tight link. This is similar to a PIC/APIC etc which are also under hw AND included from target-386/ - among others: cborntra@br96egxr:/space/qemu$ egrep "include.*hw" target-*/* | wc -l 39 [...9 >> - if (kvm_enabled()) { >> -#ifdef CONFIG_KVM >> - kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE, >> - sccb & ~3, 0, 1); >> -#endif >> - } else { >> - env->psw.addr += 4; >> - ext_interrupt(env, EXT_SERVICE, sccb & ~3, 0); >> - } >> + r = sclp_read_info(env, &work_sccb); >> > > Maybe we should have a list of callbacks that hw/ code can register for? > Like the spapr hcalls. We will have a look if thats a way to go.