Re: [Qemu-devel] [PATCH] vhost-scsi: Update 'ioeventfd_started' with host notifiers

[Felipe Franciosi]

Hello,

> On 7 Nov 2016, at 18:23, Felipe Franciosi <address@hidden> wrote:
> 
> Following the recent refactor of virtio notfiers [1], more specifically
> the patch that uses virtio_bus_set_host_notifier [2] by default, core
> virtio code requires 'ioeventfd_started' to be set to true/false when
> the host notifiers are configured. Since vhost-scsi uses the legacy
> interface, this value is not updated.
> 
> When booting a guest with a vhost-scsi backend controller, SeaBIOS will
> initially configure the device which sets all notifiers. The guest will
> continue to boot fine until the kernel virtio-scsi module reinitialises
> the device causing a stop followed by another start. Since
> ioeventfd_started was never set to true, the 'stop' operation triggered
> by virtio_bus_set_host_notifier() will not result in a call to
> virtio_pci_ioeventfd_assign(assign=false). This leaves the memory
> regions with stale notifiers and results on the next start triggering
> the following assertion:
> 
>  kvm_mem_ioeventfd_add: error adding ioeventfd: File exists
>  Aborted
> 
> This patch updates ioeventfd_started whenever the notifiers are set or
> cleared, fixing this issue.
> 
> Signed-off-by: Felipe Franciosi <address@hidden>
> 
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07748.html
> [2] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07760.html
> ---
> hw/scsi/vhost-scsi.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index 5b26946..1c6e6d4 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -95,6 +95,7 @@ static int vhost_scsi_start(VHostSCSI *s)
>     if (ret < 0) {
>         return ret;
>     }
> +    VIRTIO_BUS(qbus)->ioeventfd_started = true;

I'm not sure if it's safe to do this from vhost_dev_disable/enable_notifiers() 
directly. If you'd rather this is fixed there, please let me know and I'll send 
a v2. This e-mail is basically to flag a problem which I recently came across 
when working on vhost-user-scsi. Basically my code stopped working and I didn't 
know why, so I rolled back to test a traditional vhost-scsi on master and found 
this.

I have also not tested vhost-sock, but I imagine it might suffer from the same 
issue (even if it doesn't manifest itself this easily). Any other cases you can 
think of?

Thanks,
Felipe

> 
>     s->dev.acked_features = vdev->guest_features;
>     ret = vhost_dev_start(&s->dev, vdev);
> @@ -152,6 +153,7 @@ static void vhost_scsi_stop(VHostSCSI *s)
>     vhost_scsi_clear_endpoint(s);
>     vhost_dev_stop(&s->dev, vdev);
>     vhost_dev_disable_notifiers(&s->dev, vdev);
> +    VIRTIO_BUS(qbus)->ioeventfd_started = false;
> }
> 
> static uint64_t vhost_scsi_get_features(VirtIODevice *vdev,
> -- 
> 1.9.4
>