[dpdk-dev] [PATCH 0/7]virtio-user: introduce vhost-vdpa backend

[dpdk-dev] [PATCH 0/7]virtio-user: introduce vhost-vdpa backend

[Maxime Coquelin]

vhost-vDPA is a new vhost backend type introduced by vDPA kernel
framework, which provides abstruction to the vDPA devices and
exposes to userspace a unified control interface through char devs.

This patch set adds vhost-vdpa backend type to the virtio_user.
A set of vhost-vdpa specific ops callbacks are attached to the
virtio_user according to the runtime dynamic check result of the
backend type. DMA memory map/unmap callbacks are added to both
vdev bus driver and virtio_user pmd to support address mapping.
In addition, minor fixes to existing virtio control path are also
implemented to make the new backend work.

This is a collaborative work done with Patrick Fu from Intel and
Adrian Moreno from Red Hat. Thanks to them for their contributions.

The series has been tested with vdpasim and Intel IFC Kernel vDPA
drivers, and more lightly with Mellanox mlx5_vdpa on ConnectX-6 Dx.

Next revision will contains some clean-up for the backend type
handling.

Maxime Coquelin (7):
  bus/vdev: add DMA mapping ops
  net/virtio: introduce DMA ops
  net/virtio: introduce Vhost-vDPA backend type
  net/virtio: adapt Virtio-user status size
  net/virtio: check protocol feature in user backend
  net/virtio: split virtio-user start
  net/virtio: introduce Vhost-vDPA backend

 drivers/bus/vdev/rte_bus_vdev.h               |  46 ++-
 drivers/bus/vdev/vdev.c                       |  52 +++
 drivers/net/virtio/meson.build                |   1 +
 drivers/net/virtio/virtio_user/vhost.h        |   5 +
 drivers/net/virtio/virtio_user/vhost_user.c   |   6 +-
 drivers/net/virtio/virtio_user/vhost_vdpa.c   | 310 ++++++++++++++++++
 .../net/virtio/virtio_user/virtio_user_dev.c  | 163 ++++++---
 .../net/virtio/virtio_user/virtio_user_dev.h  |  11 +-
 drivers/net/virtio/virtio_user_ethdev.c       |  61 +++-
 9 files changed, 599 insertions(+), 56 deletions(-)
 create mode 100644 drivers/net/virtio/virtio_user/vhost_vdpa.c

-- 
2.26.2

[dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops

[Maxime Coquelin]

Add DMA map/unmap operation callbacks to the vdev bus, which
could be used by DMA capable vdev drivers.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/bus/vdev/rte_bus_vdev.h | 46 +++++++++++++++++++++++++++--
 drivers/bus/vdev/vdev.c         | 52 +++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/vdev/rte_bus_vdev.h b/drivers/bus/vdev/rte_bus_vdev.h
index 78a032cea8..3fe0b35a82 100644
--- a/drivers/bus/vdev/rte_bus_vdev.h
+++ b/drivers/bus/vdev/rte_bus_vdev.h
@@ -63,14 +63,54 @@ typedef int (rte_vdev_probe_t)(struct rte_vdev_device *dev);
  */
 typedef int (rte_vdev_remove_t)(struct rte_vdev_device *dev);
 
+/**
+ * Driver-specific DMA mapping. After a successful call the device
+ * will be able to read/write from/to this segment.
+ *
+ * @param dev
+ *   Pointer to the Virtual device.
+ * @param addr
+ *   Starting virtual address of memory to be mapped.
+ * @param iova
+ *   Starting IOVA address of memory to be mapped.
+ * @param len
+ *   Length of memory segment being mapped.
+ * @return
+ *   - 0 On success.
+ *   - Negative value and rte_errno is set otherwise.
+ */
+typedef int (rte_vdev_dma_map_t)(struct rte_vdev_device *dev, void *addr,
+			    uint64_t iova, size_t len);
+
+/**
+ * Driver-specific DMA un-mapping. After a successful call the device
+ * will not be able to read/write from/to this segment.
+ *
+ * @param dev
+ *   Pointer to the Virtual device.
+ * @param addr
+ *   Starting virtual address of memory to be unmapped.
+ * @param iova
+ *   Starting IOVA address of memory to be unmapped.
+ * @param len
+ *   Length of memory segment being unmapped.
+ * @return
+ *   - 0 On success.
+ *   - Negative value and rte_errno is set otherwise.
+ */
+typedef int (rte_vdev_dma_unmap_t)(struct rte_vdev_device *dev, void *addr,
+			      uint64_t iova, size_t len);
+
 /**
  * A virtual device driver abstraction.
  */
 struct rte_vdev_driver {
 	TAILQ_ENTRY(rte_vdev_driver) next; /**< Next in list. */
-	struct rte_driver driver;      /**< Inherited general driver. */
-	rte_vdev_probe_t *probe;       /**< Virtual device probe function. */
-	rte_vdev_remove_t *remove;     /**< Virtual device remove function. */
+	struct rte_driver driver;        /**< Inherited general driver. */
+	rte_vdev_probe_t *probe;         /**< Virtual device probe function. */
+	rte_vdev_remove_t *remove;       /**< Virtual device remove function. */
+	rte_vdev_dma_map_t *dma_map;     /**< vdev DMA map function. */
+	rte_vdev_dma_unmap_t *dma_unmap; /**< vdev DMA unmap function. */
 };
 
 /**
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index d746149a2e..455d0bd0c2 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -134,6 +134,56 @@ vdev_parse(const char *name, void *addr)
 	return driver == NULL;
 }
 
+static int
+vdev_dma_map(struct rte_device *dev, void *addr, uint64_t iova, size_t len)
+{
+	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
+	const struct rte_vdev_driver *driver;
+
+	if (!vdev) {
+		rte_errno = EINVAL;
+		return -1;
+	}
+
+	if (!vdev->device.driver) {
+		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
+		return 1;
+	}
+
+	driver = container_of(vdev->device.driver, const struct rte_vdev_driver,
+			driver);
+
+	if (driver->dma_map)
+		return driver->dma_map(vdev, addr, iova, len);
+
+	return 0;
+}
+
+static int
+vdev_dma_unmap(struct rte_device *dev, void *addr, uint64_t iova, size_t len)
+{
+	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
+	const struct rte_vdev_driver *driver;
+
+	if (!vdev) {
+		rte_errno = EINVAL;
+		return -1;
+	}
+
+	if (!vdev->device.driver) {
+		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
+		return 1;
+	}
+
+	driver = container_of(vdev->device.driver, const struct rte_vdev_driver,
+			driver);
+
+	if (driver->dma_unmap)
+		return driver->dma_unmap(vdev, addr, iova, len);
+
+	return 0;	return 0;
+}
+
 static int
 vdev_probe_all_drivers(struct rte_vdev_device *dev)
 {
@@ -551,6 +601,8 @@ static struct rte_bus rte_vdev_bus = {
 	.plug = vdev_plug,
 	.unplug = vdev_unplug,
 	.parse = vdev_parse,
+	.dma_map = vdev_dma_map,
+	.dma_unmap = vdev_dma_unmap,
 	.dev_iterate = rte_vdev_dev_iterate,
 };
 
-- 
2.26.2

Re: [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops

[Xia, Chenbo]

Hi Maxime,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> Sent: Friday, September 11, 2020 11:08 PM
> To: dev@dpdk.org; Fu, Patrick <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops
> 
> Add DMA map/unmap operation callbacks to the vdev bus, which
> could be used by DMA capable vdev drivers.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/bus/vdev/rte_bus_vdev.h | 46 +++++++++++++++++++++++++++--
>  drivers/bus/vdev/vdev.c         | 52 +++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bus/vdev/rte_bus_vdev.h
> b/drivers/bus/vdev/rte_bus_vdev.h
> index 78a032cea8..3fe0b35a82 100644
> --- a/drivers/bus/vdev/rte_bus_vdev.h
> +++ b/drivers/bus/vdev/rte_bus_vdev.h
> @@ -63,14 +63,54 @@ typedef int (rte_vdev_probe_t)(struct rte_vdev_device
> *dev);
>   */
>  typedef int (rte_vdev_remove_t)(struct rte_vdev_device *dev);
> 
> +/**
> + * Driver-specific DMA mapping. After a successful call the device
> + * will be able to read/write from/to this segment.
> + *
> + * @param dev
> + *   Pointer to the Virtual device.
> + * @param addr
> + *   Starting virtual address of memory to be mapped.
> + * @param iova
> + *   Starting IOVA address of memory to be mapped.
> + * @param len
> + *   Length of memory segment being mapped.
> + * @return
> + *   - 0 On success.
> + *   - Negative value and rte_errno is set otherwise.
> + */
> +typedef int (rte_vdev_dma_map_t)(struct rte_vdev_device *dev, void *addr,
> +			    uint64_t iova, size_t len);
> +
> +/**
> + * Driver-specific DMA un-mapping. After a successful call the device
> + * will not be able to read/write from/to this segment.
> + *
> + * @param dev
> + *   Pointer to the Virtual device.
> + * @param addr
> + *   Starting virtual address of memory to be unmapped.
> + * @param iova
> + *   Starting IOVA address of memory to be unmapped.
> + * @param len
> + *   Length of memory segment being unmapped.
> + * @return
> + *   - 0 On success.
> + *   - Negative value and rte_errno is set otherwise.
> + */
> +typedef int (rte_vdev_dma_unmap_t)(struct rte_vdev_device *dev, void
> *addr,
> +			      uint64_t iova, size_t len);
> +
>  /**
>   * A virtual device driver abstraction.
>   */
>  struct rte_vdev_driver {
>  	TAILQ_ENTRY(rte_vdev_driver) next; /**< Next in list. */
> -	struct rte_driver driver;      /**< Inherited general driver. */
> -	rte_vdev_probe_t *probe;       /**< Virtual device probe function.
> */
> -	rte_vdev_remove_t *remove;     /**< Virtual device remove function.
> */
> +	struct rte_driver driver;        /**< Inherited general driver. */
> +	rte_vdev_probe_t *probe;         /**< Virtual device probe function.
> */
> +	rte_vdev_remove_t *remove;       /**< Virtual device remove function.
> */
> +	rte_vdev_dma_map_t *dma_map;     /**< vdev DMA map function. */
> +	rte_vdev_dma_unmap_t *dma_unmap; /**< vdev DMA unmap function. */
>  };

I think we'd better unify the term 'Virtual device' or 'vdev'? Maybe just
use Virtual device.

> 
>  /**
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index d746149a2e..455d0bd0c2 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -134,6 +134,56 @@ vdev_parse(const char *name, void *addr)
>  	return driver == NULL;
>  }
> 
> +static int
> +vdev_dma_map(struct rte_device *dev, void *addr, uint64_t iova, size_t
> len)
> +{
> +	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
> +	const struct rte_vdev_driver *driver;
> +
> +	if (!vdev) {
> +		rte_errno = EINVAL;
> +		return -1;
> +	}
> +
> +	if (!vdev->device.driver) {
> +		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
> +		return 1;
> +	}
> +
> +	driver = container_of(vdev->device.driver, const struct
> rte_vdev_driver,
> +			driver);
> +
> +	if (driver->dma_map)
> +		return driver->dma_map(vdev, addr, iova, len);
> +
> +	return 0;
> +}
> +
> +static int
> +vdev_dma_unmap(struct rte_device *dev, void *addr, uint64_t iova, size_t
> len)
> +{
> +	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
> +	const struct rte_vdev_driver *driver;
> +
> +	if (!vdev) {
> +		rte_errno = EINVAL;
> +		return -1;
> +	}
> +
> +	if (!vdev->device.driver) {
> +		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
> +		return 1;
> +	}
> +
> +	driver = container_of(vdev->device.driver, const struct
> rte_vdev_driver,
> +			driver);
> +
> +	if (driver->dma_unmap)
> +		return driver->dma_unmap(vdev, addr, iova, len);
> +
> +	return 0;	return 0;

Duplicate return here 😊

Thanks!
Chenbo

> +}
> +
>  static int
>  vdev_probe_all_drivers(struct rte_vdev_device *dev)
>  {
> @@ -551,6 +601,8 @@ static struct rte_bus rte_vdev_bus = {
>  	.plug = vdev_plug,
>  	.unplug = vdev_unplug,
>  	.parse = vdev_parse,
> +	.dma_map = vdev_dma_map,
> +	.dma_unmap = vdev_dma_unmap,
>  	.dev_iterate = rte_vdev_dev_iterate,
>  };
> 
> --
> 2.26.2

Re: [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops

[Maxime Coquelin]

On 9/24/20 7:25 AM, Xia, Chenbo wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
>> Sent: Friday, September 11, 2020 11:08 PM
>> To: dev@dpdk.org; Fu, Patrick <patrick.fu@intel.com>; amorenoz@redhat.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops
>>
>> Add DMA map/unmap operation callbacks to the vdev bus, which
>> could be used by DMA capable vdev drivers.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  drivers/bus/vdev/rte_bus_vdev.h | 46 +++++++++++++++++++++++++++--
>>  drivers/bus/vdev/vdev.c         | 52 +++++++++++++++++++++++++++++++++
>>  2 files changed, 95 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bus/vdev/rte_bus_vdev.h
>> b/drivers/bus/vdev/rte_bus_vdev.h
>> index 78a032cea8..3fe0b35a82 100644
>> --- a/drivers/bus/vdev/rte_bus_vdev.h
>> +++ b/drivers/bus/vdev/rte_bus_vdev.h
>> @@ -63,14 +63,54 @@ typedef int (rte_vdev_probe_t)(struct rte_vdev_device
>> *dev);
>>   */
>>  typedef int (rte_vdev_remove_t)(struct rte_vdev_device *dev);
>>
>> +/**
>> + * Driver-specific DMA mapping. After a successful call the device
>> + * will be able to read/write from/to this segment.
>> + *
>> + * @param dev
>> + *   Pointer to the Virtual device.
>> + * @param addr
>> + *   Starting virtual address of memory to be mapped.
>> + * @param iova
>> + *   Starting IOVA address of memory to be mapped.
>> + * @param len
>> + *   Length of memory segment being mapped.
>> + * @return
>> + *   - 0 On success.
>> + *   - Negative value and rte_errno is set otherwise.
>> + */
>> +typedef int (rte_vdev_dma_map_t)(struct rte_vdev_device *dev, void *addr,
>> +			    uint64_t iova, size_t len);
>> +
>> +/**
>> + * Driver-specific DMA un-mapping. After a successful call the device
>> + * will not be able to read/write from/to this segment.
>> + *
>> + * @param dev
>> + *   Pointer to the Virtual device.
>> + * @param addr
>> + *   Starting virtual address of memory to be unmapped.
>> + * @param iova
>> + *   Starting IOVA address of memory to be unmapped.
>> + * @param len
>> + *   Length of memory segment being unmapped.
>> + * @return
>> + *   - 0 On success.
>> + *   - Negative value and rte_errno is set otherwise.
>> + */
>> +typedef int (rte_vdev_dma_unmap_t)(struct rte_vdev_device *dev, void
>> *addr,
>> +			      uint64_t iova, size_t len);
>> +
>>  /**
>>   * A virtual device driver abstraction.
>>   */
>>  struct rte_vdev_driver {
>>  	TAILQ_ENTRY(rte_vdev_driver) next; /**< Next in list. */
>> -	struct rte_driver driver;      /**< Inherited general driver. */
>> -	rte_vdev_probe_t *probe;       /**< Virtual device probe function.
>> */
>> -	rte_vdev_remove_t *remove;     /**< Virtual device remove function.
>> */
>> +	struct rte_driver driver;        /**< Inherited general driver. */
>> +	rte_vdev_probe_t *probe;         /**< Virtual device probe function.
>> */
>> +	rte_vdev_remove_t *remove;       /**< Virtual device remove function.
>> */
>> +	rte_vdev_dma_map_t *dma_map;     /**< vdev DMA map function. */
>> +	rte_vdev_dma_unmap_t *dma_unmap; /**< vdev DMA unmap function. */
>>  };
> 
> I think we'd better unify the term 'Virtual device' or 'vdev'? Maybe just
> use Virtual device.

Right, fixing it in v2.

>>
>>  /**
>> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
>> index d746149a2e..455d0bd0c2 100644
>> --- a/drivers/bus/vdev/vdev.c
>> +++ b/drivers/bus/vdev/vdev.c
>> @@ -134,6 +134,56 @@ vdev_parse(const char *name, void *addr)
>>  	return driver == NULL;
>>  }
>>
>> +static int
>> +vdev_dma_map(struct rte_device *dev, void *addr, uint64_t iova, size_t
>> len)
>> +{
>> +	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
>> +	const struct rte_vdev_driver *driver;
>> +
>> +	if (!vdev) {
>> +		rte_errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>> +	if (!vdev->device.driver) {
>> +		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
>> +		return 1;
>> +	}
>> +
>> +	driver = container_of(vdev->device.driver, const struct
>> rte_vdev_driver,
>> +			driver);
>> +
>> +	if (driver->dma_map)
>> +		return driver->dma_map(vdev, addr, iova, len);
>> +
>> +	return 0;
>> +}
>> +
>> +static int
>> +vdev_dma_unmap(struct rte_device *dev, void *addr, uint64_t iova, size_t
>> len)
>> +{
>> +	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
>> +	const struct rte_vdev_driver *driver;
>> +
>> +	if (!vdev) {
>> +		rte_errno = EINVAL;
>> +		return -1;
>> +	}
>> +
>> +	if (!vdev->device.driver) {
>> +		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
>> +		return 1;
>> +	}
>> +
>> +	driver = container_of(vdev->device.driver, const struct
>> rte_vdev_driver,
>> +			driver);
>> +
>> +	if (driver->dma_unmap)
>> +		return driver->dma_unmap(vdev, addr, iova, len);
>> +
>> +	return 0;	return 0;
> 
> Duplicate return here 😊

Oops!

Maxime

> Thanks!
> Chenbo
> 
>> +}
>> +
>>  static int
>>  vdev_probe_all_drivers(struct rte_vdev_device *dev)
>>  {
>> @@ -551,6 +601,8 @@ static struct rte_bus rte_vdev_bus = {
>>  	.plug = vdev_plug,
>>  	.unplug = vdev_unplug,
>>  	.parse = vdev_parse,
>> +	.dma_map = vdev_dma_map,
>> +	.dma_unmap = vdev_dma_unmap,
>>  	.dev_iterate = rte_vdev_dev_iterate,
>>  };
>>
>> --
>> 2.26.2
> 

[dpdk-dev] [PATCH 2/7] net/virtio: introduce DMA ops

[Maxime Coquelin]

Add DMA map/unmap callbacks to the virtio_user pmd, which could
be leveraged by vdev bus driver to map memory for backend
devices with DMA capability.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_user/vhost.h  |  4 ++
 drivers/net/virtio/virtio_user_ethdev.c | 54 +++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index 8f49ef4574..2e71995a79 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -105,6 +105,10 @@ struct virtio_user_backend_ops {
 	int (*enable_qp)(struct virtio_user_dev *dev,
 			 uint16_t pair_idx,
 			 int enable);
+	int (*dma_map)(struct virtio_user_dev *dev, void *addr,
+				  uint64_t iova, size_t len);
+	int (*dma_unmap)(struct virtio_user_dev *dev, void *addr,
+				  uint64_t iova, size_t len);
 };
 
 extern struct virtio_user_backend_ops virtio_ops_user;
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 87f6cb6950..60d17af888 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -818,9 +818,63 @@ virtio_user_pmd_remove(struct rte_vdev_device *vdev)
 	return 0;
 }
 
+static int virtio_user_pmd_dma_map(struct rte_vdev_device *vdev, void *addr,
+		uint64_t iova, size_t len)
+{
+	const char *name;
+	struct rte_eth_dev *eth_dev;
+	struct virtio_user_dev *dev;
+	struct virtio_hw *hw;
+
+	if (!vdev)
+		return -EINVAL;
+
+	name = rte_vdev_device_name(vdev);
+	eth_dev = rte_eth_dev_allocated(name);
+	/* Port has already been released by close. */
+	if (!eth_dev)
+		return 0;
+
+	hw = (struct virtio_hw *)eth_dev->data->dev_private;
+	dev = hw->virtio_user_dev;
+
+	if (dev->ops->dma_map)
+		return dev->ops->dma_map(dev, addr, iova, len);
+
+	return 0;
+}
+
+static int virtio_user_pmd_dma_unmap(struct rte_vdev_device *vdev, void *addr,
+		uint64_t iova, size_t len)
+{
+	const char *name;
+	struct rte_eth_dev *eth_dev;
+	struct virtio_user_dev *dev;
+	struct virtio_hw *hw;
+
+	if (!vdev)
+		return -EINVAL;
+
+	name = rte_vdev_device_name(vdev);
+	eth_dev = rte_eth_dev_allocated(name);
+	/* Port has already been released by close. */
+	if (!eth_dev)
+		return 0;
+
+	hw = (struct virtio_hw *)eth_dev->data->dev_private;
+	dev = hw->virtio_user_dev;
+
+	if (dev->ops->dma_unmap)
+		return dev->ops->dma_unmap(dev, addr, iova, len);
+
+	return 0;
+}
+
 static struct rte_vdev_driver virtio_user_driver = {
 	.probe = virtio_user_pmd_probe,
 	.remove = virtio_user_pmd_remove,
+	.dma_map = virtio_user_pmd_dma_map,
+	.dma_unmap = virtio_user_pmd_dma_unmap,
 };
 
 RTE_PMD_REGISTER_VDEV(net_virtio_user, virtio_user_driver);
-- 
2.26.2

[dpdk-dev] [PATCH 3/7] net/virtio: introduce Vhost-vDPA backend type

[Maxime Coquelin]

Introduce virtio_user_backend_type function and its enum in
order to support more than two backends with vDPA backend
addition.
Backend type is determined by checking the virtio-user control
file information at runtime.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 86 +++++++++++++++----
 .../net/virtio/virtio_user/virtio_user_dev.h  | 10 ++-
 drivers/net/virtio/virtio_user_ethdev.c       |  3 +-
 3 files changed, 79 insertions(+), 20 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 2a0c861085..93274b2a94 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -12,6 +12,8 @@
 #include <sys/eventfd.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/sysmacros.h>
+#include <linux/major.h>
 
 #include <rte_string_fns.h>
 #include <rte_eal_memconfig.h>
@@ -111,15 +113,55 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
 	return 0;
 }
 
-int
-is_vhost_user_by_type(const char *path)
+static uint32_t
+vdpa_dynamic_major_num(void)
+{
+	FILE *fp;
+	char *line = NULL;
+	size_t size;
+	char name[11];
+	bool found = false;
+	uint32_t num;
+
+	fp = fopen("/proc/devices", "r");
+	if (fp == NULL) {
+		PMD_INIT_LOG(ERR, "Cannot open /proc/devices: %s",
+			     strerror(errno));
+		return UNNAMED_MAJOR;
+	}
+
+	while (getline(&line, &size, fp) > 0) {
+		char *stripped = line + strspn(line, " ");
+		if ((sscanf(stripped, "%u %10s", &num, name) == 2) &&
+		    (strncmp(name, "vhost-vdpa", 10) == 0)) {
+			found = true;
+			break;
+		}
+	}
+	fclose(fp);
+	return found ? num : UNNAMED_MAJOR;
+}
+
+enum virtio_user_backend_type
+virtio_user_backend_type(const char *path)
 {
 	struct stat sb;
 
-	if (stat(path, &sb) == -1)
-		return 0;
+	if (stat(path, &sb) == -1) {
+		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
+			     strerror(errno));
+		return VIRTIO_USER_BACKEND_UNKNOWN;
+	}
 
-	return S_ISSOCK(sb.st_mode);
+	if (S_ISSOCK(sb.st_mode)) {
+		return VIRTIO_USER_BACKEND_VHOST_USER;
+	} else if (S_ISCHR(sb.st_mode)) {
+		if (major(sb.st_rdev) == MISC_MAJOR)
+			return VIRTIO_USER_BACKEND_VHOST_KERNEL;
+		if (major(sb.st_rdev) == vdpa_dynamic_major_num())
+			return VIRTIO_USER_BACKEND_VHOST_VDPA;
+	}
+	return VIRTIO_USER_BACKEND_UNKNOWN;
 }
 
 int
@@ -144,7 +186,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	rte_mcfg_mem_read_lock();
 	pthread_mutex_lock(&dev->mutex);
 
-	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
+	if (virtio_user_backend_type(dev->path) ==
+		VIRTIO_USER_BACKEND_VHOST_USER && dev->vhostfd < 0)
 		goto error;
 
 	/* Step 0: tell vhost to create queues */
@@ -359,17 +402,19 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
 	dev->vhostfds = NULL;
 	dev->tapfds = NULL;
 
+	dev->backend_type = virtio_user_backend_type(dev->path);
+
 	if (dev->is_server) {
-		if (access(dev->path, F_OK) == 0 &&
-		    !is_vhost_user_by_type(dev->path)) {
-			PMD_DRV_LOG(ERR, "Server mode doesn't support vhost-kernel!");
+		if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) {
+			PMD_DRV_LOG(ERR, "Server mode only supports vhost-user!");
 			return -1;
 		}
 		dev->ops = &virtio_ops_user;
 	} else {
-		if (is_vhost_user_by_type(dev->path)) {
+		if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
 			dev->ops = &virtio_ops_user;
-		} else {
+		} else if (dev->backend_type ==
+					VIRTIO_USER_BACKEND_VHOST_KERNEL) {
 			dev->ops = &virtio_ops_kernel;
 
 			dev->vhostfds = malloc(dev->max_queue_pairs *
@@ -457,7 +502,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		return -1;
 	}
 
-	if (!is_vhost_user_by_type(dev->path))
+	if (virtio_user_backend_type(dev->path) !=
+					VIRTIO_USER_BACKEND_VHOST_USER)
 		dev->unsupported_features |=
 			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
 
@@ -505,8 +551,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES;
 	}
 
-
-
 	if (!mrg_rxbuf)
 		dev->unsupported_features |= (1ull << VIRTIO_NET_F_MRG_RXBUF);
 
@@ -539,7 +583,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 	}
 
 	/* The backend will not report this feature, we add it explicitly */
-	if (is_vhost_user_by_type(dev->path))
+	if (virtio_user_backend_type(dev->path) ==
+					VIRTIO_USER_BACKEND_VHOST_USER)
 		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
 
 	/*
@@ -790,9 +835,11 @@ virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
 {
 	int ret;
 	uint64_t arg = status;
+	enum virtio_user_backend_type backend_type =
+				virtio_user_backend_type(dev->path);
 
 	/* Vhost-user only for now */
-	if (!is_vhost_user_by_type(dev->path))
+	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		return 0;
 
 	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
@@ -813,9 +860,11 @@ virtio_user_update_status(struct virtio_user_dev *dev)
 {
 	uint64_t ret;
 	int err;
+	enum virtio_user_backend_type backend_type =
+				virtio_user_backend_type(dev->path);
 
 	/* Vhost-user only for now */
-	if (!is_vhost_user_by_type(dev->path))
+	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		return 0;
 
 	if (!(dev->protocol_features & (1UL << VHOST_USER_PROTOCOL_F_STATUS)))
@@ -828,7 +877,8 @@ virtio_user_update_status(struct virtio_user_dev *dev)
 		return -1;
 	}
 	if (ret > UINT8_MAX) {
-		PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS response 0x%" PRIx64 "\n", ret);
+		PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS "
+				"response 0x%" PRIx64 "\n", ret);
 		return -1;
 	}
 
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 9377d5ba66..4bb7cdbd26 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -10,6 +10,13 @@
 #include "../virtio_pci.h"
 #include "../virtio_ring.h"
 
+enum virtio_user_backend_type {
+	VIRTIO_USER_BACKEND_UNKNOWN,
+	VIRTIO_USER_BACKEND_VHOST_USER,
+	VIRTIO_USER_BACKEND_VHOST_KERNEL,
+	VIRTIO_USER_BACKEND_VHOST_VDPA,
+};
+
 struct virtio_user_queue {
 	uint16_t used_idx;
 	bool avail_wrap_counter;
@@ -17,6 +24,7 @@ struct virtio_user_queue {
 };
 
 struct virtio_user_dev {
+	enum virtio_user_backend_type backend_type;
 	/* for vhost_user backend */
 	int		vhostfd;
 	int		listenfd;   /* listening fd */
@@ -60,7 +68,7 @@ struct virtio_user_dev {
 	bool		started;
 };
 
-int is_vhost_user_by_type(const char *path);
+enum virtio_user_backend_type virtio_user_backend_type(const char *path);
 int virtio_user_start_device(struct virtio_user_dev *dev);
 int virtio_user_stop_device(struct virtio_user_dev *dev);
 int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 60d17af888..2163d0e30d 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -632,7 +632,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
 	}
 
 	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) {
-		if (is_vhost_user_by_type(path)) {
+		if (virtio_user_backend_type(path) !=
+				VIRTIO_USER_BACKEND_VHOST_KERNEL) {
 			PMD_INIT_LOG(ERR,
 				"arg %s applies only to vhost-kernel backend",
 				VIRTIO_USER_ARG_INTERFACE_NAME);
-- 
2.26.2

[dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size

[Maxime Coquelin]

Set proper payload size for set/get status message. The payload
size varies according to backend types.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Patrick Fu <patrick.fu@intel.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 93274b2a94..753611ef42 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -838,14 +838,18 @@ virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
 	enum virtio_user_backend_type backend_type =
 				virtio_user_backend_type(dev->path);
 
-	/* Vhost-user only for now */
-	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
+	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
 		return 0;
 
-	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
+	if (backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
+		ret = dev->ops->send_request(dev,
+				VHOST_USER_SET_STATUS, &arg);
+	else if (backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)
+		ret = dev->ops->send_request(dev,
+				VHOST_USER_SET_STATUS, &status);
+	else
 		return 0;
 
-	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret,
 			     strerror(errno));
@@ -858,7 +862,7 @@ virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
 int
 virtio_user_update_status(struct virtio_user_dev *dev)
 {
-	uint64_t ret;
+	uint8_t ret;
 	int err;
 	enum virtio_user_backend_type backend_type =
 				virtio_user_backend_type(dev->path);
@@ -876,11 +880,6 @@ virtio_user_update_status(struct virtio_user_dev *dev)
 			     strerror(errno));
 		return -1;
 	}
-	if (ret > UINT8_MAX) {
-		PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS "
-				"response 0x%" PRIx64 "\n", ret);
-		return -1;
-	}
 
 	dev->status = ret;
 	PMD_INIT_LOG(DEBUG, "Updated Device Status(0x%08x):\n"
-- 
2.26.2

Re: [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size

[Xia, Chenbo]

Hi Maxime,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> Sent: Friday, September 11, 2020 11:08 PM
> To: dev@dpdk.org; Fu, Patrick <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size
> 
> Set proper payload size for set/get status message. The payload
> size varies according to backend types.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Patrick Fu <patrick.fu@intel.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 93274b2a94..753611ef42 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -838,14 +838,18 @@ virtio_user_send_status_update(struct
> virtio_user_dev *dev, uint8_t status)
>  	enum virtio_user_backend_type backend_type =
>  				virtio_user_backend_type(dev->path);
> 
> -	/* Vhost-user only for now */
> -	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> +	if (!(dev->protocol_features & (1ULL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
>  		return 0;
> 
> -	if (!(dev->protocol_features & (1ULL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> +	if (backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
> +		ret = dev->ops->send_request(dev,
> +				VHOST_USER_SET_STATUS, &arg);
> +	else if (backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)
> +		ret = dev->ops->send_request(dev,
> +				VHOST_USER_SET_STATUS, &status);
> +	else
>  		return 0;
> 
> -	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
>  	if (ret) {
>  		PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret,
>  			     strerror(errno));
> @@ -858,7 +862,7 @@ virtio_user_send_status_update(struct virtio_user_dev
> *dev, uint8_t status)
>  int
>  virtio_user_update_status(struct virtio_user_dev *dev)
>  {
> -	uint64_t ret;
> +	uint8_t ret;

If you use uint8_t here, for vhost-vdpa, it's ok. But for vhost-user, with Adrian's
patch, it takes the status as uint64_t, and with below logic, it may overflow, right?

		switch (req) {
		case VHOST_USER_GET_FEATURES:
		case VHOST_USER_GET_STATUS:
		case VHOST_USER_GET_PROTOCOL_FEATURES:
			if (msg.size != sizeof(m.payload.u64)) {
				PMD_DRV_LOG(ERR, "Received bad msg size");
				return -1;
			}
			*((__u64 *)arg) = msg.payload.u64;
			break;

Thanks!
Chenbo

>  	int err;
>  	enum virtio_user_backend_type backend_type =
>  				virtio_user_backend_type(dev->path);
> @@ -876,11 +880,6 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>  			     strerror(errno));
>  		return -1;
>  	}
> -	if (ret > UINT8_MAX) {
> -		PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS "
> -				"response 0x%" PRIx64 "\n", ret);
> -		return -1;
> -	}
> 
>  	dev->status = ret;
>  	PMD_INIT_LOG(DEBUG, "Updated Device Status(0x%08x):\n"
> --
> 2.26.2

Re: [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size

[Maxime Coquelin]

On 9/24/20 7:25 AM, Xia, Chenbo wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
>> Sent: Friday, September 11, 2020 11:08 PM
>> To: dev@dpdk.org; Fu, Patrick <patrick.fu@intel.com>; amorenoz@redhat.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size
>>
>> Set proper payload size for set/get status message. The payload
>> size varies according to backend types.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Signed-off-by: Patrick Fu <patrick.fu@intel.com>
>> ---
>>  .../net/virtio/virtio_user/virtio_user_dev.c  | 19 +++++++++----------
>>  1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> index 93274b2a94..753611ef42 100644
>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> @@ -838,14 +838,18 @@ virtio_user_send_status_update(struct
>> virtio_user_dev *dev, uint8_t status)
>>  	enum virtio_user_backend_type backend_type =
>>  				virtio_user_backend_type(dev->path);
>>
>> -	/* Vhost-user only for now */
>> -	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>> +	if (!(dev->protocol_features & (1ULL <<
>> VHOST_USER_PROTOCOL_F_STATUS)))
>>  		return 0;
>>
>> -	if (!(dev->protocol_features & (1ULL <<
>> VHOST_USER_PROTOCOL_F_STATUS)))
>> +	if (backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
>> +		ret = dev->ops->send_request(dev,
>> +				VHOST_USER_SET_STATUS, &arg);
>> +	else if (backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)
>> +		ret = dev->ops->send_request(dev,
>> +				VHOST_USER_SET_STATUS, &status);
>> +	else
>>  		return 0;
>>
>> -	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
>>  	if (ret) {
>>  		PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret,
>>  			     strerror(errno));
>> @@ -858,7 +862,7 @@ virtio_user_send_status_update(struct virtio_user_dev
>> *dev, uint8_t status)
>>  int
>>  virtio_user_update_status(struct virtio_user_dev *dev)
>>  {
>> -	uint64_t ret;
>> +	uint8_t ret;
> 
> If you use uint8_t here, for vhost-vdpa, it's ok. But for vhost-user, with Adrian's
> patch, it takes the status as uint64_t, and with below logic, it may overflow, right?
> 
> 		switch (req) {
> 		case VHOST_USER_GET_FEATURES:
> 		case VHOST_USER_GET_STATUS:
> 		case VHOST_USER_GET_PROTOCOL_FEATURES:
> 			if (msg.size != sizeof(m.payload.u64)) {
> 				PMD_DRV_LOG(ERR, "Received bad msg size");
> 				return -1;
> 			}
> 			*((__u64 *)arg) = msg.payload.u64;
> 			break;

Yes, it could.
I can add a dedicated case for GET_STATUS, so that it can check it does
not overflow 8bits.

Thanks,
Maxime

> Thanks!
> Chenbo
> 
>>  	int err;
>>  	enum virtio_user_backend_type backend_type =
>>  				virtio_user_backend_type(dev->path);
>> @@ -876,11 +880,6 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>>  			     strerror(errno));
>>  		return -1;
>>  	}
>> -	if (ret > UINT8_MAX) {
>> -		PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS "
>> -				"response 0x%" PRIx64 "\n", ret);
>> -		return -1;
>> -	}
>>
>>  	dev->status = ret;
>>  	PMD_INIT_LOG(DEBUG, "Updated Device Status(0x%08x):\n"
>> --
>> 2.26.2
> 

[dpdk-dev] [PATCH 5/7] net/virtio: check protocol feature in user backend

[Maxime Coquelin]

When sending set status message, move protocol feature check
to vhost_user to be compatible with different backend types.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_user/vhost_user.c      | 6 +++++-
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 -------
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index 12b6c7dbcf..ef290c357b 100644
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -277,9 +277,13 @@ vhost_user_sock(struct virtio_user_dev *dev,
 	msg.size = 0;
 
 	switch (req) {
+	case VHOST_USER_GET_STATUS:
+		if (!(dev->protocol_features &
+				(1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
+			return 0;
+		/* Fallthrough */
 	case VHOST_USER_GET_FEATURES:
 	case VHOST_USER_GET_PROTOCOL_FEATURES:
-	case VHOST_USER_GET_STATUS:
 		need_reply = 1;
 		break;
 
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 753611ef42..97baa243cd 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -838,9 +838,6 @@ virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
 	enum virtio_user_backend_type backend_type =
 				virtio_user_backend_type(dev->path);
 
-	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
-		return 0;
-
 	if (backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
 		ret = dev->ops->send_request(dev,
 				VHOST_USER_SET_STATUS, &arg);
@@ -867,13 +864,9 @@ virtio_user_update_status(struct virtio_user_dev *dev)
 	enum virtio_user_backend_type backend_type =
 				virtio_user_backend_type(dev->path);
 
-	/* Vhost-user only for now */
 	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		return 0;
 
-	if (!(dev->protocol_features & (1UL << VHOST_USER_PROTOCOL_F_STATUS)))
-		return 0;
-
 	err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
 	if (err) {
 		PMD_INIT_LOG(ERR, "VHOST_USER_GET_STATUS failed (%d): %s", err,
-- 
2.26.2

[dpdk-dev] [PATCH 6/7] net/virtio: split virtio-user start

[Maxime Coquelin]

Move feature bit settings in device start out as an standalone
function, so that feature bit could be negotiated at device
feature_ok status.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 50 ++++++++++++-------
 .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
 drivers/net/virtio/virtio_user_ethdev.c       |  4 ++
 3 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 97baa243cd..2e097a95ea 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -165,25 +165,11 @@ virtio_user_backend_type(const char *path)
 }
 
 int
-virtio_user_start_device(struct virtio_user_dev *dev)
+virtio_user_dev_set_features(struct virtio_user_dev *dev)
 {
 	uint64_t features;
-	int ret;
+	int ret = -1;
 
-	/*
-	 * XXX workaround!
-	 *
-	 * We need to make sure that the locks will be
-	 * taken in the correct order to avoid deadlocks.
-	 *
-	 * Before releasing this lock, this thread should
-	 * not trigger any memory hotplug events.
-	 *
-	 * This is a temporary workaround, and should be
-	 * replaced when we get proper supports from the
-	 * memory subsystem in the future.
-	 */
-	rte_mcfg_mem_read_lock();
 	pthread_mutex_lock(&dev->mutex);
 
 	if (virtio_user_backend_type(dev->path) ==
@@ -194,10 +180,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	if (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0)
 		goto error;
 
-	/* Step 1: negotiate protocol features & set features */
 	features = dev->features;
 
-
 	/* Strip VIRTIO_NET_F_MAC, as MAC address is handled in vdev init */
 	features &= ~(1ull << VIRTIO_NET_F_MAC);
 	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know */
@@ -207,6 +191,36 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	if (ret < 0)
 		goto error;
 	PMD_DRV_LOG(INFO, "set features: %" PRIx64, features);
+error:
+	pthread_mutex_unlock(&dev->mutex);
+
+	return ret;
+}
+
+int
+virtio_user_start_device(struct virtio_user_dev *dev)
+{
+	int ret;
+
+	/*
+	 * XXX workaround!
+	 *
+	 * We need to make sure that the locks will be
+	 * taken in the correct order to avoid deadlocks.
+	 *
+	 * Before releasing this lock, this thread should
+	 * not trigger any memory hotplug events.
+	 *
+	 * This is a temporary workaround, and should be
+	 * replaced when we get proper supports from the
+	 * memory subsystem in the future.
+	 */
+	rte_mcfg_mem_read_lock();
+	pthread_mutex_lock(&dev->mutex);
+
+	if (virtio_user_backend_type(dev->path) ==
+		VIRTIO_USER_BACKEND_VHOST_USER && dev->vhostfd < 0)
+		goto error;
 
 	/* Step 2: share memory regions */
 	ret = dev->ops->send_request(dev, VHOST_USER_SET_MEM_TABLE, NULL);
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 4bb7cdbd26..68ec9dc3f1 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -69,6 +69,7 @@ struct virtio_user_dev {
 };
 
 enum virtio_user_backend_type virtio_user_backend_type(const char *path);
+int virtio_user_dev_set_features(struct virtio_user_dev *dev);
 int virtio_user_start_device(struct virtio_user_dev *dev);
 int virtio_user_stop_device(struct virtio_user_dev *dev);
 int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 2163d0e30d..91eebbedab 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -266,7 +266,11 @@ static void
 virtio_user_set_status(struct virtio_hw *hw, uint8_t status)
 {
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
+	uint8_t old_status = dev->status;
 
+	if (status & VIRTIO_CONFIG_STATUS_FEATURES_OK &&
+			~old_status & VIRTIO_CONFIG_STATUS_FEATURES_OK)
+		virtio_user_dev_set_features(dev);
 	if (status & VIRTIO_CONFIG_STATUS_DRIVER_OK)
 		virtio_user_start_device(dev);
 	else if (status == VIRTIO_CONFIG_STATUS_RESET)
-- 
2.26.2

[dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend

[Maxime Coquelin]

vhost-vDPA is a new virtio backend type introduced by vDPA kernel
framework, which provides abstruction to the vDPA devices and
exposes an unified control interface through a char dev.

This patch adds support to the vhost-vDPA backend. As similar to
the existing vhost kernel backend, a set of virtio_user ops were
introduced for vhost-vDPA backend to handle device specific operations
such as:
 - device setup
 - ioctl message handling
 - queue pair enabling
 - dma map/unmap
vDPA relevant ioctl codes and data structures are also defined in
this patch.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/meson.build                |   1 +
 drivers/net/virtio/virtio_user/vhost.h        |   1 +
 drivers/net/virtio/virtio_user/vhost_vdpa.c   | 310 ++++++++++++++++++
 .../net/virtio/virtio_user/virtio_user_dev.c  |   9 +-
 4 files changed, 320 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/virtio/virtio_user/vhost_vdpa.c

diff --git a/drivers/net/virtio/meson.build b/drivers/net/virtio/meson.build
index 3fd6051f4b..eaed46373d 100644
--- a/drivers/net/virtio/meson.build
+++ b/drivers/net/virtio/meson.build
@@ -42,6 +42,7 @@ if is_linux
 		'virtio_user/vhost_kernel.c',
 		'virtio_user/vhost_kernel_tap.c',
 		'virtio_user/vhost_user.c',
+		'virtio_user/vhost_vdpa.c',
 		'virtio_user/virtio_user_dev.c')
 	deps += ['bus_vdev']
 endif
diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index 2e71995a79..210a3704e7 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -113,5 +113,6 @@ struct virtio_user_backend_ops {
 
 extern struct virtio_user_backend_ops virtio_ops_user;
 extern struct virtio_user_backend_ops virtio_ops_kernel;
+extern struct virtio_user_backend_ops virtio_ops_vdpa;
 
 #endif
diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
new file mode 100644
index 0000000000..d959bca0a1
--- /dev/null
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -0,0 +1,310 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Red Hat Inc.
+ */
+
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include <rte_memory.h>
+
+#include "vhost.h"
+#include "virtio_user_dev.h"
+
+struct vhost_memory_vdpa {
+	uint32_t nregions;
+	uint32_t padding;
+	struct vhost_memory_region regions[0];
+};
+
+/* vhost kernel & vdpa ioctls */
+#define VHOST_VIRTIO 0xAF
+#define VHOST_GET_FEATURES _IOR(VHOST_VIRTIO, 0x00, __u64)
+#define VHOST_SET_FEATURES _IOW(VHOST_VIRTIO, 0x00, __u64)
+#define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
+#define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)
+#define VHOST_SET_MEM_TABLE _IOW(VHOST_VIRTIO, 0x03, struct vhost_memory_vdpa)
+#define VHOST_SET_LOG_BASE _IOW(VHOST_VIRTIO, 0x04, __u64)
+#define VHOST_SET_LOG_FD _IOW(VHOST_VIRTIO, 0x07, int)
+#define VHOST_SET_VRING_NUM _IOW(VHOST_VIRTIO, 0x10, struct vhost_vring_state)
+#define VHOST_SET_VRING_ADDR _IOW(VHOST_VIRTIO, 0x11, struct vhost_vring_addr)
+#define VHOST_SET_VRING_BASE _IOW(VHOST_VIRTIO, 0x12, struct vhost_vring_state)
+#define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state)
+#define VHOST_SET_VRING_KICK _IOW(VHOST_VIRTIO, 0x20, struct vhost_vring_file)
+#define VHOST_SET_VRING_CALL _IOW(VHOST_VIRTIO, 0x21, struct vhost_vring_file)
+#define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
+#define VHOST_NET_SET_BACKEND _IOW(VHOST_VIRTIO, 0x30, struct vhost_vring_file)
+#define VHOST_VDPA_GET_DEVICE_ID _IOR(VHOST_VIRTIO, 0x70, __u32)
+#define VHOST_VDPA_GET_STATUS _IOR(VHOST_VIRTIO, 0x71, __u8)
+#define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8)
+#define VHOST_VDPA_SET_VRING_ENABLE	_IOW(VHOST_VIRTIO, 0x75, \
+					     struct vhost_vring_state)
+
+static uint64_t vhost_req_user_to_vdpa[] = {
+	[VHOST_USER_SET_OWNER] = VHOST_SET_OWNER,
+	[VHOST_USER_RESET_OWNER] = VHOST_RESET_OWNER,
+	[VHOST_USER_SET_FEATURES] = VHOST_SET_FEATURES,
+	[VHOST_USER_GET_FEATURES] = VHOST_GET_FEATURES,
+	[VHOST_USER_SET_VRING_CALL] = VHOST_SET_VRING_CALL,
+	[VHOST_USER_SET_VRING_NUM] = VHOST_SET_VRING_NUM,
+	[VHOST_USER_SET_VRING_BASE] = VHOST_SET_VRING_BASE,
+	[VHOST_USER_GET_VRING_BASE] = VHOST_GET_VRING_BASE,
+	[VHOST_USER_SET_VRING_ADDR] = VHOST_SET_VRING_ADDR,
+	[VHOST_USER_SET_VRING_KICK] = VHOST_SET_VRING_KICK,
+	[VHOST_USER_SET_MEM_TABLE] = VHOST_SET_MEM_TABLE,
+	[VHOST_USER_SET_STATUS] = VHOST_VDPA_SET_STATUS,
+	[VHOST_USER_GET_STATUS] = VHOST_VDPA_GET_STATUS,
+	[VHOST_USER_SET_VRING_ENABLE] = VHOST_VDPA_SET_VRING_ENABLE,
+};
+
+/* no alignment requirement */
+struct vhost_iotlb_msg {
+	uint64_t iova;
+	uint64_t size;
+	uint64_t uaddr;
+#define VHOST_ACCESS_RO      0x1
+#define VHOST_ACCESS_WO      0x2
+#define VHOST_ACCESS_RW      0x3
+	uint8_t perm;
+#define VHOST_IOTLB_MISS           1
+#define VHOST_IOTLB_UPDATE         2
+#define VHOST_IOTLB_INVALIDATE     3
+#define VHOST_IOTLB_ACCESS_FAIL    4
+	uint8_t type;
+};
+
+#define VHOST_IOTLB_MSG_V2 0x2
+
+struct vhost_msg {
+	uint32_t type;
+	uint32_t reserved;
+	union {
+		struct vhost_iotlb_msg iotlb;
+		uint8_t padding[64];
+	};
+};
+
+static int
+vhost_vdpa_dma_map(struct virtio_user_dev *dev, void *addr,
+				  uint64_t iova, size_t len)
+{
+	struct vhost_msg msg = {};
+
+	msg.type = VHOST_IOTLB_MSG_V2;
+	msg.iotlb.type = VHOST_IOTLB_UPDATE;
+	msg.iotlb.iova = iova;
+	msg.iotlb.uaddr = (uint64_t)(uintptr_t)addr;
+	msg.iotlb.size = len;
+	msg.iotlb.perm = VHOST_ACCESS_RW;
+
+	if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) {
+		PMD_DRV_LOG(ERR, "Failed to send IOTLB update (%s)",
+				strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
+static int
+vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, void __rte_unused *addr,
+				  uint64_t iova, size_t len)
+{
+	struct vhost_msg msg = {};
+
+	msg.type = VHOST_IOTLB_MSG_V2;
+	msg.iotlb.type = VHOST_IOTLB_INVALIDATE;
+	msg.iotlb.iova = iova;
+	msg.iotlb.size = len;
+
+	if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) {
+		PMD_DRV_LOG(ERR, "Failed to send IOTLB invalidate (%s)",
+				strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
+
+static int
+vhost_vdpa_map_contig(const struct rte_memseg_list *msl,
+		const struct rte_memseg *ms, size_t len, void *arg)
+{
+	struct virtio_user_dev *dev = arg;
+
+	if (msl->external)
+		return 0;
+
+	return vhost_vdpa_dma_map(dev, ms->addr, ms->iova, len);
+}
+
+static int
+vhost_vdpa_map(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
+		void *arg)
+{
+	struct virtio_user_dev *dev = arg;
+
+	/* skip external memory that isn't a heap */
+	if (msl->external && !msl->heap)
+		return 0;
+
+	/* skip any segments with invalid IOVA addresses */
+	if (ms->iova == RTE_BAD_IOVA)
+		return 0;
+
+	/* if IOVA mode is VA, we've already mapped the internal segments */
+	if (!msl->external && rte_eal_iova_mode() == RTE_IOVA_VA)
+		return 0;
+
+	return vhost_vdpa_dma_map(dev, ms->addr, ms->iova, ms->len);
+}
+
+static int
+vhost_vdpa_dma_map_all(struct virtio_user_dev *dev)
+{
+	vhost_vdpa_dma_unmap(dev, 0, 0ULL, 0ULL - 1);
+
+	if (rte_eal_iova_mode() == RTE_IOVA_VA) {
+		/* with IOVA as VA mode, we can get away with mapping contiguous
+		 * chunks rather than going page-by-page.
+		 */
+		int ret = rte_memseg_contig_walk_thread_unsafe(
+				vhost_vdpa_map_contig, dev);
+		if (ret)
+			return ret;
+		/* we have to continue the walk because we've skipped the
+		 * external segments during the config walk.
+		 */
+	}
+	return rte_memseg_walk_thread_unsafe(vhost_vdpa_map, dev);
+}
+
+/* with below features, vhost vdpa does not need to do the checksum and TSO,
+ * these info will be passed to virtio_user through virtio net header.
+ */
+#define VHOST_VDPA_GUEST_OFFLOADS_MASK	\
+	((1ULL << VIRTIO_NET_F_GUEST_CSUM) |	\
+	 (1ULL << VIRTIO_NET_F_GUEST_TSO4) |	\
+	 (1ULL << VIRTIO_NET_F_GUEST_TSO6) |	\
+	 (1ULL << VIRTIO_NET_F_GUEST_ECN)  |	\
+	 (1ULL << VIRTIO_NET_F_GUEST_UFO))
+
+#define VHOST_VDPA_HOST_OFFLOADS_MASK		\
+	((1ULL << VIRTIO_NET_F_HOST_TSO4) |	\
+	 (1ULL << VIRTIO_NET_F_HOST_TSO6) |	\
+	 (1ULL << VIRTIO_NET_F_CSUM))
+
+static int
+vhost_vdpa_ioctl(struct virtio_user_dev *dev,
+		   enum vhost_user_request req,
+		   void *arg)
+{
+	int ret = -1;
+	uint64_t req_vdpa;
+	struct vhost_memory_vdpa *vm = NULL;
+
+	PMD_DRV_LOG(INFO, "%s", vhost_msg_strings[req]);
+
+	req_vdpa = vhost_req_user_to_vdpa[req];
+
+	if (req_vdpa == VHOST_SET_MEM_TABLE)
+		return vhost_vdpa_dma_map_all(dev);
+
+	if (req_vdpa == VHOST_SET_FEATURES) {
+		/* WORKAROUND */
+		*(uint64_t *)arg |= 1ULL << VIRTIO_F_IOMMU_PLATFORM;
+
+		/* Multiqueue not supported for now */
+		*(uint64_t *)arg &= ~(1ULL << VIRTIO_NET_F_MQ);
+	}
+
+	switch (req_vdpa) {
+	case VHOST_SET_VRING_NUM:
+	case VHOST_SET_VRING_ADDR:
+	case VHOST_SET_VRING_BASE:
+	case VHOST_GET_VRING_BASE:
+	case VHOST_SET_VRING_KICK:
+	case VHOST_SET_VRING_CALL:
+		*(unsigned int *)arg = *(unsigned int *)arg;
+		PMD_DRV_LOG(DEBUG, "vhostfd=%d, index=%u",
+			    dev->vhostfd, *(unsigned int *)arg);
+		break;
+	default:
+		break;
+	}
+
+	ret = ioctl(dev->vhostfd, req_vdpa, arg);
+
+	if (vm)
+		free(vm);
+
+	if (ret < 0)
+		PMD_DRV_LOG(ERR, "%s failed: %s",
+			    vhost_msg_strings[req], strerror(errno));
+
+	return ret;
+}
+
+/**
+ * Set up environment to talk with a vhost vdpa backend.
+ *
+ * @return
+ *   - (-1) if fail to set up;
+ *   - (>=0) if successful.
+ */
+static int
+vhost_vdpa_setup(struct virtio_user_dev *dev)
+{
+	uint32_t did = (uint32_t)-1;
+
+	dev->vhostfd = open(dev->path, O_RDWR);
+	if (dev->vhostfd < 0) {
+		PMD_DRV_LOG(ERR, "Failed to open %s: %s\n",
+				dev->path, strerror(errno));
+		return -1;
+	}
+
+	if (ioctl(dev->vhostfd, VHOST_VDPA_GET_DEVICE_ID, &did) < 0 ||
+			did != VIRTIO_ID_NETWORK) {
+		PMD_DRV_LOG(ERR, "Invalid vdpa device ID: %u\n", did);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int
+vhost_vdpa_enable_queue_pair(struct virtio_user_dev *dev,
+			       uint16_t pair_idx,
+			       int enable)
+{
+	int i;
+
+	if (dev->qp_enabled[pair_idx] == enable)
+		return 0;
+
+	for (i = 0; i < 2; ++i) {
+		struct vhost_vring_state state = {
+			.index = pair_idx * 2 + i,
+			.num   = enable,
+		};
+
+		if (vhost_vdpa_ioctl(dev, VHOST_USER_SET_VRING_ENABLE, &state))
+			return -1;
+	}
+
+	dev->qp_enabled[pair_idx] = enable;
+
+	return 0;
+}
+
+struct virtio_user_backend_ops virtio_ops_vdpa = {
+	.setup = vhost_vdpa_setup,
+	.send_request = vhost_vdpa_ioctl,
+	.enable_qp = vhost_vdpa_enable_queue_pair,
+	.dma_map = vhost_vdpa_dma_map,
+	.dma_unmap = vhost_vdpa_dma_unmap,
+};
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 2e097a95ea..2e8311147b 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -444,6 +444,12 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
 				dev->vhostfds[q] = -1;
 				dev->tapfds[q] = -1;
 			}
+		} else if (dev->backend_type ==
+				VIRTIO_USER_BACKEND_VHOST_VDPA) {
+			dev->ops = &virtio_ops_vdpa;
+		} else {
+			PMD_DRV_LOG(ERR, "Unknown backend type");
+			return -1;
 		}
 	}
 
@@ -878,7 +884,8 @@ virtio_user_update_status(struct virtio_user_dev *dev)
 	enum virtio_user_backend_type backend_type =
 				virtio_user_backend_type(dev->path);
 
-	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
+	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER &&
+			backend_type != VIRTIO_USER_BACKEND_VHOST_VDPA)
 		return 0;
 
 	err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
-- 
2.26.2

Re: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend

[Xia, Chenbo]

Hi Maxime,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> Sent: Friday, September 11, 2020 11:08 PM
> To: dev@dpdk.org; Fu, Patrick <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend
> 
> vhost-vDPA is a new virtio backend type introduced by vDPA kernel
> framework, which provides abstruction to the vDPA devices and
> exposes an unified control interface through a char dev.
> 
> This patch adds support to the vhost-vDPA backend. As similar to
> the existing vhost kernel backend, a set of virtio_user ops were
> introduced for vhost-vDPA backend to handle device specific operations
> such as:
>  - device setup
>  - ioctl message handling
>  - queue pair enabling
>  - dma map/unmap
> vDPA relevant ioctl codes and data structures are also defined in
> this patch.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/meson.build                |   1 +
>  drivers/net/virtio/virtio_user/vhost.h        |   1 +
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 310 ++++++++++++++++++
>  .../net/virtio/virtio_user/virtio_user_dev.c  |   9 +-
>  4 files changed, 320 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/virtio/virtio_user/vhost_vdpa.c
> 
> diff --git a/drivers/net/virtio/meson.build
> b/drivers/net/virtio/meson.build
> index 3fd6051f4b..eaed46373d 100644
> --- a/drivers/net/virtio/meson.build
> +++ b/drivers/net/virtio/meson.build
> @@ -42,6 +42,7 @@ if is_linux

[snip]

> +static int
> +vhost_vdpa_ioctl(struct virtio_user_dev *dev,
> +		   enum vhost_user_request req,
> +		   void *arg)
> +{
> +	int ret = -1;
> +	uint64_t req_vdpa;
> +	struct vhost_memory_vdpa *vm = NULL;
> +
> +	PMD_DRV_LOG(INFO, "%s", vhost_msg_strings[req]);
> +
> +	req_vdpa = vhost_req_user_to_vdpa[req];
> +
> +	if (req_vdpa == VHOST_SET_MEM_TABLE)
> +		return vhost_vdpa_dma_map_all(dev);
> +
> +	if (req_vdpa == VHOST_SET_FEATURES) {
> +		/* WORKAROUND */
> +		*(uint64_t *)arg |= 1ULL << VIRTIO_F_IOMMU_PLATFORM;
> +
> +		/* Multiqueue not supported for now */
> +		*(uint64_t *)arg &= ~(1ULL << VIRTIO_NET_F_MQ);
> +	}
> +
> +	switch (req_vdpa) {
> +	case VHOST_SET_VRING_NUM:
> +	case VHOST_SET_VRING_ADDR:
> +	case VHOST_SET_VRING_BASE:
> +	case VHOST_GET_VRING_BASE:
> +	case VHOST_SET_VRING_KICK:
> +	case VHOST_SET_VRING_CALL:
> +		*(unsigned int *)arg = *(unsigned int *)arg;

Above line should be deleted?

> +		PMD_DRV_LOG(DEBUG, "vhostfd=%d, index=%u",
> +			    dev->vhostfd, *(unsigned int *)arg);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	ret = ioctl(dev->vhostfd, req_vdpa, arg);
> +
> +	if (vm)
> +		free(vm);

I think 'vm' is never changed after it's init-ed as NULL. Maybe it
should be deleted? If it's not needed, the definition of struct
vhost_memory_vdpa should also be deleted.

> +
> +	if (ret < 0)
> +		PMD_DRV_LOG(ERR, "%s failed: %s",
> +			    vhost_msg_strings[req], strerror(errno));
> +
> +	return ret;
> +}
> +
> +/**
> + * Set up environment to talk with a vhost vdpa backend.
> + *
> + * @return
> + *   - (-1) if fail to set up;
> + *   - (>=0) if successful.
> + */
> +static int
> +vhost_vdpa_setup(struct virtio_user_dev *dev)
> +{
> +	uint32_t did = (uint32_t)-1;

I see in kernel, 'did' should be u8:

static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp)

So I think here did should be uint8_t?

Besides, there are two coding style issues:
CHECK:SPACING: spaces preferred around that '*' (ctx:WxV)
#236: FILE: drivers/net/virtio/virtio_user/vhost_vdpa.c:112:
+vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, void __rte_unused *addr,
                                                                     ^

CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#298: FILE: drivers/net/virtio/virtio_user/vhost_vdpa.c:174:
+		int ret = rte_memseg_contig_walk_thread_unsafe(

Thanks!
Chenbo

> +
> +	dev->vhostfd = open(dev->path, O_RDWR);
> +	if (dev->vhostfd < 0) {
> +		PMD_DRV_LOG(ERR, "Failed to open %s: %s\n",
> +				dev->path, strerror(errno));
> +		return -1;
> +	}
> +
> +	if (ioctl(dev->vhostfd, VHOST_VDPA_GET_DEVICE_ID, &did) < 0 ||
> +			did != VIRTIO_ID_NETWORK) {
> +		PMD_DRV_LOG(ERR, "Invalid vdpa device ID: %u\n", did);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +vhost_vdpa_enable_queue_pair(struct virtio_user_dev *dev,
> +			       uint16_t pair_idx,
> +			       int enable)
> +{
> +	int i;
> +
> +	if (dev->qp_enabled[pair_idx] == enable)
> +		return 0;
> +
> +	for (i = 0; i < 2; ++i) {
> +		struct vhost_vring_state state = {
> +			.index = pair_idx * 2 + i,
> +			.num   = enable,
> +		};
> +
> +		if (vhost_vdpa_ioctl(dev, VHOST_USER_SET_VRING_ENABLE, &state))
> +			return -1;
> +	}
> +
> +	dev->qp_enabled[pair_idx] = enable;
> +
> +	return 0;
> +}
> +
> +struct virtio_user_backend_ops virtio_ops_vdpa = {
> +	.setup = vhost_vdpa_setup,
> +	.send_request = vhost_vdpa_ioctl,
> +	.enable_qp = vhost_vdpa_enable_queue_pair,
> +	.dma_map = vhost_vdpa_dma_map,
> +	.dma_unmap = vhost_vdpa_dma_unmap,
> +};
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 2e097a95ea..2e8311147b 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -444,6 +444,12 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  				dev->vhostfds[q] = -1;
>  				dev->tapfds[q] = -1;
>  			}
> +		} else if (dev->backend_type ==
> +				VIRTIO_USER_BACKEND_VHOST_VDPA) {
> +			dev->ops = &virtio_ops_vdpa;
> +		} else {
> +			PMD_DRV_LOG(ERR, "Unknown backend type");
> +			return -1;
>  		}
>  	}
> 
> @@ -878,7 +884,8 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>  	enum virtio_user_backend_type backend_type =
>  				virtio_user_backend_type(dev->path);
> 
> -	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> +	if (backend_type != VIRTIO_USER_BACKEND_VHOST_USER &&
> +			backend_type != VIRTIO_USER_BACKEND_VHOST_VDPA)
>  		return 0;
> 
>  	err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
> --
> 2.26.2

Re: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend

[Fu, Patrick]

> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Thursday, September 24, 2020 1:26 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Fu,
> Patrick <patrick.fu@intel.com>; amorenoz@redhat.com
> Subject: RE: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA
> backend
> 
> Hi Maxime,
> 
> > +
> > +/**
> > + * Set up environment to talk with a vhost vdpa backend.
> > + *
> > + * @return
> > + *   - (-1) if fail to set up;
> > + *   - (>=0) if successful.
> > + */
> > +static int
> > +vhost_vdpa_setup(struct virtio_user_dev *dev)
> > +{
> > +	uint32_t did = (uint32_t)-1;
> 
> I see in kernel, 'did' should be u8:
> 
> static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp)
> 
> So I think here did should be uint8_t?
> 

Actually `did` is u32 in kernel. argp is just a user pointer. Whether it's u8* or u32* has nothing to do with the `did` data size 

refer to control code def:
#define VHOST_VDPA_GET_DEVICE_ID	_IOR(VHOST_VIRTIO, 0x70, __u32)

Thanks,

Patrick

Re: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend

[Xia, Chenbo]

Hi Patrick,

> -----Original Message-----
> From: Fu, Patrick <patrick.fu@intel.com>
> Sent: Thursday, September 24, 2020 1:44 PM
> To: Xia, Chenbo <chenbo.xia@intel.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; dev@dpdk.org; amorenoz@redhat.com
> Subject: RE: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA
> backend
> 
> > -----Original Message-----
> > From: Xia, Chenbo <chenbo.xia@intel.com>
> > Sent: Thursday, September 24, 2020 1:26 PM
> > To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Fu,
> > Patrick <patrick.fu@intel.com>; amorenoz@redhat.com
> > Subject: RE: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA
> > backend
> >
> > Hi Maxime,
> >
> > > +
> > > +/**
> > > + * Set up environment to talk with a vhost vdpa backend.
> > > + *
> > > + * @return
> > > + *   - (-1) if fail to set up;
> > > + *   - (>=0) if successful.
> > > + */
> > > +static int
> > > +vhost_vdpa_setup(struct virtio_user_dev *dev)
> > > +{
> > > +	uint32_t did = (uint32_t)-1;
> >
> > I see in kernel, 'did' should be u8:
> >
> > static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user
> *argp)
> >
> > So I think here did should be uint8_t?
> >
> 
> Actually `did` is u32 in kernel. argp is just a user pointer. Whether it's
> u8* or u32* has nothing to do with the `did` data size
> 
> refer to control code def:
> #define VHOST_VDPA_GET_DEVICE_ID	_IOR(VHOST_VIRTIO, 0x70, __u32)

Yes, you are right!

@Maxime Coquelin Please ignore this comment 😊

> 
> Thanks,
> 
> Patrick

Re: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend

[Maxime Coquelin]

On 9/24/20 7:25 AM, Xia, Chenbo wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
>> Sent: Friday, September 11, 2020 11:08 PM
>> To: dev@dpdk.org; Fu, Patrick <patrick.fu@intel.com>; amorenoz@redhat.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend
>>
>> vhost-vDPA is a new virtio backend type introduced by vDPA kernel
>> framework, which provides abstruction to the vDPA devices and
>> exposes an unified control interface through a char dev.
>>
>> This patch adds support to the vhost-vDPA backend. As similar to
>> the existing vhost kernel backend, a set of virtio_user ops were
>> introduced for vhost-vDPA backend to handle device specific operations
>> such as:
>>  - device setup
>>  - ioctl message handling
>>  - queue pair enabling
>>  - dma map/unmap
>> vDPA relevant ioctl codes and data structures are also defined in
>> this patch.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  drivers/net/virtio/meson.build                |   1 +
>>  drivers/net/virtio/virtio_user/vhost.h        |   1 +
>>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 310 ++++++++++++++++++
>>  .../net/virtio/virtio_user/virtio_user_dev.c  |   9 +-
>>  4 files changed, 320 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/net/virtio/virtio_user/vhost_vdpa.c
>>
>> diff --git a/drivers/net/virtio/meson.build
>> b/drivers/net/virtio/meson.build
>> index 3fd6051f4b..eaed46373d 100644
>> --- a/drivers/net/virtio/meson.build
>> +++ b/drivers/net/virtio/meson.build
>> @@ -42,6 +42,7 @@ if is_linux
> 
> [snip]
> 
>> +static int
>> +vhost_vdpa_ioctl(struct virtio_user_dev *dev,
>> +		   enum vhost_user_request req,
>> +		   void *arg)
>> +{
>> +	int ret = -1;
>> +	uint64_t req_vdpa;
>> +	struct vhost_memory_vdpa *vm = NULL;
>> +
>> +	PMD_DRV_LOG(INFO, "%s", vhost_msg_strings[req]);
>> +
>> +	req_vdpa = vhost_req_user_to_vdpa[req];
>> +
>> +	if (req_vdpa == VHOST_SET_MEM_TABLE)
>> +		return vhost_vdpa_dma_map_all(dev);
>> +
>> +	if (req_vdpa == VHOST_SET_FEATURES) {
>> +		/* WORKAROUND */
>> +		*(uint64_t *)arg |= 1ULL << VIRTIO_F_IOMMU_PLATFORM;
>> +
>> +		/* Multiqueue not supported for now */
>> +		*(uint64_t *)arg &= ~(1ULL << VIRTIO_NET_F_MQ);
>> +	}
>> +
>> +	switch (req_vdpa) {
>> +	case VHOST_SET_VRING_NUM:
>> +	case VHOST_SET_VRING_ADDR:
>> +	case VHOST_SET_VRING_BASE:
>> +	case VHOST_GET_VRING_BASE:
>> +	case VHOST_SET_VRING_KICK:
>> +	case VHOST_SET_VRING_CALL:
>> +		*(unsigned int *)arg = *(unsigned int *)arg;
> 
> Above line should be deleted?

Yes!

>> +		PMD_DRV_LOG(DEBUG, "vhostfd=%d, index=%u",
>> +			    dev->vhostfd, *(unsigned int *)arg);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	ret = ioctl(dev->vhostfd, req_vdpa, arg);
>> +
>> +	if (vm)
>> +		free(vm);
> 
> I think 'vm' is never changed after it's init-ed as NULL. Maybe it
> should be deleted? If it's not needed, the definition of struct
> vhost_memory_vdpa should also be deleted.

Correct, will remove in v2.

>> +
>> +	if (ret < 0)
>> +		PMD_DRV_LOG(ERR, "%s failed: %s",
>> +			    vhost_msg_strings[req], strerror(errno));
>> +
>> +	return ret;
>> +}
>> +
>> +/**
>> + * Set up environment to talk with a vhost vdpa backend.
>> + *
>> + * @return
>> + *   - (-1) if fail to set up;
>> + *   - (>=0) if successful.
>> + */
>> +static int
>> +vhost_vdpa_setup(struct virtio_user_dev *dev)
>> +{
>> +	uint32_t did = (uint32_t)-1;
> 
> I see in kernel, 'did' should be u8:
> 
> static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp)
> 
> So I think here did should be uint8_t?
> 
> Besides, there are two coding style issues:
> CHECK:SPACING: spaces preferred around that '*' (ctx:WxV)
> #236: FILE: drivers/net/virtio/virtio_user/vhost_vdpa.c:112:
> +vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, void __rte_unused *addr,

Not sure about this one.

                                                                     ^
> 
> CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
> #298: FILE: drivers/net/virtio/virtio_user/vhost_vdpa.c:174:
> +		int ret = rte_memseg_contig_walk_thread_unsafe(

Same here.

> Thanks!
> Chenbo