[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. 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
[PATCH V4 3/3] iommu: Document usage of "/sys/kernel/iommu_groups//type" file
The default domain type of an iommu group can be changed by writing to "/sys/kernel/iommu_groups//type" file. Hence, document it's usage and more importantly spell out its 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 | 30 +++ 1 file changed, 30 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups index 017f5bc3920c..a498daffeb0c 100644 --- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups +++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups @@ -33,3 +33,33 @@ 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: June 2020 +KernelVersion: v5.8 +Contact: Sai Praneeth Prakhya +Description: Let 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 device in this group + are translated by the iommu. + 2. identity: All the DMA transactions from the device in this +group are *not* translated by the iommu. + 3. auto: Change to the type the device was booted with. 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 + 1. The group has *only* one device + 2. The device in the group is not bound to any device driver. + So, the user must first unbind the appropriate driver and + then change the default domain type. + Caution: + + Unbinding a device driver will take away the driver's 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.19.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH V4 1/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 (also known as pass_through) mode 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 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. It will be helpful if there is some way to change the
default domain of an 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 device in this group are
*not* translated by the iommu
2. DMA: all the DMA transactions from the device in this group are
translated by the iommu
3. auto: change to the type the device was booted with
Note:
1. Default domain of an iommu group with two or more devices cannot be
changed.
2. The device in the iommu group shouldn't be bound to any driver.
3. The device shouldn't be assigned to user for direct access.
4. The vendor iommu driver is required to add def_domain_type() callback.
The change request will fail if the request type conflicts with that
returned from the callback.
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 | 215 +-
1 file changed, 214 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d43120eb1dc5..b023f06f12d6 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -93,6 +93,8 @@ static void __iommu_detach_group(struct iommu_domain *domain,
static int iommu_create_device_direct_mappings(struct iommu_group *group,
struct device *dev);
static struct iommu_group *iommu_group_get_for_dev(struct device *dev);
+static ssize_t iommu_group_store_type(struct iommu_group *group,
+ const char *buf, size_t count);
#define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) \
struct iommu_group_attribute iommu_group_attr_##_name =\
@@ -525,7 +527,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)
{
@@ -2838,3 +2841,213 @@ 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 an iommu group
+ *
+ * @group: The group for which the default domain should be changed
+ * @type: The type of the new default domain that gets associated with the
group
+ *
+ * Returns 0 on success and error code on failure
+ *
+ * Note:
+ * 1. Presently, this function is called only when user requests to change the
+ *group's default domain type through
/sys/kernel/iommu_groups//type
+ *Please take a closer look if intended to use for other purposes.
+ */
+static int iommu_change_dev_def_domain(struct iommu_group *group, int type)
+{
+ struct iommu_domain *prev_dom;
+ struct group_device *grp_dev;
+ const struct iommu_ops *ops;
+ int ret, dev_def_dom;
+ struct device *dev;
+
+ if (!group)
+ return -EINVAL;
+
+ mutex_lock(>mutex);
+
+ if (group->default_domain != group->domain) {
+ pr_err_ratelimited("Group assigned to user level for direct
access\n");
+ ret = -EBUSY;
+ goto out;
+ }
+
+ /*
+* iommu group wasn't locked while acquiring device lock in
+* iommu_group_store_type(). So, make sure that the device count hasn't
+* changed while acquiring device lock.
+*
+* Changing default domain of an iommu group with two or more devices
+* isn't supported because there could be a potential deadlock. Consider
+* the following scenario. T1 is trying to acquire device locks of all
+* the devices in the group and before
[PATCH V4 2/3] iommu: Take lock before reading iommu group default domain type
"/sys/kernel/iommu_groups//type" file could be read to find out the
default domain type of an iommu group. The default domain of an iommu group
doesn't change after booting and hence could be read directly. But,
after addding support to dynamically change iommu group default domain, the
above assumption no longer stays valid.
iommu group default domain type could be changed at any time by writing to
"/sys/kernel/iommu_groups//type". So, take group mutex before
reading iommu group default domain type so that the user wouldn't see stale
values or iommu_group_show_type() doesn't try to derefernce stale pointers.
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 b023f06f12d6..eb133ee5c13a 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -501,6 +501,7 @@ static ssize_t iommu_group_show_type(struct iommu_group
*group,
{
char *type = "unknown\n";
+ mutex_lock(>mutex);
if (group->default_domain) {
switch (group->default_domain->type) {
case IOMMU_DOMAIN_BLOCKED:
@@ -517,6 +518,7 @@ static ssize_t iommu_group_show_type(struct iommu_group
*group,
break;
}
}
+ mutex_unlock(>mutex);
strcpy(buf, type);
return strlen(type);
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 3/4] iommu/arm-smmu: Add global/context fault implementation hooks
Add global/context fault hooks to allow NVIDIA SMMU implementation
handle faults across multiple SMMUs.
Signed-off-by: Krishna Reddy
---
drivers/iommu/arm-smmu-nvidia.c | 100
drivers/iommu/arm-smmu.c| 11 +++-
drivers/iommu/arm-smmu.h| 3 +
3 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu-nvidia.c b/drivers/iommu/arm-smmu-nvidia.c
index dafc293a45217..5999b6a770992 100644
--- a/drivers/iommu/arm-smmu-nvidia.c
+++ b/drivers/iommu/arm-smmu-nvidia.c
@@ -117,6 +117,104 @@ static int nsmmu_reset(struct arm_smmu_device *smmu)
return 0;
}
+static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
+{
+ return container_of(dom, struct arm_smmu_domain, domain);
+}
+
+static irqreturn_t nsmmu_global_fault_inst(int irq,
+ struct arm_smmu_device *smmu,
+ int inst)
+{
+ u32 gfsr, gfsynr0, gfsynr1, gfsynr2;
+
+ gfsr = readl_relaxed(nsmmu_page(smmu, inst, 0) + ARM_SMMU_GR0_sGFSR);
+ gfsynr0 = readl_relaxed(nsmmu_page(smmu, inst, 0) +
+ ARM_SMMU_GR0_sGFSYNR0);
+ gfsynr1 = readl_relaxed(nsmmu_page(smmu, inst, 0) +
+ ARM_SMMU_GR0_sGFSYNR1);
+ gfsynr2 = readl_relaxed(nsmmu_page(smmu, inst, 0) +
+ ARM_SMMU_GR0_sGFSYNR2);
+
+ if (!gfsr)
+ return IRQ_NONE;
+
+ dev_err_ratelimited(smmu->dev,
+ "Unexpected global fault, this could be serious\n");
+ dev_err_ratelimited(smmu->dev,
+ "\tGFSR 0x%08x, GFSYNR0 0x%08x, GFSYNR1 0x%08x, GFSYNR2
0x%08x\n",
+ gfsr, gfsynr0, gfsynr1, gfsynr2);
+
+ writel_relaxed(gfsr, nsmmu_page(smmu, inst, 0) + ARM_SMMU_GR0_sGFSR);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t nsmmu_global_fault(int irq, void *dev)
+{
+ int inst;
+ irqreturn_t irq_ret = IRQ_NONE;
+ struct arm_smmu_device *smmu = dev;
+
+ for (inst = 0; inst < to_nvidia_smmu(smmu)->num_inst; inst++) {
+ irq_ret = nsmmu_global_fault_inst(irq, smmu, inst);
+ if (irq_ret == IRQ_HANDLED)
+ return irq_ret;
+ }
+
+ return irq_ret;
+}
+
+static irqreturn_t nsmmu_context_fault_bank(int irq,
+ struct arm_smmu_device *smmu,
+ int idx, int inst)
+{
+ u32 fsr, fsynr, cbfrsynra;
+ unsigned long iova;
+
+ fsr = arm_smmu_cb_read(smmu, idx, ARM_SMMU_CB_FSR);
+ if (!(fsr & ARM_SMMU_FSR_FAULT))
+ return IRQ_NONE;
+
+ fsynr = readl_relaxed(nsmmu_page(smmu, inst, smmu->numpage + idx) +
+ ARM_SMMU_CB_FSYNR0);
+ iova = readq_relaxed(nsmmu_page(smmu, inst, smmu->numpage + idx) +
+ARM_SMMU_CB_FAR);
+ cbfrsynra = readl_relaxed(nsmmu_page(smmu, inst, 1) +
+ ARM_SMMU_GR1_CBFRSYNRA(idx));
+
+ dev_err_ratelimited(smmu->dev,
+ "Unhandled context fault: fsr=0x%x, iova=0x%08lx, fsynr=0x%x,
cbfrsynra=0x%x, cb=%d\n",
+ fsr, iova, fsynr, cbfrsynra, idx);
+
+ writel_relaxed(fsr, nsmmu_page(smmu, inst, smmu->numpage + idx) +
+ ARM_SMMU_CB_FSR);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t nsmmu_context_fault(int irq, void *dev)
+{
+ int inst, idx;
+ irqreturn_t irq_ret = IRQ_NONE;
+ struct iommu_domain *domain = dev;
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+
+ for (inst = 0; inst < to_nvidia_smmu(smmu)->num_inst; inst++) {
+ /* Interrupt line shared between all context faults.
+* Check for faults across all contexts.
+*/
+ for (idx = 0; idx < smmu->num_context_banks; idx++) {
+ irq_ret = nsmmu_context_fault_bank(irq, smmu,
+ idx, inst);
+
+ if (irq_ret == IRQ_HANDLED)
+ return irq_ret;
+ }
+ }
+
+ return irq_ret;
+}
+
static const struct arm_smmu_impl nvidia_smmu_impl = {
.read_reg = nsmmu_read_reg,
.write_reg = nsmmu_write_reg,
@@ -124,6 +222,8 @@ static const struct arm_smmu_impl nvidia_smmu_impl = {
.write_reg64 = nsmmu_write_reg64,
.reset = nsmmu_reset,
.tlb_sync = nsmmu_tlb_sync,
+ .global_fault = nsmmu_global_fault,
+ .context_fault = nsmmu_context_fault,
};
struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 243bc4cb2705b..d720e1e191176 100644
--- a/drivers/iommu/arm-smmu.c
+++
[PATCH v6 4/4] iommu/arm-smmu-nvidia: fix the warning reported by kbuild test robot
>> drivers/iommu/arm-smmu-nvidia.c:151:33: sparse: sparse: cast removes >> address space '' of expression Reported-by: kbuild test robot Signed-off-by: Krishna Reddy --- drivers/iommu/arm-smmu-nvidia.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-nvidia.c b/drivers/iommu/arm-smmu-nvidia.c index 5999b6a770992..6348d8dc17fc2 100644 --- a/drivers/iommu/arm-smmu-nvidia.c +++ b/drivers/iommu/arm-smmu-nvidia.c @@ -248,7 +248,7 @@ struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu) break; nsmmu->bases[i] = devm_ioremap_resource(dev, res); if (IS_ERR(nsmmu->bases[i])) - return (struct arm_smmu_device *)nsmmu->bases[i]; + return ERR_CAST(nsmmu->bases[i]); nsmmu->num_inst++; } -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 0/4] Nvidia Arm SMMUv2 Implementation
Changes in v6: Restricted the patch set to driver specific patches. Fixed the cast warning reported by kbuild test robot. Rebased on git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next v5 - https://lkml.org/lkml/2020/5/21/1114 v4 - https://lkml.org/lkml/2019/10/30/1054 v3 - https://lkml.org/lkml/2019/10/18/1601 v2 - https://lkml.org/lkml/2019/9/2/980 v1 - https://lkml.org/lkml/2019/8/29/1588 Krishna Reddy (4): iommu/arm-smmu: add NVIDIA implementation for dual ARM MMU-500 usage dt-bindings: arm-smmu: Add binding for Tegra194 SMMU iommu/arm-smmu: Add global/context fault implementation hooks iommu/arm-smmu-nvidia: fix the warning reported by kbuild test robot .../devicetree/bindings/iommu/arm,smmu.yaml | 5 + MAINTAINERS | 2 + drivers/iommu/Makefile| 2 +- drivers/iommu/arm-smmu-impl.c | 3 + drivers/iommu/arm-smmu-nvidia.c | 261 ++ drivers/iommu/arm-smmu.c | 11 +- drivers/iommu/arm-smmu.h | 4 + 7 files changed, 285 insertions(+), 3 deletions(-) create mode 100644 drivers/iommu/arm-smmu-nvidia.c base-commit: 431275afdc7155415254aef4bd3816a1b8a2ead0 -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 1/4] iommu/arm-smmu: add NVIDIA implementation for dual ARM MMU-500 usage
NVIDIA's Tegra194 soc uses two ARM MMU-500s together to interleave
IOVA accesses across them.
Add NVIDIA implementation for dual ARM MMU-500s and add new compatible
string for Tegra194 soc.
Signed-off-by: Krishna Reddy
---
MAINTAINERS | 2 +
drivers/iommu/Makefile | 2 +-
drivers/iommu/arm-smmu-impl.c | 3 +
drivers/iommu/arm-smmu-nvidia.c | 161
drivers/iommu/arm-smmu.h| 1 +
5 files changed, 168 insertions(+), 1 deletion(-)
create mode 100644 drivers/iommu/arm-smmu-nvidia.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 50659d76976b7..118da0893c964 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16572,9 +16572,11 @@ F: drivers/i2c/busses/i2c-tegra.c
TEGRA IOMMU DRIVERS
M: Thierry Reding
+R: Krishna Reddy
L: linux-te...@vger.kernel.org
S: Supported
F: drivers/iommu/tegra*
+F: drivers/iommu/arm-smmu-nvidia.c
TEGRA KBC DRIVER
M: Laxman Dewangan
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 57cf4ba5e27cb..35542df00da72 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -15,7 +15,7 @@ obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
amd_iommu_quirks.o
obj-$(CONFIG_AMD_IOMMU_DEBUGFS) += amd_iommu_debugfs.o
obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
obj-$(CONFIG_ARM_SMMU) += arm_smmu.o
-arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-qcom.o
+arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-qcom.o arm-smmu-nvidia.o
obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
obj-$(CONFIG_DMAR_TABLE) += dmar.o
obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index c75b9d957b702..52c84c30f83e4 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -160,6 +160,9 @@ struct arm_smmu_device *arm_smmu_impl_init(struct
arm_smmu_device *smmu)
*/
switch (smmu->model) {
case ARM_MMU500:
+ if (of_device_is_compatible(smmu->dev->of_node,
+ "nvidia,tegra194-smmu-500"))
+ return nvidia_smmu_impl_init(smmu);
smmu->impl = _mmu500_impl;
break;
case CAVIUM_SMMUV2:
diff --git a/drivers/iommu/arm-smmu-nvidia.c b/drivers/iommu/arm-smmu-nvidia.c
new file mode 100644
index 0..dafc293a45217
--- /dev/null
+++ b/drivers/iommu/arm-smmu-nvidia.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Nvidia ARM SMMU v2 implementation quirks
+// Copyright (C) 2019 NVIDIA CORPORATION. All rights reserved.
+
+#define pr_fmt(fmt) "nvidia-smmu: " fmt
+
+#include
+#include
+#include
+#include
+#include
+
+#include "arm-smmu.h"
+
+/* Tegra194 has three ARM MMU-500 Instances.
+ * Two of them are used together for Interleaved IOVA accesses and
+ * used by Non-Isochronous Hw devices for SMMU translations.
+ * Third one is used for SMMU translations from Isochronous HW devices.
+ * It is possible to use this Implementation to program either
+ * all three or two of the instances identically as desired through
+ * DT node.
+ *
+ * Programming all the three instances identically comes with redundant tlb
+ * invalidations as all three never need to be tlb invalidated for a HW device.
+ *
+ * When Linux Kernel supports multiple SMMU devices, The SMMU device used for
+ * Isochornous HW devices should be added as a separate ARM MMU-500 device
+ * in DT and be programmed independently for efficient tlb invalidates.
+ *
+ */
+#define MAX_SMMU_INSTANCES 3
+
+#define TLB_LOOP_TIMEOUT 100 /* 1s! */
+#define TLB_SPIN_COUNT 10
+
+struct nvidia_smmu {
+ struct arm_smmu_device smmu;
+ unsigned intnum_inst;
+ void __iomem*bases[MAX_SMMU_INSTANCES];
+};
+
+#define to_nvidia_smmu(s) container_of(s, struct nvidia_smmu, smmu)
+
+#define nsmmu_page(smmu, inst, page) \
+ (((inst) ? to_nvidia_smmu(smmu)->bases[(inst)] : smmu->base) + \
+ ((page) << smmu->pgshift))
+
+static u32 nsmmu_read_reg(struct arm_smmu_device *smmu,
+ int page, int offset)
+{
+ return readl_relaxed(nsmmu_page(smmu, 0, page) + offset);
+}
+
+static void nsmmu_write_reg(struct arm_smmu_device *smmu,
+ int page, int offset, u32 val)
+{
+ unsigned int i;
+
+ for (i = 0; i < to_nvidia_smmu(smmu)->num_inst; i++)
+ writel_relaxed(val, nsmmu_page(smmu, i, page) + offset);
+}
+
+static u64 nsmmu_read_reg64(struct arm_smmu_device *smmu,
+ int page, int offset)
+{
+ return readq_relaxed(nsmmu_page(smmu, 0, page) + offset);
+}
+
+static void nsmmu_write_reg64(struct arm_smmu_device *smmu,
+ int page, int offset, u64 val)
+{
+ unsigned int i;
+
+ for (i = 0; i < to_nvidia_smmu(smmu)->num_inst; i++)
+
[PATCH v6 2/4] dt-bindings: arm-smmu: Add binding for Tegra194 SMMU
Add binding for NVIDIA's Tegra194 Soc SMMU that is based on ARM MMU-500. Signed-off-by: Krishna Reddy --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index e3ef1c69d1326..8f7ffd248f303 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -37,6 +37,11 @@ properties: - qcom,sc7180-smmu-500 - qcom,sdm845-smmu-500 - const: arm,mmu-500 + - description: NVIDIA SoCs that use more than one "arm,mmu-500" +items: + - enum: + - nvdia,tegra194-smmu-500 + - const: arm,mmu-500 - items: - const: arm,mmu-500 - const: arm,smmu-v2 -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v7 3/6] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
Every Qcom Adreno GPU has an embedded SMMU for its own use. These devices depend on unique features such as split pagetables, different stall/halt requirements and other settings. Identify them with a compatible string so that they can be identified in the arm-smmu implementation specific code. Signed-off-by: Jordan Crouse --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index d7ceb4c34423..e52a1b146c97 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -38,6 +38,10 @@ properties: - qcom,sc7180-smmu-500 - qcom,sdm845-smmu-500 - const: arm,mmu-500 + - description: Qcom Adreno GPUs implementing "arm,smmu-v2" +items: + - const: qcom,adreno-smmu + - const: qcom,smmu-v2 - items: - const: arm,mmu-500 - const: arm,smmu-v2 -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v7 4/6] iommu/arm-smmu: Add implementation for the adreno GPU SMMU
Add a special implementation for the SMMU attached to most Adreno GPU
target triggered from the qcom,adreno-gpu-smmu compatible string. When
selected the driver will attempt to enable split pagetables.
Signed-off-by: Jordan Crouse
---
drivers/iommu/arm-smmu-impl.c | 5 -
drivers/iommu/arm-smmu-qcom.c | 38 +--
drivers/iommu/arm-smmu.c | 2 +-
drivers/iommu/arm-smmu.h | 3 ++-
4 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index a20e426d81ac..3bb1ef4e85f7 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -69,7 +69,7 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu)
}
static int cavium_init_context(struct arm_smmu_domain *smmu_domain,
- struct io_pgtable_cfg *pgtbl_cfg)
+ struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
{
struct cavium_smmu *cs = container_of(smmu_domain->smmu,
struct cavium_smmu, smmu);
@@ -176,5 +176,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct
arm_smmu_device *smmu)
of_device_is_compatible(np, "qcom,sc7180-smmu-500"))
return qcom_smmu_impl_init(smmu);
+ if (of_device_is_compatible(smmu->dev->of_node, "qcom,adreno-smmu"))
+ return qcom_adreno_smmu_impl_init(smmu);
+
return smmu;
}
diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
index be4318044f96..cc03f94fa458 100644
--- a/drivers/iommu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm-smmu-qcom.c
@@ -12,6 +12,22 @@ struct qcom_smmu {
struct arm_smmu_device smmu;
};
+static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
+ struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
+{
+ /*
+* All targets that use the qcom,adreno-smmu compatible string *should*
+* be AARCH64 stage 1 but double check because the arm-smmu code assumes
+* that is the case when the TTBR1 quirk is enabled
+*/
+ if (of_device_is_compatible(dev->of_node, "qcom,adreno") &&
+ (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) &&
+ (smmu_domain->cfg.fmt == ARM_SMMU_CTX_FMT_AARCH64))
+ pgtbl_cfg->quirks |= IO_PGTABLE_QUIRK_ARM_TTBR1;
+
+ return 0;
+}
+
static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
{ .compatible = "qcom,adreno" },
{ .compatible = "qcom,mdp4" },
@@ -65,7 +81,15 @@ static const struct arm_smmu_impl qcom_smmu_impl = {
.reset = qcom_smmu500_reset,
};
-struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
+static const struct arm_smmu_impl qcom_adreno_smmu_impl = {
+ .init_context = qcom_adreno_smmu_init_context,
+ .def_domain_type = qcom_smmu_def_domain_type,
+ .reset = qcom_smmu500_reset,
+};
+
+
+static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
+ const struct arm_smmu_impl *impl)
{
struct qcom_smmu *qsmmu;
@@ -75,8 +99,18 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct
arm_smmu_device *smmu)
qsmmu->smmu = *smmu;
- qsmmu->smmu.impl = _smmu_impl;
+ qsmmu->smmu.impl = impl;
devm_kfree(smmu->dev, smmu);
return >smmu;
}
+
+struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
+{
+ return qcom_smmu_create(smmu, _smmu_impl);
+}
+
+struct arm_smmu_device *qcom_adreno_smmu_impl_init(struct arm_smmu_device
*smmu)
+{
+ return qcom_smmu_create(smmu, _adreno_smmu_impl);
+}
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 048de2681670..f14dc4ecb422 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -812,7 +812,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain
*domain,
};
if (smmu->impl && smmu->impl->init_context) {
- ret = smmu->impl->init_context(smmu_domain, _cfg);
+ ret = smmu->impl->init_context(smmu_domain, _cfg, dev);
if (ret)
goto out_unlock;
}
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index 5f2de20e883b..df70d410f77d 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -397,7 +397,7 @@ struct arm_smmu_impl {
int (*cfg_probe)(struct arm_smmu_device *smmu);
int (*reset)(struct arm_smmu_device *smmu);
int (*init_context)(struct arm_smmu_domain *smmu_domain,
- struct io_pgtable_cfg *cfg);
+ struct io_pgtable_cfg *cfg, struct device *dev);
void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
int status);
int (*def_domain_type)(struct device *dev);
@@ -465,6 +465,7 @@ static inline void arm_smmu_writeq(struct arm_smmu_device
*smmu, int page,
struct
[PATCH v7 2/6] iommu/arm-smmu: Add support for split pagetables
Enable TTBR1 for a context bank if IO_PGTABLE_QUIRK_ARM_TTBR1 is selected
by the io-pgtable configuration.
Signed-off-by: Jordan Crouse
---
drivers/iommu/arm-smmu.c | 21 -
drivers/iommu/arm-smmu.h | 25 +++--
2 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 8a3a6c8c887a..048de2681670 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -555,11 +555,15 @@ static void arm_smmu_init_context_bank(struct
arm_smmu_domain *smmu_domain,
cb->ttbr[0] = pgtbl_cfg->arm_v7s_cfg.ttbr;
cb->ttbr[1] = 0;
} else {
- cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
- cb->ttbr[0] |= FIELD_PREP(ARM_SMMU_TTBRn_ASID,
- cfg->asid);
+ cb->ttbr[0] = FIELD_PREP(ARM_SMMU_TTBRn_ASID,
+ cfg->asid);
cb->ttbr[1] = FIELD_PREP(ARM_SMMU_TTBRn_ASID,
-cfg->asid);
+ cfg->asid);
+
+ if (pgtbl_cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
+ cb->ttbr[1] |= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
+ else
+ cb->ttbr[0] |= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
}
} else {
cb->ttbr[0] = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
@@ -824,7 +828,14 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
/* Update the domain's page sizes to reflect the page table format */
domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
- domain->geometry.aperture_end = (1UL << ias) - 1;
+
+ if (pgtbl_cfg.quirks & IO_PGTABLE_QUIRK_ARM_TTBR1) {
+ domain->geometry.aperture_start = ~0UL << ias;
+ domain->geometry.aperture_end = ~0UL;
+ } else {
+ domain->geometry.aperture_end = (1UL << ias) - 1;
+ }
+
domain->geometry.force_aperture = true;
/* Initialise the context bank with our page table cfg */
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index 38b041530a4f..5f2de20e883b 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -168,10 +168,12 @@ enum arm_smmu_cbar_type {
#define ARM_SMMU_CB_TCR0x30
#define ARM_SMMU_TCR_EAE BIT(31)
#define ARM_SMMU_TCR_EPD1 BIT(23)
+#define ARM_SMMU_TCR_A1BIT(22)
#define ARM_SMMU_TCR_TG0 GENMASK(15, 14)
#define ARM_SMMU_TCR_SH0 GENMASK(13, 12)
#define ARM_SMMU_TCR_ORGN0 GENMASK(11, 10)
#define ARM_SMMU_TCR_IRGN0 GENMASK(9, 8)
+#define ARM_SMMU_TCR_EPD0 BIT(7)
#define ARM_SMMU_TCR_T0SZ GENMASK(5, 0)
#define ARM_SMMU_VTCR_RES1 BIT(31)
@@ -347,12 +349,23 @@ struct arm_smmu_domain {
static inline u32 arm_smmu_lpae_tcr(struct io_pgtable_cfg *cfg)
{
- return ARM_SMMU_TCR_EPD1 |
- FIELD_PREP(ARM_SMMU_TCR_TG0, cfg->arm_lpae_s1_cfg.tcr.tg) |
- FIELD_PREP(ARM_SMMU_TCR_SH0, cfg->arm_lpae_s1_cfg.tcr.sh) |
- FIELD_PREP(ARM_SMMU_TCR_ORGN0, cfg->arm_lpae_s1_cfg.tcr.orgn) |
- FIELD_PREP(ARM_SMMU_TCR_IRGN0, cfg->arm_lpae_s1_cfg.tcr.irgn) |
- FIELD_PREP(ARM_SMMU_TCR_T0SZ, cfg->arm_lpae_s1_cfg.tcr.tsz);
+ u32 tcr = FIELD_PREP(ARM_SMMU_TCR_TG0, cfg->arm_lpae_s1_cfg.tcr.tg) |
+ FIELD_PREP(ARM_SMMU_TCR_SH0, cfg->arm_lpae_s1_cfg.tcr.sh) |
+ FIELD_PREP(ARM_SMMU_TCR_ORGN0, cfg->arm_lpae_s1_cfg.tcr.orgn) |
+ FIELD_PREP(ARM_SMMU_TCR_IRGN0, cfg->arm_lpae_s1_cfg.tcr.irgn) |
+ FIELD_PREP(ARM_SMMU_TCR_T0SZ, cfg->arm_lpae_s1_cfg.tcr.tsz);
+
+ /*
+ * When TTBR1 is selected shift the TCR fields by 16 bits and disable
+ * translation in TTBR0
+ */
+ if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1) {
+ tcr = (tcr << 16) & ~ARM_SMMU_TCR_A1;
+ tcr |= ARM_SMMU_TCR_EPD0;
+ } else
+ tcr |= ARM_SMMU_TCR_EPD1;
+
+ return tcr;
}
static inline u32 arm_smmu_lpae_tcr2(struct io_pgtable_cfg *cfg)
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v7 1/6] iommu/arm-smmu: Pass io-pgtable config to implementation specific function
Construct the io-pgtable config before calling the implementation specific
init_context function and pass it so the implementation specific function
can get a chance to change it before the io-pgtable is created.
Signed-off-by: Jordan Crouse
---
drivers/iommu/arm-smmu-impl.c | 3 ++-
drivers/iommu/arm-smmu.c | 11 ++-
drivers/iommu/arm-smmu.h | 3 ++-
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index c75b9d957b70..a20e426d81ac 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -68,7 +68,8 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu)
return 0;
}
-static int cavium_init_context(struct arm_smmu_domain *smmu_domain)
+static int cavium_init_context(struct arm_smmu_domain *smmu_domain,
+ struct io_pgtable_cfg *pgtbl_cfg)
{
struct cavium_smmu *cs = container_of(smmu_domain->smmu,
struct cavium_smmu, smmu);
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 243bc4cb2705..8a3a6c8c887a 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -797,11 +797,6 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
cfg->asid = cfg->cbndx;
smmu_domain->smmu = smmu;
- if (smmu->impl && smmu->impl->init_context) {
- ret = smmu->impl->init_context(smmu_domain);
- if (ret)
- goto out_unlock;
- }
pgtbl_cfg = (struct io_pgtable_cfg) {
.pgsize_bitmap = smmu->pgsize_bitmap,
@@ -812,6 +807,12 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
.iommu_dev = smmu->dev,
};
+ if (smmu->impl && smmu->impl->init_context) {
+ ret = smmu->impl->init_context(smmu_domain, _cfg);
+ if (ret)
+ goto out_unlock;
+ }
+
if (smmu_domain->non_strict)
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index d172c024be61..38b041530a4f 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -383,7 +383,8 @@ struct arm_smmu_impl {
u64 val);
int (*cfg_probe)(struct arm_smmu_device *smmu);
int (*reset)(struct arm_smmu_device *smmu);
- int (*init_context)(struct arm_smmu_domain *smmu_domain);
+ int (*init_context)(struct arm_smmu_domain *smmu_domain,
+ struct io_pgtable_cfg *cfg);
void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
int status);
int (*def_domain_type)(struct device *dev);
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v7 0/6] iommu/arm-smmu: Enable split pagetable support
Another iteration of the split-pagetable support for arm-smmu and the Adreno GPU SMMU. After email discussions [1] we opted to make a arm-smmu implementation for specifically for the Adreno GPU and use that to enable split pagetable support and later other implementation specific bits that we need. On the hardware side this is very close to the same code from before [2] only the TTBR1 quirk is turned on by the implementation and not a domain attribute. In drm/msm we use the returned size of the aperture as a clue to let us know which virtual address space we should use for global memory objects. There are two open items that you should be aware of. First, in the implementation specific code we have to check the compatible string of the device so that we only enable TTBR1 for the GPU (SID 0) and not the GMU (SID 4). I went back and forth trying to decide if I wanted to use the compatbile string or the SID as the filter and settled on the compatible string but I could be talked out of it. The other open item is that in drm/msm the hardware only uses 49 bits of the address space but arm-smmu expects the address to be sign extended all the way to 64 bits. This isn't a problem normally unless you look at the hardware registers that contain a IOVA and then the upper bits will be zero. I opted to restrict the internal drm/msm IOVA range to only 49 bits and then sign extend right before calling iommu_map / iommu_unmap. This is a bit wonky but I thought that matching the hardware would be less confusing when debugging a hang. [1] https://lists.linuxfoundation.org/pipermail/iommu/2020-May/044537.html [2] https://patchwork.kernel.org/patch/11482591/ Jordan Crouse (6): iommu/arm-smmu: Pass io-pgtable config to implementation specific function iommu/arm-smmu: Add support for split pagetables dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU iommu/arm-smmu: Add implementation for the adreno GPU SMMU drm/msm: Set the global virtual address range from the IOMMU domain arm6: dts: qcom: sm845: Set the compatible string for the GPU SMMU .../devicetree/bindings/iommu/arm,smmu.yaml | 4 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 13 ++- drivers/gpu/drm/msm/msm_iommu.c | 7 drivers/iommu/arm-smmu-impl.c | 6 ++- drivers/iommu/arm-smmu-qcom.c | 38 ++- drivers/iommu/arm-smmu.c | 32 +++- drivers/iommu/arm-smmu.h | 29 ++ 8 files changed, 108 insertions(+), 23 deletions(-) -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu/arm-smmu: Mark qcom_smmu_client_of_match as possibly unused
When CONFIG_OF=n of_match_device() gets pre-processed out of existence
leaving qcom-smmu_client_of_match unused. Mark it as possibly unused to
keep the compiler from warning in that case.
Fixes: 0e764a01015d ("iommu/arm-smmu: Allow client devices to select direct
mapping")
Reported-by: kbuild test robot
Acked-by: Will Deacon
Signed-off-by: Jordan Crouse
---
drivers/iommu/arm-smmu-qcom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
index cf01d0215a39..be4318044f96 100644
--- a/drivers/iommu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm-smmu-qcom.c
@@ -12,7 +12,7 @@ struct qcom_smmu {
struct arm_smmu_device smmu;
};
-static const struct of_device_id qcom_smmu_client_of_match[] = {
+static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
{ .compatible = "qcom,adreno" },
{ .compatible = "qcom,mdp4" },
{ .compatible = "qcom,mdss" },
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Nicolas,
On Thu, Jun 4, 2020 at 12:52 PM Nicolas Saenz Julienne
wrote:
>
> Hi Jim,
>
> On Thu, 2020-06-04 at 10:35 -0400, Jim Quinlan wrote:
>
> [...]
>
> > > > --- a/arch/sh/kernel/dma-coherent.c
> > > > +++ b/arch/sh/kernel/dma-coherent.c
> > > > @@ -14,6 +14,8 @@ void *arch_dma_alloc(struct device *dev, size_t size,
> > > > dma_addr_t *dma_handle,
> > > > {
> > > > void *ret, *ret_nocache;
> > > > int order = get_order(size);
> > > > + unsigned long pfn;
> > > > + phys_addr_t phys;
> > > >
> > > > gfp |= __GFP_ZERO;
> > > >
> > > > @@ -34,11 +36,14 @@ void *arch_dma_alloc(struct device *dev, size_t
> > > > size,
> > > > dma_addr_t *dma_handle,
> > > > return NULL;
> > > > }
> > > >
> > > > - split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
> > > > + phys = virt_to_phys(ret);
> > > > + pfn = phys >> PAGE_SHIFT;
> > >
> > > nit: not sure it really pays off to have a pfn variable here.
> > Did it for readability; the compiler's optimization should take care
> > of any extra variables. But I can switch if you insist.
>
> No need.
>
> [...]
>
> > > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > index 055eb0b8e396..2d66d415b6c3 100644
> > > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > > @@ -898,7 +898,10 @@ static int sun6i_csi_probe(struct platform_device
> > > > *pdev)
> > > >
> > > > sdev->dev = >dev;
> > > > /* The DMA bus has the memory mapped at 0 */
> > > > - sdev->dev->dma_pfn_offset = PHYS_OFFSET >> PAGE_SHIFT;
> > > > + ret = attach_uniform_dma_pfn_offset(sdev->dev,
> > > > + PHYS_OFFSET >> PAGE_SHIFT);
> > > > + if (ret)
> > > > + return ret;
> > > >
> > > > ret = sun6i_csi_resource_request(sdev, pdev);
> > > > if (ret)
> > > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > > index 96d8cfb14a60..c89333b0a5fb 100644
> > > > --- a/drivers/of/address.c
> > > > +++ b/drivers/of/address.c
> > > > @@ -918,6 +918,70 @@ void __iomem *of_io_request_and_map(struct
> > > > device_node
> > > > *np, int index,
> > > > }
> > > > EXPORT_SYMBOL(of_io_request_and_map);
> > > >
> > > > +static int attach_dma_pfn_offset_map(struct device *dev,
> > > > + struct device_node *node, int
> > > > num_ranges)
> > >
> > > As with the previous review, please take this comment with a grain of
> > > salt.
> > >
> > > I think there should be a clear split between what belongs to OF and what
> > > belongs to the core device infrastructure.
> > >
> > > OF's job should be to parse DT and provide a list/array of ranges, whereas
> > > the
> > > core device infrastructure should provide an API to assign a list of
> > > ranges/offset to a device.
> > >
> > > As a concrete example, you're forcing devices like the sta2x11 to build
> > > with
> > > OF
> > > support, which, being an Intel device, it's pretty odd. But I'm also
> > > thinking
> > > of how will all this fit once an ACPI device wants to use it.
> > To fix this I only have to move attach_uniform_dma_pfn_offset() from
> > of/address.c to say include/linux/dma-mapping.h. It has no
> > dependencies on OF. Do you agree?
>
> Yes that seems nicer. In case you didn't had it in mind already, I'd change
> the
> function name to match the naming scheme they use there.
>
> On the other hand, I'd also move the non OF parts of the non unifom dma_offset
> version of the function there.
>
> > > Expanding on this idea, once you have a split between the OF's and device
> > > core
> > > roles, it transpires that of_dma_get_range()'s job should only be to
> > > provide
> > > the ranges in a device understandable structure and of_dma_configre()'s to
> > > actually assign the device's parameters. This would obsolete patch #7.
> >
> > I think you mean patch #8.
>
> Yes, my bad.
>
> > I agree with you. The reason I needed a "struct device *" in the call is
> > because I wanted to make sure the memory that is alloc'd belongs to the
> > device that needs it. If I do a regular kzalloc(), this memory will become
> > a leak once someone starts unbinding/binding their device. Also, in all
> > uses of of_dma_rtange() -- there is only one -- a dev is required as one
> > can't attach an offset map to NULL.
> >
> > I do see that there are a number of functions in drivers/of/*.c that
> > take 'struct device *dev' as an argument so there is precedent for
> > something like this. Regardless, I need an owner to the memory I
> > alloc().
>
> I understand the need for dev to be around, devm_*() is key. But also it's
> important to keep the functions on purpose. And if of_dma_get_range() starts
> setting ranges it calls, for the very least, for a function rename. Although
> I'd rather split the parsing and setting
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Jim,
On Thu, 2020-06-04 at 10:35 -0400, Jim Quinlan wrote:
[...]
> > > --- a/arch/sh/kernel/dma-coherent.c
> > > +++ b/arch/sh/kernel/dma-coherent.c
> > > @@ -14,6 +14,8 @@ void *arch_dma_alloc(struct device *dev, size_t size,
> > > dma_addr_t *dma_handle,
> > > {
> > > void *ret, *ret_nocache;
> > > int order = get_order(size);
> > > + unsigned long pfn;
> > > + phys_addr_t phys;
> > >
> > > gfp |= __GFP_ZERO;
> > >
> > > @@ -34,11 +36,14 @@ void *arch_dma_alloc(struct device *dev, size_t size,
> > > dma_addr_t *dma_handle,
> > > return NULL;
> > > }
> > >
> > > - split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
> > > + phys = virt_to_phys(ret);
> > > + pfn = phys >> PAGE_SHIFT;
> >
> > nit: not sure it really pays off to have a pfn variable here.
> Did it for readability; the compiler's optimization should take care
> of any extra variables. But I can switch if you insist.
No need.
[...]
> > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > index 055eb0b8e396..2d66d415b6c3 100644
> > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > @@ -898,7 +898,10 @@ static int sun6i_csi_probe(struct platform_device
> > > *pdev)
> > >
> > > sdev->dev = >dev;
> > > /* The DMA bus has the memory mapped at 0 */
> > > - sdev->dev->dma_pfn_offset = PHYS_OFFSET >> PAGE_SHIFT;
> > > + ret = attach_uniform_dma_pfn_offset(sdev->dev,
> > > + PHYS_OFFSET >> PAGE_SHIFT);
> > > + if (ret)
> > > + return ret;
> > >
> > > ret = sun6i_csi_resource_request(sdev, pdev);
> > > if (ret)
> > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > index 96d8cfb14a60..c89333b0a5fb 100644
> > > --- a/drivers/of/address.c
> > > +++ b/drivers/of/address.c
> > > @@ -918,6 +918,70 @@ void __iomem *of_io_request_and_map(struct
> > > device_node
> > > *np, int index,
> > > }
> > > EXPORT_SYMBOL(of_io_request_and_map);
> > >
> > > +static int attach_dma_pfn_offset_map(struct device *dev,
> > > + struct device_node *node, int
> > > num_ranges)
> >
> > As with the previous review, please take this comment with a grain of salt.
> >
> > I think there should be a clear split between what belongs to OF and what
> > belongs to the core device infrastructure.
> >
> > OF's job should be to parse DT and provide a list/array of ranges, whereas
> > the
> > core device infrastructure should provide an API to assign a list of
> > ranges/offset to a device.
> >
> > As a concrete example, you're forcing devices like the sta2x11 to build with
> > OF
> > support, which, being an Intel device, it's pretty odd. But I'm also
> > thinking
> > of how will all this fit once an ACPI device wants to use it.
> To fix this I only have to move attach_uniform_dma_pfn_offset() from
> of/address.c to say include/linux/dma-mapping.h. It has no
> dependencies on OF. Do you agree?
Yes that seems nicer. In case you didn't had it in mind already, I'd change the
function name to match the naming scheme they use there.
On the other hand, I'd also move the non OF parts of the non unifom dma_offset
version of the function there.
> > Expanding on this idea, once you have a split between the OF's and device
> > core
> > roles, it transpires that of_dma_get_range()'s job should only be to provide
> > the ranges in a device understandable structure and of_dma_configre()'s to
> > actually assign the device's parameters. This would obsolete patch #7.
>
> I think you mean patch #8.
Yes, my bad.
> I agree with you. The reason I needed a "struct device *" in the call is
> because I wanted to make sure the memory that is alloc'd belongs to the
> device that needs it. If I do a regular kzalloc(), this memory will become
> a leak once someone starts unbinding/binding their device. Also, in all
> uses of of_dma_rtange() -- there is only one -- a dev is required as one
> can't attach an offset map to NULL.
>
> I do see that there are a number of functions in drivers/of/*.c that
> take 'struct device *dev' as an argument so there is precedent for
> something like this. Regardless, I need an owner to the memory I
> alloc().
I understand the need for dev to be around, devm_*() is key. But also it's
important to keep the functions on purpose. And if of_dma_get_range() starts
setting ranges it calls, for the very least, for a function rename. Although
I'd rather split the parsing and setting of ranges as mentioned earlier. That
said, I get that's a more drastic move.
Talking about drastic moves. How about getting rid of the concept of
dma_pfn_offset for drivers altogether. Let them provide dma_pfn_offset_regions
(even when there is only one). I feel it's conceptually nicer, as you'd be
dealing only in one
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Andy, On Thu, Jun 4, 2020 at 11:05 AM Andy Shevchenko wrote: > > On Thu, Jun 04, 2020 at 10:35:12AM -0400, Jim Quinlan wrote: > > On Thu, Jun 4, 2020 at 9:53 AM Nicolas Saenz Julienne > > wrote: > > > On Wed, 2020-06-03 at 15:20 -0400, Jim Quinlan wrote: > > ... > > > > > + phys = virt_to_phys(ret); > > > > + pfn = phys >> PAGE_SHIFT; > > > > > > nit: not sure it really pays off to have a pfn variable here. > > Did it for readability; the compiler's optimization should take care > > of any extra variables. But I can switch if you insist. > > One side note: please, try to get familiar with existing helpers in the > kernel. > For example, above line is like > > pfn = PFN_DOWN(phys); I just used the term in the original code; will change to PFN_DOWN(). > > ... > > > > > + if (!WARN_ON(!dev) && dev->dma_pfn_offset_map) > > > > > + *dma_handle -= PFN_PHYS( > > > > + dma_pfn_offset_from_phys_addr(dev, phys)); > > Don't do such indentation, esp. we have now 100! :-) Got it. Thanks, Jim Quinlan > > -- > With Best Regards, > Andy Shevchenko > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/mediatek: Use totalram_pages to setup enable_4GB
On 04.06.20 17:06, Christoph Hellwig wrote: > On Thu, Jun 04, 2020 at 01:32:40PM +0200, David Hildenbrand wrote: >> Just a thought: If memory hotplug is applicable as well, you might >> either want to always assume data->enable_4GB, or handle memory hotplug >> events from the memory notifier, when new memory gets onlined (not sure >> how tricky that is). > > We probably want a highest_pfn_possible() or similar API instead of > having drivers poking into random VM internals. Well, memory notifiers are a reasonable api used accross the kernel to get notified when new memory is onlined to the buddy that could be used for allocations. highest_pfn_possible() would have to default to something linked to MAX_PHYSMEM_BITS whenever memory hotplug is configured, I am not sure how helpful that is (IOW, you can just default to enable_4GB=true in that case instead in most cases). -- Thanks, David / dhildenb ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu: Mark qcom_smmu_client_of_match as possibly unused
On Wed, Jun 03, 2020 at 03:15:07PM -0600, Jordan Crouse wrote:
> When CONFIG_OF=n of_match_device() gets pre-processed out of existence
> leaving qcom-smmu_client_of_match unused. Mark it as possibly unused to
> keep the compiler from warning in that case.
>
> Fixes: 0e764a01015d ("iommu/arm-smmu: Allow client devices to select direct
> mapping")
> Reported-by: kbuild test robot
> Signed-off-by: Jordan Crouse
> ---
>
> drivers/iommu/arm-smmu-qcom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
> index cf01d0215a39..063b4388b0ff 100644
> --- a/drivers/iommu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm-smmu-qcom.c
> @@ -12,7 +12,7 @@ struct qcom_smmu {
> struct arm_smmu_device smmu;
> };
>
> -static const struct of_device_id qcom_smmu_client_of_match[] = {
> +static const struct __maybe_unused of_device_id qcom_smmu_client_of_match[]
> = {
Yikes, I've never seen that between the 'struct' and the struct name before!
I'd be inclined to stick it at the end, right before the '='.
With that:
Acked-by: Will Deacon
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: fsl_pamu: use kzfree() in fsl_pamu_probe()
Use kzfree() instead of opencoded memset with 0 followed by kfree().
Null check is not required since kzfree() checks for NULL internally.
Signed-off-by: Denis Efremov
---
drivers/iommu/fsl_pamu.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index cde281b97afa..099a11a35fb9 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -1174,10 +1174,7 @@ static int fsl_pamu_probe(struct platform_device *pdev)
if (irq != NO_IRQ)
free_irq(irq, data);
- if (data) {
- memset(data, 0, sizeof(struct pamu_isr_data));
- kfree(data);
- }
+ kzfree(data);
if (pamu_regs)
iounmap(pamu_regs);
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 10/12] of/irq: Make of_msi_map_rid() PCI bus agnostic
On Thu, May 21, 2020 at 05:17:27PM -0600, Rob Herring wrote:
> On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
> wrote:
> >
> > There is nothing PCI bus specific in the of_msi_map_rid()
> > implementation other than the requester ID tag for the input
> > ID space. Rename requester ID to a more generic ID so that
> > the translation code can be used by all busses that require
> > input/output ID translations.
> >
> > Leave a wrapper function of_msi_map_rid() in place to keep
> > existing PCI code mapping requester ID syntactically unchanged.
> >
> > No functional change intended.
> >
> > Signed-off-by: Lorenzo Pieralisi
> > Cc: Rob Herring
> > Cc: Marc Zyngier
> > ---
> > drivers/of/irq.c | 28 ++--
> > include/linux/of_irq.h | 14 --
> > 2 files changed, 26 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> > index 48a40326984f..25d17b8a1a1a 100644
> > --- a/drivers/of/irq.c
> > +++ b/drivers/of/irq.c
> > @@ -576,43 +576,43 @@ void __init of_irq_init(const struct of_device_id
> > *matches)
> > }
> > }
> >
> > -static u32 __of_msi_map_rid(struct device *dev, struct device_node **np,
> > - u32 rid_in)
> > +static u32 __of_msi_map_id(struct device *dev, struct device_node **np,
> > + u32 id_in)
> > {
> > struct device *parent_dev;
> > - u32 rid_out = rid_in;
> > + u32 id_out = id_in;
> >
> > /*
> > * Walk up the device parent links looking for one with a
> > * "msi-map" property.
> > */
> > for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent)
> > - if (!of_map_rid(parent_dev->of_node, rid_in, "msi-map",
> > - "msi-map-mask", np, _out))
> > + if (!of_map_id(parent_dev->of_node, id_in, "msi-map",
> > + "msi-map-mask", np, _out))
> > break;
> > - return rid_out;
> > + return id_out;
> > }
> >
> > /**
> > - * of_msi_map_rid - Map a MSI requester ID for a device.
> > + * of_msi_map_id - Map a MSI ID for a device.
> > * @dev: device for which the mapping is to be done.
> > * @msi_np: device node of the expected msi controller.
> > - * @rid_in: unmapped MSI requester ID for the device.
> > + * @id_in: unmapped MSI ID for the device.
> > *
> > * Walk up the device hierarchy looking for devices with a "msi-map"
> > - * property. If found, apply the mapping to @rid_in.
> > + * property. If found, apply the mapping to @id_in.
> > *
> > - * Returns the mapped MSI requester ID.
> > + * Returns the mapped MSI ID.
> > */
> > -u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32
> > rid_in)
> > +u32 of_msi_map_id(struct device *dev, struct device_node *msi_np, u32
> > id_in)
> > {
> > - return __of_msi_map_rid(dev, _np, rid_in);
> > + return __of_msi_map_id(dev, _np, id_in);
> > }
> >
> > /**
> > * of_msi_map_get_device_domain - Use msi-map to find the relevant MSI
> > domain
> > * @dev: device for which the mapping is to be done.
> > - * @rid: Requester ID for the device.
> > + * @id: Device ID.
> > * @bus_token: Bus token
> > *
> > * Walk up the device hierarchy looking for devices with a "msi-map"
> > @@ -625,7 +625,7 @@ struct irq_domain *of_msi_map_get_device_domain(struct
> > device *dev, u32 id,
> > {
> > struct device_node *np = NULL;
> >
> > - __of_msi_map_rid(dev, , id);
> > + __of_msi_map_id(dev, , id);
> > return irq_find_matching_host(np, bus_token);
> > }
> >
> > diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> > index 7142a3722758..cf9cb1e545ce 100644
> > --- a/include/linux/of_irq.h
> > +++ b/include/linux/of_irq.h
> > @@ -55,7 +55,12 @@ extern struct irq_domain
> > *of_msi_map_get_device_domain(struct device *dev,
> > u32 id,
> > u32 bus_token);
> > extern void of_msi_configure(struct device *dev, struct device_node *np);
> > -u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32
> > rid_in);
> > +u32 of_msi_map_id(struct device *dev, struct device_node *msi_np, u32
> > id_in);
> > +static inline u32 of_msi_map_rid(struct device *dev,
> > +struct device_node *msi_np, u32 rid_in)
> > +{
> > + return of_msi_map_id(dev, msi_np, rid_in);
> > +}
> > #else
> > static inline int of_irq_count(struct device_node *dev)
> > {
> > @@ -93,10 +98,15 @@ static inline struct irq_domain
> > *of_msi_map_get_device_domain(struct device *dev
> > static inline void of_msi_configure(struct device *dev, struct device_node
> > *np)
> > {
> > }
> > +static inline u32 of_msi_map_id(struct device *dev,
> > +struct device_node *msi_np, u32 id_in)
> > +{
> > + return
Re: [PATCH] iommu/mediatek: Use totalram_pages to setup enable_4GB
On Thu, Jun 04, 2020 at 01:32:40PM +0200, David Hildenbrand wrote: > Just a thought: If memory hotplug is applicable as well, you might > either want to always assume data->enable_4GB, or handle memory hotplug > events from the memory notifier, when new memory gets onlined (not sure > how tricky that is). We probably want a highest_pfn_possible() or similar API instead of having drivers poking into random VM internals. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Thu, Jun 04, 2020 at 10:35:12AM -0400, Jim Quinlan wrote: > On Thu, Jun 4, 2020 at 9:53 AM Nicolas Saenz Julienne > wrote: > > On Wed, 2020-06-03 at 15:20 -0400, Jim Quinlan wrote: ... > > > + phys = virt_to_phys(ret); > > > + pfn = phys >> PAGE_SHIFT; > > > > nit: not sure it really pays off to have a pfn variable here. > Did it for readability; the compiler's optimization should take care > of any extra variables. But I can switch if you insist. One side note: please, try to get familiar with existing helpers in the kernel. For example, above line is like pfn = PFN_DOWN(phys); ... > > > + if (!WARN_ON(!dev) && dev->dma_pfn_offset_map) > > > + *dma_handle -= PFN_PHYS( > > > + dma_pfn_offset_from_phys_addr(dev, phys)); Don't do such indentation, esp. we have now 100! :-) -- With Best Regards, Andy Shevchenko ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 07/12] of/device: Add input id to of_dma_configure()
On Thu, May 21, 2020 at 05:02:20PM -0600, Rob Herring wrote:
> On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
> wrote:
> >
> > Devices sitting on proprietary busses have a device ID space that
> > is owned by the respective bus and related firmware bindings. In order
> > to let the generic OF layer handle the input translations to
> > an IOMMU id, for such busses the current of_dma_configure() interface
> > should be extended in order to allow the bus layer to provide the
> > device input id parameter - that is retrieved/assigned in bus
> > specific code and firmware.
> >
> > Augment of_dma_configure() to add an optional input_id parameter,
> > leaving current functionality unchanged.
> >
> > Signed-off-by: Lorenzo Pieralisi
> > Cc: Rob Herring
> > Cc: Robin Murphy
> > Cc: Joerg Roedel
> > Cc: Laurentiu Tudor
> > ---
> > drivers/bus/fsl-mc/fsl-mc-bus.c | 4 ++-
> > drivers/iommu/of_iommu.c| 53 +
> > drivers/of/device.c | 8 +++--
> > include/linux/of_device.h | 16 --
> > include/linux/of_iommu.h| 6 ++--
> > 5 files changed, 60 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > index 40526da5c6a6..8ead3f0238f2 100644
> > --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > @@ -118,11 +118,13 @@ static int fsl_mc_bus_uevent(struct device *dev,
> > struct kobj_uevent_env *env)
> > static int fsl_mc_dma_configure(struct device *dev)
> > {
> > struct device *dma_dev = dev;
> > + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> > + u32 input_id = mc_dev->icid;
> >
> > while (dev_is_fsl_mc(dma_dev))
> > dma_dev = dma_dev->parent;
> >
> > - return of_dma_configure(dev, dma_dev->of_node, 0);
> > + return of_dma_configure_id(dev, dma_dev->of_node, 0, _id);
> > }
> >
> > static ssize_t modalias_show(struct device *dev, struct device_attribute
> > *attr,
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index ad96b87137d6..4516d5bf6cc9 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -139,25 +139,53 @@ static int of_pci_iommu_init(struct pci_dev *pdev,
> > u16 alias, void *data)
> > return err;
> > }
> >
> > -static int of_fsl_mc_iommu_init(struct fsl_mc_device *mc_dev,
> > - struct device_node *master_np)
> > +static int of_iommu_configure_dev_id(struct device_node *master_np,
> > +struct device *dev,
> > +const u32 *id)
>
> Should have read this patch before #6. I guess you could still make
> of_pci_iommu_init() call
> of_iommu_configure_dev_id.
Yes that makes sense, I will update it.
Thanks,
Lorenzo
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Thu, Jun 4, 2020 at 10:20 AM Dan Carpenter wrote: > > On Thu, Jun 04, 2020 at 09:48:49AM -0400, Jim Quinlan wrote: > > > > + r = devm_kcalloc(dev, 1, sizeof(struct dma_pfn_offset_region), > > > > + GFP_KERNEL); > > > > > > Use:r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL); > > Will fix. > > > > > > > > > > > > + if (!r) > > > > + return -ENOMEM; > > > > + > > > > + r->uniform_offset = true; > > > > + r->pfn_offset = pfn_offset; > > > > + > > > > + return 0; > > > > +} > > > > > > This function doesn't seem to do anything useful. Is part of it > > > missing? > > No, the uniform pfn offset is a special case. > > Sorry, I wasn't clear. We're talking about different things. The code > does: > > r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL); > if (!r) > return -ENOMEM; > > r->uniform_offset = true; > r->pfn_offset = pfn_offset; > > return 0; > > The code allocates "r" and then doesn't save it anywhere so there is > no point. You are absolutely right, sorry I missed your point. Will fix. Thanks, Jim Quinlan > > regards, > dan carpenter > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Thu, Jun 4, 2020 at 9:53 AM Nicolas Saenz Julienne
wrote:
>
> Hi Jim,
>
> On Wed, 2020-06-03 at 15:20 -0400, Jim Quinlan wrote:
> > The new field in struct device 'dma_pfn_offset_map' is used to facilitate
> > the use of multiple pfn offsets between cpu addrs and dma addrs. It
> > subsumes the role of dev->dma_pfn_offset -- a uniform offset -- and
> > designates the single offset a special case.
> >
> > of_dma_configure() is the typical manner to set pfn offsets but there
> > are a number of ad hoc assignments to dev->dma_pfn_offset in the
> > kernel code. These cases now invoke the function
> > attach_uniform_dma_pfn_offset(dev, pfn_offset).
> >
> > Signed-off-by: Jim Quinlan
> > ---
> > arch/arm/include/asm/dma-mapping.h| 9 +-
> > arch/arm/mach-keystone/keystone.c | 9 +-
> > arch/sh/drivers/pci/pcie-sh7786.c | 3 +-
> > arch/sh/kernel/dma-coherent.c | 17 ++--
> > arch/x86/pci/sta2x11-fixup.c | 7 +-
> > drivers/acpi/arm64/iort.c | 5 +-
> > drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +-
> > drivers/iommu/io-pgtable-arm.c| 2 +-
> > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
> > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +-
> > drivers/of/address.c | 93 +--
> > drivers/of/device.c | 8 +-
> > drivers/remoteproc/remoteproc_core.c | 2 +-
> > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +-
> > drivers/usb/core/message.c| 4 +-
> > drivers/usb/core/usb.c| 2 +-
> > include/linux/device.h| 4 +-
> > include/linux/dma-direct.h| 16 +++-
> > include/linux/dma-mapping.h | 45 +
> > kernel/dma/coherent.c | 11 ++-
> > 20 files changed, 210 insertions(+), 51 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-
> > mapping.h
> > index bdd80ddbca34..f1e72f99468b 100644
> > --- a/arch/arm/include/asm/dma-mapping.h
> > +++ b/arch/arm/include/asm/dma-mapping.h
> > @@ -35,8 +35,9 @@ static inline const struct dma_map_ops
> > *get_arch_dma_ops(struct bus_type *bus)
> > #ifndef __arch_pfn_to_dma
> > static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
> > {
> > - if (dev)
> > - pfn -= dev->dma_pfn_offset;
> > + if (dev && dev->dma_pfn_offset_map)
>
> Would it make sense to move the dev->dma_pfn_offset_map check into
> dma_pfn_offset_from_phys_addr() and return 0 if not available? Same for the
> opposite variant of the function. I think it'd make the code a little simpler
> on
> some of the use cases, and overall less error prone if anyone starts using the
> function elsewhere.
Yes it makes sense and I was debating doing it but I just wanted to
make it explicit that there was not much cost for this change for the
fastpath -- no dma_pfn_offset whatsoever -- as the cost goes from a
"pfn += dev->dma_pfn_offset" to a "if (dev->dma_pfn_offset_map)". I
will do what you suggest.
>
> > + pfn -= dma_pfn_offset_from_phys_addr(dev, PFN_PHYS(pfn));
> > +
> > return (dma_addr_t)__pfn_to_bus(pfn);
> > }
> >
> > @@ -44,8 +45,8 @@ static inline unsigned long dma_to_pfn(struct device *dev,
> > dma_addr_t addr)
> > {
> > unsigned long pfn = __bus_to_pfn(addr);
> >
> > - if (dev)
> > - pfn += dev->dma_pfn_offset;
> > + if (dev && dev->dma_pfn_offset_map)
> > + pfn += dma_pfn_offset_from_dma_addr(dev, addr);
> >
> > return pfn;
> > }
> > diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach-
> > keystone/keystone.c
> > index 638808c4e122..e7d3ee6e9cb5 100644
> > --- a/arch/arm/mach-keystone/keystone.c
> > +++ b/arch/arm/mach-keystone/keystone.c
> > @@ -8,6 +8,7 @@
> > */
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -38,9 +39,11 @@ static int keystone_platform_notifier(struct
> > notifier_block
> > *nb,
> > return NOTIFY_BAD;
> >
> > if (!dev->of_node) {
> > - dev->dma_pfn_offset = keystone_dma_pfn_offset;
> > - dev_err(dev, "set dma_pfn_offset%08lx\n",
> > - dev->dma_pfn_offset);
> > + int ret = attach_uniform_dma_pfn_offset
> > + (dev, keystone_dma_pfn_offset);
> > +
> > + dev_err(dev, "set dma_pfn_offset%08lx%s\n",
> > + dev->dma_pfn_offset, ret ? " failed" : "");
> > }
> > return NOTIFY_OK;
> > }
> > diff --git a/arch/sh/drivers/pci/pcie-sh7786.c b/arch/sh/drivers/pci/pcie-
> > sh7786.c
> > index e0b568aaa701..2e832a5c58c1 100644
> > --- a/arch/sh/drivers/pci/pcie-sh7786.c
> > +++ b/arch/sh/drivers/pci/pcie-sh7786.c
> > @@ -12,6 +12,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
Re: [PATCH 06/12] of/iommu: Make of_map_rid() PCI agnostic
On Thu, May 21, 2020 at 04:47:19PM -0600, Rob Herring wrote:
> On Thu, May 21, 2020 at 7:00 AM Lorenzo Pieralisi
> wrote:
> >
> > There is nothing PCI specific (other than the RID - requester ID)
> > in the of_map_rid() implementation, so the same function can be
> > reused for input/output IDs mapping for other busses just as well.
> >
> > Rename the RID instances/names to a generic "id" tag and provide
> > an of_map_rid() wrapper function so that we can leave the existing
> > (and legitimate) callers unchanged.
>
> It's not all that clear to a casual observer that RID is a PCI thing,
> so I don't know that keeping it buys much. And there's only 3 callers.
Yes I agree - I think we can remove the _rid interface.
> > No functionality change intended.
> >
> > Signed-off-by: Lorenzo Pieralisi
> > Cc: Rob Herring
> > Cc: Joerg Roedel
> > Cc: Robin Murphy
> > Cc: Marc Zyngier
> > ---
> > drivers/iommu/of_iommu.c | 2 +-
> > drivers/of/base.c| 42
> > include/linux/of.h | 17 +++-
> > 3 files changed, 38 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 20738aacac89..ad96b87137d6 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -145,7 +145,7 @@ static int of_fsl_mc_iommu_init(struct fsl_mc_device
> > *mc_dev,
> > struct of_phandle_args iommu_spec = { .args_count = 1 };
> > int err;
> >
> > - err = of_map_rid(master_np, mc_dev->icid, "iommu-map",
> > + err = of_map_id(master_np, mc_dev->icid, "iommu-map",
>
> I'm not sure this is an improvement because I'd refactor this function
> and of_pci_iommu_init() into a single function:
>
> of_bus_iommu_init(struct device *dev, struct device_node *np, u32 id)
>
> Then of_pci_iommu_init() becomes:
>
> of_pci_iommu_init()
> {
> return of_bus_iommu_init(info->dev, info->np, alias);
> }
>
> And replace of_fsl_mc_iommu_init call with:
> err = of_bus_iommu_init(dev, master_np, to_fsl_mc_device(dev)->icid);
I will follow up on this on patch 7.
> > "iommu-map-mask", _spec.np,
> > iommu_spec.args);
> > if (err)
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index ae03b1218b06..e000e17bd602 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -2201,15 +2201,15 @@ int of_find_last_cache_level(unsigned int cpu)
> > }
> >
> > /**
> > - * of_map_rid - Translate a requester ID through a downstream mapping.
> > + * of_map_id - Translate a requester ID through a downstream mapping.
>
> Still a requester ID?
Fixed, thanks.
Lorenzo
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Thu, Jun 04, 2020 at 09:48:49AM -0400, Jim Quinlan wrote: > > > + r = devm_kcalloc(dev, 1, sizeof(struct dma_pfn_offset_region), > > > + GFP_KERNEL); > > > > Use:r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL); > Will fix. > > > > > > > > + if (!r) > > > + return -ENOMEM; > > > + > > > + r->uniform_offset = true; > > > + r->pfn_offset = pfn_offset; > > > + > > > + return 0; > > > +} > > > > This function doesn't seem to do anything useful. Is part of it > > missing? > No, the uniform pfn offset is a special case. Sorry, I wasn't clear. We're talking about different things. The code does: r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL); if (!r) return -ENOMEM; r->uniform_offset = true; r->pfn_offset = pfn_offset; return 0; The code allocates "r" and then doesn't save it anywhere so there is no point. regards, dan carpenter ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Jim,
On Wed, 2020-06-03 at 15:20 -0400, Jim Quinlan wrote:
> The new field in struct device 'dma_pfn_offset_map' is used to facilitate
> the use of multiple pfn offsets between cpu addrs and dma addrs. It
> subsumes the role of dev->dma_pfn_offset -- a uniform offset -- and
> designates the single offset a special case.
>
> of_dma_configure() is the typical manner to set pfn offsets but there
> are a number of ad hoc assignments to dev->dma_pfn_offset in the
> kernel code. These cases now invoke the function
> attach_uniform_dma_pfn_offset(dev, pfn_offset).
>
> Signed-off-by: Jim Quinlan
> ---
> arch/arm/include/asm/dma-mapping.h| 9 +-
> arch/arm/mach-keystone/keystone.c | 9 +-
> arch/sh/drivers/pci/pcie-sh7786.c | 3 +-
> arch/sh/kernel/dma-coherent.c | 17 ++--
> arch/x86/pci/sta2x11-fixup.c | 7 +-
> drivers/acpi/arm64/iort.c | 5 +-
> drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +-
> drivers/iommu/io-pgtable-arm.c| 2 +-
> .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
> .../platform/sunxi/sun6i-csi/sun6i_csi.c | 5 +-
> drivers/of/address.c | 93 +--
> drivers/of/device.c | 8 +-
> drivers/remoteproc/remoteproc_core.c | 2 +-
> .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +-
> drivers/usb/core/message.c| 4 +-
> drivers/usb/core/usb.c| 2 +-
> include/linux/device.h| 4 +-
> include/linux/dma-direct.h| 16 +++-
> include/linux/dma-mapping.h | 45 +
> kernel/dma/coherent.c | 11 ++-
> 20 files changed, 210 insertions(+), 51 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-
> mapping.h
> index bdd80ddbca34..f1e72f99468b 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -35,8 +35,9 @@ static inline const struct dma_map_ops
> *get_arch_dma_ops(struct bus_type *bus)
> #ifndef __arch_pfn_to_dma
> static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
> {
> - if (dev)
> - pfn -= dev->dma_pfn_offset;
> + if (dev && dev->dma_pfn_offset_map)
Would it make sense to move the dev->dma_pfn_offset_map check into
dma_pfn_offset_from_phys_addr() and return 0 if not available? Same for the
opposite variant of the function. I think it'd make the code a little simpler on
some of the use cases, and overall less error prone if anyone starts using the
function elsewhere.
> + pfn -= dma_pfn_offset_from_phys_addr(dev, PFN_PHYS(pfn));
> +
> return (dma_addr_t)__pfn_to_bus(pfn);
> }
>
> @@ -44,8 +45,8 @@ static inline unsigned long dma_to_pfn(struct device *dev,
> dma_addr_t addr)
> {
> unsigned long pfn = __bus_to_pfn(addr);
>
> - if (dev)
> - pfn += dev->dma_pfn_offset;
> + if (dev && dev->dma_pfn_offset_map)
> + pfn += dma_pfn_offset_from_dma_addr(dev, addr);
>
> return pfn;
> }
> diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach-
> keystone/keystone.c
> index 638808c4e122..e7d3ee6e9cb5 100644
> --- a/arch/arm/mach-keystone/keystone.c
> +++ b/arch/arm/mach-keystone/keystone.c
> @@ -8,6 +8,7 @@
> */
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -38,9 +39,11 @@ static int keystone_platform_notifier(struct notifier_block
> *nb,
> return NOTIFY_BAD;
>
> if (!dev->of_node) {
> - dev->dma_pfn_offset = keystone_dma_pfn_offset;
> - dev_err(dev, "set dma_pfn_offset%08lx\n",
> - dev->dma_pfn_offset);
> + int ret = attach_uniform_dma_pfn_offset
> + (dev, keystone_dma_pfn_offset);
> +
> + dev_err(dev, "set dma_pfn_offset%08lx%s\n",
> + dev->dma_pfn_offset, ret ? " failed" : "");
> }
> return NOTIFY_OK;
> }
> diff --git a/arch/sh/drivers/pci/pcie-sh7786.c b/arch/sh/drivers/pci/pcie-
> sh7786.c
> index e0b568aaa701..2e832a5c58c1 100644
> --- a/arch/sh/drivers/pci/pcie-sh7786.c
> +++ b/arch/sh/drivers/pci/pcie-sh7786.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -487,7 +488,7 @@ int pcibios_map_platform_irq(const struct pci_dev *pdev,
> u8 slot, u8 pin)
>
> void pcibios_bus_add_device(struct pci_dev *pdev)
> {
> - pdev->dev.dma_pfn_offset = dma_pfn_offset;
> + attach_uniform_dma_pfn_offset(>dev, dma_pfn_offset);
> }
>
> static int __init sh7786_pcie_core_init(void)
> diff --git a/arch/sh/kernel/dma-coherent.c b/arch/sh/kernel/dma-coherent.c
> index d4811691b93c..5fc9e358b6c7 100644
> --- a/arch/sh/kernel/dma-coherent.c
> +++ b/arch/sh/kernel/dma-coherent.c
> @@ -14,6 +14,8 @@ void
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Dan,
On Thu, Jun 4, 2020 at 7:06 AM Dan Carpenter wrote:
>
> On Wed, Jun 03, 2020 at 03:20:41PM -0400, Jim Quinlan wrote:
> > @@ -786,7 +787,7 @@ static int sun4i_backend_bind(struct device *dev,
> > struct device *master,
> > const struct sun4i_backend_quirks *quirks;
> > struct resource *res;
> > void __iomem *regs;
> > - int i, ret;
> > + int i, ret = 0;
>
> No need for this.
Will fix.
>
> >
> > backend = devm_kzalloc(dev, sizeof(*backend), GFP_KERNEL);
> > if (!backend)
> > @@ -812,7 +813,9 @@ static int sun4i_backend_bind(struct device *dev,
> > struct device *master,
> >* on our device since the RAM mapping is at 0 for the DMA
> > bus,
> >* unlike the CPU.
> >*/
> > - drm->dev->dma_pfn_offset = PHYS_PFN_OFFSET;
> > + ret = attach_uniform_dma_pfn_offset(dev, PHYS_PFN_OFFSET);
> > + if (ret)
> > + return ret;
> > }
> >
> > backend->engine.node = dev->of_node;
> > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> > index 04fbd4bf0ff9..e9cc1c2d47cd 100644
> > --- a/drivers/iommu/io-pgtable-arm.c
> > +++ b/drivers/iommu/io-pgtable-arm.c
> > @@ -754,7 +754,7 @@ arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg)
> > if (cfg->oas > ARM_LPAE_MAX_ADDR_BITS)
> > return NULL;
> >
> > - if (!selftest_running && cfg->iommu_dev->dma_pfn_offset) {
> > + if (!selftest_running && cfg->iommu_dev->dma_pfn_offset_map) {
> > dev_err(cfg->iommu_dev, "Cannot accommodate DMA offset for
> > IOMMU page tables\n");
> > return NULL;
> > }
> > diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > index eff34ded6305..7212da5e1076 100644
> > --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > @@ -7,6 +7,7 @@
> > */
> >
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -183,7 +184,9 @@ static int sun4i_csi_probe(struct platform_device *pdev)
> > return ret;
> > } else {
> > #ifdef PHYS_PFN_OFFSET
> > - csi->dev->dma_pfn_offset = PHYS_PFN_OFFSET;
> > + ret = attach_uniform_dma_pfn_offset(dev, PHYS_PFN_OFFSET);
> > + if (ret)
> > + return ret;
> > #endif
> > }
> >
> > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > index 055eb0b8e396..2d66d415b6c3 100644
> > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > @@ -898,7 +898,10 @@ static int sun6i_csi_probe(struct platform_device
> > *pdev)
> >
> > sdev->dev = >dev;
> > /* The DMA bus has the memory mapped at 0 */
> > - sdev->dev->dma_pfn_offset = PHYS_OFFSET >> PAGE_SHIFT;
> > + ret = attach_uniform_dma_pfn_offset(sdev->dev,
> > + PHYS_OFFSET >> PAGE_SHIFT);
> > + if (ret)
> > + return ret;
> >
> > ret = sun6i_csi_resource_request(sdev, pdev);
> > if (ret)
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 96d8cfb14a60..c89333b0a5fb 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -918,6 +918,70 @@ void __iomem *of_io_request_and_map(struct device_node
> > *np, int index,
> > }
> > EXPORT_SYMBOL(of_io_request_and_map);
> >
> > +static int attach_dma_pfn_offset_map(struct device *dev,
> > + struct device_node *node, int num_ranges)
> > +{
> > + struct of_range_parser parser;
> > + struct of_range range;
> > + struct dma_pfn_offset_region *r;
> > +
> > + r = devm_kcalloc(dev, num_ranges + 1,
> > + sizeof(struct dma_pfn_offset_region), GFP_KERNEL);
> > + if (!r)
> > + return -ENOMEM;
> > + dev->dma_pfn_offset_map = r;
> > + of_dma_range_parser_init(, node);
> > +
> > + /*
> > + * Record all info for DMA ranges array. We could
> > + * just use the of_range struct, but if we did that it
> > + * would require more calculations for phys_to_dma and
> > + * dma_to_phys conversions.
> > + */
> > + for_each_of_range(, ) {
> > + r->cpu_start = range.cpu_addr;
> > + r->cpu_end = r->cpu_start + range.size - 1;
> > + r->dma_start = range.bus_addr;
> > + r->dma_end = r->dma_start + range.size - 1;
> > + r->pfn_offset = PFN_DOWN(range.cpu_addr)
> > + - PFN_DOWN(range.bus_addr);
> > + r++;
> > + }
> > + return 0;
> > +}
> > +
> > +
> > +
> > +/**
> > + * attach_dma_pfn_offset - Assign scalar offset for all addresses.
> > + * @dev: device pointer; only needed for a corner case.
>
Re: [PATCH 0/2] Introduce PCI_FIXUP_IOMMU
On 2020/6/2 上午1:41, Bjorn Helgaas wrote: On Thu, May 28, 2020 at 09:33:44AM +0200, Joerg Roedel wrote: On Wed, May 27, 2020 at 01:18:42PM -0500, Bjorn Helgaas wrote: Is this slowdown significant? We already iterate over every device when applying PCI_FIXUP_FINAL quirks, so if we used the existing PCI_FIXUP_FINAL, we wouldn't be adding a new loop. We would only be adding two more iterations to the loop in pci_do_fixups() that tries to match quirks against the current device. I doubt that would be a measurable slowdown. I don't know how significant it is, but I remember people complaining about adding new PCI quirks because it takes too long for them to run them all. That was in the discussion about the quirk disabling ATS on AMD Stoney systems. So it probably depends on how many PCI devices are in the system whether it causes any measureable slowdown. I found this [1] from Paul Menzel, which was a slowdown caused by quirk_usb_early_handoff(). I think the real problem is individual quirks that take a long time. The PCI_FIXUP_IOMMU things we're talking about should be fast, and of course, they're only run for matching devices anyway. So I'd rather keep them as PCI_FIXUP_FINAL than add a whole new phase. Thanks Bjorn for taking time for this. If so, it would be much simpler. +++ b/drivers/iommu/iommu.c @@ -2418,6 +2418,10 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode, fwspec->iommu_fwnode = iommu_fwnode; fwspec->ops = ops; dev_iommu_fwspec_set(dev, fwspec); + + if (dev_is_pci(dev)) + pci_fixup_device(pci_fixup_final, to_pci_dev(dev)); + Then pci_fixup_final will be called twice, the first in pci_bus_add_device. Here in iommu_fwspec_init is the second time, specifically for iommu_fwspec. Will send this when 5.8-rc1 is open. Thanks ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Check for deferred attach in iommu_group_do_dma_attach()
Hi Joerg,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on next-20200604]
[cannot apply to v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Joerg-Roedel/iommu-Check-for-deferred-attach-in-iommu_group_do_dma_attach/20200604-172315
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: arm-randconfig-s032-20200604 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-244-g0ee050a8-dirty
# save the attached .config to linux build tree
make W=1 C=1 ARCH=arm CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>, old ones prefixed by <<):
drivers/iommu/iommu.c:386:5: warning: no previous prototype for
'iommu_insert_resv_region' [-Wmissing-prototypes]
386 | int iommu_insert_resv_region(struct iommu_resv_region *new,
| ^~~~
drivers/iommu/iommu.c: In function 'iommu_group_do_dma_attach':
>> drivers/iommu/iommu.c:1685:32: error: 'group' undeclared (first use in this
>> function); did you mean 'cgroup'?
1685 | if (!iommu_is_attach_deferred(group->domain, dev))
|^
|cgroup
drivers/iommu/iommu.c:1685:32: note: each undeclared identifier is reported
only once for each function it appears in
drivers/iommu/iommu.c:1682:23: warning: unused variable 'domain'
[-Wunused-variable]
1682 | struct iommu_domain *domain = data;
| ^~
drivers/iommu/iommu.c: At top level:
drivers/iommu/iommu.c:2171:5: warning: no previous prototype for '__iommu_map'
[-Wmissing-prototypes]
2171 | int __iommu_map(struct iommu_domain *domain, unsigned long iova,
| ^~~
drivers/iommu/iommu.c:2322:8: warning: no previous prototype for
'__iommu_map_sg' [-Wmissing-prototypes]
2322 | size_t __iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
|^~
vim +1685 drivers/iommu/iommu.c
1679
1680 static int iommu_group_do_dma_attach(struct device *dev, void *data)
1681 {
1682 struct iommu_domain *domain = data;
1683 int ret = 0;
1684
> 1685 if (!iommu_is_attach_deferred(group->domain, dev))
1686 ret = __iommu_attach_device(group->domain, dev);
1687
1688 return ret;
1689 }
1690
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] dma-direct: provide the ability to reserve per-numa CMA
Hi Barry,
url:
https://github.com/0day-ci/linux/commits/Barry-Song/support-per-numa-CMA-for-ARM-server/20200603-104821
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
for-next/core
config: x86_64-randconfig-m001-20200603 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
kernel/dma/contiguous.c:274 dma_alloc_contiguous() warn: variable dereferenced
before check 'dev' (see line 272)
#
https://github.com/0day-ci/linux/commit/adb919e972c1cac3d8b11905d5258d23d3aac6a4
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout adb919e972c1cac3d8b11905d5258d23d3aac6a4
vim +/dev +274 kernel/dma/contiguous.c
b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 267
struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 268
{
90ae409f9eb3bc kernel/dma/contiguous.c Christoph Hellwig 2019-08-20 269
size_t count = size >> PAGE_SHIFT;
b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 270
struct page *page = NULL;
bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 271
struct cma *cma = NULL;
adb919e972c1ca kernel/dma/contiguous.c Barry Song2020-06-03 @272
int nid = dev_to_node(dev);
^^^
Dereferenced inside function.
bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 273
bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 @274
if (dev && dev->cma_area)
^^^
Too late.
bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 275
cma = dev->cma_area;
adb919e972c1ca kernel/dma/contiguous.c Barry Song2020-06-03 276
else if ((nid != NUMA_NO_NODE) && dma_contiguous_pernuma_area[nid]
adb919e972c1ca kernel/dma/contiguous.c Barry Song2020-06-03 277
&& !(gfp & (GFP_DMA | GFP_DMA32)))
adb919e972c1ca kernel/dma/contiguous.c Barry Song2020-06-03 278
cma = dma_contiguous_pernuma_area[nid];
bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 279
else if (count > 1)
bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 280
cma = dma_contiguous_default_area;
b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 281
b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 282
/* CMA can be used only in the context which permits sleeping */
b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen 2019-05-23 283
if (cma && gfpflags_allow_blocking(gfp)) {
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/mediatek: Use totalram_pages to setup enable_4GB
On 04.06.20 11:49, Miles Chen wrote: > On Thu, 2020-06-04 at 10:25 +0200, David Hildenbrand wrote: >> On 04.06.20 10:01, Miles Chen wrote: >>> To build this driver as a kernel module, we cannot use >>> the unexported symbol "max_pfn" to setup enable_4GB. >>> >>> Use totalram_pages() instead to setup enable_4GB. >>> >>> Suggested-by: Mike Rapoport >>> Signed-off-by: Miles Chen >>> Cc: David Hildenbrand >>> Cc: Yong Wu >>> Cc: Chao Hao >>> --- >>> drivers/iommu/mtk_iommu.c | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c >>> index 5f4d6df59cf6..c2798a6e0e38 100644 >>> --- a/drivers/iommu/mtk_iommu.c >>> +++ b/drivers/iommu/mtk_iommu.c >>> @@ -3,7 +3,6 @@ >>> * Copyright (c) 2015-2016 MediaTek Inc. >>> * Author: Yong Wu >>> */ >>> -#include >>> #include >>> #include >>> #include >>> @@ -626,8 +625,8 @@ static int mtk_iommu_probe(struct platform_device *pdev) >>> return -ENOMEM; >>> data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN); >>> >>> - /* Whether the current dram is over 4GB */ >>> - data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT)); >>> + /* Whether the current dram is over 4GB, note: DRAM start at 1GB */ >>> + data->enable_4GB = !!(totalram_pages() > ((SZ_2G + SZ_1G) >> >>> PAGE_SHIFT)); >> >> A similar thing seems to be done by >> drivers/media/platform/mtk-vpu/mtk_vpu.c: >> vpu->enable_4GB = !!(totalram_pages() > (SZ_2G >> PAGE_SHIFT)); >> >> I do wonder if some weird memory hotplug setups might give you false >> negatives. >> >> E.g., start a VM with 1GB and hotplug 1GB - it will be hotplugged on >> x86-64 above 4GB, turning max_pfn into 5GB. totalram_pages() should >> return something < 2GB. >> >> Same can happen when you have a VM and use ballooning to fake-unplug >> memory, making totalram_pages() return something < 4GB, but leaving >> usable pfns >= 4GB > > Yes. Yingjoe also told me that this patch is not correct. > > Thanks for pointing this out. totalram_pages() does not work > for some cases: > Just a thought: If memory hotplug is applicable as well, you might either want to always assume data->enable_4GB, or handle memory hotplug events from the memory notifier, when new memory gets onlined (not sure how tricky that is). -- Thanks, David / dhildenb ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Wed, Jun 03, 2020 at 03:20:41PM -0400, Jim Quinlan wrote:
> @@ -786,7 +787,7 @@ static int sun4i_backend_bind(struct device *dev, struct
> device *master,
> const struct sun4i_backend_quirks *quirks;
> struct resource *res;
> void __iomem *regs;
> - int i, ret;
> + int i, ret = 0;
No need for this.
>
> backend = devm_kzalloc(dev, sizeof(*backend), GFP_KERNEL);
> if (!backend)
> @@ -812,7 +813,9 @@ static int sun4i_backend_bind(struct device *dev, struct
> device *master,
>* on our device since the RAM mapping is at 0 for the DMA bus,
>* unlike the CPU.
>*/
> - drm->dev->dma_pfn_offset = PHYS_PFN_OFFSET;
> + ret = attach_uniform_dma_pfn_offset(dev, PHYS_PFN_OFFSET);
> + if (ret)
> + return ret;
> }
>
> backend->engine.node = dev->of_node;
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 04fbd4bf0ff9..e9cc1c2d47cd 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -754,7 +754,7 @@ arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg)
> if (cfg->oas > ARM_LPAE_MAX_ADDR_BITS)
> return NULL;
>
> - if (!selftest_running && cfg->iommu_dev->dma_pfn_offset) {
> + if (!selftest_running && cfg->iommu_dev->dma_pfn_offset_map) {
> dev_err(cfg->iommu_dev, "Cannot accommodate DMA offset for
> IOMMU page tables\n");
> return NULL;
> }
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> index eff34ded6305..7212da5e1076 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -183,7 +184,9 @@ static int sun4i_csi_probe(struct platform_device *pdev)
> return ret;
> } else {
> #ifdef PHYS_PFN_OFFSET
> - csi->dev->dma_pfn_offset = PHYS_PFN_OFFSET;
> + ret = attach_uniform_dma_pfn_offset(dev, PHYS_PFN_OFFSET);
> + if (ret)
> + return ret;
> #endif
> }
>
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> index 055eb0b8e396..2d66d415b6c3 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> @@ -898,7 +898,10 @@ static int sun6i_csi_probe(struct platform_device *pdev)
>
> sdev->dev = >dev;
> /* The DMA bus has the memory mapped at 0 */
> - sdev->dev->dma_pfn_offset = PHYS_OFFSET >> PAGE_SHIFT;
> + ret = attach_uniform_dma_pfn_offset(sdev->dev,
> + PHYS_OFFSET >> PAGE_SHIFT);
> + if (ret)
> + return ret;
>
> ret = sun6i_csi_resource_request(sdev, pdev);
> if (ret)
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 96d8cfb14a60..c89333b0a5fb 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -918,6 +918,70 @@ void __iomem *of_io_request_and_map(struct device_node
> *np, int index,
> }
> EXPORT_SYMBOL(of_io_request_and_map);
>
> +static int attach_dma_pfn_offset_map(struct device *dev,
> + struct device_node *node, int num_ranges)
> +{
> + struct of_range_parser parser;
> + struct of_range range;
> + struct dma_pfn_offset_region *r;
> +
> + r = devm_kcalloc(dev, num_ranges + 1,
> + sizeof(struct dma_pfn_offset_region), GFP_KERNEL);
> + if (!r)
> + return -ENOMEM;
> + dev->dma_pfn_offset_map = r;
> + of_dma_range_parser_init(, node);
> +
> + /*
> + * Record all info for DMA ranges array. We could
> + * just use the of_range struct, but if we did that it
> + * would require more calculations for phys_to_dma and
> + * dma_to_phys conversions.
> + */
> + for_each_of_range(, ) {
> + r->cpu_start = range.cpu_addr;
> + r->cpu_end = r->cpu_start + range.size - 1;
> + r->dma_start = range.bus_addr;
> + r->dma_end = r->dma_start + range.size - 1;
> + r->pfn_offset = PFN_DOWN(range.cpu_addr)
> + - PFN_DOWN(range.bus_addr);
> + r++;
> + }
> + return 0;
> +}
> +
> +
> +
> +/**
> + * attach_dma_pfn_offset - Assign scalar offset for all addresses.
> + * @dev: device pointer; only needed for a corner case.
> + * @dma_pfn_offset: offset to apply when converting from phys addr
^^^
This parameter name does not match.
> + * to dma addr and vice versa.
> + *
> + * It returns -ENOMEM if out of memory, otherwise 0.
It can also return -ENODEV. Why are we passing NULL dev pointers to
all these
Re: [PATCH] iommu/mediatek: Use totalram_pages to setup enable_4GB
On Thu, 2020-06-04 at 10:25 +0200, David Hildenbrand wrote: > On 04.06.20 10:01, Miles Chen wrote: > > To build this driver as a kernel module, we cannot use > > the unexported symbol "max_pfn" to setup enable_4GB. > > > > Use totalram_pages() instead to setup enable_4GB. > > > > Suggested-by: Mike Rapoport > > Signed-off-by: Miles Chen > > Cc: David Hildenbrand > > Cc: Yong Wu > > Cc: Chao Hao > > --- > > drivers/iommu/mtk_iommu.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > > index 5f4d6df59cf6..c2798a6e0e38 100644 > > --- a/drivers/iommu/mtk_iommu.c > > +++ b/drivers/iommu/mtk_iommu.c > > @@ -3,7 +3,6 @@ > > * Copyright (c) 2015-2016 MediaTek Inc. > > * Author: Yong Wu > > */ > > -#include > > #include > > #include > > #include > > @@ -626,8 +625,8 @@ static int mtk_iommu_probe(struct platform_device *pdev) > > return -ENOMEM; > > data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN); > > > > - /* Whether the current dram is over 4GB */ > > - data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT)); > > + /* Whether the current dram is over 4GB, note: DRAM start at 1GB */ > > + data->enable_4GB = !!(totalram_pages() > ((SZ_2G + SZ_1G) >> > > PAGE_SHIFT)); > > A similar thing seems to be done by > drivers/media/platform/mtk-vpu/mtk_vpu.c: > vpu->enable_4GB = !!(totalram_pages() > (SZ_2G >> PAGE_SHIFT)); > > I do wonder if some weird memory hotplug setups might give you false > negatives. > > E.g., start a VM with 1GB and hotplug 1GB - it will be hotplugged on > x86-64 above 4GB, turning max_pfn into 5GB. totalram_pages() should > return something < 2GB. > > Same can happen when you have a VM and use ballooning to fake-unplug > memory, making totalram_pages() return something < 4GB, but leaving > usable pfns >= 4GB Yes. Yingjoe also told me that this patch is not correct. Thanks for pointing this out. totalram_pages() does not work for some cases: e.g., DRAM start @0x4000_ and DRAM size is 0x1__ but we reserve large amount of memory, which makes totalram_pages() < 3GB but it is possible to allocate a pfn >= 4GB. I will discuss this internally. Miles > . > > but > ... I don't know if I understood what "enable_4GB" needs/implies > ... I don't know if this is applicable to VMs > at all (on real HW such > memory hotplug setups should not exist) > ... I don't know how this code would react to memory hotplug, so if the > condition changes after the driver loaded and enable_4GB would > suddenly apply. Again, most probably not relevant on real HW, only > for VMs. > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Check for deferred attach in iommu_group_do_dma_attach()
On Thu, Jun 04, 2020 at 11:19:44AM +0200, Joerg Roedel wrote:
> From: Joerg Roedel
>
> The iommu_group_do_dma_attach() must not attach devices which have
> deferred_attach set. Otherwise devices could cause IOMMU faults when
> re-initialized in a kdump kernel.
>
> Fixes: deac0b3bed26 ("iommu: Split off default domain allocation from group
> assignment")
> Reported-by: Jerry Snitselaar
> Reviewed-by: Jerry Snitselaar
> Tested-by: Jerry Snitselaar
> Signed-off-by: Joerg Roedel
> ---
> drivers/iommu/iommu.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index b5ea203f6c68..5a6d509f72b6 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1680,8 +1680,12 @@ static void probe_alloc_default_domain(struct bus_type
> *bus,
> static int iommu_group_do_dma_attach(struct device *dev, void *data)
> {
> struct iommu_domain *domain = data;
> + int ret = 0;
>
> - return __iommu_attach_device(domain, dev);
> + if (!iommu_is_attach_deferred(group->domain, dev))
> + ret = __iommu_attach_device(group->domain, dev);
And of course with the same bug as my original diff. Fixed that up
before applying it. (group->domain -> domain).
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: Check for deferred attach in iommu_group_do_dma_attach()
From: Joerg Roedel
The iommu_group_do_dma_attach() must not attach devices which have
deferred_attach set. Otherwise devices could cause IOMMU faults when
re-initialized in a kdump kernel.
Fixes: deac0b3bed26 ("iommu: Split off default domain allocation from group
assignment")
Reported-by: Jerry Snitselaar
Reviewed-by: Jerry Snitselaar
Tested-by: Jerry Snitselaar
Signed-off-by: Joerg Roedel
---
drivers/iommu/iommu.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b5ea203f6c68..5a6d509f72b6 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1680,8 +1680,12 @@ static void probe_alloc_default_domain(struct bus_type
*bus,
static int iommu_group_do_dma_attach(struct device *dev, void *data)
{
struct iommu_domain *domain = data;
+ int ret = 0;
- return __iommu_attach_device(domain, dev);
+ if (!iommu_is_attach_deferred(group->domain, dev))
+ ret = __iommu_attach_device(group->domain, dev);
+
+ return ret;
}
static int __iommu_group_dma_attach(struct iommu_group *group)
--
2.26.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Don't attach deferred device in iommu_group_do_dma_attach
Hi Jerry, On Thu, Jun 04, 2020 at 12:31:42AM -0700, Jerry Snitselaar wrote: > Attaching a deferred device should be delayed until dma api is called. > > Cc: iommu@lists.linux-foundation.org > Suggested-by: Joerg Roedel > Signed-off-by: Jerry Snitselaar > --- > If you already have thrown a patch together, then ignore this. Also > feel free to swap out the signed-off-by with your's since > this is more your patch than mine. You can put a reviewed-by > and tested-by instead for me. Yeah, I already have a patch, just not sent out yet. I updated it with your tags and will send it out shortly. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/mediatek: Use totalram_pages to setup enable_4GB
On 04.06.20 10:01, Miles Chen wrote: > To build this driver as a kernel module, we cannot use > the unexported symbol "max_pfn" to setup enable_4GB. > > Use totalram_pages() instead to setup enable_4GB. > > Suggested-by: Mike Rapoport > Signed-off-by: Miles Chen > Cc: David Hildenbrand > Cc: Yong Wu > Cc: Chao Hao > --- > drivers/iommu/mtk_iommu.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 5f4d6df59cf6..c2798a6e0e38 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -3,7 +3,6 @@ > * Copyright (c) 2015-2016 MediaTek Inc. > * Author: Yong Wu > */ > -#include > #include > #include > #include > @@ -626,8 +625,8 @@ static int mtk_iommu_probe(struct platform_device *pdev) > return -ENOMEM; > data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN); > > - /* Whether the current dram is over 4GB */ > - data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT)); > + /* Whether the current dram is over 4GB, note: DRAM start at 1GB */ > + data->enable_4GB = !!(totalram_pages() > ((SZ_2G + SZ_1G) >> > PAGE_SHIFT)); A similar thing seems to be done by drivers/media/platform/mtk-vpu/mtk_vpu.c: vpu->enable_4GB = !!(totalram_pages() > (SZ_2G >> PAGE_SHIFT)); I do wonder if some weird memory hotplug setups might give you false negatives. E.g., start a VM with 1GB and hotplug 1GB - it will be hotplugged on x86-64 above 4GB, turning max_pfn into 5GB. totalram_pages() should return something < 2GB. Same can happen when you have a VM and use ballooning to fake-unplug memory, making totalram_pages() return something < 4GB, but leaving usable pfns >= 4GB. but ... I don't know if I understood what "enable_4GB" needs/implies ... I don't know if this is applicable to VMs at all (on real HW such memory hotplug setups should not exist) ... I don't know how this code would react to memory hotplug, so if the condition changes after the driver loaded and enable_4GB would suddenly apply. Again, most probably not relevant on real HW, only for VMs. -- Thanks, David / dhildenb ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/mediatek: Use totalram_pages to setup enable_4GB
To build this driver as a kernel module, we cannot use the unexported symbol "max_pfn" to setup enable_4GB. Use totalram_pages() instead to setup enable_4GB. Suggested-by: Mike Rapoport Signed-off-by: Miles Chen Cc: David Hildenbrand Cc: Yong Wu Cc: Chao Hao --- drivers/iommu/mtk_iommu.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index 5f4d6df59cf6..c2798a6e0e38 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -3,7 +3,6 @@ * Copyright (c) 2015-2016 MediaTek Inc. * Author: Yong Wu */ -#include #include #include #include @@ -626,8 +625,8 @@ static int mtk_iommu_probe(struct platform_device *pdev) return -ENOMEM; data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN); - /* Whether the current dram is over 4GB */ - data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT)); + /* Whether the current dram is over 4GB, note: DRAM start at 1GB */ + data->enable_4GB = !!(totalram_pages() > ((SZ_2G + SZ_1G) >> PAGE_SHIFT)); if (!data->plat_data->has_4gb_mode) data->enable_4GB = false; -- 2.18.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: Don't attach deferred device in iommu_group_do_dma_attach
Attaching a deferred device should be delayed until dma api is called.
Cc: iommu@lists.linux-foundation.org
Suggested-by: Joerg Roedel
Signed-off-by: Jerry Snitselaar
---
If you already have thrown a patch together, then ignore this. Also
feel free to swap out the signed-off-by with your's since
this is more your patch than mine. You can put a reviewed-by
and tested-by instead for me.
drivers/iommu/iommu.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b5ea203f6c68..d43120eb1dc5 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1680,8 +1680,12 @@ static void probe_alloc_default_domain(struct bus_type
*bus,
static int iommu_group_do_dma_attach(struct device *dev, void *data)
{
struct iommu_domain *domain = data;
+ int ret = 0;
- return __iommu_attach_device(domain, dev);
+ if (!iommu_is_attach_deferred(domain, dev))
+ ret = __iommu_attach_device(domain, dev);
+
+ return ret;
}
static int __iommu_group_dma_attach(struct iommu_group *group)
--
2.24.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
