* [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops
2020-09-11 15:07 [dpdk-dev] [PATCH 0/7]virtio-user: introduce vhost-vdpa backend Maxime Coquelin
@ 2020-09-11 15:07 ` Maxime Coquelin
2020-09-24 5:25 ` Xia, Chenbo
2020-09-11 15:08 ` [dpdk-dev] [PATCH 2/7] net/virtio: introduce DMA ops Maxime Coquelin
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-11 15:07 UTC (permalink / raw)
To: dev, patrick.fu, amorenoz; +Cc: 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops
2020-09-11 15:07 ` [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops Maxime Coquelin
@ 2020-09-24 5:25 ` Xia, Chenbo
2020-09-24 7:40 ` Maxime Coquelin
0 siblings, 1 reply; 16+ messages in thread
From: Xia, Chenbo @ 2020-09-24 5:25 UTC (permalink / raw)
To: Maxime Coquelin, dev, Fu, Patrick, amorenoz
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops
2020-09-24 5:25 ` Xia, Chenbo
@ 2020-09-24 7:40 ` Maxime Coquelin
0 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-24 7:40 UTC (permalink / raw)
To: Xia, Chenbo, dev, Fu, Patrick, amorenoz
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH 2/7] net/virtio: introduce DMA ops
2020-09-11 15:07 [dpdk-dev] [PATCH 0/7]virtio-user: introduce vhost-vdpa backend Maxime Coquelin
2020-09-11 15:07 ` [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops Maxime Coquelin
@ 2020-09-11 15:08 ` Maxime Coquelin
2020-09-11 15:08 ` [dpdk-dev] [PATCH 3/7] net/virtio: introduce Vhost-vDPA backend type Maxime Coquelin
` (4 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-11 15:08 UTC (permalink / raw)
To: dev, patrick.fu, amorenoz; +Cc: 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH 3/7] net/virtio: introduce Vhost-vDPA backend type
2020-09-11 15:07 [dpdk-dev] [PATCH 0/7]virtio-user: introduce vhost-vdpa backend Maxime Coquelin
2020-09-11 15:07 ` [dpdk-dev] [PATCH 1/7] bus/vdev: add DMA mapping ops Maxime Coquelin
2020-09-11 15:08 ` [dpdk-dev] [PATCH 2/7] net/virtio: introduce DMA ops Maxime Coquelin
@ 2020-09-11 15:08 ` Maxime Coquelin
2020-09-11 15:08 ` [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size Maxime Coquelin
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-11 15:08 UTC (permalink / raw)
To: dev, patrick.fu, amorenoz; +Cc: 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size
2020-09-11 15:07 [dpdk-dev] [PATCH 0/7]virtio-user: introduce vhost-vdpa backend Maxime Coquelin
` (2 preceding siblings ...)
2020-09-11 15:08 ` [dpdk-dev] [PATCH 3/7] net/virtio: introduce Vhost-vDPA backend type Maxime Coquelin
@ 2020-09-11 15:08 ` Maxime Coquelin
2020-09-24 5:25 ` Xia, Chenbo
2020-09-11 15:08 ` [dpdk-dev] [PATCH 5/7] net/virtio: check protocol feature in user backend Maxime Coquelin
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-11 15:08 UTC (permalink / raw)
To: dev, patrick.fu, amorenoz; +Cc: 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size
2020-09-11 15:08 ` [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size Maxime Coquelin
@ 2020-09-24 5:25 ` Xia, Chenbo
2020-09-24 8:05 ` Maxime Coquelin
0 siblings, 1 reply; 16+ messages in thread
From: Xia, Chenbo @ 2020-09-24 5:25 UTC (permalink / raw)
To: Maxime Coquelin, dev, Fu, Patrick, amorenoz
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size
2020-09-24 5:25 ` Xia, Chenbo
@ 2020-09-24 8:05 ` Maxime Coquelin
0 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-24 8:05 UTC (permalink / raw)
To: Xia, Chenbo, dev, Fu, Patrick, amorenoz
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH 5/7] net/virtio: check protocol feature in user backend
2020-09-11 15:07 [dpdk-dev] [PATCH 0/7]virtio-user: introduce vhost-vdpa backend Maxime Coquelin
` (3 preceding siblings ...)
2020-09-11 15:08 ` [dpdk-dev] [PATCH 4/7] net/virtio: adapt Virtio-user status size Maxime Coquelin
@ 2020-09-11 15:08 ` Maxime Coquelin
2020-09-11 15:08 ` [dpdk-dev] [PATCH 6/7] net/virtio: split virtio-user start Maxime Coquelin
2020-09-11 15:08 ` [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend Maxime Coquelin
6 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-11 15:08 UTC (permalink / raw)
To: dev, patrick.fu, amorenoz; +Cc: 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH 6/7] net/virtio: split virtio-user start
2020-09-11 15:07 [dpdk-dev] [PATCH 0/7]virtio-user: introduce vhost-vdpa backend Maxime Coquelin
` (4 preceding siblings ...)
2020-09-11 15:08 ` [dpdk-dev] [PATCH 5/7] net/virtio: check protocol feature in user backend Maxime Coquelin
@ 2020-09-11 15:08 ` Maxime Coquelin
2020-09-11 15:08 ` [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend Maxime Coquelin
6 siblings, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-11 15:08 UTC (permalink / raw)
To: dev, patrick.fu, amorenoz; +Cc: 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend
2020-09-11 15:07 [dpdk-dev] [PATCH 0/7]virtio-user: introduce vhost-vdpa backend Maxime Coquelin
` (5 preceding siblings ...)
2020-09-11 15:08 ` [dpdk-dev] [PATCH 6/7] net/virtio: split virtio-user start Maxime Coquelin
@ 2020-09-11 15:08 ` Maxime Coquelin
2020-09-24 5:25 ` Xia, Chenbo
6 siblings, 1 reply; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-11 15:08 UTC (permalink / raw)
To: dev, patrick.fu, amorenoz; +Cc: 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend
2020-09-11 15:08 ` [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend Maxime Coquelin
@ 2020-09-24 5:25 ` Xia, Chenbo
2020-09-24 5:43 ` Fu, Patrick
2020-09-24 8:07 ` Maxime Coquelin
0 siblings, 2 replies; 16+ messages in thread
From: Xia, Chenbo @ 2020-09-24 5:25 UTC (permalink / raw)
To: Maxime Coquelin, dev, Fu, Patrick, amorenoz
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend
2020-09-24 5:25 ` Xia, Chenbo
@ 2020-09-24 5:43 ` Fu, Patrick
2020-09-24 5:52 ` Xia, Chenbo
2020-09-24 8:07 ` Maxime Coquelin
1 sibling, 1 reply; 16+ messages in thread
From: Fu, Patrick @ 2020-09-24 5:43 UTC (permalink / raw)
To: Xia, Chenbo, Maxime Coquelin, dev, amorenoz
> -----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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend
2020-09-24 5:43 ` Fu, Patrick
@ 2020-09-24 5:52 ` Xia, Chenbo
0 siblings, 0 replies; 16+ messages in thread
From: Xia, Chenbo @ 2020-09-24 5:52 UTC (permalink / raw)
To: Fu, Patrick, Maxime Coquelin, dev, amorenoz
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [dpdk-dev] [PATCH 7/7] net/virtio: introduce Vhost-vDPA backend
2020-09-24 5:25 ` Xia, Chenbo
2020-09-24 5:43 ` Fu, Patrick
@ 2020-09-24 8:07 ` Maxime Coquelin
1 sibling, 0 replies; 16+ messages in thread
From: Maxime Coquelin @ 2020-09-24 8:07 UTC (permalink / raw)
To: Xia, Chenbo, dev, Fu, Patrick, amorenoz
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
^ permalink raw reply [flat|nested] 16+ messages in thread