[dpdk-dev] [PATCH 01/2] vfio: expose clear group function for internal usages

[dpdk-dev] [PATCH 01/2] vfio: expose clear group function for internal usages

[Hemant Agrawal]

other vfio based module e.g. fslmc will also need to use
the clear_group call.
So, exposing it and renaming it to *rte_vfio_clear_group*

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 lib/librte_eal/linuxapp/eal/eal_vfio.c         | 18 +++++++++---------
 lib/librte_eal/linuxapp/eal/eal_vfio.h         |  2 +-
 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c |  2 +-
 lib/librte_eal/rte_eal_version.map             |  7 +++++++
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c
index 58f0123..76184d1 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -226,7 +226,7 @@ vfio_group_device_count(int vfio_group_fd)
 }
 
 int
-clear_group(int vfio_group_fd)
+rte_vfio_clear_group(int vfio_group_fd)
 {
 	int i;
 	int socket_fd, ret;
@@ -329,12 +329,12 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 		RTE_LOG(ERR, EAL, "  %s cannot get group status, "
 				"error %i (%s)\n", dev_addr, errno, strerror(errno));
 		close(vfio_group_fd);
-		clear_group(vfio_group_fd);
+		rte_vfio_clear_group(vfio_group_fd);
 		return -1;
 	} else if (!(group_status.flags & VFIO_GROUP_FLAGS_VIABLE)) {
 		RTE_LOG(ERR, EAL, "  %s VFIO group is not viable!\n", dev_addr);
 		close(vfio_group_fd);
-		clear_group(vfio_group_fd);
+		rte_vfio_clear_group(vfio_group_fd);
 		return -1;
 	}
 
@@ -348,7 +348,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 			RTE_LOG(ERR, EAL, "  %s cannot add VFIO group to container, "
 					"error %i (%s)\n", dev_addr, errno, strerror(errno));
 			close(vfio_group_fd);
-			clear_group(vfio_group_fd);
+			rte_vfio_clear_group(vfio_group_fd);
 			return -1;
 		}
 
@@ -370,7 +370,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 					"  %s failed to select IOMMU type\n",
 					dev_addr);
 				close(vfio_group_fd);
-				clear_group(vfio_group_fd);
+				rte_vfio_clear_group(vfio_group_fd);
 				return -1;
 			}
 			ret = t->dma_map_func(vfio_cfg.vfio_container_fd);
@@ -379,7 +379,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 					"  %s DMA remapping failed, error %i (%s)\n",
 					dev_addr, errno, strerror(errno));
 				close(vfio_group_fd);
-				clear_group(vfio_group_fd);
+				rte_vfio_clear_group(vfio_group_fd);
 				return -1;
 			}
 		}
@@ -395,7 +395,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 		RTE_LOG(WARNING, EAL, "Getting a vfio_dev_fd for %s failed\n",
 				dev_addr);
 		close(vfio_group_fd);
-		clear_group(vfio_group_fd);
+		rte_vfio_clear_group(vfio_group_fd);
 		return -1;
 	}
 
@@ -407,7 +407,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 				strerror(errno));
 		close(*vfio_dev_fd);
 		close(vfio_group_fd);
-		clear_group(vfio_group_fd);
+		rte_vfio_clear_group(vfio_group_fd);
 		return -1;
 	}
 	vfio_group_device_get(vfio_group_fd);
@@ -467,7 +467,7 @@ rte_vfio_release_device(const char *sysfs_base, const char *dev_addr,
 			return -1;
 		}
 
-		if (clear_group(vfio_group_fd) < 0) {
+		if (rte_vfio_clear_group(vfio_group_fd) < 0) {
 			RTE_LOG(INFO, EAL, "Error when clearing group for %s\n",
 					   dev_addr);
 			return -1;
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h
index ba7892b..12e38cc 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.h
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h
@@ -179,7 +179,7 @@ vfio_get_group_fd(int iommu_group_no);
 
 /* remove group fd from internal VFIO group fd array */
 int
-clear_group(int vfio_group_fd);
+rte_vfio_clear_group(int vfio_group_fd);
 
 int vfio_mp_sync_setup(void);
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
index b53ed7e..e32f2e3 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -333,7 +333,7 @@ vfio_mp_sync_thread(void __rte_unused * arg)
 				continue;
 			}
 
-			ret = clear_group(vfio_data);
+			ret = rte_vfio_clear_group(vfio_data);
 
 			if (ret < 0)
 				vfio_mp_sync_send_request(conn_sock, SOCKET_NO_FD);
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index f4f46c1..5af887b 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -236,3 +236,10 @@ EXPERIMENTAL {
 	rte_service_start_with_defaults;
 
 } DPDK_17.11;
+
+DPDK_18.02 {
+	global:
+
+	rte_vfio_clear_group;
+
+} DPDK_17.11;
\ No newline at end of file
-- 
2.7.4

[dpdk-dev] [PATCH 02/2] bus/fslmc: clear the vfio group on error

[Hemant Agrawal]

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 drivers/bus/fslmc/fslmc_vfio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index 7831201..25c87ad 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -660,12 +660,14 @@ fslmc_vfio_setup_group(void)
 	if (ret) {
 		FSLMC_VFIO_LOG(ERR, "VFIO error getting group status");
 		close(vfio_group.fd);
+		rte_vfio_clear_group(vfio_group.fd);
 		return ret;
 	}
 
 	if (!(status.flags & VFIO_GROUP_FLAGS_VIABLE)) {
 		FSLMC_VFIO_LOG(ERR, "VFIO group not viable");
 		close(vfio_group.fd);
+		rte_vfio_clear_group(vfio_group.fd);
 		return -EPERM;
 	}
 	/* Since Group is VIABLE, Store the groupid */
@@ -680,6 +682,7 @@ fslmc_vfio_setup_group(void)
 				"Error connecting container with groupid %d",
 				groupid);
 			close(vfio_group.fd);
+			rte_vfio_clear_group(vfio_group.fd);
 			return ret;
 		}
 	}
@@ -690,6 +693,7 @@ fslmc_vfio_setup_group(void)
 		FSLMC_VFIO_LOG(ERR, "Error getting device %s fd from group %d",
 			       g_container, vfio_group.groupid);
 		close(vfio_group.fd);
+		rte_vfio_clear_group(vfio_group.fd);
 		return ret;
 	}
 	container_device_fd = ret;
-- 
2.7.4

Re: [dpdk-dev] [PATCH 01/2] vfio: expose clear group function for internal usages

[Burakov, Anatoly]

On 07-Dec-17 10:28 AM, Hemant Agrawal wrote:
> other vfio based module e.g. fslmc will also need to use
> the clear_group call.
> So, exposing it and renaming it to *rte_vfio_clear_group*
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH 01/2] vfio: expose clear group function for internal usages

[Thomas Monjalon]

07/12/2017 11:28, Hemant Agrawal:
> other vfio based module e.g. fslmc will also need to use
> the clear_group call.
> So, exposing it and renaming it to *rte_vfio_clear_group*
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h
> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h
> @@ -179,7 +179,7 @@ vfio_get_group_fd(int iommu_group_no);
>  
>  /* remove group fd from internal VFIO group fd array */
>  int
> -clear_group(int vfio_group_fd);
> +rte_vfio_clear_group(int vfio_group_fd);

The function should be moved to lib/librte_eal/common/include/rte_vfio.h
and an empty implementation must be added for BSD.

Related note: in drivers/bus/fslmc/fslmc_vfio.h there is
	#include "eal_vfio.h"
Can it be removed?

Re: [dpdk-dev] [PATCH 01/2] vfio: expose clear group function for internal usages

[Hemant Agrawal]

On 1/12/2018 10:06 PM, Thomas Monjalon wrote:
> 07/12/2017 11:28, Hemant Agrawal:
>> other vfio based module e.g. fslmc will also need to use
>> the clear_group call.
>> So, exposing it and renaming it to *rte_vfio_clear_group*
>>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.h
>> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h
>> @@ -179,7 +179,7 @@ vfio_get_group_fd(int iommu_group_no);
>>
>>  /* remove group fd from internal VFIO group fd array */
>>  int
>> -clear_group(int vfio_group_fd);
>> +rte_vfio_clear_group(int vfio_group_fd);
>
> The function should be moved to lib/librte_eal/common/include/rte_vfio.h
> and an empty implementation must be added for BSD.
>
I am sending v2 for that.

> Related note: in drivers/bus/fslmc/fslmc_vfio.h there is
> 	#include "eal_vfio.h"
> Can it be removed?
>
Yes but it will be different change.
Currently fslmc_vfio.h is is using few "vfio_get_xxx" functions. I will 
plan that cleanup in different patch.

[dpdk-dev] [PATCH v2 1/2] vfio: expose clear group function for internal usages

[Hemant Agrawal]

other vfio based module e.g. fslmc will also need to use
the clear_group call.
So, exposing it and renaming it to *rte_vfio_clear_group*

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/bsdapp/eal/eal.c                |  5 +++++
 lib/librte_eal/common/include/rte_vfio.h       |  4 ++++
 lib/librte_eal/linuxapp/eal/eal_vfio.c         | 18 +++++++++---------
 lib/librte_eal/linuxapp/eal/eal_vfio.h         |  4 ----
 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c |  2 +-
 lib/librte_eal/rte_eal_version.map             |  1 +
 6 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 369a682..7239243 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -778,3 +778,8 @@ int rte_vfio_noiommu_is_enabled(void)
 {
 	return 0;
 }
+
+int rte_vfio_clear_group(int vfio_group_fd)
+{
+	return 0;
+}
diff --git a/lib/librte_eal/common/include/rte_vfio.h b/lib/librte_eal/common/include/rte_vfio.h
index a69c4ff..daa0dbd 100644
--- a/lib/librte_eal/common/include/rte_vfio.h
+++ b/lib/librte_eal/common/include/rte_vfio.h
@@ -148,6 +148,10 @@ int rte_vfio_is_enabled(const char *modname);
  */
 int rte_vfio_noiommu_is_enabled(void);
 
+/* remove group fd from internal VFIO group fd array */
+int
+rte_vfio_clear_group(int vfio_group_fd);
+
 #endif /* VFIO_PRESENT */
 
 #endif /* _RTE_VFIO_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c
index 681eab5..5371ec2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -197,7 +197,7 @@ vfio_group_device_count(int vfio_group_fd)
 }
 
 int
-clear_group(int vfio_group_fd)
+rte_vfio_clear_group(int vfio_group_fd)
 {
 	int i;
 	int socket_fd, ret;
@@ -300,12 +300,12 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 		RTE_LOG(ERR, EAL, "  %s cannot get group status, "
 				"error %i (%s)\n", dev_addr, errno, strerror(errno));
 		close(vfio_group_fd);
-		clear_group(vfio_group_fd);
+		rte_vfio_clear_group(vfio_group_fd);
 		return -1;
 	} else if (!(group_status.flags & VFIO_GROUP_FLAGS_VIABLE)) {
 		RTE_LOG(ERR, EAL, "  %s VFIO group is not viable!\n", dev_addr);
 		close(vfio_group_fd);
-		clear_group(vfio_group_fd);
+		rte_vfio_clear_group(vfio_group_fd);
 		return -1;
 	}
 
@@ -319,7 +319,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 			RTE_LOG(ERR, EAL, "  %s cannot add VFIO group to container, "
 					"error %i (%s)\n", dev_addr, errno, strerror(errno));
 			close(vfio_group_fd);
-			clear_group(vfio_group_fd);
+			rte_vfio_clear_group(vfio_group_fd);
 			return -1;
 		}
 
@@ -341,7 +341,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 					"  %s failed to select IOMMU type\n",
 					dev_addr);
 				close(vfio_group_fd);
-				clear_group(vfio_group_fd);
+				rte_vfio_clear_group(vfio_group_fd);
 				return -1;
 			}
 			ret = t->dma_map_func(vfio_cfg.vfio_container_fd);
@@ -350,7 +350,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 					"  %s DMA remapping failed, error %i (%s)\n",
 					dev_addr, errno, strerror(errno));
 				close(vfio_group_fd);
-				clear_group(vfio_group_fd);
+				rte_vfio_clear_group(vfio_group_fd);
 				return -1;
 			}
 		}
