[RFC 0/4] vDPA: support VHOST_GET/SET_VRING_ENDIAN

[RFC 0/4] vDPA: support VHOST_GET/SET_VRING_ENDIAN

[Zhu Lingshan]

This series add ioctl VHOST_GET_VRING_ENDIAN and
VHOST_SET_VRING_ENDIAN support for vDPA.

Endian-ness is a device wide attribute, so QEMU
can get/set endian-ness of the device by these
ioctls, and QEMU can be aware of the endian-ness
of the device.

To support these ioctls, vendor driver needs to implement:
vdpa_config_ops.get_vq_endian: set vq endian-ness
vdpa_config_ops.set_vq_endian: get vq endian-ness

This series also call vdpa_config_ops.get_config()
instead of vdpa_get_config_unlocked() in
vdpa_dev_net_config_fill(). Then do endian-ness
covert properly through __virtio16_to_cpu by
support of vdpa_config_ops.get_vq_endian.
So there are no race on set_features() any more.

Zhu Lingshan (4):
  vDPA/ifcvf: add get/set_vq_endian support for vDPA
  vDPA: support ioctl VHOST_GET/SET_VRING_ENDIAN
  vDPA: detect device endian in _net_config_fill() and _fill_stats_rec()
  vDPA: report device endian-ness to userspace through netlink

 drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
 drivers/vdpa/ifcvf/ifcvf_main.c | 15 +++++++++++++++
 drivers/vdpa/vdpa.c             | 31 +++++++++++++++++++++++++++----
 drivers/vhost/vdpa.c            | 32 +++++++++++++++++++++++++++++++-
 include/linux/vdpa.h            | 13 +++++++++++++
 include/uapi/linux/vdpa.h       |  1 +
 6 files changed, 88 insertions(+), 5 deletions(-)

-- 
2.31.1

[RFC 1/4] vDPA/ifcvf: add get/set_vq_endian support for vDPA

[Zhu Lingshan]

This commit introuduces new config operatoions for vDPA:
vdpa_config_ops.get_vq_endian: set vq endian-ness
vdpa_config_ops.set_vq_endian: get vq endian-ness

Because the endian-ness is a device wide attribute,
so seting a vq's endian-ness will result in changing
the device endian-ness, including all vqs and the config space.

These two operations are implemented in ifcvf in this commit.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
 drivers/vdpa/ifcvf/ifcvf_main.c | 15 +++++++++++++++
 include/linux/vdpa.h            | 13 +++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
index f5563f665cc6..640238b95033 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -19,6 +19,7 @@
 #include <uapi/linux/virtio_blk.h>
 #include <uapi/linux/virtio_config.h>
 #include <uapi/linux/virtio_pci.h>
+#include <uapi/linux/vhost.h>
 
 #define N3000_DEVICE_ID		0x1041
 #define N3000_SUBSYS_DEVICE_ID	0x001A
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index f9c0044c6442..270637d0f3a5 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -684,6 +684,19 @@ static struct vdpa_notification_area ifcvf_get_vq_notification(struct vdpa_devic
 	return area;
 }
 
+static u8 ifcvf_vdpa_get_vq_endian(struct vdpa_device *vdpa_dev, u16 idx)
+{
+	return VHOST_VRING_LITTLE_ENDIAN;
+}
+
+static int ifcvf_vdpa_set_vq_endian(struct vdpa_device *vdpa_dev, u16 idx, u8 endian)
+{
+	if (endian != VHOST_VRING_LITTLE_ENDIAN)
+		return -EFAULT;
+
+	return 0;
+}
+
 /*
  * IFCVF currently doesn't have on-chip IOMMU, so not
  * implemented set_map()/dma_map()/dma_unmap()
@@ -715,6 +728,8 @@ static const struct vdpa_config_ops ifc_vdpa_ops = {
 	.set_config	= ifcvf_vdpa_set_config,
 	.set_config_cb  = ifcvf_vdpa_set_config_cb,
 	.get_vq_notification = ifcvf_get_vq_notification,
+	.get_vq_endian	= ifcvf_vdpa_get_vq_endian,
+	.set_vq_endian	= ifcvf_vdpa_set_vq_endian,
 };
 
 static struct virtio_device_id id_table_net[] = {
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index d282f464d2f1..5eb83453ba86 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -174,6 +174,17 @@ struct vdpa_map_file {
  *				@idx: virtqueue index
  *				Returns int: irq number of a virtqueue,
  *				negative number if no irq assigned.
+ * @set_vq_endian:		set endian-ness for a virtqueue
+ *				@vdev: vdpa device
+ *				@idx: virtqueue index
+ *				@endian: the endian-ness to set,
+ *				can be VHOST_VRING_LITTLE_ENDIAN or VHOST_VRING_BIG_ENDIAN
+ *				Returns integer: success (0) or error (< 0)
+ * @get_vq_endian:		get the endian-ness of a virtqueue
+ *				@vdev: vdpa device
+ *				@idx: virtqueue index
+ *				Returns u8, the endian-ness of the virtqueue,
+ *				can be VHOST_VRING_LITTLE_ENDIAN or VHOST_VRING_BIG_ENDIAN
  * @get_vq_align:		Get the virtqueue align requirement
  *				for the device
  *				@vdev: vdpa device
@@ -306,6 +317,8 @@ struct vdpa_config_ops {
 	(*get_vq_notification)(struct vdpa_device *vdev, u16 idx);
 	/* vq irq is not expected to be changed once DRIVER_OK is set */
 	int (*get_vq_irq)(struct vdpa_device *vdev, u16 idx);
+	int (*set_vq_endian)(struct vdpa_device *vdev, u16 idx, u8 endian);
+	u8 (*get_vq_endian)(struct vdpa_device *vdev, u16 idx);
 
 	/* Device ops */
 	u32 (*get_vq_align)(struct vdpa_device *vdev);
-- 
2.31.1

[RFC 2/4] vDPA: support ioctl VHOST_GET/SET_VRING_ENDIAN

[Zhu Lingshan]

This commit add ioctl VHOST_GET_VRING_ENDIAN and
VHOST_SET_VRING_ENDIAN support for vhost_vdpa.

So that QEMU can be aware of the endian-ness of the vDPA device.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vhost/vdpa.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 166044642fd5..084fbf04c6bb 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -547,11 +547,41 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
 
 		vq->last_avail_idx = vq_state.split.avail_index;
 		break;
-	}
+	case VHOST_SET_VRING_ENDIAN:
+		if (!ops->set_vq_endian)
+			return -EOPNOTSUPP;
+
+		if (copy_from_user(&s, argp, sizeof(s)))
+			return -EFAULT;
+
+		if (s.num != VHOST_VRING_LITTLE_ENDIAN &&
+		    s.num != VHOST_VRING_BIG_ENDIAN)
+			return -EINVAL;
+
+		if (ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK)
+			return -EFAULT;
+
+		r = ops->set_vq_endian(vdpa, s.index, s.num);
+		if (r)
+			return -EFAULT;
+
+		return 0;
+	case VHOST_GET_VRING_ENDIAN:
+		if (!ops->get_vq_endian)
+			return -EOPNOTSUPP;
+
+		s.index = idx;
+		s.num = ops->get_vq_endian(vdpa, idx);
+
+		if (copy_to_user(argp, &s, sizeof(s)))
+			return -EFAULT;
+
+		return 0;
 
 	r = vhost_vring_ioctl(&v->vdev, cmd, argp);
 	if (r)
 		return r;
+	}
 
 	switch (cmd) {
 	case VHOST_SET_VRING_ADDR:
-- 
2.31.1

[RFC 3/4] vDPA: detect device endian in _net_config_fill() and _fill_stats_rec()

[Zhu Lingshan]

This commit detect endian-ness of the device by
vdpa_config_ops.get_vq_endian(), so that functions
vdpa_dev_net_config_fill() and vdpa_fill_stats_rec()
can convert the endian-ness correctly by __virtio16_to_cpu().

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vdpa/vdpa.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index c06c02704461..8a08caf573d1 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/vdpa.h>
 #include <uapi/linux/vdpa.h>
+#include <uapi/linux/vhost.h>
 #include <net/genetlink.h>
 #include <linux/mod_devicetable.h>
 #include <linux/virtio_ids.h>
@@ -814,21 +815,31 @@ static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev,
 
 static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *msg)
 {
+	const struct vdpa_config_ops *ops = vdev->config;
 	struct virtio_net_config config = {};
 	u64 features;
 	u16 val_u16;
+	u8 le;
 
-	vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config));
+	ops->get_config(vdev, 0, &config, sizeof(config));
+
+	/* endian-ness is a device wide attr, so reading the endian-ness of vq0
+	 * can get the endian-ness of the device config space.
+	 */
+	if (ops->get_vq_endian(vdev, 0) == VHOST_VRING_LITTLE_ENDIAN)
+		le = true;
+	else
+		le = false;
 
 	if (nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR, sizeof(config.mac),
 		    config.mac))
 		return -EMSGSIZE;
 
-	val_u16 = __virtio16_to_cpu(true, config.status);
+	val_u16 = __virtio16_to_cpu(le, config.status);
 	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16))
 		return -EMSGSIZE;
 
-	val_u16 = __virtio16_to_cpu(true, config.mtu);
+	val_u16 = __virtio16_to_cpu(le, config.mtu);
 	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16))
 		return -EMSGSIZE;
 
@@ -897,6 +908,7 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg,
 	u16 max_vqp;
 	u8 status;
 	int err;
+	u8 le;
 
 	status = vdev->config->get_status(vdev);
 	if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) {
@@ -905,7 +917,15 @@ static int vdpa_fill_stats_rec(struct vdpa_device *vdev, struct sk_buff *msg,
 	}
 	vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config));
 
-	max_vqp = __virtio16_to_cpu(true, config.max_virtqueue_pairs);
+	/* endian-ness is a device wide attr, so reading the endian-ness of vq0
+	 * can get the endian-ness of the device config space.
+	 */
+	if (vdev->config->get_vq_endian(vdev, 0) == VHOST_VRING_LITTLE_ENDIAN)
+		le = true;
+	else
+		le = false;
+
+	max_vqp = __virtio16_to_cpu(le, config.max_virtqueue_pairs);
 	if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, max_vqp))
 		return -EMSGSIZE;
 
-- 
2.31.1

[RFC 4/4] vDPA: report device endian-ness to userspace through netlink

[Zhu Lingshan]

This commit introduces a new netlink attr VDPA_ATTR_DEV_ENDIAN
to report device endian-ness to usersapce.

So the userspace tools can be aware of the endian-ness of
the device, even uninitialized legacy/transitional devices.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vdpa/vdpa.c       | 3 +++
 include/uapi/linux/vdpa.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 8a08caf573d1..d361f951ff63 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -848,6 +848,9 @@ static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff *ms
 			      VDPA_ATTR_PAD))
 		return -EMSGSIZE;
 
+	if (nla_put_u8(msg, VDPA_ATTR_DEV_ENDIAN, le))
+		return -EMSGSIZE;
+
 	return vdpa_dev_net_mq_config_fill(vdev, msg, features, &config);
 }
 
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 25c55cab3d7c..bb9797781f97 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -51,6 +51,7 @@ enum vdpa_attr {
 	VDPA_ATTR_DEV_QUEUE_INDEX,              /* u32 */
 	VDPA_ATTR_DEV_VENDOR_ATTR_NAME,		/* string */
 	VDPA_ATTR_DEV_VENDOR_ATTR_VALUE,        /* u64 */
+	VDPA_ATTR_DEV_ENDIAN,			/* u8 */
 
 	/* new attributes must be added above here */
 	VDPA_ATTR_MAX,
-- 
2.31.1

Re: [RFC 1/4] vDPA/ifcvf: add get/set_vq_endian support for vDPA

[Jason Wang]

在 2022/9/1 18:15, Zhu Lingshan 写道:
> This commit introuduces new config operatoions for vDPA:
> vdpa_config_ops.get_vq_endian: set vq endian-ness
> vdpa_config_ops.set_vq_endian: get vq endian-ness
>
> Because the endian-ness is a device wide attribute,
> so seting a vq's endian-ness will result in changing
> the device endian-ness, including all vqs and the config space.
>
> These two operations are implemented in ifcvf in this commit.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> ---
>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>   drivers/vdpa/ifcvf/ifcvf_main.c | 15 +++++++++++++++
>   include/linux/vdpa.h            | 13 +++++++++++++
>   3 files changed, 29 insertions(+)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index f5563f665cc6..640238b95033 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -19,6 +19,7 @@
>   #include <uapi/linux/virtio_blk.h>
>   #include <uapi/linux/virtio_config.h>
>   #include <uapi/linux/virtio_pci.h>
> +#include <uapi/linux/vhost.h>
>   
>   #define N3000_DEVICE_ID		0x1041
>   #define N3000_SUBSYS_DEVICE_ID	0x001A
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index f9c0044c6442..270637d0f3a5 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -684,6 +684,19 @@ static struct vdpa_notification_area ifcvf_get_vq_notification(struct vdpa_devic
>   	return area;
>   }
>   
> +static u8 ifcvf_vdpa_get_vq_endian(struct vdpa_device *vdpa_dev, u16 idx)
> +{
> +	return VHOST_VRING_LITTLE_ENDIAN;
> +}
> +
> +static int ifcvf_vdpa_set_vq_endian(struct vdpa_device *vdpa_dev, u16 idx, u8 endian)
> +{
> +	if (endian != VHOST_VRING_LITTLE_ENDIAN)
> +		return -EFAULT;


I'm worrying that this basically make the proposed API not much useful.

For example, what would userspace do if it meet this failure?

Thanks


> +
> +	return 0;
> +}
> +
>   /*
>    * IFCVF currently doesn't have on-chip IOMMU, so not
>    * implemented set_map()/dma_map()/dma_unmap()
> @@ -715,6 +728,8 @@ static const struct vdpa_config_ops ifc_vdpa_ops = {
>   	.set_config	= ifcvf_vdpa_set_config,
>   	.set_config_cb  = ifcvf_vdpa_set_config_cb,
>   	.get_vq_notification = ifcvf_get_vq_notification,
> +	.get_vq_endian	= ifcvf_vdpa_get_vq_endian,
> +	.set_vq_endian	= ifcvf_vdpa_set_vq_endian,
>   };
>   
>   static struct virtio_device_id id_table_net[] = {
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index d282f464d2f1..5eb83453ba86 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -174,6 +174,17 @@ struct vdpa_map_file {
>    *				@idx: virtqueue index
>    *				Returns int: irq number of a virtqueue,
>    *				negative number if no irq assigned.
> + * @set_vq_endian:		set endian-ness for a virtqueue
> + *				@vdev: vdpa device
> + *				@idx: virtqueue index
> + *				@endian: the endian-ness to set,
> + *				can be VHOST_VRING_LITTLE_ENDIAN or VHOST_VRING_BIG_ENDIAN
> + *				Returns integer: success (0) or error (< 0)
> + * @get_vq_endian:		get the endian-ness of a virtqueue
> + *				@vdev: vdpa device
> + *				@idx: virtqueue index
> + *				Returns u8, the endian-ness of the virtqueue,
> + *				can be VHOST_VRING_LITTLE_ENDIAN or VHOST_VRING_BIG_ENDIAN
>    * @get_vq_align:		Get the virtqueue align requirement
>    *				for the device
>    *				@vdev: vdpa device
> @@ -306,6 +317,8 @@ struct vdpa_config_ops {
>   	(*get_vq_notification)(struct vdpa_device *vdev, u16 idx);
>   	/* vq irq is not expected to be changed once DRIVER_OK is set */
>   	int (*get_vq_irq)(struct vdpa_device *vdev, u16 idx);
> +	int (*set_vq_endian)(struct vdpa_device *vdev, u16 idx, u8 endian);
> +	u8 (*get_vq_endian)(struct vdpa_device *vdev, u16 idx);
>   
>   	/* Device ops */
>   	u32 (*get_vq_align)(struct vdpa_device *vdev);

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

Re: [RFC 1/4] vDPA/ifcvf: add get/set_vq_endian support for vDPA

[Jason Wang]

在 2022/9/1 18:15, Zhu Lingshan 写道:
> This commit introuduces new config operatoions for vDPA:
> vdpa_config_ops.get_vq_endian: set vq endian-ness
> vdpa_config_ops.set_vq_endian: get vq endian-ness
>
> Because the endian-ness is a device wide attribute,
> so seting a vq's endian-ness will result in changing
> the device endian-ness, including all vqs and the config space.
>
> These two operations are implemented in ifcvf in this commit.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> ---
>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>   drivers/vdpa/ifcvf/ifcvf_main.c | 15 +++++++++++++++
>   include/linux/vdpa.h            | 13 +++++++++++++
>   3 files changed, 29 insertions(+)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index f5563f665cc6..640238b95033 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -19,6 +19,7 @@
>   #include <uapi/linux/virtio_blk.h>
>   #include <uapi/linux/virtio_config.h>
>   #include <uapi/linux/virtio_pci.h>
> +#include <uapi/linux/vhost.h>
>   
>   #define N3000_DEVICE_ID		0x1041
>   #define N3000_SUBSYS_DEVICE_ID	0x001A
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index f9c0044c6442..270637d0f3a5 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -684,6 +684,19 @@ static struct vdpa_notification_area ifcvf_get_vq_notification(struct vdpa_devic
>   	return area;
>   }
>   
> +static u8 ifcvf_vdpa_get_vq_endian(struct vdpa_device *vdpa_dev, u16 idx)
> +{
> +	return VHOST_VRING_LITTLE_ENDIAN;
> +}
> +
> +static int ifcvf_vdpa_set_vq_endian(struct vdpa_device *vdpa_dev, u16 idx, u8 endian)
> +{
> +	if (endian != VHOST_VRING_LITTLE_ENDIAN)
> +		return -EFAULT;


I'm worrying that this basically make the proposed API not much useful.

For example, what would userspace do if it meet this failure?

Thanks


> +
> +	return 0;
> +}
> +
>   /*
>    * IFCVF currently doesn't have on-chip IOMMU, so not
>    * implemented set_map()/dma_map()/dma_unmap()
> @@ -715,6 +728,8 @@ static const struct vdpa_config_ops ifc_vdpa_ops = {
>   	.set_config	= ifcvf_vdpa_set_config,
>   	.set_config_cb  = ifcvf_vdpa_set_config_cb,
>   	.get_vq_notification = ifcvf_get_vq_notification,
> +	.get_vq_endian	= ifcvf_vdpa_get_vq_endian,
> +	.set_vq_endian	= ifcvf_vdpa_set_vq_endian,
>   };
>   
>   static struct virtio_device_id id_table_net[] = {
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index d282f464d2f1..5eb83453ba86 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -174,6 +174,17 @@ struct vdpa_map_file {
>    *				@idx: virtqueue index
>    *				Returns int: irq number of a virtqueue,
>    *				negative number if no irq assigned.
> + * @set_vq_endian:		set endian-ness for a virtqueue
> + *				@vdev: vdpa device
> + *				@idx: virtqueue index
> + *				@endian: the endian-ness to set,
> + *				can be VHOST_VRING_LITTLE_ENDIAN or VHOST_VRING_BIG_ENDIAN
> + *				Returns integer: success (0) or error (< 0)
> + * @get_vq_endian:		get the endian-ness of a virtqueue
> + *				@vdev: vdpa device
> + *				@idx: virtqueue index
> + *				Returns u8, the endian-ness of the virtqueue,
> + *				can be VHOST_VRING_LITTLE_ENDIAN or VHOST_VRING_BIG_ENDIAN
>    * @get_vq_align:		Get the virtqueue align requirement
>    *				for the device
>    *				@vdev: vdpa device
> @@ -306,6 +317,8 @@ struct vdpa_config_ops {
>   	(*get_vq_notification)(struct vdpa_device *vdev, u16 idx);
>   	/* vq irq is not expected to be changed once DRIVER_OK is set */
>   	int (*get_vq_irq)(struct vdpa_device *vdev, u16 idx);
> +	int (*set_vq_endian)(struct vdpa_device *vdev, u16 idx, u8 endian);
> +	u8 (*get_vq_endian)(struct vdpa_device *vdev, u16 idx);
>   
>   	/* Device ops */
>   	u32 (*get_vq_align)(struct vdpa_device *vdev);

Re: [RFC 1/4] vDPA/ifcvf: add get/set_vq_endian support for vDPA

[Zhu, Lingshan]

On 9/5/2022 4:34 PM, Jason Wang wrote:
>
> 在 2022/9/1 18:15, Zhu Lingshan 写道:
>> This commit introuduces new config operatoions for vDPA:
>> vdpa_config_ops.get_vq_endian: set vq endian-ness
>> vdpa_config_ops.set_vq_endian: get vq endian-ness
>>
>> Because the endian-ness is a device wide attribute,
>> so seting a vq's endian-ness will result in changing
>> the device endian-ness, including all vqs and the config space.
>>
>> These two operations are implemented in ifcvf in this commit.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>> ---
>>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>>   drivers/vdpa/ifcvf/ifcvf_main.c | 15 +++++++++++++++
>>   include/linux/vdpa.h            | 13 +++++++++++++
>>   3 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 
>> b/drivers/vdpa/ifcvf/ifcvf_base.h
>> index f5563f665cc6..640238b95033 100644
>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
>> @@ -19,6 +19,7 @@
>>   #include <uapi/linux/virtio_blk.h>
>>   #include <uapi/linux/virtio_config.h>
>>   #include <uapi/linux/virtio_pci.h>
>> +#include <uapi/linux/vhost.h>
>>     #define N3000_DEVICE_ID        0x1041
>>   #define N3000_SUBSYS_DEVICE_ID    0x001A
>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
>> b/drivers/vdpa/ifcvf/ifcvf_main.c
>> index f9c0044c6442..270637d0f3a5 100644
>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
>> @@ -684,6 +684,19 @@ static struct vdpa_notification_area 
>> ifcvf_get_vq_notification(struct vdpa_devic
>>       return area;
>>   }
>>   +static u8 ifcvf_vdpa_get_vq_endian(struct vdpa_device *vdpa_dev, 
>> u16 idx)
>> +{
>> +    return VHOST_VRING_LITTLE_ENDIAN;
>> +}
>> +
>> +static int ifcvf_vdpa_set_vq_endian(struct vdpa_device *vdpa_dev, 
>> u16 idx, u8 endian)
>> +{
>> +    if (endian != VHOST_VRING_LITTLE_ENDIAN)
>> +        return -EFAULT;
>
>
> I'm worrying that this basically make the proposed API not much useful.
>
> For example, what would userspace do if it meet this failure?
This means the device only support LE, so if the user space does not 
work with LE,
I think it should explicitly fails on the device.

And we see the device only support LE, so no need to set anything to the 
device
if LE is to set.

Thanks,
Zhu Lingshan
>
> Thanks
>
>
>> +
>> +    return 0;
>> +}
>> +
>>   /*
>>    * IFCVF currently doesn't have on-chip IOMMU, so not
>>    * implemented set_map()/dma_map()/dma_unmap()
>> @@ -715,6 +728,8 @@ static const struct vdpa_config_ops ifc_vdpa_ops = {
>>       .set_config    = ifcvf_vdpa_set_config,
>>       .set_config_cb  = ifcvf_vdpa_set_config_cb,
>>       .get_vq_notification = ifcvf_get_vq_notification,
>> +    .get_vq_endian    = ifcvf_vdpa_get_vq_endian,
>> +    .set_vq_endian    = ifcvf_vdpa_set_vq_endian,
>>   };
>>     static struct virtio_device_id id_table_net[] = {
>> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
>> index d282f464d2f1..5eb83453ba86 100644
>> --- a/include/linux/vdpa.h
>> +++ b/include/linux/vdpa.h
>> @@ -174,6 +174,17 @@ struct vdpa_map_file {
>>    *                @idx: virtqueue index
>>    *                Returns int: irq number of a virtqueue,
>>    *                negative number if no irq assigned.
>> + * @set_vq_endian:        set endian-ness for a virtqueue
>> + *                @vdev: vdpa device
>> + *                @idx: virtqueue index
>> + *                @endian: the endian-ness to set,
>> + *                can be VHOST_VRING_LITTLE_ENDIAN or 
>> VHOST_VRING_BIG_ENDIAN
>> + *                Returns integer: success (0) or error (< 0)
>> + * @get_vq_endian:        get the endian-ness of a virtqueue
>> + *                @vdev: vdpa device
>> + *                @idx: virtqueue index
>> + *                Returns u8, the endian-ness of the virtqueue,
>> + *                can be VHOST_VRING_LITTLE_ENDIAN or 
>> VHOST_VRING_BIG_ENDIAN
>>    * @get_vq_align:        Get the virtqueue align requirement
>>    *                for the device
>>    *                @vdev: vdpa device
>> @@ -306,6 +317,8 @@ struct vdpa_config_ops {
>>       (*get_vq_notification)(struct vdpa_device *vdev, u16 idx);
>>       /* vq irq is not expected to be changed once DRIVER_OK is set */
>>       int (*get_vq_irq)(struct vdpa_device *vdev, u16 idx);
>> +    int (*set_vq_endian)(struct vdpa_device *vdev, u16 idx, u8 endian);
>> +    u8 (*get_vq_endian)(struct vdpa_device *vdev, u16 idx);
>>         /* Device ops */
>>       u32 (*get_vq_align)(struct vdpa_device *vdev);
>