From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45597) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvSgN-0002Yv-Az for qemu-devel@nongnu.org; Wed, 16 Jan 2013 08:05:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TvSgH-0003jS-Br for qemu-devel@nongnu.org; Wed, 16 Jan 2013 08:05:31 -0500 Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:51194) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvSgH-0003j7-3r for qemu-devel@nongnu.org; Wed, 16 Jan 2013 08:05:25 -0500 Received: from /spool/local by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 16 Jan 2013 13:03:43 -0000 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 r0GD53jH4063572 for ; Wed, 16 Jan 2013 13:05:03 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 r0GD5AI8015265 for ; Wed, 16 Jan 2013 06:05:11 -0700 Date: Wed, 16 Jan 2013 14:05:07 +0100 From: Cornelia Huck Message-ID: <20130116140507.554aa427@gondolin> In-Reply-To: <50F69FE6.9030908@suse.de> References: <1358337445-53555-1-git-send-email-cornelia.huck@de.ibm.com> <1358337445-53555-3-git-send-email-cornelia.huck@de.ibm.com> <50F69FE6.9030908@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2] s390-virtio: Factor out some initialization code. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?UTF-8?B?RsOkcmJlcg==?= Cc: Christian Borntraeger , Jens Freimann , Alexander Graf , qemu-devel On Wed, 16 Jan 2013 13:41:10 +0100 Andreas F=C3=A4rber wrote: > Am 16.01.2013 12:57, schrieb Cornelia Huck: > > diff --git a/hw/s390-virtio.h b/hw/s390-virtio.h > > index cd88179..acd4846 100644 > > --- a/hw/s390-virtio.h > > +++ b/hw/s390-virtio.h > > @@ -20,4 +20,10 @@ typedef int (*s390_virtio_fn)(uint64_t reg2, uint64_= t reg3, uint64_t reg4, > > uint64_t reg5, uint64_t reg6, uint64_t r= eg7); > > void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn); > > =20 > > +CPUS390XState *s390_init_cpus(const char *cpu_model, uint8_t *storage_= keys); > > +void s390_set_up_kernel(CPUS390XState *env, > > + const char *kernel_filename, > > + const char *kernel_cmdline, > > + const char *initrd_filename); >=20 > I don't like this interface: It reads "cpus" but appears to return a > single CPUS390XState. Can't you at least use S390CPU* instead? >=20 > Alternatively it would be possible (although at some point to be > changed) to use global first_cpu and to iterate over the CPUs rather > than returning one from one function to the other. An alternative might be to use s390_cpu_addr2state(0) to effectively get to the same cpu. >=20 > However since the only usage I spot in the patch without looking up the > file myself is s390_add_running_cpu(), can the call be moved out of the > kernel setup function to avoid this dependency? s390_set_up_kernel() uses it to specify the initial psw, and for virtio-ccw, it will be needed to issue an ioctl. But both use cases could be covered by grabbing cpu 0. >=20 > Andreas >=20 > > +void s390_create_virtio_net(BusState *bus, const char *name); > > #endif >=20