On Thu, Jan 13, 2022 at 12:10:49AM -0500, Si-Wei Liu wrote:
> No functional change introduced. vdpa bus driver such as virtio_vdpa
> or vhost_vdpa is not supposed to take care of the locking for core
> by its own. The locked API vdpa_set_features should suffice the
> bus driver's need.
> 
> Signed-off-by: Si-Wei Liu<[email protected]>

Not sure I get the explanation here. But I like separate APIs better
than a flag, so there.


> ---
>  drivers/vdpa/vdpa.c          |  2 +-
>  drivers/vhost/vdpa.c         |  2 +-
>  drivers/virtio/virtio_vdpa.c |  2 +-
>  include/linux/vdpa.h         | 18 ++++++++++++------
>  4 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 9846c9d..1ea5254 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -393,7 +393,7 @@ static void vdpa_get_config_unlocked(struct vdpa_device 
> *vdev,
>        * If it does happen we assume a legacy guest.
>        */
>       if (!vdev->features_valid)
> -             vdpa_set_features(vdev, 0, true);
> +             vdpa_set_features_unlocked(vdev, 0);
>       ops->get_config(vdev, offset, buf, len);
>  }
>  
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 8515398..ec5249e 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -286,7 +286,7 @@ static long vhost_vdpa_set_features(struct vhost_vdpa *v, 
> u64 __user *featurep)
>       if (copy_from_user(&features, featurep, sizeof(features)))
>               return -EFAULT;
>  
> -     if (vdpa_set_features(vdpa, features, false))
> +     if (vdpa_set_features(vdpa, features))
>               return -EINVAL;
>  
>       return 0;
> diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> index 7767a7f..7650455 100644
> --- a/drivers/virtio/virtio_vdpa.c
> +++ b/drivers/virtio/virtio_vdpa.c
> @@ -317,7 +317,7 @@ static int virtio_vdpa_finalize_features(struct 
> virtio_device *vdev)
>       /* Give virtio_ring a chance to accept features. */
>       vring_transport_features(vdev);
>  
> -     return vdpa_set_features(vdpa, vdev->features, false);
> +     return vdpa_set_features(vdpa, vdev->features);
>  }
>  
>  static const char *virtio_vdpa_bus_name(struct virtio_device *vdev)
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 2de442e..721089b 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -401,18 +401,24 @@ static inline int vdpa_reset(struct vdpa_device *vdev)
>       return ret;
>  }
>  
> -static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features, 
> bool locked)
> +static inline int vdpa_set_features_unlocked(struct vdpa_device *vdev, u64 
> features)
>  {
>       const struct vdpa_config_ops *ops = vdev->config;
>       int ret;
>  
> -     if (!locked)
> -             mutex_lock(&vdev->cf_mutex);
> -
>       vdev->features_valid = true;
>       ret = ops->set_driver_features(vdev, features);
> -     if (!locked)
> -             mutex_unlock(&vdev->cf_mutex);
> +
> +     return ret;
> +}
> +
> +static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
> +{
> +     int ret;
> +
> +     mutex_lock(&vdev->cf_mutex);
> +     ret = vdpa_set_features_unlocked(vdev, features);
> +     mutex_unlock(&vdev->cf_mutex);
>  
>       return ret;
>  }
> -- 
> 1.8.3.1

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to