Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1405156 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH V2] vfio: platform: support No-IOMMU mode Peng Fan <van.freenix@gmail.com> - 2016-05-23 10:20 +0200
    Re: [PATCH V2] vfio: platform: support No-IOMMU mode Eric Auger <eric.auger@linaro.org> - 2016-05-23 11:10 +0200
      Re: [PATCH V2] vfio: platform: support No-IOMMU mode Peng Fan <van.freenix@gmail.com> - 2016-05-23 11:30 +0200

#1405156 — [PATCH V2] vfio: platform: support No-IOMMU mode

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.

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Cc: Eric Auger <eric.auger@linaro.org>
Cc: Baptiste Reynal <b.reynal@virtualopensystems.com>
Cc: Alex Williamson <alex.williamson@redhat.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;
-- 
2.6.2

[toc] | [next] | [standalone]

#1405176

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@gmail.com>
> Cc: Eric Auger <eric.auger@linaro.org>
> Cc: Baptiste Reynal <b.reynal@virtualopensystems.com>
> Cc: Alex Williamson <alex.williamson@redhat.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;
> 

[toc] | [prev] | [next] | [standalone]

#1405191

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@gmail.com>
>> Cc: Eric Auger <eric.auger@linaro.org>
>> Cc: Baptiste Reynal <b.reynal@virtualopensystems.com>
>> Cc: Alex Williamson <alex.williamson@redhat.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;
>> 
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web