From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH v2] vfio: Support for no-IOMMU mode
Date: Wed, 27 Jan 2016 10:05:04 +0100
Message-ID: <1584319.QMRQbZMs1h@xps13>
References: <1450728967-9686-1-git-send-email-anatoly.burakov@intel.com>
 <1452688569-14695-1-git-send-email-anatoly.burakov@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org
To: Anatoly Burakov <anatoly.burakov@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f45.google.com (mail-wm0-f45.google.com [74.125.82.45])
 by dpdk.org (Postfix) with ESMTP id 4FFED9577
 for <dev@dpdk.org>; Wed, 27 Jan 2016 10:06:15 +0100 (CET)
Received: by mail-wm0-f45.google.com with SMTP id p63so15758858wmp.1
 for <dev@dpdk.org>; Wed, 27 Jan 2016 01:06:15 -0800 (PST)
In-Reply-To: <1452688569-14695-1-git-send-email-anatoly.burakov@intel.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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?