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] [day] [month] [year] [list]

Date:	Mon, 23 May 2016 17:23:53 +0800
From:	Peng Fan <van.freenix@...il.com>
To:	Eric Auger <eric.auger@...aro.org>
Cc:	alex.williamson@...hat.com, b.reynal@...tualopensystems.com,
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] vfio: platform: support No-IOMMU mode

Hi Eric,

On Mon, May 23, 2016 at 10:59:24AM +0200, Eric Auger wrote:
>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?

Sorry. My commit log maybe misleading, I mean we can not use mmap + VFIO_IOMMU_MAP_DMA
for noiommu.

The platform device can still do DMA, if we program the DMA address for the device correctly,
such as a reserved memory region.

I can discard the last sentence of the commit log in V3.

Thanks,
Peng.
>
>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;
>> 
>