[v3,02/44] bus/vdev: add driver IOVA VA mode requirement

Message ID 20210125171444.167241-3-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series net/virtio: Virtio PMD rework |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Maxime Coquelin Jan. 25, 2021, 5:14 p.m. UTC
  This patch adds driver flag in vdev bus driver so that
vdev drivers can require VA IOVA mode to be used, which
for example the case of Virtio-user PMD.

The patch implements the .get_iommu_class() callback, that
is called before devices probing to determine the IOVA mode
to be used.

It also adds a check right before the device is probed to
ensure compatible IOVa mode has been selected.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/bus/vdev/rte_bus_vdev.h |  4 ++++
 drivers/bus/vdev/vdev.c         | 29 +++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)
  

Comments

Chenbo Xia Jan. 26, 2021, 9:23 a.m. UTC | #1
Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, January 26, 2021 1:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; olivier.matz@6wind.com;
> amorenoz@redhat.com; david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 02/44] bus/vdev: add driver IOVA VA mode requirement
> 
> This patch adds driver flag in vdev bus driver so that
> vdev drivers can require VA IOVA mode to be used, which
> for example the case of Virtio-user PMD.
> 
> The patch implements the .get_iommu_class() callback, that
> is called before devices probing to determine the IOVA mode
> to be used.
> 
> It also adds a check right before the device is probed to
> ensure compatible IOVa mode has been selected.

s/IOVa/IOVA

> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/bus/vdev/rte_bus_vdev.h |  4 ++++
>  drivers/bus/vdev/vdev.c         | 29 +++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/bus/vdev/rte_bus_vdev.h b/drivers/bus/vdev/rte_bus_vdev.h
> index f99a41f825..c8b41e649c 100644
> --- a/drivers/bus/vdev/rte_bus_vdev.h
> +++ b/drivers/bus/vdev/rte_bus_vdev.h
> @@ -113,8 +113,12 @@ struct rte_vdev_driver {
>  	rte_vdev_remove_t *remove;       /**< Virtual device remove function. */
>  	rte_vdev_dma_map_t *dma_map;     /**< Virtual device DMA map function.
> */
>  	rte_vdev_dma_unmap_t *dma_unmap; /**< Virtual device DMA unmap function.
> */
> +	uint32_t drv_flags;                /**< Flags RTE_VDEV_DRV_*. */

I remember David mentioned that the comment above should be consistent with others, which
also makes sense to me

Thanks,
Chenbo

>  };
> 
> +/** Device driver needs IOVA as VA and cannot work with IOVA as PA */
> +#define RTE_VDEV_DRV_NEED_IOVA_AS_VA 0x0001
> +
>  /**
>   * Register a virtual device driver.
>   *
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index acfd78828f..9a673347ae 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -189,6 +189,7 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
>  {
>  	const char *name;
>  	struct rte_vdev_driver *driver;
> +	enum rte_iova_mode iova_mode;
>  	int ret;
> 
>  	if (rte_dev_is_probed(&dev->device))
> @@ -199,6 +200,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
> 
>  	if (vdev_parse(name, &driver))
>  		return -1;
> +
> +	iova_mode = rte_eal_iova_mode();
> +	if ((driver->drv_flags & RTE_VDEV_DRV_NEED_IOVA_AS_VA) && (iova_mode ==
> RTE_IOVA_PA)) {
> +		VDEV_LOG(ERR, "%s requires VA IOVA mode but current mode is PA,
> not initializing",
> +				name);
> +		return -1;
> +	}
> +
>  	ret = driver->probe(dev);
>  	if (ret == 0)
>  		dev->device.driver = &driver->driver;
> @@ -594,6 +603,25 @@ vdev_unplug(struct rte_device *dev)
>  	return rte_vdev_uninit(dev->name);
>  }
> 
> +static enum rte_iova_mode
> +vdev_get_iommu_class(void)
> +{
> +	const char *name;
> +	struct rte_vdev_device *dev;
> +	struct rte_vdev_driver *driver;
> +
> +	TAILQ_FOREACH(dev, &vdev_device_list, next) {
> +		name = rte_vdev_device_name(dev);
> +		if (vdev_parse(name, &driver))
> +			continue;
> +
> +		if (driver->drv_flags & RTE_VDEV_DRV_NEED_IOVA_AS_VA)
> +			return RTE_IOVA_VA;
> +	}
> +
> +	return RTE_IOVA_DC;
> +}
> +
>  static struct rte_bus rte_vdev_bus = {
>  	.scan = vdev_scan,
>  	.probe = vdev_probe,
> @@ -603,6 +631,7 @@ static struct rte_bus rte_vdev_bus = {
>  	.parse = vdev_parse,
>  	.dma_map = vdev_dma_map,
>  	.dma_unmap = vdev_dma_unmap,
> +	.get_iommu_class = vdev_get_iommu_class,
>  	.dev_iterate = rte_vdev_dev_iterate,
>  };
> 
> --
> 2.29.2
  
Maxime Coquelin Jan. 26, 2021, 9:24 a.m. UTC | #2
On 1/26/21 10:23 AM, Xia, Chenbo wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Tuesday, January 26, 2021 1:14 AM
>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; olivier.matz@6wind.com;
>> amorenoz@redhat.com; david.marchand@redhat.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [PATCH v3 02/44] bus/vdev: add driver IOVA VA mode requirement
>>
>> This patch adds driver flag in vdev bus driver so that
>> vdev drivers can require VA IOVA mode to be used, which
>> for example the case of Virtio-user PMD.
>>
>> The patch implements the .get_iommu_class() callback, that
>> is called before devices probing to determine the IOVA mode
>> to be used.
>>
>> It also adds a check right before the device is probed to
>> ensure compatible IOVa mode has been selected.
> 
> s/IOVa/IOVA

Just spotted it while reworking the commit message :)

>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  drivers/bus/vdev/rte_bus_vdev.h |  4 ++++
>>  drivers/bus/vdev/vdev.c         | 29 +++++++++++++++++++++++++++++
>>  2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/bus/vdev/rte_bus_vdev.h b/drivers/bus/vdev/rte_bus_vdev.h
>> index f99a41f825..c8b41e649c 100644
>> --- a/drivers/bus/vdev/rte_bus_vdev.h
>> +++ b/drivers/bus/vdev/rte_bus_vdev.h
>> @@ -113,8 +113,12 @@ struct rte_vdev_driver {
>>  	rte_vdev_remove_t *remove;       /**< Virtual device remove function. */
>>  	rte_vdev_dma_map_t *dma_map;     /**< Virtual device DMA map function.
>> */
>>  	rte_vdev_dma_unmap_t *dma_unmap; /**< Virtual device DMA unmap function.
>> */
>> +	uint32_t drv_flags;                /**< Flags RTE_VDEV_DRV_*. */
> 
> I remember David mentioned that the comment above should be consistent with others, which
> also makes sense to me
> 
> Thanks,
> Chenbo
> 
>>  };
>>
>> +/** Device driver needs IOVA as VA and cannot work with IOVA as PA */
>> +#define RTE_VDEV_DRV_NEED_IOVA_AS_VA 0x0001
>> +
>>  /**
>>   * Register a virtual device driver.
>>   *
>> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
>> index acfd78828f..9a673347ae 100644
>> --- a/drivers/bus/vdev/vdev.c
>> +++ b/drivers/bus/vdev/vdev.c
>> @@ -189,6 +189,7 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
>>  {
>>  	const char *name;
>>  	struct rte_vdev_driver *driver;
>> +	enum rte_iova_mode iova_mode;
>>  	int ret;
>>
>>  	if (rte_dev_is_probed(&dev->device))
>> @@ -199,6 +200,14 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)
>>
>>  	if (vdev_parse(name, &driver))
>>  		return -1;
>> +
>> +	iova_mode = rte_eal_iova_mode();
>> +	if ((driver->drv_flags & RTE_VDEV_DRV_NEED_IOVA_AS_VA) && (iova_mode ==
>> RTE_IOVA_PA)) {
>> +		VDEV_LOG(ERR, "%s requires VA IOVA mode but current mode is PA,
>> not initializing",
>> +				name);
>> +		return -1;
>> +	}
>> +
>>  	ret = driver->probe(dev);
>>  	if (ret == 0)
>>  		dev->device.driver = &driver->driver;
>> @@ -594,6 +603,25 @@ vdev_unplug(struct rte_device *dev)
>>  	return rte_vdev_uninit(dev->name);
>>  }
>>
>> +static enum rte_iova_mode
>> +vdev_get_iommu_class(void)
>> +{
>> +	const char *name;
>> +	struct rte_vdev_device *dev;
>> +	struct rte_vdev_driver *driver;
>> +
>> +	TAILQ_FOREACH(dev, &vdev_device_list, next) {
>> +		name = rte_vdev_device_name(dev);
>> +		if (vdev_parse(name, &driver))
>> +			continue;
>> +
>> +		if (driver->drv_flags & RTE_VDEV_DRV_NEED_IOVA_AS_VA)
>> +			return RTE_IOVA_VA;
>> +	}
>> +
>> +	return RTE_IOVA_DC;
>> +}
>> +
>>  static struct rte_bus rte_vdev_bus = {
>>  	.scan = vdev_scan,
>>  	.probe = vdev_probe,
>> @@ -603,6 +631,7 @@ static struct rte_bus rte_vdev_bus = {
>>  	.parse = vdev_parse,
>>  	.dma_map = vdev_dma_map,
>>  	.dma_unmap = vdev_dma_unmap,
>> +	.get_iommu_class = vdev_get_iommu_class,
>>  	.dev_iterate = rte_vdev_dev_iterate,
>>  };
>>
>> --
>> 2.29.2
>
  

Patch

diff --git a/drivers/bus/vdev/rte_bus_vdev.h b/drivers/bus/vdev/rte_bus_vdev.h
index f99a41f825..c8b41e649c 100644
--- a/drivers/bus/vdev/rte_bus_vdev.h
+++ b/drivers/bus/vdev/rte_bus_vdev.h
@@ -113,8 +113,12 @@  struct rte_vdev_driver {
 	rte_vdev_remove_t *remove;       /**< Virtual device remove function. */
 	rte_vdev_dma_map_t *dma_map;     /**< Virtual device DMA map function. */
 	rte_vdev_dma_unmap_t *dma_unmap; /**< Virtual device DMA unmap function. */
+	uint32_t drv_flags;                /**< Flags RTE_VDEV_DRV_*. */
 };
 
+/** Device driver needs IOVA as VA and cannot work with IOVA as PA */
+#define RTE_VDEV_DRV_NEED_IOVA_AS_VA 0x0001
+
 /**
  * Register a virtual device driver.
  *
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index acfd78828f..9a673347ae 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -189,6 +189,7 @@  vdev_probe_all_drivers(struct rte_vdev_device *dev)
 {
 	const char *name;
 	struct rte_vdev_driver *driver;
+	enum rte_iova_mode iova_mode;
 	int ret;
 
 	if (rte_dev_is_probed(&dev->device))
@@ -199,6 +200,14 @@  vdev_probe_all_drivers(struct rte_vdev_device *dev)
 
 	if (vdev_parse(name, &driver))
 		return -1;
+
+	iova_mode = rte_eal_iova_mode();
+	if ((driver->drv_flags & RTE_VDEV_DRV_NEED_IOVA_AS_VA) && (iova_mode == RTE_IOVA_PA)) {
+		VDEV_LOG(ERR, "%s requires VA IOVA mode but current mode is PA, not initializing",
+				name);
+		return -1;
+	}
+
 	ret = driver->probe(dev);
 	if (ret == 0)
 		dev->device.driver = &driver->driver;
@@ -594,6 +603,25 @@  vdev_unplug(struct rte_device *dev)
 	return rte_vdev_uninit(dev->name);
 }
 
+static enum rte_iova_mode
+vdev_get_iommu_class(void)
+{
+	const char *name;
+	struct rte_vdev_device *dev;
+	struct rte_vdev_driver *driver;
+
+	TAILQ_FOREACH(dev, &vdev_device_list, next) {
+		name = rte_vdev_device_name(dev);
+		if (vdev_parse(name, &driver))
+			continue;
+
+		if (driver->drv_flags & RTE_VDEV_DRV_NEED_IOVA_AS_VA)
+			return RTE_IOVA_VA;
+	}
+
+	return RTE_IOVA_DC;
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
@@ -603,6 +631,7 @@  static struct rte_bus rte_vdev_bus = {
 	.parse = vdev_parse,
 	.dma_map = vdev_dma_map,
 	.dma_unmap = vdev_dma_unmap,
+	.get_iommu_class = vdev_get_iommu_class,
 	.dev_iterate = rte_vdev_dev_iterate,
 };