Re: [PATCH v6 03/13] vdpa: Sync calls set/get config/status with cf_mutex

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/22/2021 9:46 PM, Eli Cohen wrote:
On Wed, Dec 22, 2021 at 02:58:18PM -0800, Si-Wei Liu wrote:

On 12/22/2021 6:20 AM, Eli Cohen wrote:
Add wrappers to get/set status and protect these operations with
cf_mutex to serialize these operations with respect to get/set config
operations.
Need to protect vdpa_reset() which is essentially vdpa_set_status(0)
And what about vdpa_set_features()?
Oh, that was missed... Indeed, it also needs to take the lock.

Maybe it should be done as part of another patch to address this?
Isn't this patch the one adding the helpers for the locked API? I think it's fine if you'd like specific ones for reset or set_features.


Shouldn't all these serializations requirements be handled by the caller?
If the caller remains in the vdpa core I think it should be fine. Or you want the vdpa bus drivers to take the serialization requirement? Perhaps not so good.

Thanks,
-Siwei


-Siwei
Signed-off-by: Eli Cohen <elic@xxxxxxxxxx>
---
   drivers/vdpa/vdpa.c          | 19 +++++++++++++++++++
   drivers/vhost/vdpa.c         |  7 +++----
   drivers/virtio/virtio_vdpa.c |  3 +--
   include/linux/vdpa.h         |  3 +++
   4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 42d71d60d5dc..5134c83c4a22 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -21,6 +21,25 @@ static LIST_HEAD(mdev_head);
   static DEFINE_MUTEX(vdpa_dev_mutex);
   static DEFINE_IDA(vdpa_index_ida);
+u8 vdpa_get_status(struct vdpa_device *vdev)
+{
+	u8 status;
+
+	mutex_lock(&vdev->cf_mutex);
+	status = vdev->config->get_status(vdev);
+	mutex_unlock(&vdev->cf_mutex);
+	return status;
+}
+EXPORT_SYMBOL(vdpa_get_status);
+
+void vdpa_set_status(struct vdpa_device *vdev, u8 status)
+{
+	mutex_lock(&vdev->cf_mutex);
+	vdev->config->set_status(vdev, status);
+	mutex_unlock(&vdev->cf_mutex);
+}
+EXPORT_SYMBOL(vdpa_set_status);
+
   static struct genl_family vdpa_nl_family;
   static int vdpa_dev_probe(struct device *d)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index ebaa373e9b82..d9d499465e2e 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -142,10 +142,9 @@ static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp)
   static long vhost_vdpa_get_status(struct vhost_vdpa *v, u8 __user *statusp)
   {
   	struct vdpa_device *vdpa = v->vdpa;
-	const struct vdpa_config_ops *ops = vdpa->config;
   	u8 status;
-	status = ops->get_status(vdpa);
+	status = vdpa_get_status(vdpa);
   	if (copy_to_user(statusp, &status, sizeof(status)))
   		return -EFAULT;
@@ -164,7 +163,7 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
   	if (copy_from_user(&status, statusp, sizeof(status)))
   		return -EFAULT;
-	status_old = ops->get_status(vdpa);
+	status_old = vdpa_get_status(vdpa);
   	/*
   	 * Userspace shouldn't remove status bits unless reset the
@@ -182,7 +181,7 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp)
   		if (ret)
   			return ret;
   	} else
-		ops->set_status(vdpa, status);
+		vdpa_set_status(vdpa, status);
   	if ((status & VIRTIO_CONFIG_S_DRIVER_OK) && !(status_old & VIRTIO_CONFIG_S_DRIVER_OK))
   		for (i = 0; i < nvqs; i++)
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index a84b04ba3195..76504559bc25 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -91,9 +91,8 @@ static u8 virtio_vdpa_get_status(struct virtio_device *vdev)
   static void virtio_vdpa_set_status(struct virtio_device *vdev, u8 status)
   {
   	struct vdpa_device *vdpa = vd_get_vdpa(vdev);
-	const struct vdpa_config_ops *ops = vdpa->config;
-	return ops->set_status(vdpa, status);
+	return vdpa_set_status(vdpa, status);
   }
   static void virtio_vdpa_reset(struct virtio_device *vdev)
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 9cc4291a79b3..ae047fae2603 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -408,6 +408,9 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
   		     void *buf, unsigned int len);
   void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
   		     const void *buf, unsigned int length);
+u8 vdpa_get_status(struct vdpa_device *vdev);
+void vdpa_set_status(struct vdpa_device *vdev, u8 status);
+
   /**
    * struct vdpa_mgmtdev_ops - vdpa device ops
    * @dev_add: Add a vdpa device using alloc and register

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization



[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux