[dpdk-dev] [PATCH v2] vfio: Support for no-IOMMU mode

2016-01-27 Thread David Marchand
On Wed, Jan 27, 2016 at 11:12 AM, Thomas Monjalon
 wrote:
> 2016-01-27 10:08, Burakov, Anatoly:
>> > Why a new file for these functions?
>>
>> Well, my thought was to make future extensions easier by way of avoiding 
>> mixing irrelevant and/or general code with driver-specific code. I can 
>> change it back if that's not OK.
>
> No strong opinion here.
> David?

Hum, no strong opinion either, but I don't think we really need to
split this file for this much code.
Besides, if we keep all code in eal_pci_vfio.c, there is no need to
expose those structures through eal_pci_init.h.


-- 
David Marchand


[dpdk-dev] [PATCH v2] vfio: Support for no-IOMMU mode

2016-01-27 Thread Thomas Monjalon
2016-01-27 10:08, Burakov, Anatoly:
> > Why a new file for these functions?
> 
> Well, my thought was to make future extensions easier by way of avoiding 
> mixing irrelevant and/or general code with driver-specific code. I can change 
> it back if that's not OK.

No strong opinion here.
David?


[dpdk-dev] [PATCH v2] vfio: Support for no-IOMMU mode

2016-01-27 Thread Burakov, Anatoly
> >> > Why a new file for these functions?
> >>
> >> Well, my thought was to make future extensions easier by way of
> avoiding mixing irrelevant and/or general code with driver-specific code. I 
> can
> change it back if that's not OK.
> >
> > No strong opinion here.
> > David?
> 
> Hum, no strong opinion either, but I don't think we really need to split this
> file for this much code.
> Besides, if we keep all code in eal_pci_vfio.c, there is no need to expose
> those structures through eal_pci_init.h.

OK then, I'll merge it back into the eal_pci_vfio.c

Thanks,
Anatoly


[dpdk-dev] [PATCH v2] vfio: Support for no-IOMMU mode

2016-01-27 Thread Thomas Monjalon
Hi Anatoly,

Few small comments.

The comments "function pointer typedef" or "structure to hold" don't
bring new information. Please keep it short.

2016-01-13 12:36, Anatoly Burakov:
> +/* function pointer typedef for DMA mapping functions */

->  DMA mapping function type
It would be relevant to describe the return and the parameter.

> +typedef  int (*vfio_dma_func_t)(int);
> +
> +/* Structure to hold supported IOMMU types */

This comment seems useless.

