On 10/11/2016 15:35, Christian Borntraeger wrote:
> On 10/30/2016 10:23 PM, Michael S. Tsirkin wrote:
>> From: Paolo Bonzini <pbonz...@redhat.com>
>>
>> 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 <cornelia.h...@de.ibm.com>
>> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>> Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
>> ---
>> 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);
>> }
>>
>
