On Wed, 16 Jan 2013 13:41:10 +0100 Andreas Färber <[email protected]> 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 reg7); > > void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn); > > > > +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); > > I don't like this interface: It reads "cpus" but appears to return a > single CPUS390XState. Can't you at least use S390CPU* instead? > > 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. > > 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. > > Andreas > > > +void s390_create_virtio_net(BusState *bus, const char *name); > > #endif >
