Re: [RFC PATCH 10/10] vhost/vdpa: return configuration bytes read and written to user space

On Tue, Mar 02, 2021 at 12:05:35PM +0800, Jason Wang wrote:

On 2021/2/16 5:44 下午, Stefano Garzarella wrote:

quoted

vdpa_get_config() and vdpa_set_config() now return the amount
of bytes read and written, so let's return them to the user space.

We also modify vhost_vdpa_config_validate() to return 0 (bytes read
or written) instead of an error, when the buffer length is 0.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vhost/vdpa.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 21eea2be5afa..b754c53171a7 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c

@@ -191,9 +191,6 @@ static ssize_t vhost_vdpa_config_validate(struct vhost_vdpa *v,
 	struct vdpa_device *vdpa = v->vdpa;
 	u32 size = vdpa->config->get_config_size(vdpa);
-	if (c->len == 0)
-		return -EINVAL;
-
 	return min(c->len, size);
 }

@@ -204,6 +201,7 @@ static long vhost_vdpa_get_config(struct vhost_vdpa *v,
 	struct vhost_vdpa_config config;
 	unsigned long size = offsetof(struct vhost_vdpa_config, buf);
 	ssize_t config_size;
+	long ret;
 	u8 *buf;
 	if (copy_from_user(&config, c, size))

@@ -217,15 +215,18 @@ static long vhost_vdpa_get_config(struct vhost_vdpa *v,
 	if (!buf)
 		return -ENOMEM;
-	vdpa_get_config(vdpa, config.off, buf, config_size);
-
-	if (copy_to_user(c->buf, buf, config_size)) {
-		kvfree(buf);
-		return -EFAULT;
+	ret = vdpa_get_config(vdpa, config.off, buf, config_size);
+	if (ret < 0) {
+		ret = -EFAULT;
+		goto out;
 	}
+	if (copy_to_user(c->buf, buf, config_size))
+		ret = -EFAULT;
+
+out:
 	kvfree(buf);
-	return 0;
+	return ret;
 }
 static long vhost_vdpa_set_config(struct vhost_vdpa *v,

@@ -235,6 +236,7 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
 	struct vhost_vdpa_config config;
 	unsigned long size = offsetof(struct vhost_vdpa_config, buf);
 	ssize_t config_size;
+	long ret;
 	u8 *buf;
 	if (copy_from_user(&config, c, size))

@@ -248,10 +250,12 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
 	if (IS_ERR(buf))
 		return PTR_ERR(buf);
-	vdpa_set_config(vdpa, config.off, buf, config_size);
+	ret = vdpa_set_config(vdpa, config.off, buf, config_size);
+	if (ret < 0)
+		ret = -EFAULT;
 	kvfree(buf);
-	return 0;
+	return ret;
 }

So I wonder whether it's worth to return the number of bytes since we 
can't propogate the result to driver or driver doesn't care about 
that.

Okay, but IIUC user space application that issue VHOST_VDPA_GET_CONFIG 
ioctl can use the return value.

Should we change also 'struct virtio_config_ops' to propagate this value 
also to virtio drivers?

Thanks,
Stefano

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization