[dpdk-stable] [PATCH] vfio: allow secondary process to query IOMMU type

[dpdk-stable] [PATCH] vfio: allow secondary process to query IOMMU type

[Anatoly Burakov]

It is only possible to know IOMMU type of a given VFIO container
by attempting to initialize it. Since secondary process never
attempts to set up VFIO container itself (because they're shared
between primary and secondary), it never knows which IOMMU type
the container is using, and never sets up the appropriate config
structures. This results in inability to perform DMA mappings in
secondary process.

Fix this by allowing secondary process to query IOMMU type of
primary's default container at device initialization.

Note that this fix is assuming we're only interested in default
container.

Bugzilla ID: 174

Fixes: 6bcb7c95fe14 ("vfio: share default container in multi-process")
Cc: dariusz.stojaczyk@intel.com
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_vfio.c        | 89 +++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_vfio.h        | 12 ++-
 .../linuxapp/eal/eal_vfio_mp_sync.c           | 16 ++++
 3 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c
index 72cc65151..17648b1ae 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -549,6 +549,66 @@ vfio_mem_event_callback(enum rte_mem_event type, const void *addr, size_t len,
 	}
 }
 
+static int
+vfio_sync_default_container(void)
+{
+	struct rte_mp_msg mp_req, *mp_rep;
+	struct rte_mp_reply mp_reply;
+	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
+	struct vfio_mp_param *p = (struct vfio_mp_param *)mp_req.param;
+	int vfio_container_fd, iommu_type_id;
+	unsigned int i;
+
+	/* cannot be called from primary */
+	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
+		return -1;
+
+	/* get default container from the primary */
+	vfio_container_fd = vfio_get_default_container_fd();
+	if (vfio_container_fd < 0) {
+		RTE_LOG(ERR, EAL, "Cannot get default container fd\n");
+		return -1;
+	}
+
+	/* find default container's IOMMU type */
+	p->req = SOCKET_REQ_IOMMU_TYPE;
+	strcpy(mp_req.name, EAL_VFIO_MP);
+	mp_req.len_param = sizeof(*p);
+	mp_req.num_fds = 0;
+
+	iommu_type_id = -1;
+	if (rte_mp_request_sync(&mp_req, &mp_reply, &ts) == 0 &&
+			mp_reply.nb_received == 1) {
+		mp_rep = &mp_reply.msgs[0];
+		p = (struct vfio_mp_param *)mp_rep->param;
+		if (p->result == SOCKET_OK)
+			iommu_type_id = p->iommu_type_id;
+		free(mp_reply.msgs);
+	}
+	if (iommu_type_id < 0) {
+		RTE_LOG(ERR, EAL, "Could not get IOMMU type for default container\n");
+		close(vfio_container_fd);
+		return -1;
+	}
+
+	/* we now have an fd for default container, as well as its IOMMU type.
+	 * now, set up default VFIO container config to match.
+	 */
+	for (i = 0; i < RTE_DIM(iommu_types); i++) {
+		const struct vfio_iommu_type *t = &iommu_types[i];
+		if (t->type_id != iommu_type_id)
+			continue;
+
+		/* we found our IOMMU type */
+		default_vfio_cfg->vfio_enabled = 1;
+		default_vfio_cfg->vfio_container_fd = vfio_container_fd;
+		default_vfio_cfg->vfio_iommu_type = t;
+		break;
+	}
+
+	return 0;
+}
+
 int
 rte_vfio_clear_group(int vfio_group_fd)
 {
@@ -745,6 +805,26 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 			else
 				RTE_LOG(DEBUG, EAL, "Installed memory event callback for VFIO\n");
 		}
+	} else if (rte_eal_process_type() != RTE_PROC_PRIMARY &&
+			vfio_cfg == default_vfio_cfg &&
+			vfio_cfg->vfio_iommu_type == NULL) {
+		/* if we're not a primary process, we do not set up the VFIO
+		 * container because it's already been set up by the primary
+		 * process. instead, we simply ask the primary about VFIO type
+		 * we are using, and set the VFIO config up appropriately.
+		 */
+		ret = vfio_sync_default_container();
+		if (ret < 0) {
+			RTE_LOG(ERR, EAL, "Could not sync default VFIO container\n");
+			close(vfio_group_fd);
+			rte_vfio_clear_group(vfio_group_fd);
+			return -1;
+		}
+		/* we have successfully initialized VFIO, notify user */
+		const struct vfio_iommu_type *t =
+				default_vfio_cfg->vfio_iommu_type;
+		RTE_LOG(NOTICE, EAL, "  using IOMMU type %d (%s)\n",
+				t->type_id, t->name);
 	}
 
 	/* get a file descriptor for the device */
@@ -978,6 +1058,15 @@ vfio_get_default_container_fd(void)
 	return -1;
 }
 
+int
+vfio_get_iommu_type(void)
+{
+	if (default_vfio_cfg->vfio_iommu_type == NULL)
+		return -1;
+
+	return default_vfio_cfg->vfio_iommu_type->type_id;
+}
+
 const struct vfio_iommu_type *
 vfio_set_iommu_type(int vfio_container_fd)
 {
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h
index 63ae115c3..cb2d35fb1 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.h
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h
@@ -5,6 +5,8 @@
 #ifndef EAL_VFIO_H_
 #define EAL_VFIO_H_
 
+#include <rte_common.h>
+
 /*
  * determine if VFIO is present on the system
  */
@@ -122,6 +124,9 @@ int vfio_get_default_container_fd(void);
 const struct vfio_iommu_type *
 vfio_set_iommu_type(int vfio_container_fd);
 
+int
+vfio_get_iommu_type(void);
+
 /* check if we have any supported extensions */
 int
 vfio_has_supported_extensions(int vfio_container_fd);
@@ -133,6 +138,7 @@ int vfio_mp_sync_setup(void);
 #define SOCKET_REQ_CONTAINER 0x100
 #define SOCKET_REQ_GROUP 0x200
 #define SOCKET_REQ_DEFAULT_CONTAINER 0x400
+#define SOCKET_REQ_IOMMU_TYPE 0x800
 #define SOCKET_OK 0x0
 #define SOCKET_NO_FD 0x1
 #define SOCKET_ERR 0xFF
@@ -140,7 +146,11 @@ int vfio_mp_sync_setup(void);
 struct vfio_mp_param {
 	int req;
 	int result;
-	int group_num;
+	RTE_STD_C11
+	union {
+		int group_num;
+		int iommu_type_id;
+	};
 };
 
 #endif /* VFIO_PRESENT */
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 a1e8c834f..2a47f29d5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -77,6 +77,22 @@ vfio_mp_primary(const struct rte_mp_msg *msg, const void *peer)
 			reply.fds[0] = fd;
 		}
 		break;
+	case SOCKET_REQ_IOMMU_TYPE:
+	{
+		int iommu_type_id;
+
+		r->req = SOCKET_REQ_IOMMU_TYPE;
+
+		iommu_type_id = vfio_get_iommu_type();
+
+		if (iommu_type_id < 0)
+			r->result = SOCKET_ERR;
+		else {
+			r->iommu_type_id = iommu_type_id;
+			r->result = SOCKET_OK;
+		}
+		break;
+	}
 	default:
 		RTE_LOG(ERR, EAL, "vfio received invalid message!\n");
 		return -1;
-- 
2.17.1

Re: [dpdk-stable] [dpdk-dev] [PATCH] vfio: allow secondary process to query IOMMU type

[Burakov, Anatoly]

On 17-Jan-19 5:30 PM, Anatoly Burakov wrote:
> It is only possible to know IOMMU type of a given VFIO container
> by attempting to initialize it. Since secondary process never
> attempts to set up VFIO container itself (because they're shared
> between primary and secondary), it never knows which IOMMU type
> the container is using, and never sets up the appropriate config
> structures. This results in inability to perform DMA mappings in
> secondary process.
> 
> Fix this by allowing secondary process to query IOMMU type of
> primary's default container at device initialization.
> 
> Note that this fix is assuming we're only interested in default
> container.
> 
> Bugzilla ID: 174
> 
> Fixes: 6bcb7c95fe14 ("vfio: share default container in multi-process")
> Cc: dariusz.stojaczyk@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

<...>

> +	/* we now have an fd for default container, as well as its IOMMU type.
> +	 * now, set up default VFIO container config to match.
> +	 */
> +	for (i = 0; i < RTE_DIM(iommu_types); i++) {
> +		const struct vfio_iommu_type *t = &iommu_types[i];
> +		if (t->type_id != iommu_type_id)
> +			continue;
> +
> +		/* we found our IOMMU type */
> +		default_vfio_cfg->vfio_enabled = 1;
> +		default_vfio_cfg->vfio_container_fd = vfio_container_fd;
> +		default_vfio_cfg->vfio_iommu_type = t;
> +		break;
> +	}

Self-review: if IOMMU type wasn't found, we still return 0. Will fix in v2.

> +
> +	return 0;
> +}
> +
>   int
>   rte_vfio_clear_group(int vfio_group_fd)
>   {
> @@ -745,6 +805,26 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
>   			else
>   				RTE_LOG(DEBUG, EAL, "Installed memory event callback for VFIO\n");

-- 
Thanks,
Anatoly

[dpdk-stable] [PATCH v2] vfio: allow secondary process to query IOMMU type

[Anatoly Burakov]

It is only possible to know IOMMU type of a given VFIO container
by attempting to initialize it. Since secondary process never
attempts to set up VFIO container itself (because they're shared
between primary and secondary), it never knows which IOMMU type
the container is using, and never sets up the appropriate config
structures. This results in inability to perform DMA mappings in
secondary process.

Fix this by allowing secondary process to query IOMMU type of
primary's default container at device initialization.

Note that this fix is assuming we're only interested in default
container.

Bugzilla ID: 174

Fixes: 6bcb7c95fe14 ("vfio: share default container in multi-process")
Cc: dariusz.stojaczyk@intel.com
Cc: stable@dpdk.org

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

Notes:
    v2:
    - Check if we found our IOMMU type within list of IOMMU types
    - Don't request new default container fd as this should have
      been done during rte_vfio_enable()

 lib/librte_eal/linuxapp/eal/eal_vfio.c        | 88 +++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_vfio.h        | 12 ++-
 .../linuxapp/eal/eal_vfio_mp_sync.c           | 16 ++++
 3 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c
index 72cc65151..c821e8382 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -549,6 +549,65 @@ vfio_mem_event_callback(enum rte_mem_event type, const void *addr, size_t len,
 	}
 }
 
+static int
+vfio_sync_default_container(void)
+{
+	struct rte_mp_msg mp_req, *mp_rep;
+	struct rte_mp_reply mp_reply;
+	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
+	struct vfio_mp_param *p = (struct vfio_mp_param *)mp_req.param;
+	int iommu_type_id;
+	unsigned int i;
+
+	/* cannot be called from primary */
+	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
+		return -1;
+
+	/* default container fd should have been opened in rte_vfio_enable() */
+	if (!default_vfio_cfg->vfio_enabled ||
+			default_vfio_cfg->vfio_container_fd < 0) {
+		RTE_LOG(ERR, EAL, "VFIO support is not initialized\n");
+		return -1;
+	}
+
+	/* find default container's IOMMU type */
+	p->req = SOCKET_REQ_IOMMU_TYPE;
+	strcpy(mp_req.name, EAL_VFIO_MP);
+	mp_req.len_param = sizeof(*p);
+	mp_req.num_fds = 0;
+
+	iommu_type_id = -1;
+	if (rte_mp_request_sync(&mp_req, &mp_reply, &ts) == 0 &&
+			mp_reply.nb_received == 1) {
+		mp_rep = &mp_reply.msgs[0];
+		p = (struct vfio_mp_param *)mp_rep->param;
+		if (p->result == SOCKET_OK)
+			iommu_type_id = p->iommu_type_id;
+		free(mp_reply.msgs);
+	}
+	if (iommu_type_id < 0) {
+		RTE_LOG(ERR, EAL, "Could not get IOMMU type for default container\n");
+		return -1;
+	}
+
+	/* we now have an fd for default container, as well as its IOMMU type.
+	 * now, set up default VFIO container config to match.
+	 */
+	for (i = 0; i < RTE_DIM(iommu_types); i++) {
+		const struct vfio_iommu_type *t = &iommu_types[i];
+		if (t->type_id != iommu_type_id)
+			continue;
+
+		/* we found our IOMMU type */
+		default_vfio_cfg->vfio_iommu_type = t;
+
+		return 0;
+	}
+	RTE_LOG(ERR, EAL, "Could not find IOMMU type id (%i)\n",
+			iommu_type_id);
+	return -1;
+}
+
 int
 rte_vfio_clear_group(int vfio_group_fd)
 {
@@ -745,6 +804,26 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
 			else
 				RTE_LOG(DEBUG, EAL, "Installed memory event callback for VFIO\n");
 		}
+	} else if (rte_eal_process_type() != RTE_PROC_PRIMARY &&
+			vfio_cfg == default_vfio_cfg &&
+			vfio_cfg->vfio_iommu_type == NULL) {
+		/* if we're not a primary process, we do not set up the VFIO
+		 * container because it's already been set up by the primary
+		 * process. instead, we simply ask the primary about VFIO type
+		 * we are using, and set the VFIO config up appropriately.
+		 */
+		ret = vfio_sync_default_container();
+		if (ret < 0) {
+			RTE_LOG(ERR, EAL, "Could not sync default VFIO container\n");
+			close(vfio_group_fd);
+			rte_vfio_clear_group(vfio_group_fd);
+			return -1;
+		}
+		/* we have successfully initialized VFIO, notify user */
+		const struct vfio_iommu_type *t =
+				default_vfio_cfg->vfio_iommu_type;
+		RTE_LOG(NOTICE, EAL, "  using IOMMU type %d (%s)\n",
+				t->type_id, t->name);
 	}
 
 	/* get a file descriptor for the device */
@@ -978,6 +1057,15 @@ vfio_get_default_container_fd(void)
 	return -1;
 }
 
+int
+vfio_get_iommu_type(void)
+{
+	if (default_vfio_cfg->vfio_iommu_type == NULL)
+		return -1;
+
+	return default_vfio_cfg->vfio_iommu_type->type_id;
+}
+
 const struct vfio_iommu_type *
 vfio_set_iommu_type(int vfio_container_fd)
 {
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h
index 63ae115c3..cb2d35fb1 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.h
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h
@@ -5,6 +5,8 @@
 #ifndef EAL_VFIO_H_
 #define EAL_VFIO_H_
 
+#include <rte_common.h>
+
 /*
  * determine if VFIO is present on the system
  */
@@ -122,6 +124,9 @@ int vfio_get_default_container_fd(void);
 const struct vfio_iommu_type *
 vfio_set_iommu_type(int vfio_container_fd);
 
+int
+vfio_get_iommu_type(void);
+
 /* check if we have any supported extensions */
 int
 vfio_has_supported_extensions(int vfio_container_fd);
@@ -133,6 +138,7 @@ int vfio_mp_sync_setup(void);
 #define SOCKET_REQ_CONTAINER 0x100
 #define SOCKET_REQ_GROUP 0x200
 #define SOCKET_REQ_DEFAULT_CONTAINER 0x400
+#define SOCKET_REQ_IOMMU_TYPE 0x800
 #define SOCKET_OK 0x0
 #define SOCKET_NO_FD 0x1
 #define SOCKET_ERR 0xFF
@@ -140,7 +146,11 @@ int vfio_mp_sync_setup(void);
 struct vfio_mp_param {
 	int req;
 	int result;
-	int group_num;
+	RTE_STD_C11
+	union {
+		int group_num;
+		int iommu_type_id;
+	};
 };
 
 #endif /* VFIO_PRESENT */
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 a1e8c834f..2a47f29d5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
@@ -77,6 +77,22 @@ vfio_mp_primary(const struct rte_mp_msg *msg, const void *peer)
 			reply.fds[0] = fd;
 		}
 		break;
+	case SOCKET_REQ_IOMMU_TYPE:
+	{
+		int iommu_type_id;
+
+		r->req = SOCKET_REQ_IOMMU_TYPE;
+
+		iommu_type_id = vfio_get_iommu_type();
+
+		if (iommu_type_id < 0)
+			r->result = SOCKET_ERR;
+		else {
+			r->iommu_type_id = iommu_type_id;
+			r->result = SOCKET_OK;
+		}
+		break;
+	}
 	default:
 		RTE_LOG(ERR, EAL, "vfio received invalid message!\n");
 		return -1;
-- 
2.17.1

Re: [dpdk-stable] [PATCH v2] vfio: allow secondary process to query IOMMU type

[Wang, Xiao W]

Hi Anatoly,

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Friday, January 18, 2019 6:25 PM
> To: dev@dpdk.org
> Cc: Wang, Xiao W <xiao.w.wang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; qingfu.cqf@alibaba-inc.com; thomas@monjalon.net;
> Stojaczyk, Dariusz <dariusz.stojaczyk@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] vfio: allow secondary process to query IOMMU type
> 
> It is only possible to know IOMMU type of a given VFIO container
> by attempting to initialize it. Since secondary process never
> attempts to set up VFIO container itself (because they're shared
> between primary and secondary), it never knows which IOMMU type
> the container is using, and never sets up the appropriate config
> structures. This results in inability to perform DMA mappings in
> secondary process.
> 
> Fix this by allowing secondary process to query IOMMU type of
> primary's default container at device initialization.
> 
> Note that this fix is assuming we're only interested in default
> container.
> 
> Bugzilla ID: 174
> 
> Fixes: 6bcb7c95fe14 ("vfio: share default container in multi-process")
> Cc: dariusz.stojaczyk@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> Notes:
>     v2:
>     - Check if we found our IOMMU type within list of IOMMU types
>     - Don't request new default container fd as this should have
>       been done during rte_vfio_enable()
> 
>  lib/librte_eal/linuxapp/eal/eal_vfio.c        | 88 +++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_vfio.h        | 12 ++-
>  .../linuxapp/eal/eal_vfio_mp_sync.c           | 16 ++++
>  3 files changed, 115 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
> b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> index 72cc65151..c821e8382 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> @@ -549,6 +549,65 @@ vfio_mem_event_callback(enum rte_mem_event
> type, const void *addr, size_t len,
>  	}
>  }
> 
> +static int
> +vfio_sync_default_container(void)
> +{
> +	struct rte_mp_msg mp_req, *mp_rep;
> +	struct rte_mp_reply mp_reply;
> +	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
> +	struct vfio_mp_param *p = (struct vfio_mp_param *)mp_req.param;
> +	int iommu_type_id;
> +	unsigned int i;
> +
> +	/* cannot be called from primary */
> +	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
> +		return -1;
> +
> +	/* default container fd should have been opened in rte_vfio_enable()
> */
> +	if (!default_vfio_cfg->vfio_enabled ||
> +			default_vfio_cfg->vfio_container_fd < 0) {
> +		RTE_LOG(ERR, EAL, "VFIO support is not initialized\n");
> +		return -1;
> +	}
> +
> +	/* find default container's IOMMU type */
> +	p->req = SOCKET_REQ_IOMMU_TYPE;

Since this function is to sync IOMMU type for the default container, should we make the req type as
SOCKET_REQ_DEFAULT_IOMMU_TYPE?

BRs,
Xiao

Re: [dpdk-stable] [PATCH v2] vfio: allow secondary process to query IOMMU type

[Burakov, Anatoly]

On 19-Jan-19 3:23 AM, Wang, Xiao W wrote:
> Hi Anatoly,
> 
>> -----Original Message-----
>> From: Burakov, Anatoly
>> Sent: Friday, January 18, 2019 6:25 PM
>> To: dev@dpdk.org
>> Cc: Wang, Xiao W <xiao.w.wang@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; qingfu.cqf@alibaba-inc.com; thomas@monjalon.net;
>> Stojaczyk, Dariusz <dariusz.stojaczyk@intel.com>; stable@dpdk.org
>> Subject: [PATCH v2] vfio: allow secondary process to query IOMMU type
>>
>> It is only possible to know IOMMU type of a given VFIO container
>> by attempting to initialize it. Since secondary process never
>> attempts to set up VFIO container itself (because they're shared
>> between primary and secondary), it never knows which IOMMU type
>> the container is using, and never sets up the appropriate config
>> structures. This results in inability to perform DMA mappings in
>> secondary process.
>>
>> Fix this by allowing secondary process to query IOMMU type of
>> primary's default container at device initialization.
>>
>> Note that this fix is assuming we're only interested in default
>> container.
>>
>> Bugzilla ID: 174
>>
>> Fixes: 6bcb7c95fe14 ("vfio: share default container in multi-process")
>> Cc: dariusz.stojaczyk@intel.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>
>> Notes:
>>      v2:
>>      - Check if we found our IOMMU type within list of IOMMU types
>>      - Don't request new default container fd as this should have
>>        been done during rte_vfio_enable()
>>
>>   lib/librte_eal/linuxapp/eal/eal_vfio.c        | 88 +++++++++++++++++++
>>   lib/librte_eal/linuxapp/eal/eal_vfio.h        | 12 ++-
>>   .../linuxapp/eal/eal_vfio_mp_sync.c           | 16 ++++
>>   3 files changed, 115 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> index 72cc65151..c821e8382 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> @@ -549,6 +549,65 @@ vfio_mem_event_callback(enum rte_mem_event
>> type, const void *addr, size_t len,
>>   	}
>>   }
>>
>> +static int
>> +vfio_sync_default_container(void)
>> +{
>> +	struct rte_mp_msg mp_req, *mp_rep;
>> +	struct rte_mp_reply mp_reply;
>> +	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
>> +	struct vfio_mp_param *p = (struct vfio_mp_param *)mp_req.param;
>> +	int iommu_type_id;
>> +	unsigned int i;
>> +
>> +	/* cannot be called from primary */
>> +	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
>> +		return -1;
>> +
>> +	/* default container fd should have been opened in rte_vfio_enable()
>> */
>> +	if (!default_vfio_cfg->vfio_enabled ||
>> +			default_vfio_cfg->vfio_container_fd < 0) {
>> +		RTE_LOG(ERR, EAL, "VFIO support is not initialized\n");
>> +		return -1;
>> +	}
>> +
>> +	/* find default container's IOMMU type */
>> +	p->req = SOCKET_REQ_IOMMU_TYPE;
> 
> Since this function is to sync IOMMU type for the default container, should we make the req type as
> SOCKET_REQ_DEFAULT_IOMMU_TYPE?

Hi,

Sure, that can be done. However, i don't think it warrants a respin 
unless there's more important stuff to fix also. This patch is a 
stop-gap, and this stuff will be rewritten for 19.05, so getting this 
right is not that important :).

> 
> BRs,
> Xiao
> 


-- 
Thanks,
Anatoly

Re: [dpdk-stable] [PATCH v2] vfio: allow secondary process to query IOMMU type

[Thomas Monjalon]

21/01/2019 11:13, Burakov, Anatoly:
> On 19-Jan-19 3:23 AM, Wang, Xiao W wrote:
> > Hi Anatoly,
> > 
> > From: Burakov, Anatoly
> >>
> >> It is only possible to know IOMMU type of a given VFIO container
> >> by attempting to initialize it. Since secondary process never
> >> attempts to set up VFIO container itself (because they're shared
> >> between primary and secondary), it never knows which IOMMU type
> >> the container is using, and never sets up the appropriate config
> >> structures. This results in inability to perform DMA mappings in
> >> secondary process.
> >>
> >> Fix this by allowing secondary process to query IOMMU type of
> >> primary's default container at device initialization.
> >>
> >> Note that this fix is assuming we're only interested in default
> >> container.
> >>
> >> Bugzilla ID: 174
> >>
> >> Fixes: 6bcb7c95fe14 ("vfio: share default container in multi-process")
> >> Cc: dariusz.stojaczyk@intel.com
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >> ---
> >>
> >> Notes:
> >>      v2:
> >>      - Check if we found our IOMMU type within list of IOMMU types
> >>      - Don't request new default container fd as this should have
> >>        been done during rte_vfio_enable()
> >>
> >>   lib/librte_eal/linuxapp/eal/eal_vfio.c        | 88 +++++++++++++++++++
> >>   lib/librte_eal/linuxapp/eal/eal_vfio.h        | 12 ++-
> >>   .../linuxapp/eal/eal_vfio_mp_sync.c           | 16 ++++
> >>   3 files changed, 115 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
> >> b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> >> index 72cc65151..c821e8382 100644
> >> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
> >> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
> >> @@ -549,6 +549,65 @@ vfio_mem_event_callback(enum rte_mem_event
> >> type, const void *addr, size_t len,
> >>   	}
> >>   }
> >>
> >> +static int
> >> +vfio_sync_default_container(void)
> >> +{
> >> +	struct rte_mp_msg mp_req, *mp_rep;
> >> +	struct rte_mp_reply mp_reply;
> >> +	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
> >> +	struct vfio_mp_param *p = (struct vfio_mp_param *)mp_req.param;
> >> +	int iommu_type_id;
> >> +	unsigned int i;
> >> +
> >> +	/* cannot be called from primary */
> >> +	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
> >> +		return -1;
> >> +
> >> +	/* default container fd should have been opened in rte_vfio_enable()
> >> */
> >> +	if (!default_vfio_cfg->vfio_enabled ||
> >> +			default_vfio_cfg->vfio_container_fd < 0) {
> >> +		RTE_LOG(ERR, EAL, "VFIO support is not initialized\n");
> >> +		return -1;
> >> +	}
> >> +
> >> +	/* find default container's IOMMU type */
> >> +	p->req = SOCKET_REQ_IOMMU_TYPE;
> > 
> > Since this function is to sync IOMMU type for the default container, should we make the req type as
> > SOCKET_REQ_DEFAULT_IOMMU_TYPE?
> 
> Hi,
> 
> Sure, that can be done. However, i don't think it warrants a respin 
> unless there's more important stuff to fix also. This patch is a 
> stop-gap, and this stuff will be rewritten for 19.05, so getting this 
> right is not that important :).

