Re: [Patch V8 0/3] iommu: Add support to change default domain of an iommu group
On 11/18/20 9:52 PM, Will Deacon wrote: On Fri, Sep 25, 2020 at 12:06:17PM -0700, Ashok Raj wrote: 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. Modulo my minor comments, I think this looks good for 5.11 if you can please send a version 9. Robin -- please can you give it the once-over too? I think root can break things quite badly with this interface, but root can do that in other ways anyway... Sure. I will send a v9 after Robin's review. Will Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Patch V8 0/3] iommu: Add support to change default domain of an iommu group
On Fri, Sep 25, 2020 at 12:06:17PM -0700, Ashok Raj wrote: > 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. Modulo my minor comments, I think this looks good for 5.11 if you can please send a version 9. Robin -- please can you give it the once-over too? I think root can break things quite badly with this interface, but root can do that in other ways anyway... Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Patch V8 0/3] iommu: Add support to change default domain of an iommu group
Hi Joerg On Thu, Oct 01, 2020 at 02:58:41PM +0200, Joerg Roedel wrote: > Hi Ashok, > > On Fri, Sep 25, 2020 at 12:06:17PM -0700, Ashok Raj wrote: > > 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 | 227 > > - > > 2 files changed, 256 insertions(+), 1 deletion(-) > > Thanks for the repost, I can grab it just fine with b4. But this nees > some more testing on my side and some time in linux-next, so it is too > late now to queue it for v5.10. Can you please remind me after the next > merge window? I'll pick it up then and do the testing and it will > hopefully spend enough time in linux-next. Yes, I'll try to remind you after the next merge window. Cheers, Ashok ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Patch V8 0/3] iommu: Add support to change default domain of an iommu group
Hi Ashok, On Fri, Sep 25, 2020 at 12:06:17PM -0700, Ashok Raj wrote: > 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 | 227 > - > 2 files changed, 256 insertions(+), 1 deletion(-) Thanks for the repost, I can grab it just fine with b4. But this nees some more testing on my side and some time in linux-next, so it is too late now to queue it for v5.10. Can you please remind me after the next merge window? I'll pick it up then and do the testing and it will hopefully spend enough time in linux-next. Thanks, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[Patch V8 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 V6,v7: 1. None except for version bump. https://lore.kernel.org/linux-iommu/20200925073423.gt27...@8bytes.org/ Changes from V5: 1. None except for version bump because Joerg had asked to resend the patches after the merge window closes. Changes from V4: 1. Created device direct mappings before attaching the device to the domain 2. Used list_first_entry() instead of list_for_each_entry() to get the first element of a linked list. 3. Used get_device() and put_device() before and after device_lock() 4. Passed device as an argument to iommu_change_dev_def_domain() to check that the device hasn't changed between calls. 5. Changed error message from "Group assigned to user level for direct access" to "Group not assigned to default domain". 6. Changed error message from "Cannot change default domain of a group with two or more devices" to "Cannot change default domain: Group has more than one device". 7. Removed printing error message "'def_domain_type' call back isn't registered" 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 Sai Praneeth Prakhya (3):
