* Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.
2013-02-26 11:18 ` Michael S. Tsirkin
@ 2013-02-26 11:54 ` Christian Borntraeger
2013-02-26 12:13 ` Christian Borntraeger
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Christian Borntraeger @ 2013-02-26 11:54 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Martin Schwidefsky, virtualization
On 26/02/13 12:18, Michael S. Tsirkin wrote:
> On Tue, Feb 26, 2013 at 01:04:21PM +0200, Michael S. Tsirkin wrote:
>> On Mon, Feb 25, 2013 at 04:27:45PM +0100, Cornelia Huck wrote:
>>> Here's the latest version of my patch series enabling ioeventfds
>>> on s390, again against kvm-next.
>>>
>>> Patches 1 and 2 (cleaning up initialization and exporting the virtio-ccw
>>> api) would make sense even independent of the ioeventfd enhancements.
>>>
>>> Patches 3-5 are concerned with adding a new type of ioeventfds for
>>> virtio-ccw notifications on s390. The naming is now hopefully clearer.
>>> We won't add ioeventfd support for the legacy s390-virtio transport.
>>>
>>> Please consider applying.
>>
>> I just had a thought: this makes us lookup the device on the bus
>> for each notification. It would be better to simply get the
>> device index from guest instead.
>>
>> We could validate that it matches the correct device,
>> if not - fallback to the current linear scan.
>>
>> We could return the index to guest for the next call.
>>
>> I know this needs guest changes but it's still not too late to
>> fix this for 3.9 guests so that we won't need to worry
>> about compatibility going forward.
>>
>> Hmm?
>
> And just to clarify, here's what I mean (BTW, why doesn't
> this code use the interfaces from kvm_para.h?)
> I think it's a good idea to merge this before 3.9 so we don't
> need to worry about legacy going forward.
>
> Completely untested, just to give you the idea.
Thinking more about it: Isnt the index on the kvm bus just an implementation
detail? In other words, what happens if we want to re-arrange the kvm io bus
to a tree like structure. Then the index becomes pretty much useless. Do we
really want to put such an internal thing into an interface?
CHristian
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.
2013-02-26 11:18 ` Michael S. Tsirkin
2013-02-26 11:54 ` Christian Borntraeger
@ 2013-02-26 12:13 ` Christian Borntraeger
2013-02-26 13:29 ` Cornelia Huck
` (2 more replies)
2013-02-27 19:49 ` Christian Borntraeger
2013-03-07 18:02 ` virtio-s390: document GPR4/GPR2 cookie values Michael S. Tsirkin
3 siblings, 3 replies; 15+ messages in thread
From: Christian Borntraeger @ 2013-02-26 12:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Martin Schwidefsky, virtualization
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>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.
2013-02-26 12:13 ` Christian Borntraeger
@ 2013-02-26 13:29 ` Cornelia Huck
2013-02-26 13:41 ` Michael S. Tsirkin
[not found] ` <20130226142907.3a38659f@gondolin>
2 siblings, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2013-02-26 13:29 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Carsten Otte, KVM, Michael S. Tsirkin, linux-s390, Heiko Carstens,
qemu-devel, Martin Schwidefsky, virtualization
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.
I'll do a v4 later.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.
2013-02-26 12:13 ` Christian Borntraeger
2013-02-26 13:29 ` Cornelia Huck
@ 2013-02-26 13:41 ` Michael S. Tsirkin
2013-02-26 13:48 ` Christian Borntraeger
[not found] ` <20130226142907.3a38659f@gondolin>
2 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-26 13:41 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Martin Schwidefsky, virtualization
On Tue, Feb 26, 2013 at 01:13:39PM +0100, Christian Borntraeger 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>
So let's apply the patch for 3.9 and avoid caring about
"old guests" much?
--
MST
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.
2013-02-26 13:41 ` Michael S. Tsirkin
@ 2013-02-26 13:48 ` Christian Borntraeger
2013-02-26 13:57 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Christian Borntraeger @ 2013-02-26 13:48 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Martin Schwidefsky, virtualization
On 26/02/13 14:41, Michael S. Tsirkin wrote:
>> So...
>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> So let's apply the patch for 3.9 and
Give us 1 or 2 days testing for regression and then this can go for 3.9.
The host changes can then be deferred to a later point in time.
> avoid caring about "old guests" much?
Well caring about old guests is related to caring about malicious guests.
Maybe we should make it explicit in the spec that the token can be wrong
or omitted. Just to avoid that anybody starts to optimize things too
aggressive.
Ok?
Christian
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.
2013-02-26 13:48 ` Christian Borntraeger
@ 2013-02-26 13:57 ` Michael S. Tsirkin
0 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-26 13:57 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Martin Schwidefsky, virtualization
On Tue, Feb 26, 2013 at 02:48:38PM +0100, Christian Borntraeger wrote:
> On 26/02/13 14:41, Michael S. Tsirkin wrote:
> >> So...
> >> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >
> > So let's apply the patch for 3.9 and
>
> Give us 1 or 2 days testing for regression and then this can go for 3.9.
> The host changes can then be deferred to a later point in time.
Nod.
> > avoid caring about "old guests" much?
>
> Well caring about old guests is related to caring about malicious guests.
> Maybe we should make it explicit in the spec that the token can be wrong
> or omitted. Just to avoid that anybody starts to optimize things too
> aggressive.
>
> Ok?
>
> Christian
Absolutely, we can't trust the guest anyway. It's just an optimization
hint, must validate and if it fails do a linear lookup.
--
MST
^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <20130226142907.3a38659f@gondolin>]
* Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.
[not found] ` <20130226142907.3a38659f@gondolin>
@ 2013-02-26 13:56 ` Michael S. Tsirkin
2013-02-26 14:05 ` Cornelia Huck
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-02-26 13:56 UTC (permalink / raw)
To: Cornelia Huck
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Christian Borntraeger, Martin Schwidefsky, virtualization
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.
Want me to write you a spec patch too?
> I'll do a v4 later.
Right, just return 0 and it'll work.
--
MST
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.
2013-02-26 13:56 ` Michael S. Tsirkin
@ 2013-02-26 14:05 ` Cornelia Huck
0 siblings, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2013-02-26 14:05 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Christian Borntraeger, Martin Schwidefsky, virtualization
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.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/5] kvm: Make ioeventfd usable on s390.
2013-02-26 11:18 ` Michael S. Tsirkin
2013-02-26 11:54 ` Christian Borntraeger
2013-02-26 12:13 ` Christian Borntraeger
@ 2013-02-27 19:49 ` Christian Borntraeger
2013-03-07 18:02 ` virtio-s390: document GPR4/GPR2 cookie values Michael S. Tsirkin
3 siblings, 0 replies; 15+ messages in thread
From: Christian Borntraeger @ 2013-02-27 19:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Martin Schwidefsky, virtualization
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>
Seems to work fine. (as expected).
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.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,
>
^ permalink raw reply [flat|nested] 15+ messages in thread* virtio-s390: document GPR4/GPR2 cookie values
2013-02-26 11:18 ` Michael S. Tsirkin
` (2 preceding siblings ...)
2013-02-27 19:49 ` Christian Borntraeger
@ 2013-03-07 18:02 ` Michael S. Tsirkin
2013-03-08 7:55 ` Cornelia Huck
3 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2013-03-07 18:02 UTC (permalink / raw)
To: Cornelia Huck
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Christian Borntraeger, Martin Schwidefsky, virtualization
virtio-s390 on kvm can use a cookie value passed to guest
to optimize channel/VQ lookups.
Document this.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 72d956c..91aed06 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -10627,7 +10626,252 @@ Guest->Host Notification
For notifying the host of virtqueue buffers, the guest unfortunately can't
use a channel command (the asynchronous characteristics of channel I/O
interact badly with the host block I/O backend).
- Instead, it uses a diagnose 0x500 call with subcode 3 specifying the queue.
+ Instead, it uses a diagnose 0x500 call with subcode 3 specifying the queue
+\change_inserted 1986246365 1362677938
+, as follows:
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 1986246365 1362677973
+\begin_inset Tabular
+<lyxtabular version="3" rows="5" columns="3">
+<features tabularvalignment="middle">
+<column alignment="center" valignment="top" width="0">
+<column alignment="center" valignment="top" width="0">
+<column alignment="center" valignment="top" width="0">
+<row>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362677978
+GPR
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678629
+Input Value
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678627
+Output Value
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362677991
+1
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678208
+0x3
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678620
+
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678226
+2
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678498
+Subchannel ID
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678642
+Host Cookie
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678500
+3
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678540
+Virtqueue number
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678620
+
+\end_layout
+
+\end_inset
+</cell>
+</row>
+<row>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678529
+4
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678644
+Host Cookie
+\change_unchanged
+
+\end_layout
+
+\end_inset
+</cell>
+<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" rightline="true" usebox="none">
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362678620
+
+\end_layout
+
+\end_inset
+</cell>
+</row>
+</lyxtabular>
+
+\end_inset
+
+
+\change_deleted 1986246365 1362677938
+.
+\change_inserted 1986246365 1362678646
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 1986246365 1362679010
+Host cookie is an optional per-virtqueue 64 bit value that can be used by
+ the hypervisor to speed up the notification execution.
+ For each notification, the output value is returned in GPR2 and should
+ be passed in GPR4 for the next notification:
+\end_layout
+
+\begin_layout LyX-Code
+
+\change_inserted 1986246365 1362679011
+\begin_inset listings
+inline false
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 1986246365 1362679033
+
+info->cookie = do_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
+\end_layout
+
+\end_inset
+
+
\end_layout
\begin_layout Subsubsection*
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: virtio-s390: document GPR4/GPR2 cookie values
2013-03-07 18:02 ` virtio-s390: document GPR4/GPR2 cookie values Michael S. Tsirkin
@ 2013-03-08 7:55 ` Cornelia Huck
2013-03-12 3:47 ` Rusty Russell
0 siblings, 1 reply; 15+ messages in thread
From: Cornelia Huck @ 2013-03-08 7:55 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Christian Borntraeger, Martin Schwidefsky, virtualization
On Thu, 7 Mar 2013 20:02:21 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> virtio-s390 on kvm can use a cookie value passed to guest
s/virtio-s390/virtio-ccw/ (to avoid confusion with s390-virtio, which
was never specced)
> to optimize channel/VQ lookups.
> Document this.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Otherwise:
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Rusty, could you please apply?
>
> ---
> diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> index 72d956c..91aed06 100644
> --- a/virtio-spec.lyx
> +++ b/virtio-spec.lyx
> @@ -10627,7 +10626,252 @@ Guest->Host Notification
> For notifying the host of virtqueue buffers, the guest unfortunately can't
> use a channel command (the asynchronous characteristics of channel I/O
> interact badly with the host block I/O backend).
> - Instead, it uses a diagnose 0x500 call with subcode 3 specifying the queue.
> + Instead, it uses a diagnose 0x500 call with subcode 3 specifying the queue
> +\change_inserted 1986246365 1362677938
> +, as follows:
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 1986246365 1362677973
> +\begin_inset Tabular
> +<lyxtabular version="3" rows="5" columns="3">
> +<features tabularvalignment="middle">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<row>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362677978
> +GPR
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678629
> +Input Value
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678627
> +Output Value
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +<row>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362677991
> +1
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678208
> +0x3
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678620
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +<row>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678226
> +2
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678498
> +Subchannel ID
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678642
> +Host Cookie
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +<row>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678500
> +3
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678540
> +Virtqueue number
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678620
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +<row>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678529
> +4
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678644
> +Host Cookie
> +\change_unchanged
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +<cell alignment="center" valignment="top" topline="true" bottomline="true" leftline="true" rightline="true" usebox="none">
> +\begin_inset Text
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362678620
> +
> +\end_layout
> +
> +\end_inset
> +</cell>
> +</row>
> +</lyxtabular>
> +
> +\end_inset
> +
> +
> +\change_deleted 1986246365 1362677938
> +.
> +\change_inserted 1986246365 1362678646
> +
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 1986246365 1362679010
> +Host cookie is an optional per-virtqueue 64 bit value that can be used by
> + the hypervisor to speed up the notification execution.
> + For each notification, the output value is returned in GPR2 and should
> + be passed in GPR4 for the next notification:
> +\end_layout
> +
> +\begin_layout LyX-Code
> +
> +\change_inserted 1986246365 1362679011
> +\begin_inset listings
> +inline false
> +status open
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 1986246365 1362679033
> +
> +info->cookie = do_notify(schid, virtqueue_get_queue_index(vq), info->cookie);
> +\end_layout
> +
> +\end_inset
> +
> +
> \end_layout
>
> \begin_layout Subsubsection*
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: virtio-s390: document GPR4/GPR2 cookie values
2013-03-08 7:55 ` Cornelia Huck
@ 2013-03-12 3:47 ` Rusty Russell
0 siblings, 0 replies; 15+ messages in thread
From: Rusty Russell @ 2013-03-12 3:47 UTC (permalink / raw)
To: Cornelia Huck, Michael S. Tsirkin
Cc: Carsten Otte, KVM, linux-s390, Heiko Carstens, qemu-devel,
Christian Borntraeger, Martin Schwidefsky, virtualization
Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> On Thu, 7 Mar 2013 20:02:21 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> virtio-s390 on kvm can use a cookie value passed to guest
>
> s/virtio-s390/virtio-ccw/ (to avoid confusion with s390-virtio, which
> was never specced)
>
>> to optimize channel/VQ lookups.
>> Document this.
>>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Otherwise:
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>
> Rusty, could you please apply?
Modified and committed.
Thanks!
Rusty.
^ permalink raw reply [flat|nested] 15+ messages in thread