[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio: allow broken device to notify guest
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PATCH] virtio: allow broken device to notify guest |
Date: |
Wed, 26 Apr 2017 15:15:48 +0200 |
On Wed, 26 Apr 2017 14:51:17 +0200
Greg Kurz <address@hidden> wrote:
> According to section 2.1.2 of the virtio-1 specification:
>
> "The device SHOULD set DEVICE_NEEDS_RESET when it enters an error state that
> a reset is needed. If DRIVER_OK is set, after it sets DEVICE_NEEDS_RESET,
> the device MUST send a device configuration change notification to the
> driver."
>
> Commit "f5ed36635d8f virtio: stop virtqueue processing if device is broken"
> introduced a virtio_error() call that just does that:
>
> - internally mark the device as broken
> - set the DEVICE_NEEDS_RESET bit in the status
> - send a configuration change notification
>
> Unfortunately, virtio_notify_vector(), called by virtio_notify_config(),
> returns right away when the device is marked as broken and the notification
> isn't sent in this case.
>
> The spec doesn't say whether a broken device can send notifications
> in other situations or not. But since the driver isn't supposed to do
> anything but to reset the device, it makes sense to keep the check in
> virtio_notify_config().
>
> Marking the device as broken AFTER the configuration change notification was
> sent is enough to fix the issue.
>
> Signed-off-by: Greg Kurz <address@hidden>
> ---
> hw/virtio/virtio.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 03592c542a55..890b4d7eb751 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2451,12 +2451,12 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice
> *vdev, const char *fmt, ...)
> error_vreport(fmt, ap);
> va_end(ap);
>
> - vdev->broken = true;
> -
> if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> virtio_set_status(vdev, vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET);
> virtio_notify_config(vdev);
> }
> +
> + vdev->broken = true;
> }
>
> static void virtio_memory_listener_commit(MemoryListener *listener)
>
Good catch.
Reviewed-by: Cornelia Huck <address@hidden>
Should this be cc:stable, as it's a spec violation?