Re: [PATCH 2/2] iommu/mediatek: add support for MT8167
+Yong.Wu. On Fri, 2020-01-03 at 17:26 +0100, Fabien Parent wrote: > Add support for the IOMMU on MT8167 > > Signed-off-by: Fabien Parent > --- > drivers/iommu/mtk_iommu.c | 11 ++- > drivers/iommu/mtk_iommu.h | 1 + > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 6fc1f5ecf91e..5fc6178a82dc 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -569,7 +569,8 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data > *data) > F_INT_PRETETCH_TRANSATION_FIFO_FAULT; > writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL); > > - if (data->plat_data->m4u_plat == M4U_MT8173) > + if (data->plat_data->m4u_plat == M4U_MT8173 || > + data->plat_data->m4u_plat == M4U_MT8167) > regval = (data->protect_base >> 1) | (data->enable_4GB << 31); > else > regval = lower_32_bits(data->protect_base) | > @@ -782,6 +783,13 @@ static const struct mtk_iommu_plat_data mt2712_data = { > .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, > }; > > +static const struct mtk_iommu_plat_data mt8167_data = { > + .m4u_plat = M4U_MT8167, > + .has_4gb_mode = true, > + .reset_axi= true, > + .larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */ > +}; > + > static const struct mtk_iommu_plat_data mt8173_data = { > .m4u_plat = M4U_MT8173, > .has_4gb_mode = true, > @@ -798,6 +806,7 @@ static const struct mtk_iommu_plat_data mt8183_data = { > > static const struct of_device_id mtk_iommu_of_ids[] = { > { .compatible = "mediatek,mt2712-m4u", .data = &mt2712_data}, > + { .compatible = "mediatek,mt8167-m4u", .data = &mt8167_data}, > { .compatible = "mediatek,mt8173-m4u", .data = &mt8173_data}, > { .compatible = "mediatek,mt8183-m4u", .data = &mt8183_data}, > {} > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > index ea949a324e33..cb8fd5970cd4 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -30,6 +30,7 @@ struct mtk_iommu_suspend_reg { > enum mtk_iommu_plat { > M4U_MT2701, > M4U_MT2712, > + M4U_MT8167, > M4U_MT8173, > M4U_MT8183, > }; ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 2/2] iommu/mediatek: add support for MT8167
___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: arm-smmu.1.auto: Unhandled context fault starting with 5.4-rc1
On Fri Feb 14 20, Robin Murphy wrote: Hi Jerry, On 2020-02-14 8:13 pm, Jerry Snitselaar wrote: Hi Will, On a gigabyte system with Cavium CN8xx, when doing a fio test against an nvme drive we are seeing the following: [ 637.161194] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x8010003f6000, fsynr=0x70091, cbfrsynra=0x9000, cb=7 [ 637.174329] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x80136000, fsynr=0x70091, cbfrsynra=0x9000, cb=7 [ 637.186887] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x8010002ee000, fsynr=0x70091, cbfrsynra=0x9000, cb=7 [ 637.199275] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x8010003c7000, fsynr=0x70091, cbfrsynra=0x9000, cb=7 [ 637.211885] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x801000392000, fsynr=0x70091, cbfrsynra=0x9000, cb=7 [ 637.224580] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x80118000, fsynr=0x70091, cbfrsynra=0x9000, cb=7 [ 637.237241] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x80100036, fsynr=0x70091, cbfrsynra=0x9000, cb=7 [ 637.249657] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x801ba000, fsynr=0x70091, cbfrsynra=0x9000, cb=7 [ 637.262120] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x8013e000, fsynr=0x70091, cbfrsynra=0x9000, cb=7 [ 637.274468] arm-smmu arm-smmu.1.auto: Unhandled context fault: fsr=0x8402, iova=0x801000304000, fsynr=0x70091, cbfrsynra=0x9000, cb=7 Those "IOVAs" don't look much like IOVAs from the DMA allocator - if they were physical addresses, would they correspond to an expected region of the physical memory map? I would suspect that this is most likely misbehaviour in the NVMe driver (issuing a write to a non-DMA-mapped address), and the SMMU is just doing its job in blocking and reporting it. I also reproduced with 5.5-rc7, and will check 5.6-rc1 later today. I couldn't narrow it down further into 5.4-rc1. I don't know smmu or the code well, any thoughts on where to start digging into this? fio test that is being run is: #fio -filename=/dev/nvme0n1 -iodepth=64 -thread -rw=randwrite -ioengine=libaio -bs=4k -runtime=43200 -size=-group_reporting -name=mytest -numjobs=32 Just to clarify, do other tests work OK on the same device? Thanks, Robin. I was able to get back on the system today. I think I know what the problem is: [0.036189] iommu: Gigabyte R120-T34-00 detected, force iommu passthrough mode [6.324282] iommu: Default domain type: Translated So the new default domain code in 5.4 overrides the iommu quirk code setting default passthrough. Testing a quick patch that tracks whether the default domain was set in the quirk code, and leaves it alone if it was. So far it seems to be working. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH V2 3/5] iommu: Add support to change default domain of an iommu_group
Presently, the default domain of an iommu_group is allocated during boot time (i.e. when a device is being added to a group) and it cannot be changed later. So, the device would typically be either in identity (also known as pass_through) mode (controlled by "iommu=pt" kernel command line argument) or the device would be in DMA mode as long as the machine 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. But, assume a use case wherein the user trusts the device and also believes that the OS is secure enough and hence wants *only* this device to bypass IOMMU (so that it could be high performing) whereas all the other devices to go through IOMMU (so that the system is protected). Presently, this use case is not supported. Hence it will be helpful if there is some way to change the default domain of a B:D.F dynamically. Since, linux iommu subsystem prefers to deal at iommu_group level instead of B:D.F level, it might be helpful if there is some way to change the default domain of a *iommu_group* dynamically. Hence, add such support. A privileged user could request the kernel to change the default domain type of a iommu_group by writing to "/sys/kernel/iommu_groups//type" file. Presently, only three values are supported 1. identity: all the DMA transactions from the devices in this group are *not* translated by the iommu 2. DMA: all the DMA transactions from the devices in this group are translated by the iommu 3. auto: kernel decides one among DMA/identity Also please note that a group type could be modified only when *all* the devices in the group are not binded to any device driver. Please see "Documentation/ABI/testing/sysfs-kernel-iommu_groups" for more information. 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 --- drivers/iommu/iommu.c | 227 +- 1 file changed, 226 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 3e3528436e0b..56a29076871f 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -225,6 +225,8 @@ static int __iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group); static void __iommu_detach_group(struct iommu_domain *domain, struct iommu_group *group); +static ssize_t iommu_group_store_type(struct iommu_group *group, + const char *buf, size_t count); static int __init iommu_set_def_domain_type(char *str) { @@ -448,7 +450,8 @@ static IOMMU_GROUP_ATTR(name, S_IRUGO, iommu_group_show_name, NULL); static IOMMU_GROUP_ATTR(reserved_regions, 0444, iommu_group_show_resv_regions, NULL); -static IOMMU_GROUP_ATTR(type, 0444, iommu_group_show_type, NULL); +static IOMMU_GROUP_ATTR(type, 0644, iommu_group_show_type, + iommu_group_store_type); static void iommu_group_release(struct kobject *kobj) { @@ -2654,3 +2657,225 @@ int iommu_sva_get_pasid(struct iommu_sva *handle) return ops->sva_get_pasid(handle); } EXPORT_SYMBOL_GPL(iommu_sva_get_pasid); + +/* + * Changes the default domain of a group + * + * @group: The group for which the default domain should be changed + * @prv_dom: The previous domain that is being switched from + * @type: The type of the new default domain that gets associated with the group + * + * Returns 0 on success and error code on failure + */ +static int iommu_group_change_def_domain(struct iommu_group *group, +struct iommu_domain *prv_dom, +int type) +{ + struct group_device *grp_dev, *temp; + struct iommu_domain *new_domain; + int ret; + + /* +* iommu_domain_alloc() takes "struct bus_type" as an argument which is +* a member in "struct device". Changing a group's default domain type +* deals at iommu_group level rather than device level and hence there +* is no straight forward way to get "bus_type" of an iommu_group that +* could be passed to iommu_domain_alloc(). So, instead of directly +* calling iommu_domain_alloc(), use iommu_ops from previous default +* domain. +*/ + if (!prv_dom || !prv_dom->ops || !prv_dom->ops->domain_alloc || !type) + return -EINVAL; + + /* Allocate a new domain of requested type */ + new_domain = prv_dom->ops->domain_alloc(type); + if (!new_domain) { + pr_err("Unable to allocate memory for the new domain\n"); + return -ENOMEM; + } + + new_domain->type = type; + new_domain->ops = prv_dom->ops; + new_domain->pgsize_bitmap = prv_dom->pgsize_bitmap; + +
[PATCH V2 4/5] iommu: Take lock before reading iommu_group default domain type
Since user could change default domain type of an iommu group through sysfs, reading the default domain type now requires taking the lock first. 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 --- drivers/iommu/iommu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 56a29076871f..10e14cb9c32b 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -424,6 +424,7 @@ static ssize_t iommu_group_show_type(struct iommu_group *group, { char *type = "unknown\n"; + mutex_lock(&group->mutex); if (group->default_domain) { switch (group->default_domain->type) { case IOMMU_DOMAIN_BLOCKED: @@ -440,6 +441,7 @@ static ssize_t iommu_group_show_type(struct iommu_group *group, break; } } + mutex_unlock(&group->mutex); strcpy(buf, type); return strlen(type); -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH V2 1/5] iommu: Add dev_def_domain_type() call back function to iommu_ops
When user requests kernel to change the default domain type of a group through sysfs, kernel has to make sure that it's ok to change the domain type of every device in the group to the requested domain (every device may not support both the domain types i.e. DMA and identity). Hence, add a call back function that could be implemented per architecture that performs the above check. 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 --- include/linux/iommu.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/iommu.h b/include/linux/iommu.h index d1b5f4d98569..3f4aaad0aeb7 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -248,6 +248,7 @@ struct iommu_iotlb_gather { * @cache_invalidate: invalidate translation caches * @sva_bind_gpasid: bind guest pasid and mm * @sva_unbind_gpasid: unbind guest pasid and mm + * @dev_def_domain_type: Return the required default domain type for a device * @pgsize_bitmap: bitmap of all possible supported page sizes * @owner: Driver module providing these ops */ @@ -318,6 +319,8 @@ struct iommu_ops { int (*sva_unbind_gpasid)(struct device *dev, int pasid); + int (*dev_def_domain_type)(struct device *dev); + unsigned long pgsize_bitmap; struct module *owner; }; -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH V2 0/5] iommu: Add support to change default domain of a group
Presently, the default domain of a 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 a group dynamically. Support this through a sysfs file, namely "/sys/kernel/iommu_groups//type". Hi Joerg, Sorry! for _huge_ delay in posting a V2 of this patch set. Was stuck with some internal PoC work. Will be consistent from now. 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. 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. Based on iommu maintainer's 'next' branch. Sai Praneeth Prakhya (5): iommu: Add dev_def_domain_type() call back function to iommu_ops iommu/vt-d: Rename device_def_domain_type() to intel_iommu_dev_def_domain_type() 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 | 29 +++ drivers/iommu/intel-iommu.c| 8 +- drivers/iommu/iommu.c | 229 - include/linux/iommu.h | 3 + 4 files changed, 265 insertions(+), 4 deletions(-) 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.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH V2 5/5] iommu: Document usage of "/sys/kernel/iommu_groups//type" file
The default domain type of an iommu group can be changed using file "/sys/kernel/iommu_groups//type". Hence, document it's usage and more importantly spell out it's limitations. 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 --- .../ABI/testing/sysfs-kernel-iommu_groups | 29 ++ 1 file changed, 29 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups index 017f5bc3920c..808a9507b281 100644 --- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups +++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups @@ -33,3 +33,32 @@ Description:In case an RMRR is used only by graphics or USB devices it is now exposed as "direct-relaxable" instead of "direct". In device assignment use case, for instance, those RMRR are considered to be relaxable and safe. + +What: /sys/kernel/iommu_groups//type +Date: February 2020 +KernelVersion: v5.6 +Contact: Sai Praneeth Prakhya +Description: Lets the user know the type of default domain in use by iommu + for this group. A privileged user could request kernel to change + the group type by writing to this file. Presently, only three + types are supported + 1. DMA: All the DMA transactions from the devices in this group + are translated by the iommu. + 2. identity: All the DMA transactions from the devices in this +group are *not* translated by the iommu. + 3. auto: Kernel decides one among DMA/identity mode and hence +when the user reads the file he would never see "auto". +This is just a write only value. + Note: + - + A group type could be modified only when *all* the devices in + the group are not binded to any device driver. So, the user has + to first unbind the appropriate drivers and then change the + default domain type. + Caution: + + Unbinding a device driver will take away the drivers control + over the device and if done on devices that host root file + system could lead to catastrophic effects (the user might + need to reboot the machine to get it to normal state). So, it's + expected that the user understands what he is doing. -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH V2 2/5] iommu/vt-d: Rename device_def_domain_type() to intel_iommu_dev_def_domain_type()
The functionality needed for iommu_ops->dev_def_domain_type() is already provided by device_def_domain_type() in intel_iommu.c. But, every call back function in intel_iommu_ops starts with intel_iommu prefix, hence rename device_def_domain_type() to intel_iommu_dev_def_domain_type() so that it follows the same semantics. 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 --- drivers/iommu/intel-iommu.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 64ddccf1d5fe..68f10d271ac0 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -3034,7 +3034,7 @@ static bool device_is_rmrr_locked(struct device *dev) * - IOMMU_DOMAIN_IDENTITY: device requires an identical mapping domain * - 0: both identity and dynamic domains work for this device */ -static int device_def_domain_type(struct device *dev) +static int intel_iommu_dev_def_domain_type(struct device *dev) { if (dev_is_pci(dev)) { struct pci_dev *pdev = to_pci_dev(dev); @@ -5796,7 +5796,8 @@ static int intel_iommu_add_device(struct device *dev) domain = iommu_get_domain_for_dev(dev); dmar_domain = to_dmar_domain(domain); if (domain->type == IOMMU_DOMAIN_DMA) { - if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) { + if (intel_iommu_dev_def_domain_type(dev) == + IOMMU_DOMAIN_IDENTITY) { ret = iommu_request_dm_for_dev(dev); if (ret) { dmar_remove_one_dev_info(dev); @@ -5807,7 +5808,7 @@ static int intel_iommu_add_device(struct device *dev) } } } else { - if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) { + if (intel_iommu_dev_def_domain_type(dev) == IOMMU_DOMAIN_DMA) { ret = iommu_request_dma_domain_for_dev(dev); if (ret) { dmar_remove_one_dev_info(dev); @@ -6194,6 +6195,7 @@ const struct iommu_ops intel_iommu_ops = { .dev_enable_feat= intel_iommu_dev_enable_feat, .dev_disable_feat = intel_iommu_dev_disable_feat, .is_attach_deferred = intel_iommu_is_attach_deferred, + .dev_def_domain_type= intel_iommu_dev_def_domain_type, .pgsize_bitmap = INTEL_IOMMU_PGSIZES, }; -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/3] iommu/virtio: Enable x86 support
On Fri, Feb 14, 2020 at 04:57:11PM +, Robin Murphy wrote: > On 14/02/2020 4:04 pm, Jean-Philippe Brucker wrote: > > With the built-in topology description in place, x86 platforms can now > > use the virtio-iommu. > > > > Signed-off-by: Jean-Philippe Brucker > > --- > > drivers/iommu/Kconfig | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > > index 068d4e0e3541..adcbda44d473 100644 > > --- a/drivers/iommu/Kconfig > > +++ b/drivers/iommu/Kconfig > > @@ -508,8 +508,9 @@ config HYPERV_IOMMU > > config VIRTIO_IOMMU > > bool "Virtio IOMMU driver" > > depends on VIRTIO=y > > - depends on ARM64 > > + depends on (ARM64 || X86) > > select IOMMU_API > > + select IOMMU_DMA > > Can that have an "if X86" for clarity? AIUI it's not necessary for > virtio-iommu itself (and really shouldn't be), but is merely to satisfy the > x86 arch code's expectation that IOMMU drivers bring their own DMA ops, > right? > > Robin. In fact does not this work on any platform now? > > select INTERVAL_TREE > > help > > Para-virtualised IOMMU driver with virtio. > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu