RE: [PATCH V4 0/3] iommu: Add support to change default domain of an iommu group

2020-06-26 Thread Prakhya, Sai Praneeth
Hi Joerg,

> -Original Message-
> From: Prakhya, Sai Praneeth 
> Sent: Thursday, June 4, 2020 6:32 PM
> To: iommu@lists.linux-foundation.org
> Cc: Prakhya, Sai Praneeth ; Christoph Hellwig
> ; Joerg Roedel ; Raj, Ashok
> ; Will Deacon ; Lu Baolu
> ; Mehta, Sohil ; Robin
> Murphy ; Jacob Pan 
> Subject: [PATCH V4 0/3] iommu: Add support to change default domain of an
> iommu group
> 
> Presently, the default domain of an iommu group is allocated during boot time
> and it cannot be changed later. So, the device would typically be either in
> identity (pass_through) mode or the device would be in DMA mode as long as
> the system is up and running. There is no way to change the default domain 
> type
> dynamically i.e. after booting, a device cannot switch between identity mode
> and DMA mode.
> 
> Assume a use case wherein the privileged user would want to use the device in
> pass-through mode when the device is used for host so that it would be high
> performing. Presently, this is not supported. Hence add support to change the
> default domain of an iommu group dynamically.
> 
> Support this by writing to a sysfs file, namely
> "/sys/kernel/iommu_groups//type".
> 
> Testing:
> 
> Tested by dynamically changing storage device (nvme) from 1. identity mode to
> DMA and making sure file transfer works 2. DMA mode to identity mode and
> making sure file transfer works Tested only for intel_iommu/vt-d. Would
> appreciate if someone could test on AMD and ARM based machines.
> 
> Based on iommu maintainer's 'next' branch.
> 
> Changes from V3:
> 
> 1. Made changes to commit message as suggested by Baolu.
> 2. Don't pass "prev_dom" and "dev" as parameters to
>iommu_change_dev_def_domain(). Instead get them from group.
> 3. Sanitize the logic to validate user default domain type request. The logic
>remains same but is implmented differently.
> 4. Push lot of error checking into iommu_change_dev_def_domain() from
>iommu_group_store_type().
> 5. iommu_change_dev_def_domain() takes/releases group mutex as needed.
> So, it
>shouldn't be called holding a group mutex.
> 6. Use pr_err_ratelimited() instead of pr_err() to avoid DOS attack.

Could you please review this patch set and let me know if you have any comments?

Regards,
Sai
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH V4 0/3] iommu: Add support to change default domain of an iommu group

2020-06-04 Thread Sai Praneeth Prakhya
Presently, the default domain of an iommu group is allocated during boot time
and it cannot be changed later. So, the device would typically be either in
identity (pass_through) mode or the device would be in DMA mode as long as the
system is up and running. There is no way to change the default domain type
dynamically i.e. after booting, a device cannot switch between identity mode and
DMA mode.

Assume a use case wherein the privileged user would want to use the device in
pass-through mode when the device is used for host so that it would be high
performing. Presently, this is not supported. Hence add support to change the
default domain of an iommu group dynamically.

Support this by writing to a sysfs file, namely
"/sys/kernel/iommu_groups//type".

Testing:

Tested by dynamically changing storage device (nvme) from
1. identity mode to DMA and making sure file transfer works
2. DMA mode to identity mode and making sure file transfer works
Tested only for intel_iommu/vt-d. Would appreciate if someone could test on AMD
and ARM based machines.

Based on iommu maintainer's 'next' branch.

Changes from V3:

1. Made changes to commit message as suggested by Baolu.
2. Don't pass "prev_dom" and "dev" as parameters to
   iommu_change_dev_def_domain(). Instead get them from group.
3. Sanitize the logic to validate user default domain type request. The logic
   remains same but is implmented differently.
4. Push lot of error checking into iommu_change_dev_def_domain() from
   iommu_group_store_type().
5. iommu_change_dev_def_domain() takes/releases group mutex as needed. So, it
   shouldn't be called holding a group mutex.
6. Use pr_err_ratelimited() instead of pr_err() to avoid DOS attack.

Changes from V2:

1. Change the logic of updating default domain from V2 because
   ops->probe_finalize() could be used to update dma_ops.
2. Drop 1st and 2nd patch of V2 series because they are no longer needed on
   iommu maintainer's 'next' branch.
3. Limit this feature to iommu groups with only one device.
4. Hold device_lock and group mutex until the default domain is changed.

Changes from V1:

1. V1 patch set wasn't updating dma_ops for some vendors (Eg: AMD), hence,
   change the logic of updating default domain as below (because adding a device
   to iommu_group automatically updates dma_ops)
   a. Allocate a new domain
   b. For every device in the group
i. Remove the device from the group
ii. Add the device back to the group
   c. Free previous domain
2. Drop 1st patch of V1 (iommu/vt-d: Modify device_def_domain_type() to use at
   runtime) because "iommu=pt" has no effect on this function anymore.
3. Added a patch to take/release lock while reading 
iommu_group->default_domain->type
   because it can be changed any time by user.
4. Before changing default domain type of a group, check if the group is
   directly assigned for user level access. If so, abort.
5. Sanitize return path (using ternary operator) in iommu_group_store_type()
6. Split 2nd patch of V1 (iommu: Add device_def_domain_type() call back function
   to iommu_ops) into two patches such that iommu generic changes are now in 1st
   patch of V2 and vt-d specific changes are in 2nd patch of V2.
7. Rename device_def_domain_type() to dev_def_domain_type()
8. Remove example from documentation
9. Change the value written to file "/sys/kernel/iommu_groups//type"
   from "dma" to "DMA".

Changes from RFC:
-
1. Added support for "auto" type, so that kernel selects one among identity or
   dma mode.
2. Use "system_state" in device_def_domain_type() instead of an argument.

Sai Praneeth Prakhya (3):
  iommu: Add support to change default domain of an iommu_group
  iommu: Take lock before reading iommu_group default domain type
  iommu: Document usage of "/sys/kernel/iommu_groups//type" file

 .../ABI/testing/sysfs-kernel-iommu_groups |  30 +++
 drivers/iommu/iommu.c | 217 +-
 2 files changed, 246 insertions(+), 1 deletion(-)

Cc: Christoph Hellwig 
Cc: Joerg Roedel 
Cc: Ashok Raj 
Cc: Will Deacon 
Cc: Lu Baolu 
Cc: Sohil Mehta 
Cc: Robin Murphy 
Cc: Jacob Pan 
Signed-off-by: Sai Praneeth Prakhya 

-- 
2.19.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu