Re: [PATCH] virtio_config: add device ready status check

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Mar 15, 2024 at 06:31:25PM +0800, gavin.liu wrote:
> From: Yuxue Liu <yuxue.liu@jaguarmicro.com>
> 
> Under the VDPA framework, when the VDPA driver of a device sets the status
> to DRIVER OK,success is not guaranteed. The virtio driver should provide
> some protection or notification.
> 
> Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>
> ---
>  drivers/block/virtio_blk.c    |  4 +++-
>  drivers/net/virtio_net.c      |  7 ++++++-
>  include/linux/virtio_config.h | 14 ++++++++++++++
>  3 files changed, 23 insertions(+), 2 deletions(-)


And then what we are expected to change all drivers for this hack to
work? Suggest you instead have vdpa set broken flag and be done with it.

> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 42dea7601d87..14cabf662705 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -1535,7 +1535,9 @@ static int virtblk_probe(struct virtio_device *vdev)
>  		set_disk_ro(vblk->disk, 1);
>  
>  	virtblk_update_capacity(vblk, false);
> -	virtio_device_ready(vdev);
> +	err = virtio_device_ready_safe(vdev);
> +	if (err)
> +		goto out_cleanup_disk;
>  
>  	/*
>  	 * All steps that follow use the VQs therefore they need to be
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index d7ce4a1011ea..0431fc9095ae 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -4808,7 +4808,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>  		goto free_failover;
>  	}
>  
> -	virtio_device_ready(vdev);
> +	err = virtio_device_ready_safe(vdev);
> +	if (err) {
> +		pr_debug("virtio_net: setting device ready failed\n");
> +		rtnl_unlock();
> +		goto free_unregister_netdev;
> +	}
>  
>  	_virtnet_set_queues(vi, vi->curr_queue_pairs);
>  
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index da9b271b54db..de4e864952ef 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -301,6 +301,20 @@ void virtio_device_ready(struct virtio_device *dev)
>  	dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
>  }
>  
> +static inline
> +int virtio_device_ready_safe(struct virtio_device *dev)
> +{
> +	u8 status;
> +
> +	virtio_device_ready(dev);
> +	status = dev->config->get_status(dev);
> +	if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> +		WARN_ON(1);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
>  static inline
>  const char *virtio_bus_name(struct virtio_device *vdev)
>  {
> -- 
> 2.43.0