From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54724) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SeZm1-0005U2-FQ for qemu-devel@nongnu.org; Tue, 12 Jun 2012 18:41:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SeZlz-0007Il-Ik for qemu-devel@nongnu.org; Tue, 12 Jun 2012 18:41:17 -0400 Received: from mail-pz0-f45.google.com ([209.85.210.45]:61799) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SeZlz-0007IY-Cw for qemu-devel@nongnu.org; Tue, 12 Jun 2012 18:41:15 -0400 Received: by dadv2 with SMTP id v2so80340dad.4 for ; Tue, 12 Jun 2012 15:41:13 -0700 (PDT) Message-ID: <4FD7C586.8090607@codemonkey.ws> Date: Tue, 12 Jun 2012 17:41:10 -0500 From: Anthony Liguori 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> <4FD73507.3040204@de.ibm.com> <4FD736DF.60301@suse.de> In-Reply-To: <4FD736DF.60301@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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 , Christian Borntraeger , Jens Freimann , Cornelia Huck , =?ISO-8859-1?Q?Andreas_F=E4rber?= On 06/12/2012 07:32 AM, Alexander Graf wrote: > On 06/12/2012 02:24 PM, Christian Borntraeger wrote: >> 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. > > Yeah, or have KVM hook into the tcg interrupt dispatch loop at > cpu_exec.c:cpu_exec(). Not sure which way is easier. > >> >> >> 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? > > Yes, please :). > >> >>>> } >>>> >>>> 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: > > Which is exactly why Anthony is suggesting for years now to pull the APIC code > into target-i386. Indeed :-) > > To me, the SCLP interface is similar to PIO, MMIO, SPAPR hypercalls, you name > it. Yeah, the SPAPR hypercalls is a good one I think but I don't know enough about SCLP yet. From what's here, it would be pretty easy to model with qemu_irq I think. We do that for target-i386 for things like the a20 line which is another case where random hardware interacts with the cpu in a far too personal fashion. Regards, Anthony Liguori We can certainly have sclp awareness in target-s390x, but please don't just > blindly include headers from hw/. Split the few bits of information that we need > in target-s390x into a separate header (clean) or target-s390x/cpu.h (hacky, but > ok for now) and rather include that from hw/. > > > Alex > > >