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

From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Anatoly Burakov <anatoly.burakov@intel.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2] vfio: Support for no-IOMMU mode
Date: Wed, 27 Jan 2016 10:05:04 +0100	[thread overview]
Message-ID: <1584319.QMRQbZMs1h@xps13> (raw)
In-Reply-To: <1452688569-14695-1-git-send-email-anatoly.burakov@intel.com>

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", &vfio_iommu_type1_dma_map},
> +		/* IOMMU-less mode */
> +		{ VFIO_NOIOMMU_IOMMU, "No-IOMMU", &vfio_iommu_noiommu_dma_map},
> +};

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

Why a new file for these functions?