Anatoly,
I don't understand what is the use case.
It does not look critical enough to be merged late in 19.02.

Re: [dpdk-stable] [PATCH v2] vfio: allow secondary process to query IOMMU type

[Burakov, Anatoly]

On 21-Jan-19 10:21 AM, Thomas Monjalon wrote:
> 21/01/2019 11:13, Burakov, Anatoly:
>> On 19-Jan-19 3:23 AM, Wang, Xiao W wrote:
>>> Hi Anatoly,
>>>
>>> From: Burakov, Anatoly
>>>>
>>>> It is only possible to know IOMMU type of a given VFIO container
>>>> by attempting to initialize it. Since secondary process never
>>>> attempts to set up VFIO container itself (because they're shared
>>>> between primary and secondary), it never knows which IOMMU type
>>>> the container is using, and never sets up the appropriate config
>>>> structures. This results in inability to perform DMA mappings in
>>>> secondary process.
>>>>
>>>> Fix this by allowing secondary process to query IOMMU type of
>>>> primary's default container at device initialization.
>>>>
>>>> Note that this fix is assuming we're only interested in default
>>>> container.
>>>>
>>>> Bugzilla ID: 174
>>>>
>>>> Fixes: 6bcb7c95fe14 ("vfio: share default container in multi-process")
>>>> Cc: dariusz.stojaczyk@intel.com
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>> ---
>>>>
>>>> Notes:
>>>>       v2:
>>>>       - Check if we found our IOMMU type within list of IOMMU types
>>>>       - Don't request new default container fd as this should have
>>>>         been done during rte_vfio_enable()
>>>>
>>>>    lib/librte_eal/linuxapp/eal/eal_vfio.c        | 88 +++++++++++++++++++
>>>>    lib/librte_eal/linuxapp/eal/eal_vfio.h        | 12 ++-
>>>>    .../linuxapp/eal/eal_vfio_mp_sync.c           | 16 ++++
>>>>    3 files changed, 115 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>>>> b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>>>> index 72cc65151..c821e8382 100644
>>>> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>>>> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>>>> @@ -549,6 +549,65 @@ vfio_mem_event_callback(enum rte_mem_event
>>>> type, const void *addr, size_t len,
>>>>    	}
>>>>    }
>>>>
>>>> +static int
>>>> +vfio_sync_default_container(void)
>>>> +{
>>>> +	struct rte_mp_msg mp_req, *mp_rep;
>>>> +	struct rte_mp_reply mp_reply;
>>>> +	struct timespec ts = {.tv_sec = 5, .tv_nsec = 0};
>>>> +	struct vfio_mp_param *p = (struct vfio_mp_param *)mp_req.param;
>>>> +	int iommu_type_id;
>>>> +	unsigned int i;
>>>> +
>>>> +	/* cannot be called from primary */
>>>> +	if (rte_eal_process_type() != RTE_PROC_SECONDARY)
>>>> +		return -1;
>>>> +
>>>> +	/* default container fd should have been opened in rte_vfio_enable()
>>>> */
>>>> +	if (!default_vfio_cfg->vfio_enabled ||
>>>> +			default_vfio_cfg->vfio_container_fd < 0) {
>>>> +		RTE_LOG(ERR, EAL, "VFIO support is not initialized\n");
>>>> +		return -1;
>>>> +	}
>>>> +
>>>> +	/* find default container's IOMMU type */
>>>> +	p->req = SOCKET_REQ_IOMMU_TYPE;
>>>
>>> Since this function is to sync IOMMU type for the default container, should we make the req type as
>>> SOCKET_REQ_DEFAULT_IOMMU_TYPE?
>>
>> Hi,
>>
>> Sure, that can be done. However, i don't think it warrants a respin
>> unless there's more important stuff to fix also. This patch is a
>> stop-gap, and this stuff will be rewritten for 19.05, so getting this
>> right is not that important :).
> 
> Anatoly,
> I don't understand what is the use case.
> It does not look critical enough to be merged late in 19.02.
> 

This isn't "a use case", it's a bug fix. This was supposed to work, but 
doesn't.

-- 
Thanks,
Anatoly

Re: [dpdk-stable] [PATCH v2] vfio: allow secondary process to query IOMMU type

[Stojaczyk, Dariusz]

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Friday, January 18, 2019 11:25 AM
> Subject: [PATCH v2] vfio: allow secondary process to query IOMMU type
> 
> It is only possible to know IOMMU type of a given VFIO container
> by attempting to initialize it. Since secondary process never
> attempts to set up VFIO container itself (because they're shared
> between primary and secondary), it never knows which IOMMU type
> the container is using, and never sets up the appropriate config
> structures. This results in inability to perform DMA mappings in
> secondary process.
> 
> Fix this by allowing secondary process to query IOMMU type of
> primary's default container at device initialization.
> 
> Note that this fix is assuming we're only interested in default
> container.
> 
> Bugzilla ID: 174
> 
> Fixes: 6bcb7c95fe14 ("vfio: share default container in multi-process")
> Cc: dariusz.stojaczyk@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>

Thanks!
In commit 6bcb7c95fe1 (vfio: share default container in multi-process) we fixed just the container fd and not the rte_vfio mapping APIs because we did not even use those APIs in SPDK.

Re: [dpdk-stable] [PATCH v2] vfio: allow secondary process to query IOMMU type

[Thomas Monjalon]

21/01/2019 11:29, Stojaczyk, Dariusz:
> > It is only possible to know IOMMU type of a given VFIO container
> > by attempting to initialize it. Since secondary process never
> > attempts to set up VFIO container itself (because they're shared
> > between primary and secondary), it never knows which IOMMU type
> > the container is using, and never sets up the appropriate config
> > structures. This results in inability to perform DMA mappings in
> > secondary process.
> > 
> > Fix this by allowing secondary process to query IOMMU type of
> > primary's default container at device initialization.
> > 
> > Note that this fix is assuming we're only interested in default
> > container.
> > 
> > Bugzilla ID: 174
> > 
> > Fixes: 6bcb7c95fe14 ("vfio: share default container in multi-process")
> > Cc: dariusz.stojaczyk@intel.com
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > ---
> 
> Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> 
> Thanks!
> In commit 6bcb7c95fe1 (vfio: share default container in multi-process) we fixed just the container fd and not the rte_vfio mapping APIs because we did not even use those APIs in SPDK.

Applied, thanks