Re: [PATCH master] virtio: pci: don't re-enable with 0 as argument

[Sascha Hauer <sha@pengutronix.de>]

On Wed, Jan 03, 2024 at 11:13:54AM +0100, Ahmad Fatoum wrote:
> Qemu prints an error during barebox shutdown when virtio-net was used:
> 
>   qemu-system-aarch64: wrong value for queue_enable 0
> 
> This warning was added a few years back in Qemu commit 10d35e5819:
> 
>  | virtio-pci: fix queue_enable write
>  |
>  | Spec said: The driver uses this to selectively prevent the device from
>  | executing requests from this virtqueue. 1 - enabled; 0 - disabled.
>  |
>  | Though write 0 to queue_enable is forbidden by the spec, we should not
>  | assume that the value is 1.
>  |
>  | Fix this by ignore the write value other than 1.
>  |
>  | Signed-off-by: Jason Wang <jasowang@redhat.com>
>  | Message-Id: <20200610054351.15811-1-jasowang@redhat.com>
>  | Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>  | Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>  | Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>  | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>  | Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>  | Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Even older Qemu versions actually marked the virtqueue enabled when
> queue_enable was written with any value, we should really stop writing
> anything, but 1 into queue_enable in the removal path.
> 
> We already reset before deleting the virtqueues, which disables them.
> This aligns us with what Linux is doing, except that Linux has some
> MSIX cleanup logic in virtio_pci_del_vq. We don't have that, but we
> will keep the function anyway to simplify future synchronization.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/virtio/virtio_pci_modern.c | 8 --------
>  1 file changed, 8 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 26eefba85bea..2dd369b02e9a 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -223,14 +223,6 @@ static struct virtqueue *virtio_pci_setup_vq(struct virtio_device *vdev,
>  
>  static void virtio_pci_del_vq(struct virtqueue *vq)
>  {
> -	struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
> -	unsigned int index = vq->index;
> -
> -	iowrite16(index, &vp_dev->common->queue_select);
> -
> -	/* Select and deactivate the queue */
> -	iowrite16(0, &vp_dev->common->queue_enable);
> -
>  	vring_del_virtqueue(vq);
>  }
>  
> -- 
> 2.39.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |