On Wed, Sep 19, 2012 at 06:12:55PM +0200, Alexander Graf wrote: > Just a really quick glimpse. This patch is huge :). > > On 04.09.2012, at 17:13, Cornelia Huck wrote: > > > Add a new virtio transport that uses channel commands to perform > > virtio operations. > > > > Add a new machine type s390-ccw that uses this virtio-ccw transport > > and make it the default machine for s390. > > > > Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com> > > --- > > > > Changes v1->v2: > > - update to virtio-ccw interface changes > > > > --- > > hw/qdev-monitor.c | 5 + > > hw/s390-virtio.c | 277 ++++++++++++---- > > hw/s390x/Makefile.objs | 1 + > > hw/s390x/css.c | 45 +++ > > hw/s390x/css.h | 3 + > > hw/s390x/virtio-ccw.c | 875 > > +++++++++++++++++++++++++++++++++++++++++++++++++ > > hw/s390x/virtio-ccw.h | 79 +++++ > > vl.c | 1 + > > 8 files changed, 1215 insertions(+), 71 deletions(-) > > create mode 100644 hw/s390x/virtio-ccw.c > > create mode 100644 hw/s390x/virtio-ccw.h > > > > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c > > index 33b7f79..92b7c59 100644 > > --- a/hw/qdev-monitor.c > > +++ b/hw/qdev-monitor.c > > @@ -42,6 +42,11 @@ static const QDevAlias qdev_alias_table[] = { > > { "virtio-blk-s390", "virtio-blk", QEMU_ARCH_S390X }, > > { "virtio-net-s390", "virtio-net", QEMU_ARCH_S390X }, > > { "virtio-serial-s390", "virtio-serial", QEMU_ARCH_S390X }, > > + { "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_S390X }, > > + { "virtio-net-ccw", "virtio-net", QEMU_ARCH_S390X }, > > + { "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_S390X }, > > + { "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_S390X }, > > + { "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_S390X }, > > How does this work? We want to default to virtio-xxx-pci on !s390, to > virtio-xxx-s390 on the legacy machine and to virtio-xxx-ccw for the s390-ccw > machine, right?
See patch 5. As you said, we need to find a nice way to solve this. > > [...] > > + > > +static void virtio_ccw_notify(void *opaque, uint16_t vector) > > +{ > > + VirtioCcwData *dev = opaque; > > + SubchDev *sch = dev->sch; > > + uint64_t indicators; > > + > > + if (vector >= VIRTIO_PCI_QUEUE_MAX) { > > + return; > > + } > > + > > + qemu_mutex_lock(&sch->mutex); > > Why do you need this lock? The locks are probably a leftover from an earlier version where Conny used a separate thread for subchannel work. We will double check if the lock would be needed when the big QEMU lock is gone and, if yes, leave a comment in the code as a reminder. Jens