Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.

Cornelia Huck
2013-02-26T14:05:50Z

On Tue, 26 Feb 2013 15:56:43 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Feb 26, 2013 at 02:29:07PM +0100, Cornelia Huck wrote:
> > On Tue, 26 Feb 2013 13:13:39 +0100
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >
> > > On 26/02/13 12:18, Michael S. Tsirkin wrote:
> > > > virtio_ccw: pass a cookie value to kvm hypercall
> > > >
> > > > Lookups by channel/vq pair on host during virtio notifications might be
> > > > expensive. Interpret hypercall return value as a cookie which host can
> > > > use to do device lookups for the next notification more efficiently.
> > > >
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > >
> > > > ---
> > > >
> > > > diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> > > > index 2029b6c..1054f3a 100644
> > > > --- a/drivers/s390/kvm/virtio_ccw.c
> > > > +++ b/drivers/s390/kvm/virtio_ccw.c
> > > > @@ -77,6 +77,7 @@ struct virtio_ccw_vq_info {
> > > > void *queue;
> > > > struct vq_info_block *info_block;
> > > > struct list_head node;
> > > > + long cookie;
> > > > };
> > > >
> > > > #define KVM_VIRTIO_CCW_RING_ALIGN 4096
> > > > @@ -145,15 +146,18 @@ static int ccw_io_helper(struct virtio_ccw_device *vcdev,
> > > > }
> > > >
> > > > static inline long do_kvm_notify(struct subchannel_id schid,
> > > > - unsigned long queue_index)
> > > > + unsigned long queue_index,
> > > > + long cookie)
> > > > {
> > > > register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
> > > > register struct subchannel_id __schid asm("2") = schid;
> > > > register unsigned long __index asm("3") = queue_index;
> > > > register long __rc asm("2");
> > > > + register long __cookie asm("4") = cookie;
> > > >
> > > > asm volatile ("diag 2,4,0x500\n"
> > > > - : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index)
> > > > + : "=d" (__rc) : "d" (__nr), "d" (__schid), "d" (__index),
> > > > + "d"(__cookie)
> > > > : "memory", "cc");
> > > > return __rc;
> > > > }
> > > > @@ -166,7 +170,7 @@ static void virtio_ccw_kvm_notify(struct virtqueue *vq)
> > > >
> > > > vcdev = to_vc_device(info->vq->vdev);
> > > > ccw_device_get_schid(vcdev->cdev, &schid);
> > > > - do_kvm_notify(schid, virtqueue_get_queue_index(vq));
> > > > + info->cookie = do_kvm_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
> > > > }
> > > >
> > > > static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev,
> > >
> > >
> > > Hmmm, forget my last mail. This actually could be even forward and backward compatible.
> > > In the virtio spec we will not define the cookie format (just 64bit int). That will allow
> > > qemu or future kernels to use that for other things (as long as a validity check is
> > > possible) if we dont have a kvm bus.
> > >
> > > Now:
> > >
> > > old guest, old host:
> > > works.
> > >
> > > old guest, new host:
> > > the cookie from the guest contains junk, the host needs to detect that the cookie is
> > > junk and ignores it. It will return the new cookie anyway.
> > >
> > > new guest, old host:
> > > The guest will get a junk cookie and pass it back to the host. But the host will ignore
> > > it anyway.
> > >
> > > new guest, new host:
> > > works.
> > >
> > > So...
> > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >
> > Yes, that sounds sane; I'll give it a try later.
> >
> > However, I'd rather not want to rush this; I'd prefer to get the
> > initial version in first.
>
> Well planning to obsolete an interface from the start sounds wrong
> somehow. We could always drop ccw in 3.9 if we feel we need more
> time, but to me, this looks like a minor enough change to do even after
> the merge window closed.

This was aimed at the exploitation; I'm fine with this patch for 3.9
after we let it mature for a day or two.

>
> Want me to write you a spec patch too?

That would be great.

>
> > I'll do a v4 later.
>
> Right, just return 0 and it'll work.
>