@@ -366,7 +366,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 		RTE_LOG(WARNING, EAL, "Getting a vfio_dev_fd for %s failed\n",
 				dev_addr);
 		close(vfio_group_fd);
-		clear_group(vfio_group_fd);
+		rte_vfio_clear_group(vfio_group_fd);
 		return -1;
 	}
 
@@ -378,7 +378,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 				strerror(errno));
 		close(*vfio_dev_fd);
 		close(vfio_group_fd);
-		clear_group(vfio_group_fd);
+		rte_vfio_clear_group(vfio_group_fd);
 		return -1;
 	}
 	vfio_group_device_get(vfio_group_fd);
@@ -438,7 +438,7 @@ rte_vfio_release_device(const char *sysfs_base, const char *dev_addr,
 			return -1;
 		}
 
-		if (clear_group(vfio_group_fd) < 0) {
+		if (rte_vfio_clear_group(vfio_group_fd) < 0) {
 			RTE_LOG(INFO, EAL, "Error when clearing group for %s\n",
 					   dev_addr);
 			return -1;
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h
index b34d5d0..359589e 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.h
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h
@@ -148,10 +148,6 @@ vfio_get_group_no(const char *sysfs_base,
 int
 vfio_get_group_fd(int iommu_group_no);
 
-/* remove group fd from internal VFIO group fd array */
-int
-clear_group(int vfio_group_fd);
-
 int vfio_mp_sync_setup(void);
 
 #define SOCKET_REQ_CONTAINER 0x100
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
index 9b474dc..7cc3c15 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -304,7 +304,7 @@ vfio_mp_sync_thread(void __rte_unused * arg)
 				continue;
 			}
 
-			ret = clear_group(vfio_data);
+			ret = rte_vfio_clear_group(vfio_data);
 
 			if (ret < 0)
 				vfio_mp_sync_send_request(conn_sock, SOCKET_NO_FD);
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 3fa1e13..7088b72 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -205,6 +205,7 @@ DPDK_18.02 {
 
 	rte_hypervisor_get;
 	rte_hypervisor_get_name;
+	rte_vfio_clear_group;
 
 } DPDK_17.11;
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/2] bus/fslmc: clear the vfio group on error

[Hemant Agrawal]

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 drivers/bus/fslmc/fslmc_vfio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/bus/fslmc/fslmc_vfio.c b/drivers/bus/fslmc/fslmc_vfio.c
index fb2f403..12b254a 100644
--- a/drivers/bus/fslmc/fslmc_vfio.c
+++ b/drivers/bus/fslmc/fslmc_vfio.c
@@ -634,12 +634,14 @@ fslmc_vfio_setup_group(void)
 	if (ret) {
 		FSLMC_VFIO_LOG(ERR, "VFIO error getting group status");
 		close(vfio_group.fd);
+		rte_vfio_clear_group(vfio_group.fd);
 		return ret;
 	}
 
 	if (!(status.flags & VFIO_GROUP_FLAGS_VIABLE)) {
 		FSLMC_VFIO_LOG(ERR, "VFIO group not viable");
 		close(vfio_group.fd);
+		rte_vfio_clear_group(vfio_group.fd);
 		return -EPERM;
 	}
 	/* Since Group is VIABLE, Store the groupid */
@@ -654,6 +656,7 @@ fslmc_vfio_setup_group(void)
 				"Error connecting container with groupid %d",
 				groupid);
 			close(vfio_group.fd);
+			rte_vfio_clear_group(vfio_group.fd);
 			return ret;
 		}
 	}
@@ -664,6 +667,7 @@ fslmc_vfio_setup_group(void)
 		FSLMC_VFIO_LOG(ERR, "Error getting device %s fd from group %d",
 			       g_container, vfio_group.groupid);
 		close(vfio_group.fd);
+		rte_vfio_clear_group(vfio_group.fd);
 		return ret;
 	}
 	container_device_fd = ret;
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 1/2] vfio: expose clear group function for internal usages

[Thomas Monjalon]

15/01/2018 06:11, Hemant Agrawal:
> other vfio based module e.g. fslmc will also need to use
> the clear_group call.
> So, exposing it and renaming it to *rte_vfio_clear_group*
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Series applied, thanks