> +struct vfio_iommu_type {

[...]
> +/* function prototypes for different IOMMU types */

idem

> +int vfio_iommu_type1_dma_map(int container_fd);
> +int vfio_iommu_noiommu_dma_map(int container_fd);
> +
> +/* IOMMU types we support */
> +static const struct vfio_iommu_type iommu_types[] = {
> + /* x86 IOMMU, otherwise known as type 1 */
> + { VFIO_TYPE1_IOMMU, "Type 1", _iommu_type1_dma_map},
> + /* IOMMU-less mode */
> + { VFIO_NOIOMMU_IOMMU, "No-IOMMU", _iommu_noiommu_dma_map},
> +};

[...]
> --- /dev/null
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c

Why a new file for these functions?



[dpdk-dev] [PATCH v2] vfio: Support for no-IOMMU mode

2016-01-14 Thread Burakov, Anatoly
Hi Stephen,

> > +/* IOMMU types we support */
> > +static const struct vfio_iommu_type iommu_types[] = {
> > +   /* x86 IOMMU, otherwise known as type 1 */
> > +   { VFIO_TYPE1_IOMMU, "Type 1",
> _iommu_type1_dma_map},
> > +   /* IOMMU-less mode */
> > +   { VFIO_NOIOMMU_IOMMU, "No-IOMMU",
> _iommu_noiommu_dma_map},
> > +};
> > +
> 
> Nit.. Why full-tab indent here?

Readability mainly... at least it's more readable to me that way. I can change 
that if necessary.

Thanks,
Anatoly


[dpdk-dev] [PATCH v2] vfio: Support for no-IOMMU mode

2016-01-13 Thread Anatoly Burakov
This commit is adding a generic mechanism to support multiple IOMMU
types. For now, it's only type 1 (x86 IOMMU) and no-IOMMU (a special
VFIO mode that doesn't use IOMMU at all), but it's easily extended
by adding necessary definitions into eal_pci_init.h and a DMA
mapping function to eal_pci_vfio_dma.c.

Since type 1 IOMMU module is no longer necessary to have VFIO,
we fix the module check to check for vfio-pci instead. It's not
ideal and triggers VFIO checks more often (and thus produces more
error output, which was the reason behind the module check in the
first place), so we compensate for that by providing more verbose
logging, indicating whether VFIO initialization has succeeded or
failed.

Signed-off-by: Anatoly Burakov 
Signed-off-by: Santosh Shukla 
Tested-by: Santosh Shukla 
---
v2 changes:
  Compile fix (hat-tip to Santosh Shukla)
  Tested-by is provisional, since only superficial testing was done
---
 lib/librte_eal/linuxapp/eal/Makefile   |   1 +
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |  22 
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 143 -
 lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c |  84 +++
 lib/librte_eal/linuxapp/eal/eal_vfio.h |   5 +
 5 files changed, 202 insertions(+), 53 deletions(-)
 create mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c

diff --git a/lib/librte_eal/linuxapp/eal/Makefile 
b/lib/librte_eal/linuxapp/eal/Makefile
index 26eced5..5c9e9d9 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -59,6 +59,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_log.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_uio.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_vfio.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_vfio_dma.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_vfio_mp_sync.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_debug.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_lcore.c
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h 
b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index a17c708..da1c431 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -106,6 +106,28 @@ struct vfio_config {
struct vfio_group vfio_groups[VFIO_MAX_GROUPS];
 };

+/* function pointer typedef for DMA mapping functions */
+typedef  int (*vfio_dma_func_t)(int);
+
+/* Structure to hold supported IOMMU types */
+struct vfio_iommu_type {
+   int type_id;
+   const char *name;
+   vfio_dma_func_t dma_map_func;
+};
+
+/* function prototypes for different IOMMU types */
+int vfio_iommu_type1_dma_map(int container_fd);
+int vfio_iommu_noiommu_dma_map(int container_fd);
+
+/* IOMMU types we support */
+static const struct vfio_iommu_type iommu_types[] = {
+   /* x86 IOMMU, otherwise known as type 1 */
+   { VFIO_TYPE1_IOMMU, "Type 1", _iommu_type1_dma_map},
+   /* IOMMU-less mode */
+   { VFIO_NOIOMMU_IOMMU, "No-IOMMU", _iommu_noiommu_dma_map},
+};
+
 #endif

 #endif /* EAL_PCI_INIT_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c 
b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 74f91ba..5eb6cd0 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -72,6 +72,7 @@ EAL_REGISTER_TAILQ(rte_vfio_tailq)
 #define VFIO_DIR "/dev/vfio"
 #define VFIO_CONTAINER_PATH "/dev/vfio/vfio"
 #define VFIO_GROUP_FMT "/dev/vfio/%u"
+#define VFIO_NOIOMMU_GROUP_FMT "/dev/vfio/noiommu-%u"
 #define VFIO_GET_REGION_ADDR(x) ((uint64_t) x << 40ULL)

 /* per-process VFIO config */
@@ -208,42 +209,58 @@ pci_vfio_set_bus_master(int dev_fd)
return 0;
 }

-/* set up DMA mappings */
-static int
-pci_vfio_setup_dma_maps(int vfio_container_fd)
-{
-   const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-   int i, ret;
-
-   ret = ioctl(vfio_container_fd, VFIO_SET_IOMMU,
-   VFIO_TYPE1_IOMMU);
-   if (ret) {
-   RTE_LOG(ERR, EAL, "  cannot set IOMMU type, "
-   "error %i (%s)\n", errno, strerror(errno));
-   return -1;
+/* pick IOMMU type. returns a pointer to vfio_iommu_type or NULL for error */
+static const struct vfio_iommu_type *
+pci_vfio_set_iommu_type(int vfio_container_fd) {
+   unsigned idx;
+   for (idx = 0; idx < RTE_DIM(iommu_types); idx++) {
+   const struct vfio_iommu_type *t = _types[idx];
+
+   int ret = ioctl(vfio_container_fd, VFIO_SET_IOMMU,
+   t->type_id);
+   if (!ret) {
+   RTE_LOG(NOTICE, EAL, "  using IOMMU type %d (%s)\n",
+   t->type_id, t->name);
+   return t;
+   }
+   /* not an error, there may be more supported IOMMU types */
+   

[dpdk-dev] [PATCH v2] vfio: Support for no-IOMMU mode

2016-01-13 Thread Stephen Hemminger
On Wed, 13 Jan 2016 12:36:09 +
Anatoly Burakov  wrote:

> +/* IOMMU types we support */
> +static const struct vfio_iommu_type iommu_types[] = {
> + /* x86 IOMMU, otherwise known as type 1 */
> + { VFIO_TYPE1_IOMMU, "Type 1", _iommu_type1_dma_map},
> + /* IOMMU-less mode */
> + { VFIO_NOIOMMU_IOMMU, "No-IOMMU", _iommu_noiommu_dma_map},
> +};
> +

Nit.. Why full-tab indent here?