On Wed, 26 Apr 2017 15:15:48 +0200 Cornelia Huck <cornelia.h...@de.ibm.com> wrote:
> On Wed, 26 Apr 2017 14:51:17 +0200 > Greg Kurz <gr...@kaod.org> 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 <gr...@kaod.org> > > --- > > 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 <cornelia.h...@de.ibm.com> > > Should this be cc:stable, as it's a spec violation? > I don't know if this qualifies for stable, but if it does then it affects all versions >= 2.8.0.
pgpPpp254wLM8.pgp
Description: OpenPGP digital signature