linux-kernel - Re: [PATCH V2] vfio: platform: support No-IOMMU mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5742C66C.90203@linaro.org>
Date:	Mon, 23 May 2016 10:59:24 +0200
From:	Eric Auger <eric.auger@...aro.org>
To:	Peng Fan <van.freenix@...il.com>, alex.williamson@...hat.com,
	b.reynal@...tualopensystems.com
Cc:	kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] vfio: platform: support No-IOMMU mode

Hi Peng,
On 05/23/2016 10:14 AM, Peng Fan wrote:
> The vfio No-IOMMU mode was supported by this
> 'commit 03a76b60f8ba2797 ("vfio: Include No-IOMMU mode")',
> but it only support vfio-pci.
> 
> Using vfio_iommu_group_get/put, but not iommu_group_get/put,
> the platform devices can be exposed to userspace with
> CONFIG_VFIO_NOIOMMU and the "enable_unsafe_noiommu_mode"
> option enabled.
> 
> From 'commit 03a76b60f8ba2797 ("vfio: Include No-IOMMU mode")',
> "This should make it very clear that this mode is not safe.
> Additionally, CAP_SYS_RAWIO privileges are necessary to work
> with groups and containers using this mode.  Groups making
> use of this support are named /dev/vfio/noiommu-$GROUP and
> can only make use of the special VFIO_NOIOMMU_IOMMU for the
> container.  Use of this mode, specifically binding a device
> without a native IOMMU group to a VFIO bus driver will taint
> the kernel and should therefore not be considered supported.",
> 
> Actually, for vfio-platform No-IOMMU mode, the userspace can
> not do DMA, because the ioctl API of noiommu container only
> supports VFIO_CHECK_EXTENSION and VFIO_IOMMU_MAP_DMA is not
> supported.
I did not play with no-iommu mode yet but I am surprised by this last
sentence. Without IOMMU the VFIO_IOMMU_MAP_DMA ioctl is not relevant
since you do not need to and cannot map anything but does this really
mean the device cannot perform DMA transfers towards physical memory, if
programmed to do so; I don't think so?

Best Regards

Eric
> 
> Signed-off-by: Peng Fan <van.freenix@...il.com>
> Cc: Eric Auger <eric.auger@...aro.org>
> Cc: Baptiste Reynal <b.reynal@...tualopensystems.com>
> Cc: Alex Williamson <alex.williamson@...hat.com>
> ---
> 
> V2:
>  Rename subject to support No-IOMMU
>  Add more commit log.
> 
>  I wrote a simple program following this
>  https://github.com/virtualopensystems/vfio-host-test/blob/master/src_test/vfio_device_test.c
>  ,no dma support. The device's register can be
>  accessed in userspace using command './vfio_dev_test 30b60000.usdhc 0 1 platform'
> 
>  drivers/vfio/platform/vfio_platform_common.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index e65b142..993b2f9 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -561,7 +561,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>  
>  	vdev->device = dev;
>  
> -	group = iommu_group_get(dev);
> +	group = vfio_iommu_group_get(dev);
>  	if (!group) {
>  		pr_err("VFIO: No IOMMU group for device %s\n", vdev->name);
>  		return -EINVAL;
> @@ -569,7 +569,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>  
>  	ret = vfio_add_group_dev(dev, &vfio_platform_ops, vdev);
>  	if (ret) {
> -		iommu_group_put(group);
> +		vfio_iommu_group_put(group, dev);
>  		return ret;
>  	}
>  
> @@ -589,7 +589,7 @@ struct vfio_platform_device *vfio_platform_remove_common(struct device *dev)
>  
>  	if (vdev) {
>  		vfio_platform_put_reset(vdev);
> -		iommu_group_put(dev->iommu_group);
> +		vfio_iommu_group_put(dev->iommu_group, dev);
>  	}
>  
>  	return vdev;
>