Re: [Qemu-devel] Qemu-devel Digest, Vol 149, Issue 266

Re: [Qemu-devel] Qemu-devel Digest, Vol 149, Issue 266

[Ouyang, Changchun]

> -----Original Message-----
> Date: Wed, 12 Aug 2015 14:15:54 +0300
> From: "Michael S. Tsirkin" <mst@redhat.com>
> To: Marcel Apfelbaum <marcel@redhat.com>
> Cc: qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH] virtio/vhost: drop unnecessary
> 	VHOST_SET_VRING call
> Message-ID: <20150812141448-mutt-send-email-mst@redhat.com>
> Content-Type: text/plain; charset=us-ascii
> 
> On Wed, Aug 12, 2015 at 02:10:56PM +0300, Marcel Apfelbaum wrote:
> > On 08/12/2015 01:34 PM, Michael S. Tsirkin wrote:
> > >On Wed, Aug 12, 2015 at 01:19:51PM +0300, Marcel Apfelbaum wrote:
> > >>No need to send VHOST_SET_VRING_CALL to backend before the
> > >>negotiation with the guest is finished.
> > >>
> > >>Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> > >
> > >Well - we do need to set it to the masked notifier initially to avoid
> > >losing events.  You can't just drop it - need to move this call
> > >somewhere else.

Agree with m.s.t.
We could not drop it.
Vhost-user multi queue also need this.

> > What do we need to set?
> > I just dropped the call to VHOST_SET_VRING_CALL.
> >
> > Thanks,
> > Marcel
> 
> We use two eventfds: masked and unmasked one.
> We switch dynamically dependent on msi mask value.
> Code assumes we start out masked, so we need to match that.
> 
> 
> > >
> > >>---
> > >>  hw/virtio/vhost.c | 13 +------------
> > >>  1 file changed, 1 insertion(+), 12 deletions(-)
> > >>
> > >>diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index
> > >>2712c6f..b448542 100644
> > >>--- a/hw/virtio/vhost.c
> > >>+++ b/hw/virtio/vhost.c
> > >>@@ -875,24 +875,13 @@ static void
> vhost_eventfd_del(MemoryListener
> > >>*listener,
> > >>  static int vhost_virtqueue_init(struct vhost_dev *dev,
> > >>                                  struct vhost_virtqueue *vq, int n)
> > >>  {
> > >>-    struct vhost_vring_file file = {
> > >>-        .index = n,
> > >>-    };
> > >>      int r = event_notifier_init(&vq->masked_notifier, 0);
> > >>+
> > >>      if (r < 0) {
> > >>          return r;
> > >>      }
> > >>
> > >>-    file.fd = event_notifier_get_fd(&vq->masked_notifier);
> > >>-    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_CALL, &file);
> > >>-    if (r) {
> > >>-        r = -errno;
> > >>-        goto fail_call;
> > >>-    }
> > >>      return 0;
> > >>-fail_call:
> > >>-    event_notifier_cleanup(&vq->masked_notifier);
> > >>-    return r;
> > >>  }
> > >>
> > >>  static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
> > >>--
> > >>2.1.0
> 
> 

Re: [Qemu-devel] Qemu-devel Digest, Vol 149, Issue 266

[Marcel Apfelbaum]

On 08/13/2015 08:32 AM, Ouyang, Changchun wrote:
>
>
>> -----Original Message-----
>> Date: Wed, 12 Aug 2015 14:15:54 +0300
>> From: "Michael S. Tsirkin" <mst@redhat.com>
>> To: Marcel Apfelbaum <marcel@redhat.com>
>> Cc: qemu-devel@nongnu.org
>> Subject: Re: [Qemu-devel] [PATCH] virtio/vhost: drop unnecessary
>> 	VHOST_SET_VRING call
>> Message-ID: <20150812141448-mutt-send-email-mst@redhat.com>
>> Content-Type: text/plain; charset=us-ascii
>>
>> On Wed, Aug 12, 2015 at 02:10:56PM +0300, Marcel Apfelbaum wrote:
>>> On 08/12/2015 01:34 PM, Michael S. Tsirkin wrote:
>>>> On Wed, Aug 12, 2015 at 01:19:51PM +0300, Marcel Apfelbaum wrote:
>>>>> No need to send VHOST_SET_VRING_CALL to backend before the
>>>>> negotiation with the guest is finished.
>>>>>
>>>>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>>>>
>>>> Well - we do need to set it to the masked notifier initially to avoid
>>>> losing events.  You can't just drop it - need to move this call
>>>> somewhere else.
>
> Agree with m.s.t.
> We could not drop it.
> Vhost-user multi queue also need this.
I'll try to call it a little bit later instead of dropping it.

Thanks,
Marcel

>
>>> What do we need to set?
>>> I just dropped the call to VHOST_SET_VRING_CALL.
>>>
>>> Thanks,
>>> Marcel
>>
>> We use two eventfds: masked and unmasked one.
>> We switch dynamically dependent on msi mask value.
>> Code assumes we start out masked, so we need to match that.
>>
>>
>>>>
>>>>> ---
>>>>>   hw/virtio/vhost.c | 13 +------------
>>>>>   1 file changed, 1 insertion(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index
>>>>> 2712c6f..b448542 100644
>>>>> --- a/hw/virtio/vhost.c
>>>>> +++ b/hw/virtio/vhost.c
>>>>> @@ -875,24 +875,13 @@ static void
>> vhost_eventfd_del(MemoryListener
>>>>> *listener,
>>>>>   static int vhost_virtqueue_init(struct vhost_dev *dev,
>>>>>                                   struct vhost_virtqueue *vq, int n)
>>>>>   {
>>>>> -    struct vhost_vring_file file = {
>>>>> -        .index = n,
>>>>> -    };
>>>>>       int r = event_notifier_init(&vq->masked_notifier, 0);
>>>>> +
>>>>>       if (r < 0) {
>>>>>           return r;
>>>>>       }
>>>>>
>>>>> -    file.fd = event_notifier_get_fd(&vq->masked_notifier);
>>>>> -    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_CALL, &file);
>>>>> -    if (r) {
>>>>> -        r = -errno;
>>>>> -        goto fail_call;
>>>>> -    }
>>>>>       return 0;
>>>>> -fail_call:
>>>>> -    event_notifier_cleanup(&vq->masked_notifier);
>>>>> -    return r;
>>>>>   }
>>>>>
>>>>>   static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
>>>>> --
>>>>> 2.1.0
>>
>>