From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46487) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c4qVO-0007MO-P3 for qemu-devel@nongnu.org; Thu, 10 Nov 2016 09:39:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c4qVL-0004Lh-J8 for qemu-devel@nongnu.org; Thu, 10 Nov 2016 09:39:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55864) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c4qVL-0004LA-9E for qemu-devel@nongnu.org; Thu, 10 Nov 2016 09:39:03 -0500 References: <1477850917-1214-1-git-send-email-mst@redhat.com> <1477850917-1214-13-git-send-email-mst@redhat.com> <925e80d1-f936-4519-9490-9c40d129392e@de.ibm.com> From: Paolo Bonzini Message-ID: <75f9936a-443a-79ee-8e44-ff4b93ab66bc@redhat.com> Date: Thu, 10 Nov 2016 15:38:57 +0100 MIME-Version: 1.0 In-Reply-To: <925e80d1-f936-4519-9490-9c40d129392e@de.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 12/47] virtio: remove ioeventfd_disabled altogether List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger , "Michael S. Tsirkin" , qemu-devel@nongnu.org Cc: Cornelia Huck , Peter Maydell On 10/11/2016 15:35, Christian Borntraeger wrote: > On 10/30/2016 10:23 PM, Michael S. Tsirkin wrote: >> From: Paolo Bonzini >> >> Now that there is not anymore a switch from the generic ioeventfd handler >> to the dataplane handler, virtio_bus_set_host_notifier(assign=true) is >> always called with !bus->ioeventfd_started, hence virtio_bus_stop_ioeventfd >> does nothing in this case. Move the invocation to vhost.c, which is the >> only place that needs it. >> >> Reviewed-by: Cornelia Huck >> Signed-off-by: Paolo Bonzini >> Reviewed-by: Michael S. Tsirkin >> Signed-off-by: Michael S. Tsirkin >> --- >> include/hw/virtio/virtio-bus.h | 6 ------ >> hw/virtio/vhost.c | 3 +++ >> hw/virtio/virtio-bus.c | 23 ++++++++--------------- >> 3 files changed, 11 insertions(+), 21 deletions(-) > > This breaks vhost-net for s390/kvm after rebooting the guest. (ping fails and > ifconfig shows no packets is TXed) > > Any idea? Patch from Felipe: [PATCH v2] vhost: Update 'ioeventfd_started' with host notifiers Paolo >> >> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h >> index af6b5c4..cbdf745 100644 >> --- a/include/hw/virtio/virtio-bus.h >> +++ b/include/hw/virtio/virtio-bus.h >> @@ -94,12 +94,6 @@ struct VirtioBusState { >> BusState parent_obj; >> >> /* >> - * Set if the default ioeventfd handlers are disabled by vhost >> - * or dataplane. >> - */ >> - bool ioeventfd_disabled; >> - >> - /* >> * Set if ioeventfd has been started. >> */ >> bool ioeventfd_started; >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c >> index 501a5f4..131f164 100644 >> --- a/hw/virtio/vhost.c >> +++ b/hw/virtio/vhost.c >> @@ -1196,6 +1196,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) >> goto fail; >> } >> >> + virtio_device_stop_ioeventfd(vdev); >> for (i = 0; i < hdev->nvqs; ++i) { >> r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, >> true); >> @@ -1215,6 +1216,7 @@ fail_vq: >> } >> assert (e >= 0); >> } >> + virtio_device_start_ioeventfd(vdev); >> fail: >> return r; >> } >> @@ -1237,6 +1239,7 @@ void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) >> } >> assert (r >= 0); >> } >> + virtio_device_start_ioeventfd(vdev); >> } >> >> /* Test and clear event pending status. >> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c >> index a619445..b0e4544 100644 >> --- a/hw/virtio/virtio-bus.c >> +++ b/hw/virtio/virtio-bus.c >> @@ -190,7 +190,7 @@ int virtio_bus_start_ioeventfd(VirtioBusState *bus) >> if (!k->ioeventfd_assign || !k->ioeventfd_enabled(proxy)) { >> return -ENOSYS; >> } >> - if (bus->ioeventfd_started || bus->ioeventfd_disabled) { >> + if (bus->ioeventfd_started) { >> return 0; >> } >> r = vdc->start_ioeventfd(vdev); >> @@ -226,8 +226,8 @@ bool virtio_bus_ioeventfd_enabled(VirtioBusState *bus) >> } >> >> /* >> - * This function switches from/to the generic ioeventfd handler. >> - * assign==false means 'use generic ioeventfd handler'. >> + * This function switches ioeventfd on/off in the device. >> + * The caller must set or clear the handlers for the EventNotifier. >> */ >> int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign) >> { >> @@ -237,19 +237,12 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign) >> if (!k->ioeventfd_assign) { >> return -ENOSYS; >> } >> - bus->ioeventfd_disabled = assign; >> if (assign) { >> - /* >> - * Stop using the generic ioeventfd, we are doing eventfd handling >> - * ourselves below >> - * >> - * FIXME: We should just switch the handler and not deassign the >> - * ioeventfd. >> - * Otherwise, there's a window where we don't have an >> - * ioeventfd and we may end up with a notification where >> - * we don't expect one. >> - */ >> - virtio_bus_stop_ioeventfd(bus); >> + assert(!bus->ioeventfd_started); >> + } else { >> + if (!bus->ioeventfd_started) { >> + return 0; >> + } >> } >> return set_host_notifier_internal(proxy, bus, n, assign); >> } >> >