[PATCH v3 0/2] Properly initialize speed/duplex and remove vDPA config updates

[PATCH v3 0/2] Properly initialize speed/duplex and remove vDPA config updates

[Carlos Bilbao]

From: Carlos Bilbao <cbilbao@digitalocean.com>

Initialize speed and duplex for virtio_net_config to UNKNOWN (mlx5_vdpa
vDPA devices currently do not support VIRTIO_NET_F_SPEED_DUPLEX). Remove
ioctl VHOST_VDPA_SET_CONFIG and its related logic as it is not supported;
see: https://docs.oasis-open.org/virtio/virtio/v1.3/virtio-v1.3.html

Carlos:
  vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN
  vdpa: Remove ioctl VHOST_VDPA_SET_CONFIG per spec compliance

---

Changes since v1:
 Link: https://lkml.org/lkml/2024/8/29/1368
 - Fix prefix of the first commit and add Reviewed-By tag.
 - Redo second commit completely: instead of attempting to add support to
   set configuration fields, remove ioctl and support entirely from vDPA
   implementations -- because it's not allowed by spec.

Changes since v2:
 Link: https://lkml.org/lkml/2024/9/3/1407
 - Fix first commit by changing 4 spaces for a tab.
 - In second commit, ENI is legacy and should keep set_config(). Change it
   to set_config_legacy() to avoid future confusion and erroneous
   implementations.

---
 drivers/vdpa/alibaba/eni_vdpa.c    |  2 +-
 drivers/vdpa/ifcvf/ifcvf_main.c    | 10 ----------
 drivers/vdpa/mlx5/net/mlx5_vnet.c  | 19 ++++++++++++-------
 drivers/vdpa/pds/vdpa_dev.c        | 16 ----------------
 drivers/vdpa/solidrun/snet_main.c  | 18 ------------------
 drivers/vdpa/vdpa.c                | 16 ----------------
 drivers/vdpa/vdpa_sim/vdpa_sim.c   | 16 ----------------
 drivers/vdpa/vdpa_sim/vdpa_sim.h   |  1 -
 drivers/vdpa/vdpa_user/vduse_dev.c |  7 -------
 drivers/vdpa/virtio_pci/vp_vdpa.c  | 14 --------------
 drivers/vhost/vdpa.c               | 26 --------------------------
 drivers/virtio/virtio_vdpa.c       |  9 ---------
 include/linux/vdpa.h               |  7 ++++---
 include/uapi/linux/vhost.h         |  8 ++++----
 14 files changed, 21 insertions(+), 148 deletions(-)

[PATCH v3 1/2] vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN

[Carlos Bilbao]

From: Carlos Bilbao <cbilbao@digitalocean.com>

Initialize the speed and duplex fields in virtio_net_config to UNKNOWN.
This is needed because mlx5_vdpa vDPA devices currently do not support the
VIRTIO_NET_F_SPEED_DUPLEX feature which reports speed and duplex. Add
needed helper cpu_to_mlx5vdpa32() to convert endianness of speed.

Signed-off-by: Carlos Bilbao <cbilbao@digitalocean.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index b56aae3f7be3..41ca268d43ff 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -173,6 +173,11 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
 	return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
 }
 
+static __virtio32 cpu_to_mlx5vdpa32(struct mlx5_vdpa_dev *mvdev, u32 val)
+{
+	return __cpu_to_virtio32(mlx5_vdpa_is_little_endian(mvdev), val);
+}
+
 static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
 {
 	if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
@@ -3433,6 +3438,13 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
 	init_rwsem(&ndev->reslock);
 	config = &ndev->config;
 
+	/*
+	 * mlx5_vdpa vDPA devices currently don't support reporting or
+	 * setting the speed or duplex.
+	 */
+	config->speed  = cpu_to_mlx5vdpa32(mvdev, SPEED_UNKNOWN);
+	config->duplex = DUPLEX_UNKNOWN;
+
 	if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU)) {
 		err = config_func_mtu(mdev, add_config->net.mtu);
 		if (err)
-- 
2.34.1

[PATCH v3 2/2] vdpa: Remove ioctl VHOST_VDPA_SET_CONFIG per spec compliance

[Carlos Bilbao]

From: Carlos Bilbao <cbilbao@digitalocean.com>

Remove invalid ioctl VHOST_VDPA_SET_CONFIG and all its implementations with
vdpa_config_ops->set_config(). This is needed per virtio spec requirements:
virtio-spec v1.3 Sec 5.1.4 states that "All of the device configuration
fields are read-only for the driver.". This excludes legacy Alibaba's ENI
so make it use vda_config_ops->set_config_legacy() to avoid future
confusion.

Signed-off-by: Carlos Bilbao <cbilbao@digitalocean.com>
---
 drivers/vdpa/alibaba/eni_vdpa.c    |  2 +-
 drivers/vdpa/ifcvf/ifcvf_main.c    | 10 ----------
 drivers/vdpa/mlx5/net/mlx5_vnet.c  |  7 -------
 drivers/vdpa/pds/vdpa_dev.c        | 16 ----------------
 drivers/vdpa/solidrun/snet_main.c  | 18 ------------------
 drivers/vdpa/vdpa.c                | 16 ----------------
 drivers/vdpa/vdpa_sim/vdpa_sim.c   | 16 ----------------
 drivers/vdpa/vdpa_sim/vdpa_sim.h   |  1 -
 drivers/vdpa/vdpa_user/vduse_dev.c |  7 -------
 drivers/vdpa/virtio_pci/vp_vdpa.c  | 14 --------------
 drivers/vhost/vdpa.c               | 26 --------------------------
 drivers/virtio/virtio_vdpa.c       |  9 ---------
 include/linux/vdpa.h               |  7 ++++---
 include/uapi/linux/vhost.h         |  8 ++++----
 14 files changed, 9 insertions(+), 148 deletions(-)

diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c
index cce3d1837104..64b0c0be89ae 100644
--- a/drivers/vdpa/alibaba/eni_vdpa.c
+++ b/drivers/vdpa/alibaba/eni_vdpa.c
@@ -429,7 +429,7 @@ static const struct vdpa_config_ops eni_vdpa_ops = {
 	.get_vq_align	= eni_vdpa_get_vq_align,
 	.get_config_size = eni_vdpa_get_config_size,
 	.get_config	= eni_vdpa_get_config,
-	.set_config	= eni_vdpa_set_config,
+	.set_config_legacy = eni_vdpa_set_config,
 	.set_config_cb  = eni_vdpa_set_config_cb,
 	.get_vq_irq	= eni_vdpa_get_vq_irq,
 };
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index e98fa8100f3c..7cbac787ad5f 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -568,15 +568,6 @@ static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev,
 	ifcvf_read_dev_config(vf, offset, buf, len);
 }
 
-static void ifcvf_vdpa_set_config(struct vdpa_device *vdpa_dev,
-				  unsigned int offset, const void *buf,
-				  unsigned int len)
-{
-	struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
-
-	ifcvf_write_dev_config(vf, offset, buf, len);
-}
-
 static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev,
 				     struct vdpa_callback *cb)
 {
@@ -640,7 +631,6 @@ static const struct vdpa_config_ops ifc_vdpa_ops = {
 	.get_vq_group	= ifcvf_vdpa_get_vq_group,
 	.get_config_size	= ifcvf_vdpa_get_config_size,
 	.get_config	= ifcvf_vdpa_get_config,
-	.set_config	= ifcvf_vdpa_set_config,
 	.set_config_cb  = ifcvf_vdpa_set_config_cb,
 	.get_vq_notification = ifcvf_get_vq_notification,
 };
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 41ca268d43ff..35ed46c65b4d 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2918,12 +2918,6 @@ static void mlx5_vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
 		memcpy(buf, (u8 *)&ndev->config + offset, len);
 }
 
-static void mlx5_vdpa_set_config(struct vdpa_device *vdev, unsigned int offset, const void *buf,
-				 unsigned int len)
-{
-	/* not supported */
-}
-
 static u32 mlx5_vdpa_get_generation(struct vdpa_device *vdev)
 {
 	struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
@@ -3218,7 +3212,6 @@ static const struct vdpa_config_ops mlx5_vdpa_ops = {
 	.reset = mlx5_vdpa_reset,
 	.get_config_size = mlx5_vdpa_get_config_size,
 	.get_config = mlx5_vdpa_get_config,
-	.set_config = mlx5_vdpa_set_config,
 	.get_generation = mlx5_vdpa_get_generation,
 	.set_map = mlx5_vdpa_set_map,
 	.set_group_asid = mlx5_set_group_asid,
diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
index 25c0fe5ec3d5..553dcd2aa065 100644
--- a/drivers/vdpa/pds/vdpa_dev.c
+++ b/drivers/vdpa/pds/vdpa_dev.c
@@ -553,21 +553,6 @@ static void pds_vdpa_get_config(struct vdpa_device *vdpa_dev,
 	memcpy_fromio(buf, device + offset, len);
 }
 
-static void pds_vdpa_set_config(struct vdpa_device *vdpa_dev,
-				unsigned int offset, const void *buf,
-				unsigned int len)
-{
-	struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
-	void __iomem *device;
-
-	if (offset + len > sizeof(struct virtio_net_config)) {
-		WARN(true, "%s: bad read, offset %d len %d\n", __func__, offset, len);
-		return;
-	}
-
-	device = pdsv->vdpa_aux->vd_mdev.device;
-	memcpy_toio(device + offset, buf, len);
-}
 
 static const struct vdpa_config_ops pds_vdpa_ops = {
 	.set_vq_address		= pds_vdpa_set_vq_address,
@@ -595,7 +580,6 @@ static const struct vdpa_config_ops pds_vdpa_ops = {
 	.reset			= pds_vdpa_reset,
 	.get_config_size	= pds_vdpa_get_config_size,
 	.get_config		= pds_vdpa_get_config,
-	.set_config		= pds_vdpa_set_config,
 };
 static struct virtio_device_id pds_vdpa_id_table[] = {
 	{VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID},
diff --git a/drivers/vdpa/solidrun/snet_main.c b/drivers/vdpa/solidrun/snet_main.c
index 99428a04068d..141740269b6c 100644
--- a/drivers/vdpa/solidrun/snet_main.c
+++ b/drivers/vdpa/solidrun/snet_main.c
@@ -478,23 +478,6 @@ static void snet_get_config(struct vdpa_device *vdev, unsigned int offset,
 		*buf_ptr++ = ioread8(cfg_ptr + i);
 }
 
-static void snet_set_config(struct vdpa_device *vdev, unsigned int offset,
-			    const void *buf, unsigned int len)
-{
-	struct snet *snet = vdpa_to_snet(vdev);
-	void __iomem *cfg_ptr = snet->cfg->virtio_cfg + offset;
-	const u8 *buf_ptr = buf;
-	u32 i;
-
-	/* check for offset error */
-	if (offset + len > snet->cfg->cfg_size)
-		return;
-
-	/* Write into PCI BAR */
-	for (i = 0; i < len; i++)
-		iowrite8(*buf_ptr++, cfg_ptr + i);
-}
-
 static int snet_suspend(struct vdpa_device *vdev)
 {
 	struct snet *snet = vdpa_to_snet(vdev);
@@ -548,7 +531,6 @@ static const struct vdpa_config_ops snet_config_ops = {
 	.get_status             = snet_get_status,
 	.set_status             = snet_set_status,
 	.get_config             = snet_get_config,
-	.set_config             = snet_set_config,
 	.suspend		= snet_suspend,
 	.resume			= snet_resume,
 };
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index a7612e0783b3..a9eac31f3757 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -401,22 +401,6 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
 }
 EXPORT_SYMBOL_GPL(vdpa_get_config);
 
-/**
- * vdpa_set_config - Set one or more device configuration fields.
- * @vdev: vdpa device to operate on
- * @offset: starting byte offset of the field
- * @buf: buffer pointer to read from
- * @length: length of the configuration fields in bytes
- */
-void vdpa_set_config(struct vdpa_device *vdev, unsigned int offset,
-		     const void *buf, unsigned int length)
-{
-	down_write(&vdev->cf_lock);
-	vdev->config->set_config(vdev, offset, buf, length);
-	up_write(&vdev->cf_lock);
-}
-EXPORT_SYMBOL_GPL(vdpa_set_config);
-
 static bool mgmtdev_handle_match(const struct vdpa_mgmt_dev *mdev,
 				 const char *busname, const char *devname)
 {
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 421ab01ef06b..c2e14bcc01f6 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -546,20 +546,6 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
 	memcpy(buf, vdpasim->config + offset, len);
 }
 
-static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
-			     const void *buf, unsigned int len)
-{
-	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
-
-	if (offset + len > vdpasim->dev_attr.config_size)
-		return;
-
-	memcpy(vdpasim->config + offset, buf, len);
-
-	if (vdpasim->dev_attr.set_config)
-		vdpasim->dev_attr.set_config(vdpasim, vdpasim->config);
-}
-
 static u32 vdpasim_get_generation(struct vdpa_device *vdpa)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
@@ -754,7 +740,6 @@ static const struct vdpa_config_ops vdpasim_config_ops = {
 	.resume			= vdpasim_resume,
 	.get_config_size        = vdpasim_get_config_size,
 	.get_config             = vdpasim_get_config,
-	.set_config             = vdpasim_set_config,
 	.get_generation         = vdpasim_get_generation,
 	.get_iova_range         = vdpasim_get_iova_range,
 	.set_group_asid         = vdpasim_set_group_asid,
@@ -792,7 +777,6 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
 	.resume			= vdpasim_resume,
 	.get_config_size        = vdpasim_get_config_size,
 	.get_config             = vdpasim_get_config,
-	.set_config             = vdpasim_set_config,
 	.get_generation         = vdpasim_get_generation,
 	.get_iova_range         = vdpasim_get_iova_range,
 	.set_group_asid         = vdpasim_set_group_asid,
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h
index bb137e479763..b48bf954a3bb 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.h
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h
@@ -46,7 +46,6 @@ struct vdpasim_dev_attr {
 
 	void (*work_fn)(struct vdpasim *vdpasim);
 	void (*get_config)(struct vdpasim *vdpasim, void *config);
-	void (*set_config)(struct vdpasim *vdpasim, const void *config);
 	int (*get_stats)(struct vdpasim *vdpasim, u16 idx,
 			 struct sk_buff *msg,
 			 struct netlink_ext_ack *extack);
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index df7869537ef1..4fe69cb5b156 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -698,12 +698,6 @@ static void vduse_vdpa_get_config(struct vdpa_device *vdpa, unsigned int offset,
 	memcpy(buf, dev->config + offset, len);
 }
 
-static void vduse_vdpa_set_config(struct vdpa_device *vdpa, unsigned int offset,
-			const void *buf, unsigned int len)
-{
-	/* Now we only support read-only configuration space */
-}
-
 static int vduse_vdpa_reset(struct vdpa_device *vdpa)
 {
 	struct vduse_dev *dev = vdpa_to_vduse(vdpa);
@@ -790,7 +784,6 @@ static const struct vdpa_config_ops vduse_vdpa_config_ops = {
 	.set_status		= vduse_vdpa_set_status,
 	.get_config_size	= vduse_vdpa_get_config_size,
 	.get_config		= vduse_vdpa_get_config,
-	.set_config		= vduse_vdpa_set_config,
 	.get_generation		= vduse_vdpa_get_generation,
 	.set_vq_affinity	= vduse_vdpa_set_vq_affinity,
 	.get_vq_affinity	= vduse_vdpa_get_vq_affinity,
diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c
index 281287fae89f..5e8ff91475e3 100644
--- a/drivers/vdpa/virtio_pci/vp_vdpa.c
+++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
@@ -400,19 +400,6 @@ static void vp_vdpa_get_config(struct vdpa_device *vdpa,
 	} while (old != new);
 }
 
-static void vp_vdpa_set_config(struct vdpa_device *vdpa,
-			       unsigned int offset, const void *buf,
-			       unsigned int len)
-{
-	struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
-	struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa);
-	const u8 *p = buf;
-	int i;
-
-	for (i = 0; i < len; i++)
-		vp_iowrite8(*p++, mdev->device + offset + i);
-}
-
 static void vp_vdpa_set_config_cb(struct vdpa_device *vdpa,
 				  struct vdpa_callback *cb)
 {
@@ -457,7 +444,6 @@ static const struct vdpa_config_ops vp_vdpa_ops = {
 	.get_vq_align	= vp_vdpa_get_vq_align,
 	.get_config_size = vp_vdpa_get_config_size,
 	.get_config	= vp_vdpa_get_config,
-	.set_config	= vp_vdpa_set_config,
 	.set_config_cb  = vp_vdpa_set_config_cb,
 	.get_vq_irq	= vp_vdpa_get_vq_irq,
 };
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index fb590e346e43..c6fcd54f59be 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -350,29 +350,6 @@ static long vhost_vdpa_get_config(struct vhost_vdpa *v,
 	return 0;
 }
 
-static long vhost_vdpa_set_config(struct vhost_vdpa *v,
-				  struct vhost_vdpa_config __user *c)
-{
-	struct vdpa_device *vdpa = v->vdpa;
-	struct vhost_vdpa_config config;
-	unsigned long size = offsetof(struct vhost_vdpa_config, buf);
-	u8 *buf;
-
-	if (copy_from_user(&config, c, size))
-		return -EFAULT;
-	if (vhost_vdpa_config_validate(v, &config))
-		return -EINVAL;
-
-	buf = vmemdup_user(c->buf, config.len);
-	if (IS_ERR(buf))
-		return PTR_ERR(buf);
-
-	vdpa_set_config(vdpa, config.off, buf, config.len);
-
-	kvfree(buf);
-	return 0;
-}
-
 static bool vhost_vdpa_can_suspend(const struct vhost_vdpa *v)
 {
 	struct vdpa_device *vdpa = v->vdpa;
@@ -719,9 +696,6 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 	case VHOST_VDPA_GET_CONFIG:
 		r = vhost_vdpa_get_config(v, argp);
 		break;
-	case VHOST_VDPA_SET_CONFIG:
-		r = vhost_vdpa_set_config(v, argp);
-		break;
 	case VHOST_GET_FEATURES:
 		r = vhost_vdpa_get_features(v, argp);
 		break;
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index 06ce6d8c2e00..481ded50c916 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -62,14 +62,6 @@ static void virtio_vdpa_get(struct virtio_device *vdev, unsigned int offset,
 	vdpa_get_config(vdpa, offset, buf, len);
 }
 
-static void virtio_vdpa_set(struct virtio_device *vdev, unsigned int offset,
-			    const void *buf, unsigned int len)
-{
-	struct vdpa_device *vdpa = vd_get_vdpa(vdev);
-
-	vdpa_set_config(vdpa, offset, buf, len);
-}
-
 static u32 virtio_vdpa_generation(struct virtio_device *vdev)
 {
 	struct vdpa_device *vdpa = vd_get_vdpa(vdev);
@@ -462,7 +454,6 @@ virtio_vdpa_get_vq_affinity(struct virtio_device *vdev, int index)
 
 static const struct virtio_config_ops virtio_vdpa_config_ops = {
 	.get		= virtio_vdpa_get,
-	.set		= virtio_vdpa_set,
 	.generation	= virtio_vdpa_generation,
 	.get_status	= virtio_vdpa_get_status,
 	.set_status	= virtio_vdpa_set_status,
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 0e652026b776..9346fa9e3d97 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -259,7 +259,8 @@ struct vdpa_map_file {
  *				@buf: buffer used to read to
  *				@len: the length to read from
  *				configuration space
- * @set_config:			Write to device specific configuration space
+ * @set_config_legacy:		Write to device specific configuration space
+ *				Legacy functionality for virtio-spec < v1.3
  *				@vdev: vdpa device
  *				@offset: offset from the beginning of
  *				configuration space
@@ -378,8 +379,8 @@ struct vdpa_config_ops {
 	size_t (*get_config_size)(struct vdpa_device *vdev);
 	void (*get_config)(struct vdpa_device *vdev, unsigned int offset,
 			   void *buf, unsigned int len);
-	void (*set_config)(struct vdpa_device *vdev, unsigned int offset,
-			   const void *buf, unsigned int len);
+	void (*set_config_legacy)(struct vdpa_device *vdev,
+			unsigned int offset, const void *buf, unsigned int len);
 	u32 (*get_generation)(struct vdpa_device *vdev);
 	struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
 	int (*set_vq_affinity)(struct vdpa_device *vdev, u16 idx,
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index f5c48b61ab62..b7977f9ae596 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -157,13 +157,13 @@
  */
 #define VHOST_VDPA_GET_STATUS		_IOR(VHOST_VIRTIO, 0x71, __u8)
 #define VHOST_VDPA_SET_STATUS		_IOW(VHOST_VIRTIO, 0x72, __u8)
-/* Get and set the device config. The device config follows the same
- * definition of the device config defined in virtio-spec.
+/* Get the device config. The device config follows the same
+ * definition of the device config defined in virtio-spec. According to
+ * virtio-spec v1.3, all device configuration fields are read-only for the
+ * driver, and thus we do not have VHOST_VDPA_SET_CONFIG.
  */
 #define VHOST_VDPA_GET_CONFIG		_IOR(VHOST_VIRTIO, 0x73, \
 					     struct vhost_vdpa_config)
-#define VHOST_VDPA_SET_CONFIG		_IOW(VHOST_VIRTIO, 0x74, \
-					     struct vhost_vdpa_config)
 /* Enable/disable the ring. */
 #define VHOST_VDPA_SET_VRING_ENABLE	_IOW(VHOST_VIRTIO, 0x75, \
 					     struct vhost_vring_state)
-- 
2.34.1

Re: [PATCH v3 2/2] vdpa: Remove ioctl VHOST_VDPA_SET_CONFIG per spec compliance

[Jason Wang]

On Wed, Sep 4, 2024 at 11:18 PM Carlos Bilbao
<carlos.bilbao.osdev@gmail.com> wrote:
>
> From: Carlos Bilbao <cbilbao@digitalocean.com>
>
> Remove invalid ioctl VHOST_VDPA_SET_CONFIG and all its implementations with
> vdpa_config_ops->set_config(). This is needed per virtio spec requirements:
> virtio-spec v1.3 Sec 5.1.4 states that "All of the device configuration
> fields are read-only for the driver.".

Just to make sure we are on the same page.

That part is specific to the virtio-net device. Other types of device
may have a field that could be written. For example the writeback
field of the virtio-blk device when WCE is enabled.

Even for the networking device, it doesn't mean we can't have a
writable field in the future.

So I tend to leave the code as-is.

Thanks

> This excludes legacy Alibaba's ENI
> so make it use vda_config_ops->set_config_legacy() to avoid future
> confusion.
>
> Signed-off-by: Carlos Bilbao <cbilbao@digitalocean.com>
> ---
>  drivers/vdpa/alibaba/eni_vdpa.c    |  2 +-
>  drivers/vdpa/ifcvf/ifcvf_main.c    | 10 ----------
>  drivers/vdpa/mlx5/net/mlx5_vnet.c  |  7 -------
>  drivers/vdpa/pds/vdpa_dev.c        | 16 ----------------
>  drivers/vdpa/solidrun/snet_main.c  | 18 ------------------
>  drivers/vdpa/vdpa.c                | 16 ----------------
>  drivers/vdpa/vdpa_sim/vdpa_sim.c   | 16 ----------------
>  drivers/vdpa/vdpa_sim/vdpa_sim.h   |  1 -
>  drivers/vdpa/vdpa_user/vduse_dev.c |  7 -------
>  drivers/vdpa/virtio_pci/vp_vdpa.c  | 14 --------------
>  drivers/vhost/vdpa.c               | 26 --------------------------
>  drivers/virtio/virtio_vdpa.c       |  9 ---------
>  include/linux/vdpa.h               |  7 ++++---
>  include/uapi/linux/vhost.h         |  8 ++++----
>  14 files changed, 9 insertions(+), 148 deletions(-)
>
> diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c
> index cce3d1837104..64b0c0be89ae 100644
> --- a/drivers/vdpa/alibaba/eni_vdpa.c
> +++ b/drivers/vdpa/alibaba/eni_vdpa.c
> @@ -429,7 +429,7 @@ static const struct vdpa_config_ops eni_vdpa_ops = {
>         .get_vq_align   = eni_vdpa_get_vq_align,
>         .get_config_size = eni_vdpa_get_config_size,
>         .get_config     = eni_vdpa_get_config,
> -       .set_config     = eni_vdpa_set_config,
> +       .set_config_legacy = eni_vdpa_set_config,
>         .set_config_cb  = eni_vdpa_set_config_cb,
>         .get_vq_irq     = eni_vdpa_get_vq_irq,
>  };
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index e98fa8100f3c..7cbac787ad5f 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -568,15 +568,6 @@ static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev,
>         ifcvf_read_dev_config(vf, offset, buf, len);
>  }
>
> -static void ifcvf_vdpa_set_config(struct vdpa_device *vdpa_dev,
> -                                 unsigned int offset, const void *buf,
> -                                 unsigned int len)
> -{
> -       struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
> -
> -       ifcvf_write_dev_config(vf, offset, buf, len);
> -}
> -
>  static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev,
>                                      struct vdpa_callback *cb)
>  {
> @@ -640,7 +631,6 @@ static const struct vdpa_config_ops ifc_vdpa_ops = {
>         .get_vq_group   = ifcvf_vdpa_get_vq_group,
>         .get_config_size        = ifcvf_vdpa_get_config_size,
>         .get_config     = ifcvf_vdpa_get_config,
> -       .set_config     = ifcvf_vdpa_set_config,
>         .set_config_cb  = ifcvf_vdpa_set_config_cb,
>         .get_vq_notification = ifcvf_get_vq_notification,
>  };
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 41ca268d43ff..35ed46c65b4d 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -2918,12 +2918,6 @@ static void mlx5_vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
>                 memcpy(buf, (u8 *)&ndev->config + offset, len);
>  }
>
> -static void mlx5_vdpa_set_config(struct vdpa_device *vdev, unsigned int offset, const void *buf,
> -                                unsigned int len)
> -{
> -       /* not supported */
> -}
> -
>  static u32 mlx5_vdpa_get_generation(struct vdpa_device *vdev)
>  {
>         struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> @@ -3218,7 +3212,6 @@ static const struct vdpa_config_ops mlx5_vdpa_ops = {
>         .reset = mlx5_vdpa_reset,
>         .get_config_size = mlx5_vdpa_get_config_size,
>         .get_config = mlx5_vdpa_get_config,
> -       .set_config = mlx5_vdpa_set_config,
>         .get_generation = mlx5_vdpa_get_generation,
>         .set_map = mlx5_vdpa_set_map,
>         .set_group_asid = mlx5_set_group_asid,
> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
> index 25c0fe5ec3d5..553dcd2aa065 100644
> --- a/drivers/vdpa/pds/vdpa_dev.c
> +++ b/drivers/vdpa/pds/vdpa_dev.c
> @@ -553,21 +553,6 @@ static void pds_vdpa_get_config(struct vdpa_device *vdpa_dev,
>         memcpy_fromio(buf, device + offset, len);
>  }
>
> -static void pds_vdpa_set_config(struct vdpa_device *vdpa_dev,
> -                               unsigned int offset, const void *buf,
> -                               unsigned int len)
> -{
> -       struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
> -       void __iomem *device;
> -
> -       if (offset + len > sizeof(struct virtio_net_config)) {
> -               WARN(true, "%s: bad read, offset %d len %d\n", __func__, offset, len);
> -               return;
> -       }
> -
> -       device = pdsv->vdpa_aux->vd_mdev.device;
> -       memcpy_toio(device + offset, buf, len);
> -}
>
>  static const struct vdpa_config_ops pds_vdpa_ops = {
>         .set_vq_address         = pds_vdpa_set_vq_address,
> @@ -595,7 +580,6 @@ static const struct vdpa_config_ops pds_vdpa_ops = {
>         .reset                  = pds_vdpa_reset,
>         .get_config_size        = pds_vdpa_get_config_size,
>         .get_config             = pds_vdpa_get_config,
> -       .set_config             = pds_vdpa_set_config,
>  };
>  static struct virtio_device_id pds_vdpa_id_table[] = {
>         {VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID},
> diff --git a/drivers/vdpa/solidrun/snet_main.c b/drivers/vdpa/solidrun/snet_main.c
> index 99428a04068d..141740269b6c 100644
> --- a/drivers/vdpa/solidrun/snet_main.c
> +++ b/drivers/vdpa/solidrun/snet_main.c
> @@ -478,23 +478,6 @@ static void snet_get_config(struct vdpa_device *vdev, unsigned int offset,
>                 *buf_ptr++ = ioread8(cfg_ptr + i);
>  }
>
> -static void snet_set_config(struct vdpa_device *vdev, unsigned int offset,
> -                           const void *buf, unsigned int len)
> -{
> -       struct snet *snet = vdpa_to_snet(vdev);
> -       void __iomem *cfg_ptr = snet->cfg->virtio_cfg + offset;
> -       const u8 *buf_ptr = buf;
> -       u32 i;
> -
> -       /* check for offset error */
> -       if (offset + len > snet->cfg->cfg_size)
> -               return;
> -
> -       /* Write into PCI BAR */
> -       for (i = 0; i < len; i++)
> -               iowrite8(*buf_ptr++, cfg_ptr + i);
> -}
> -
>  static int snet_suspend(struct vdpa_device *vdev)
>  {
>         struct snet *snet = vdpa_to_snet(vdev);
> @@ -548,7 +531,6 @@ static const struct vdpa_config_ops snet_config_ops = {
>         .get_status             = snet_get_status,
>         .set_status             = snet_set_status,
>         .get_config             = snet_get_config,
> -       .set_config             = snet_set_config,
>         .suspend                = snet_suspend,
>         .resume                 = snet_resume,
>  };
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index a7612e0783b3..a9eac31f3757 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -401,22 +401,6 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
>  }
>  EXPORT_SYMBOL_GPL(vdpa_get_config);
>
> -/**
> - * vdpa_set_config - Set one or more device configuration fields.
> - * @vdev: vdpa device to operate on
> - * @offset: starting byte offset of the field
> - * @buf: buffer pointer to read from
> - * @length: length of the configuration fields in bytes
> - */
> -void vdpa_set_config(struct vdpa_device *vdev, unsigned int offset,
> -                    const void *buf, unsigned int length)
> -{
> -       down_write(&vdev->cf_lock);
> -       vdev->config->set_config(vdev, offset, buf, length);
> -       up_write(&vdev->cf_lock);
> -}
> -EXPORT_SYMBOL_GPL(vdpa_set_config);
> -
>  static bool mgmtdev_handle_match(const struct vdpa_mgmt_dev *mdev,
>                                  const char *busname, const char *devname)
>  {
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index 421ab01ef06b..c2e14bcc01f6 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -546,20 +546,6 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
>         memcpy(buf, vdpasim->config + offset, len);
>  }
>
> -static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
> -                            const void *buf, unsigned int len)
> -{
> -       struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> -
> -       if (offset + len > vdpasim->dev_attr.config_size)
> -               return;
> -
> -       memcpy(vdpasim->config + offset, buf, len);
> -
> -       if (vdpasim->dev_attr.set_config)
> -               vdpasim->dev_attr.set_config(vdpasim, vdpasim->config);
> -}
> -
>  static u32 vdpasim_get_generation(struct vdpa_device *vdpa)
>  {
>         struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> @@ -754,7 +740,6 @@ static const struct vdpa_config_ops vdpasim_config_ops = {
>         .resume                 = vdpasim_resume,
>         .get_config_size        = vdpasim_get_config_size,
>         .get_config             = vdpasim_get_config,
> -       .set_config             = vdpasim_set_config,
>         .get_generation         = vdpasim_get_generation,
>         .get_iova_range         = vdpasim_get_iova_range,
>         .set_group_asid         = vdpasim_set_group_asid,
> @@ -792,7 +777,6 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
>         .resume                 = vdpasim_resume,
>         .get_config_size        = vdpasim_get_config_size,
>         .get_config             = vdpasim_get_config,
> -       .set_config             = vdpasim_set_config,
>         .get_generation         = vdpasim_get_generation,
>         .get_iova_range         = vdpasim_get_iova_range,
>         .set_group_asid         = vdpasim_set_group_asid,
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h b/drivers/vdpa/vdpa_sim/vdpa_sim.h
> index bb137e479763..b48bf954a3bb 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h
> @@ -46,7 +46,6 @@ struct vdpasim_dev_attr {
>
>         void (*work_fn)(struct vdpasim *vdpasim);
>         void (*get_config)(struct vdpasim *vdpasim, void *config);
> -       void (*set_config)(struct vdpasim *vdpasim, const void *config);
>         int (*get_stats)(struct vdpasim *vdpasim, u16 idx,
>                          struct sk_buff *msg,
>                          struct netlink_ext_ack *extack);
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index df7869537ef1..4fe69cb5b156 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -698,12 +698,6 @@ static void vduse_vdpa_get_config(struct vdpa_device *vdpa, unsigned int offset,
>         memcpy(buf, dev->config + offset, len);
>  }
>
> -static void vduse_vdpa_set_config(struct vdpa_device *vdpa, unsigned int offset,
> -                       const void *buf, unsigned int len)
> -{
> -       /* Now we only support read-only configuration space */
> -}
> -
>  static int vduse_vdpa_reset(struct vdpa_device *vdpa)
>  {
>         struct vduse_dev *dev = vdpa_to_vduse(vdpa);
> @@ -790,7 +784,6 @@ static const struct vdpa_config_ops vduse_vdpa_config_ops = {
>         .set_status             = vduse_vdpa_set_status,
>         .get_config_size        = vduse_vdpa_get_config_size,
>         .get_config             = vduse_vdpa_get_config,
> -       .set_config             = vduse_vdpa_set_config,
>         .get_generation         = vduse_vdpa_get_generation,
>         .set_vq_affinity        = vduse_vdpa_set_vq_affinity,
>         .get_vq_affinity        = vduse_vdpa_get_vq_affinity,
> diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c b/drivers/vdpa/virtio_pci/vp_vdpa.c
> index 281287fae89f..5e8ff91475e3 100644
> --- a/drivers/vdpa/virtio_pci/vp_vdpa.c
> +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
> @@ -400,19 +400,6 @@ static void vp_vdpa_get_config(struct vdpa_device *vdpa,
>         } while (old != new);
>  }
>
> -static void vp_vdpa_set_config(struct vdpa_device *vdpa,
> -                              unsigned int offset, const void *buf,
> -                              unsigned int len)
> -{
> -       struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
> -       struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa);
> -       const u8 *p = buf;
> -       int i;
> -
> -       for (i = 0; i < len; i++)
> -               vp_iowrite8(*p++, mdev->device + offset + i);
> -}
> -
>  static void vp_vdpa_set_config_cb(struct vdpa_device *vdpa,
>                                   struct vdpa_callback *cb)
>  {
> @@ -457,7 +444,6 @@ static const struct vdpa_config_ops vp_vdpa_ops = {
>         .get_vq_align   = vp_vdpa_get_vq_align,
>         .get_config_size = vp_vdpa_get_config_size,
>         .get_config     = vp_vdpa_get_config,
> -       .set_config     = vp_vdpa_set_config,
>         .set_config_cb  = vp_vdpa_set_config_cb,
>         .get_vq_irq     = vp_vdpa_get_vq_irq,
>  };
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index fb590e346e43..c6fcd54f59be 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -350,29 +350,6 @@ static long vhost_vdpa_get_config(struct vhost_vdpa *v,
>         return 0;
>  }
>
> -static long vhost_vdpa_set_config(struct vhost_vdpa *v,
> -                                 struct vhost_vdpa_config __user *c)
> -{
> -       struct vdpa_device *vdpa = v->vdpa;
> -       struct vhost_vdpa_config config;
> -       unsigned long size = offsetof(struct vhost_vdpa_config, buf);
> -       u8 *buf;
> -
> -       if (copy_from_user(&config, c, size))
> -               return -EFAULT;
> -       if (vhost_vdpa_config_validate(v, &config))
> -               return -EINVAL;
> -
> -       buf = vmemdup_user(c->buf, config.len);
> -       if (IS_ERR(buf))
> -               return PTR_ERR(buf);
> -
> -       vdpa_set_config(vdpa, config.off, buf, config.len);
> -
> -       kvfree(buf);
> -       return 0;
> -}
> -
>  static bool vhost_vdpa_can_suspend(const struct vhost_vdpa *v)
>  {
>         struct vdpa_device *vdpa = v->vdpa;
> @@ -719,9 +696,6 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>         case VHOST_VDPA_GET_CONFIG:
>                 r = vhost_vdpa_get_config(v, argp);
>                 break;
> -       case VHOST_VDPA_SET_CONFIG:
> -               r = vhost_vdpa_set_config(v, argp);
> -               break;
>         case VHOST_GET_FEATURES:
>                 r = vhost_vdpa_get_features(v, argp);
>                 break;
> diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> index 06ce6d8c2e00..481ded50c916 100644
> --- a/drivers/virtio/virtio_vdpa.c
> +++ b/drivers/virtio/virtio_vdpa.c
> @@ -62,14 +62,6 @@ static void virtio_vdpa_get(struct virtio_device *vdev, unsigned int offset,
>         vdpa_get_config(vdpa, offset, buf, len);
>  }
>
> -static void virtio_vdpa_set(struct virtio_device *vdev, unsigned int offset,
> -                           const void *buf, unsigned int len)
> -{
> -       struct vdpa_device *vdpa = vd_get_vdpa(vdev);
> -
> -       vdpa_set_config(vdpa, offset, buf, len);
> -}
> -
>  static u32 virtio_vdpa_generation(struct virtio_device *vdev)
>  {
>         struct vdpa_device *vdpa = vd_get_vdpa(vdev);
> @@ -462,7 +454,6 @@ virtio_vdpa_get_vq_affinity(struct virtio_device *vdev, int index)
>
>  static const struct virtio_config_ops virtio_vdpa_config_ops = {
>         .get            = virtio_vdpa_get,
> -       .set            = virtio_vdpa_set,
>         .generation     = virtio_vdpa_generation,
>         .get_status     = virtio_vdpa_get_status,
>         .set_status     = virtio_vdpa_set_status,
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 0e652026b776..9346fa9e3d97 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -259,7 +259,8 @@ struct vdpa_map_file {
>   *                             @buf: buffer used to read to
>   *                             @len: the length to read from
>   *                             configuration space
> - * @set_config:                        Write to device specific configuration space
> + * @set_config_legacy:         Write to device specific configuration space
> + *                             Legacy functionality for virtio-spec < v1.3
>   *                             @vdev: vdpa device
>   *                             @offset: offset from the beginning of
>   *                             configuration space
> @@ -378,8 +379,8 @@ struct vdpa_config_ops {
>         size_t (*get_config_size)(struct vdpa_device *vdev);
>         void (*get_config)(struct vdpa_device *vdev, unsigned int offset,
>                            void *buf, unsigned int len);
> -       void (*set_config)(struct vdpa_device *vdev, unsigned int offset,
> -                          const void *buf, unsigned int len);
> +       void (*set_config_legacy)(struct vdpa_device *vdev,
> +                       unsigned int offset, const void *buf, unsigned int len);
>         u32 (*get_generation)(struct vdpa_device *vdev);
>         struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
>         int (*set_vq_affinity)(struct vdpa_device *vdev, u16 idx,
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index f5c48b61ab62..b7977f9ae596 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -157,13 +157,13 @@
>   */
>  #define VHOST_VDPA_GET_STATUS          _IOR(VHOST_VIRTIO, 0x71, __u8)
>  #define VHOST_VDPA_SET_STATUS          _IOW(VHOST_VIRTIO, 0x72, __u8)
> -/* Get and set the device config. The device config follows the same
> - * definition of the device config defined in virtio-spec.
> +/* Get the device config. The device config follows the same
> + * definition of the device config defined in virtio-spec. According to
> + * virtio-spec v1.3, all device configuration fields are read-only for the
> + * driver, and thus we do not have VHOST_VDPA_SET_CONFIG.
>   */
>  #define VHOST_VDPA_GET_CONFIG          _IOR(VHOST_VIRTIO, 0x73, \
>                                              struct vhost_vdpa_config)
> -#define VHOST_VDPA_SET_CONFIG          _IOW(VHOST_VIRTIO, 0x74, \
> -                                            struct vhost_vdpa_config)
>  /* Enable/disable the ring. */
>  #define VHOST_VDPA_SET_VRING_ENABLE    _IOW(VHOST_VIRTIO, 0x75, \
>                                              struct vhost_vring_state)
> --
> 2.34.1
>

Re: [PATCH v3 1/2] vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN

[Jason Wang]

On Wed, Sep 4, 2024 at 11:18 PM Carlos Bilbao
<carlos.bilbao.osdev@gmail.com> wrote:
>
> From: Carlos Bilbao <cbilbao@digitalocean.com>
>
> Initialize the speed and duplex fields in virtio_net_config to UNKNOWN.
> This is needed because mlx5_vdpa vDPA devices currently do not support the
> VIRTIO_NET_F_SPEED_DUPLEX feature which reports speed and duplex. Add
> needed helper cpu_to_mlx5vdpa32() to convert endianness of speed.
>
> Signed-off-by: Carlos Bilbao <cbilbao@digitalocean.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index b56aae3f7be3..41ca268d43ff 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -173,6 +173,11 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
>         return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
>  }
>
> +static __virtio32 cpu_to_mlx5vdpa32(struct mlx5_vdpa_dev *mvdev, u32 val)
> +{
> +       return __cpu_to_virtio32(mlx5_vdpa_is_little_endian(mvdev), val);
> +}
> +
>  static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
>  {
>         if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
> @@ -3433,6 +3438,13 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
>         init_rwsem(&ndev->reslock);
>         config = &ndev->config;
>
> +       /*
> +        * mlx5_vdpa vDPA devices currently don't support reporting or
> +        * setting the speed or duplex.
> +        */
> +       config->speed  = cpu_to_mlx5vdpa32(mvdev, SPEED_UNKNOWN);
> +       config->duplex = DUPLEX_UNKNOWN;
> +
>         if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU)) {
>                 err = config_func_mtu(mdev, add_config->net.mtu);
>                 if (err)
> --
> 2.34.1
>

Re: [PATCH v3 0/2] Properly initialize speed/duplex and remove vDPA config updates

[Michael S. Tsirkin]

On Wed, Sep 04, 2024 at 10:11:13AM -0500, Carlos Bilbao wrote:
> From: Carlos Bilbao <cbilbao@digitalocean.com>
> 
> Initialize speed and duplex for virtio_net_config to UNKNOWN (mlx5_vdpa
> vDPA devices currently do not support VIRTIO_NET_F_SPEED_DUPLEX). Remove
> ioctl VHOST_VDPA_SET_CONFIG and its related logic as it is not supported;
> see: https://docs.oasis-open.org/virtio/virtio/v1.3/virtio-v1.3.html
> 
> Carlos:
>   vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN
>   vdpa: Remove ioctl VHOST_VDPA_SET_CONFIG per spec compliance

This will need a rebase. Will apply once you post one.
Thanks!

> ---
> 
> Changes since v1:
>  Link: https://lkml.org/lkml/2024/8/29/1368
>  - Fix prefix of the first commit and add Reviewed-By tag.
>  - Redo second commit completely: instead of attempting to add support to
>    set configuration fields, remove ioctl and support entirely from vDPA
>    implementations -- because it's not allowed by spec.
> 
> Changes since v2:
>  Link: https://lkml.org/lkml/2024/9/3/1407
>  - Fix first commit by changing 4 spaces for a tab.
>  - In second commit, ENI is legacy and should keep set_config(). Change it
>    to set_config_legacy() to avoid future confusion and erroneous
>    implementations.
> 
> ---
>  drivers/vdpa/alibaba/eni_vdpa.c    |  2 +-
>  drivers/vdpa/ifcvf/ifcvf_main.c    | 10 ----------
>  drivers/vdpa/mlx5/net/mlx5_vnet.c  | 19 ++++++++++++-------
>  drivers/vdpa/pds/vdpa_dev.c        | 16 ----------------
>  drivers/vdpa/solidrun/snet_main.c  | 18 ------------------
>  drivers/vdpa/vdpa.c                | 16 ----------------
>  drivers/vdpa/vdpa_sim/vdpa_sim.c   | 16 ----------------
>  drivers/vdpa/vdpa_sim/vdpa_sim.h   |  1 -
>  drivers/vdpa/vdpa_user/vduse_dev.c |  7 -------
>  drivers/vdpa/virtio_pci/vp_vdpa.c  | 14 --------------
>  drivers/vhost/vdpa.c               | 26 --------------------------
>  drivers/virtio/virtio_vdpa.c       |  9 ---------
>  include/linux/vdpa.h               |  7 ++++---
>  include/uapi/linux/vhost.h         |  8 ++++----
>  14 files changed, 21 insertions(+), 148 deletions(-)

Re: [PATCH v3 0/2] Properly initialize speed/duplex and remove vDPA config updates

[Jason Wang]

On Tue, Sep 10, 2024 at 2:29 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Sep 04, 2024 at 10:11:13AM -0500, Carlos Bilbao wrote:
> > From: Carlos Bilbao <cbilbao@digitalocean.com>
> >
> > Initialize speed and duplex for virtio_net_config to UNKNOWN (mlx5_vdpa
> > vDPA devices currently do not support VIRTIO_NET_F_SPEED_DUPLEX). Remove
> > ioctl VHOST_VDPA_SET_CONFIG and its related logic as it is not supported;
> > see: https://docs.oasis-open.org/virtio/virtio/v1.3/virtio-v1.3.html
> >
> > Carlos:
> >   vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN
> >   vdpa: Remove ioctl VHOST_VDPA_SET_CONFIG per spec compliance
>
> This will need a rebase. Will apply once you post one.
> Thanks!

Note that I think patch 2 is probably not right as we indeed allow
config write for some device.

Thanks

Re: [PATCH v3 0/2] Properly initialize speed/duplex and remove vDPA config updates

[Carlos Bilbao]

Hello,

On 9/10/24 10:42 PM, Jason Wang wrote:
> On Tue, Sep 10, 2024 at 2:29 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Wed, Sep 04, 2024 at 10:11:13AM -0500, Carlos Bilbao wrote:
>>> From: Carlos Bilbao <cbilbao@digitalocean.com>
>>>
>>> Initialize speed and duplex for virtio_net_config to UNKNOWN (mlx5_vdpa
>>> vDPA devices currently do not support VIRTIO_NET_F_SPEED_DUPLEX). Remove
>>> ioctl VHOST_VDPA_SET_CONFIG and its related logic as it is not supported;
>>> see: https://docs.oasis-open.org/virtio/virtio/v1.3/virtio-v1.3.html
>>>
>>> Carlos:
>>>   vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN
>>>   vdpa: Remove ioctl VHOST_VDPA_SET_CONFIG per spec compliance
>> This will need a rebase. Will apply once you post one.
>> Thanks!
> Note that I think patch 2 is probably not right as we indeed allow
> config write for some device.


I'll rebase patch 1 and drop patch 2.


>
> Thanks
>

Thanks, Carlos

Re: [PATCH v3 0/2] Properly initialize speed/duplex and remove vDPA config updates

[Carlos Bilbao]

On 9/11/24 11:55 AM, Carlos Bilbao wrote:

> Hello,
>
> On 9/10/24 10:42 PM, Jason Wang wrote:
>> On Tue, Sep 10, 2024 at 2:29 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Wed, Sep 04, 2024 at 10:11:13AM -0500, Carlos Bilbao wrote:
>>>> From: Carlos Bilbao <cbilbao@digitalocean.com>
>>>>
>>>> Initialize speed and duplex for virtio_net_config to UNKNOWN (mlx5_vdpa
>>>> vDPA devices currently do not support VIRTIO_NET_F_SPEED_DUPLEX). Remove
>>>> ioctl VHOST_VDPA_SET_CONFIG and its related logic as it is not supported;
>>>> see: https://docs.oasis-open.org/virtio/virtio/v1.3/virtio-v1.3.html
>>>>
>>>> Carlos:
>>>>   vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN
>>>>   vdpa: Remove ioctl VHOST_VDPA_SET_CONFIG per spec compliance
>>> This will need a rebase. Will apply once you post one.
>>> Thanks!


I successfully patched linux-next without any issues. Could you please
specify the repo/branch you need me to rebase onto? Thanks in advance


>> Note that I think patch 2 is probably not right as we indeed allow
>> config write for some device.
>
> I'll rebase patch 1 and drop patch 2.
>
>
>> Thanks
>>
> Thanks, Carlos
>

Re: [PATCH v3 1/2] vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN

[Michael S. Tsirkin]

On Wed, Sep 04, 2024 at 10:11:14AM -0500, Carlos Bilbao wrote:
> From: Carlos Bilbao <cbilbao@digitalocean.com>
> 
> Initialize the speed and duplex fields in virtio_net_config to UNKNOWN.
> This is needed because mlx5_vdpa vDPA devices currently do not support the
> VIRTIO_NET_F_SPEED_DUPLEX feature which reports speed and duplex.

I see no logic here. Without this feature bit, guests will not read
this field, why do we suddenly need to initialize it?

> Add
> needed helper cpu_to_mlx5vdpa32() to convert endianness of speed.
> 
> Signed-off-by: Carlos Bilbao <cbilbao@digitalocean.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index b56aae3f7be3..41ca268d43ff 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -173,6 +173,11 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
>  	return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
>  }
>  
> +static __virtio32 cpu_to_mlx5vdpa32(struct mlx5_vdpa_dev *mvdev, u32 val)
> +{
> +	return __cpu_to_virtio32(mlx5_vdpa_is_little_endian(mvdev), val);
> +}
> +
>  static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
>  {
>  	if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
> @@ -3433,6 +3438,13 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
>  	init_rwsem(&ndev->reslock);
>  	config = &ndev->config;
>  
> +	/*
> +	 * mlx5_vdpa vDPA devices currently don't support reporting or
> +	 * setting the speed or duplex.
> +	 */
> +	config->speed  = cpu_to_mlx5vdpa32(mvdev, SPEED_UNKNOWN);
> +	config->duplex = DUPLEX_UNKNOWN;
> +
>  	if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU)) {
>  		err = config_func_mtu(mdev, add_config->net.mtu);
>  		if (err)
> -- 
> 2.34.1

Re: [PATCH v3 1/2] vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN

[Dragos Tatulea]

On 07.11.24 22:50, Michael S. Tsirkin wrote:
> On Wed, Sep 04, 2024 at 10:11:14AM -0500, Carlos Bilbao wrote:
>> From: Carlos Bilbao <cbilbao@digitalocean.com>
>>
>> Initialize the speed and duplex fields in virtio_net_config to UNKNOWN.
>> This is needed because mlx5_vdpa vDPA devices currently do not support the
>> VIRTIO_NET_F_SPEED_DUPLEX feature which reports speed and duplex.
> 
> I see no logic here. Without this feature bit, guests will not read
> this field, why do we suddenly need to initialize it?
> 
IIRC, Carlos was reading data via ioctl VHOST_VDPA_GET_CONFIG which calls
.get_config() directly, always exposing the speed and duplex config fields [0].
Carlos, was this the case?

[0] https://lore.kernel.org/lkml/afcbf041-7613-48e6-8088-9d52edd907ff@nvidia.com/T/

Thanks,
Dragos

>> Add
>> needed helper cpu_to_mlx5vdpa32() to convert endianness of speed.
>>
>> Signed-off-by: Carlos Bilbao <cbilbao@digitalocean.com>
>> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
>> ---
>>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>> index b56aae3f7be3..41ca268d43ff 100644
>> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>> @@ -173,6 +173,11 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
>>  	return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
>>  }
>>  
>> +static __virtio32 cpu_to_mlx5vdpa32(struct mlx5_vdpa_dev *mvdev, u32 val)
>> +{
>> +	return __cpu_to_virtio32(mlx5_vdpa_is_little_endian(mvdev), val);
>> +}
>> +
>>  static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
>>  {
>>  	if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
>> @@ -3433,6 +3438,13 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
>>  	init_rwsem(&ndev->reslock);
>>  	config = &ndev->config;
>>  
>> +	/*
>> +	 * mlx5_vdpa vDPA devices currently don't support reporting or
>> +	 * setting the speed or duplex.
>> +	 */
>> +	config->speed  = cpu_to_mlx5vdpa32(mvdev, SPEED_UNKNOWN);
>> +	config->duplex = DUPLEX_UNKNOWN;
>> +
>>  	if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU)) {
>>  		err = config_func_mtu(mdev, add_config->net.mtu);
>>  		if (err)
>> -- 
>> 2.34.1
> 

Re: [PATCH v3 1/2] vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN

[Michael S. Tsirkin]

On Fri, Nov 08, 2024 at 10:31:58AM +0100, Dragos Tatulea wrote:
> 
> 
> On 07.11.24 22:50, Michael S. Tsirkin wrote:
> > On Wed, Sep 04, 2024 at 10:11:14AM -0500, Carlos Bilbao wrote:
> >> From: Carlos Bilbao <cbilbao@digitalocean.com>
> >>
> >> Initialize the speed and duplex fields in virtio_net_config to UNKNOWN.
> >> This is needed because mlx5_vdpa vDPA devices currently do not support the
> >> VIRTIO_NET_F_SPEED_DUPLEX feature which reports speed and duplex.
> > 
> > I see no logic here. Without this feature bit, guests will not read
> > this field, why do we suddenly need to initialize it?
> > 
> IIRC, Carlos was reading data via ioctl VHOST_VDPA_GET_CONFIG which calls
> .get_config() directly, always exposing the speed and duplex config fields [0].
> Carlos, was this the case?
> 
> [0] https://lore.kernel.org/lkml/afcbf041-7613-48e6-8088-9d52edd907ff@nvidia.com/T/
> 
> Thanks,
> Dragos

Basically, driver should ignore these if feature is not set.


> >> Add
> >> needed helper cpu_to_mlx5vdpa32() to convert endianness of speed.
> >>
> >> Signed-off-by: Carlos Bilbao <cbilbao@digitalocean.com>
> >> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> >> ---
> >>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >> index b56aae3f7be3..41ca268d43ff 100644
> >> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >> @@ -173,6 +173,11 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
> >>  	return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
> >>  }
> >>  
> >> +static __virtio32 cpu_to_mlx5vdpa32(struct mlx5_vdpa_dev *mvdev, u32 val)
> >> +{
> >> +	return __cpu_to_virtio32(mlx5_vdpa_is_little_endian(mvdev), val);
> >> +}
> >> +
> >>  static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
> >>  {
> >>  	if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
> >> @@ -3433,6 +3438,13 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
> >>  	init_rwsem(&ndev->reslock);
> >>  	config = &ndev->config;
> >>  
> >> +	/*
> >> +	 * mlx5_vdpa vDPA devices currently don't support reporting or
> >> +	 * setting the speed or duplex.
> >> +	 */
> >> +	config->speed  = cpu_to_mlx5vdpa32(mvdev, SPEED_UNKNOWN);
> >> +	config->duplex = DUPLEX_UNKNOWN;
> >> +
> >>  	if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU)) {
> >>  		err = config_func_mtu(mdev, add_config->net.mtu);
> >>  		if (err)
> >> -- 
> >> 2.34.1
> > 

Re: [PATCH v3 1/2] vdpa/mlx5: Set speed and duplex of vDPA devices to UNKNOWN

[Carlos Bilbao]

Hello,

On 11/8/24 5:51 AM, Michael S. Tsirkin wrote:
> On Fri, Nov 08, 2024 at 10:31:58AM +0100, Dragos Tatulea wrote:
>>
>> On 07.11.24 22:50, Michael S. Tsirkin wrote:
>>> On Wed, Sep 04, 2024 at 10:11:14AM -0500, Carlos Bilbao wrote:
>>>> From: Carlos Bilbao <cbilbao@digitalocean.com>
>>>>
>>>> Initialize the speed and duplex fields in virtio_net_config to UNKNOWN.
>>>> This is needed because mlx5_vdpa vDPA devices currently do not support the
>>>> VIRTIO_NET_F_SPEED_DUPLEX feature which reports speed and duplex.
>>> I see no logic here. Without this feature bit, guests will not read
>>> this field, why do we suddenly need to initialize it?
>>>
>> IIRC, Carlos was reading data via ioctl VHOST_VDPA_GET_CONFIG which calls
>> .get_config() directly, always exposing the speed and duplex config fields [0].
>> Carlos, was this the case?
>>
>> [0] https://lore.kernel.org/lkml/afcbf041-7613-48e6-8088-9d52edd907ff@nvidia.com/T/
>>
>> Thanks,
>> Dragos
> Basically, driver should ignore these if feature is not set.


There _is_ a chance the guest could read this field; As Dragos mentioned, I
was using the VHOST_VDPA_GET_CONFIG ioctl from userspace, and the incorrect
field initialization led me to believe I was in half-duplex mode --
something people told me they hadn't seen since the 80s. But as Andrew
(CCed) mentioned, "If the speed is 0, does duplex even matter?".

I also tried to remove the pointless ioctl call altogether to avoid further
confusion [0] but that wasn't viable due to spec compliance.

Initializing the fields seems the simple solution here (and, IMHO, more
generally, it's as a good programming practice)

Thanks for looking into this!

Best,
Carlos

[0] https://lore.kernel.org/lkml/CACGkMEvfdUYLjx-Z+oB11XW-54ErJsQMKcnu2p=dsj5N_BiEKw@mail.gmail.com/


>
>
>>>> Add
>>>> needed helper cpu_to_mlx5vdpa32() to convert endianness of speed.
>>>>
>>>> Signed-off-by: Carlos Bilbao <cbilbao@digitalocean.com>
>>>> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
>>>> ---
>>>>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 ++++++++++++
>>>>  1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>>> index b56aae3f7be3..41ca268d43ff 100644
>>>> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>>> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>>> @@ -173,6 +173,11 @@ static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)
>>>>  	return __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev), val);
>>>>  }
>>>>  
>>>> +static __virtio32 cpu_to_mlx5vdpa32(struct mlx5_vdpa_dev *mvdev, u32 val)
>>>> +{
>>>> +	return __cpu_to_virtio32(mlx5_vdpa_is_little_endian(mvdev), val);
>>>> +}
>>>> +
>>>>  static u16 ctrl_vq_idx(struct mlx5_vdpa_dev *mvdev)
>>>>  {
>>>>  	if (!(mvdev->actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
>>>> @@ -3433,6 +3438,13 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
>>>>  	init_rwsem(&ndev->reslock);
>>>>  	config = &ndev->config;
>>>>  
>>>> +	/*
>>>> +	 * mlx5_vdpa vDPA devices currently don't support reporting or
>>>> +	 * setting the speed or duplex.
>>>> +	 */
>>>> +	config->speed  = cpu_to_mlx5vdpa32(mvdev, SPEED_UNKNOWN);
>>>> +	config->duplex = DUPLEX_UNKNOWN;
>>>> +
>>>>  	if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU)) {
>>>>  		err = config_func_mtu(mdev, add_config->net.mtu);
>>>>  		if (err)
>>>> -- 
>>>> 2.34.1