Re: [PATCH v3 0/5] enhance DMA CMA on x86
On 09/30/2014 07:45 PM, Thomas Gleixner wrote: > Whether the proposed patchset is the correct solution to support it is > a completely different question. This patchset has been in mainline since 3.16 and has already caused regressions, so the question of whether this is the correct solution has already been answered. > So either you stop this right now and help Akinobu to find the proper > solution If this is only a test platform for ARM parts then I don't think it unreasonable to suggest forking x86 swiotlb support into a iommu=cma selector that gets DMA mapping working for this test platform and doesn't cause a bunch of breakage. Which is different than if the plan is to ship production units for x86; then a general purpose solution will be required. As to the good design of a general purpose solution for allocating and mapping huge order pages, you are certainly more qualified to help Akinobu than I am. Regards, Peter Hurley ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 1/2] iopoll: Introduce memory-mapped IO polling macros
From: Matt Wagantall
It is sometimes necessary to poll a memory-mapped register until its value
satisfies some condition. Introduce a family of convenience macros that do
this. Tight-looping, sleeping, and timing out can all be accomplished using
these macros.
Cc: Thierry Reding
Cc: Will Deacon
Signed-off-by: Matt Wagantall
Signed-off-by: Mitchel Humpherys
---
Changes since v3:
- Updated commit message to better reflect the patch content
---
include/linux/iopoll.h | 77 ++
1 file changed, 77 insertions(+)
create mode 100644 include/linux/iopoll.h
diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
new file mode 100644
index 00..594b0d4f03
--- /dev/null
+++ b/include/linux/iopoll.h
@@ -0,0 +1,77 @@
+/*
+ * Copyright (c) 2012-2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _LINUX_IOPOLL_H
+#define _LINUX_IOPOLL_H
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * readl_poll_timeout - Periodically poll an address until a condition is met
or a timeout occurs
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @sleep_us: Maximum time to sleep between reads in us (0 tight-loops)
+ * @timeout_us: Timeout in us, 0 means never timeout
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
+ * case, the last read value at @addr is stored in @val. Must not
+ * be called from atomic context if sleep_us or timeout_us are used.
+ */
+#define readl_poll_timeout(addr, val, cond, sleep_us, timeout_us) \
+({ \
+ ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+ might_sleep_if(timeout_us); \
+ for (;;) { \
+ (val) = readl(addr); \
+ if (cond) \
+ break; \
+ if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+ (val) = readl(addr); \
+ break; \
+ } \
+ if (sleep_us) \
+ usleep_range(DIV_ROUND_UP(sleep_us, 4), sleep_us); \
+ } \
+ (cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**
+ * readl_poll_timeout_atomic - Periodically poll an address until a condition
is met or a timeout occurs
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @max_reads: Maximum number of reads before giving up
+ * @time_between_us: Time to udelay() between successive reads
+ *
+ * Returns 0 on success and -ETIMEDOUT upon a timeout.
+ */
+#define readl_poll_timeout_atomic(addr, val, cond, max_reads, time_between_us)
\
+({ \
+ int count; \
+ for (count = (max_reads); count > 0; count--) { \
+ (val) = readl(addr); \
+ if (cond) \
+ break; \
+ udelay(time_between_us); \
+ } \
+ (cond) ? 0 : -ETIMEDOUT; \
+})
+
+#endif /* _LINUX_IOPOLL_H */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/2] iommu/arm-smmu: add support for iova_to_phys through ATS1PR
On Tue, Sep 30 2014 at 03:23:34 AM, Will Deacon wrote:
> Hi Mitch,
>
> On Sun, Sep 28, 2014 at 04:27:29AM +0100, Mitchel Humpherys wrote:
>> Currently, we provide the iommu_ops.iova_to_phys service by doing a
>> table walk in software to translate IO virtual addresses to physical
>> addresses. On SMMUs that support it, it can be useful to ask the SMMU
>> itself to do the translation. This can be used to warm the TLBs for an
>> SMMU. It can also be useful for testing and hardware validation.
>>
>> Since the address translation registers are optional on SMMUv2, only
>> enable hardware translations when using SMMUv1 or when SMMU_IDR0.S1TS=1
>> and SMMU_IDR0.ATOSNS=0, as described in the ARM SMMU v1-v2 spec.
>
> [...]
>
>> +static phys_addr_t arm_smmu_iova_to_phys_hard(struct iommu_domain *domain,
>> +dma_addr_t iova)
>> +{
>> +struct arm_smmu_domain *smmu_domain = domain->priv;
>> +struct arm_smmu_device *smmu = smmu_domain->smmu;
>> +struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>> +struct device *dev = smmu->dev;
>> +void __iomem *cb_base;
>> +u32 tmp;
>> +u64 phys;
>> +
>> +cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
>> +
>> +if (smmu->version == 1) {
>> +u32 reg = iova & ~0xFFF;
>
> Cosmetic comment, but hex constants are lowercase everywhere else in the
> file.
Ah, woops. Let me fix that.
>
>> +writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
>> +} else {
>> +u32 reg = iova & ~0xFFF;
>> +writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
>> +reg = (iova & ~0xFFF) >> 32;
>> +writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_HI);
>> +}
>> +
>> +if (readl_poll_timeout(cb_base + ARM_SMMU_CB_ATSR, tmp,
>> +!(tmp & ATSR_ACTIVE), 10, ATSR_LOOP_TIMEOUT)) {
>> +dev_err(dev,
>> +"iova to phys timed out on 0x%pa. Falling back to
>> software table walk.\n",
>> +&iova);
>> +return arm_smmu_iova_to_phys_soft(domain, iova);
>> +}
>> +
>> +phys = readl_relaxed(cb_base + ARM_SMMU_CB_PAR_LO);
>> +phys |= ((u64) readl_relaxed(cb_base + ARM_SMMU_CB_PAR_HI)) << 32;
>
> The absence of locking in this function concerns me a bit. For the software
> implementation, we're just reading page tables, but here we're writing ATS
> registers and I think we need to ensure serialisation against another
> iova_to_phys on the same domain.
Good catch, let me take the domain lock here. I'll also have to move to
readl_poll_timeout_atomic since the domain lock is a spinlock.
>
>> +if (phys & CB_PAR_F) {
>> +dev_err(dev, "translation fault!\n");
>> +dev_err(dev, "PAR = 0x%llx\n", phys);
>> +}
>> +phys = (phys & 0xFFF000ULL) | (iova & 0x0FFF);
>> +
>> +return phys;
>
> You can return phys == 0 on failure (at least, the callers in kvm and vfio
> treat this as an error).
Ah yes, I agree that a 0 return value from iommu_iova_to_phys appears to
be treated as an error. Let me fix that.
-Mitch
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 2/2] iommu/arm-smmu: add support for iova_to_phys through ATS1PR
Currently, we provide the iommu_ops.iova_to_phys service by doing a
table walk in software to translate IO virtual addresses to physical
addresses. On SMMUs that support it, it can be useful to ask the SMMU
itself to do the translation. This can be used to warm the TLBs for an
SMMU. It can also be useful for testing and hardware validation.
Since the address translation registers are optional on SMMUv2, only
enable hardware translations when using SMMUv1 or when SMMU_IDR0.S1TS=1
and SMMU_IDR0.ATOSNS=0, as described in the ARM SMMU v1-v2 spec.
Signed-off-by: Mitchel Humpherys
---
Changes since v3:
- Added locking around address translation op
- Return 0 on iova_to_phys failure
---
drivers/iommu/arm-smmu.c | 79 +++-
1 file changed, 78 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 37dc3dd0df..c80c12a104 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -140,6 +141,7 @@
#define ID0_S2TS (1 << 29)
#define ID0_NTS(1 << 28)
#define ID0_SMS(1 << 27)
+#define ID0_ATOSNS (1 << 26)
#define ID0_PTFS_SHIFT 24
#define ID0_PTFS_MASK 0x2
#define ID0_PTFS_V8_ONLY 0x2
@@ -233,11 +235,16 @@
#define ARM_SMMU_CB_TTBR0_HI 0x24
#define ARM_SMMU_CB_TTBCR 0x30
#define ARM_SMMU_CB_S1_MAIR0 0x38
+#define ARM_SMMU_CB_PAR_LO 0x50
+#define ARM_SMMU_CB_PAR_HI 0x54
#define ARM_SMMU_CB_FSR0x58
#define ARM_SMMU_CB_FAR_LO 0x60
#define ARM_SMMU_CB_FAR_HI 0x64
#define ARM_SMMU_CB_FSYNR0 0x68
#define ARM_SMMU_CB_S1_TLBIASID0x610
+#define ARM_SMMU_CB_ATS1PR_LO 0x800
+#define ARM_SMMU_CB_ATS1PR_HI 0x804
+#define ARM_SMMU_CB_ATSR 0x8f0
#define SCTLR_S1_ASIDPNE (1 << 12)
#define SCTLR_CFCFG(1 << 7)
@@ -249,6 +256,10 @@
#define SCTLR_M(1 << 0)
#define SCTLR_EAE_SBOP (SCTLR_AFE | SCTLR_TRE)
+#define CB_PAR_F (1 << 0)
+
+#define ATSR_ACTIVE(1 << 0)
+
#define RESUME_RETRY (0 << 0)
#define RESUME_TERMINATE (1 << 0)
@@ -366,6 +377,7 @@ struct arm_smmu_device {
#define ARM_SMMU_FEAT_TRANS_S1 (1 << 2)
#define ARM_SMMU_FEAT_TRANS_S2 (1 << 3)
#define ARM_SMMU_FEAT_TRANS_NESTED (1 << 4)
+#define ARM_SMMU_FEAT_TRANS_OPS(1 << 5)
u32 features;
#define ARM_SMMU_OPT_SECURE_CFG_ACCESS (1 << 0)
@@ -1524,7 +1536,7 @@ static size_t arm_smmu_unmap(struct iommu_domain *domain,
unsigned long iova,
return ret ? 0 : size;
}
-static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain,
+static phys_addr_t arm_smmu_iova_to_phys_soft(struct iommu_domain *domain,
dma_addr_t iova)
{
pgd_t *pgdp, pgd;
@@ -1557,6 +1569,66 @@ static phys_addr_t arm_smmu_iova_to_phys(struct
iommu_domain *domain,
return __pfn_to_phys(pte_pfn(pte)) | (iova & ~PAGE_MASK);
}
+static phys_addr_t arm_smmu_iova_to_phys_hard(struct iommu_domain *domain,
+ dma_addr_t iova)
+{
+ struct arm_smmu_domain *smmu_domain = domain->priv;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
+ struct device *dev = smmu->dev;
+ void __iomem *cb_base;
+ u32 tmp;
+ u64 phys;
+ unsigned long flags;
+
+ cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
+
+ spin_lock_irqsave(&smmu_domain->lock, flags);
+
+ if (smmu->version == 1) {
+ u32 reg = iova & ~0xfff;
+ writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
+ } else {
+ u32 reg = iova & ~0xfff;
+ writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
+ reg = (iova & ~0xfff) >> 32;
+ writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_HI);
+ }
+
+ if (readl_poll_timeout_atomic(cb_base + ARM_SMMU_CB_ATSR, tmp,
+ !(tmp & ATSR_ACTIVE), 50, 100)) {
+ dev_err(dev,
+ "iova to phys timed out on 0x%pa. Falling back to
software table walk.\n",
+ &iova);
+ return arm_smmu_iova_to_phys_soft(domain, iova);
+ }
+
+ phys = readl_relaxed(cb_base + ARM_SMMU_CB_PAR_LO);
+ phys |= ((u64) readl_relaxed(cb_base + ARM_SMMU_CB_PAR_HI)) << 32;
+
+ spin_unlock_irqrestore(&smmu_domain->lock, flags);
+
+ if (phys & CB_PAR_F) {
+
[PATCH v4 0/2] iommu/arm-smmu: hard iova_to_phys
This series introduces support for performing iova-to-phys translations via the ARM SMMU hardware on supported implementations. We also make use of some new generic macros for polling hardware registers. v1..v2: - Renamed one of the iopoll macros to use the more standard `_atomic' suffix - Removed some convenience iopoll wrappers to encourage explicitness v2..v3: - Removed unnecessary `dev_name's v3..v4: - Updated the iopoll commit message to reflect the patch better - Added locking around address translation op - Return 0 on iova_to_phys failure Matt Wagantall (1): iopoll: Introduce memory-mapped IO polling macros Mitchel Humpherys (1): iommu/arm-smmu: add support for iova_to_phys through ATS1PR drivers/iommu/arm-smmu.c | 79 +++- include/linux/iopoll.h | 77 ++ 2 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 include/linux/iopoll.h -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/5] enhance DMA CMA on x86
On 09/30/2014 07:45 PM, Thomas Gleixner wrote: > On Tue, 30 Sep 2014, Peter Hurley wrote: >> I read the UFS Unified Memory Extension v1.0 (JESD220-1) specification and >> it is not clear to me that using DMA mapping is the right approach to >> supporting UM, at least on x86. >> >> And without a mainline user, the merits of this approach are not evident. >> I cannot even find a production x86 UFS controller, much less one that >> supports UME. >> >> The only PCI UFS controller I could find (and that mainline supports) is >> Samsung's x86 FPGA-based test unit for developing UFS devices in a x86 test >> environment, and not a production x86 design. > > And how is that relevant? That device exists and you have no reason to > deny it to be supported just because you are not interested in it. > >> Unless there's something else I've missed, I don't think these patches >> belong in mainline. > > You missed that there is no reason WHY such a device should not be > supported in mainline. Mainline already supports this card right now without these patches. >> Samsung's own roadmap >> (http://www.slideshare.net/linaroorg/next-gen-mobilestorageufs) >> mentions nothing about bringing UFS to x86 designs. > > And that's telling you what? > >- That we should deny Samsung proper support for their obviously > x86 based test card > >- That we should ignore a JEDEC Standard which is obviously never > going to hit x86 land just because you decide it? > > Your argumentation is just ass backwards. Linux wants to support the > full zoo of hardware including this particular PCI card. Period. > > Whether the proposed patchset is the correct solution to support it is > a completely different question. And there is currently no way to determine that because there is no user in mainline that requires this support. Which you would understand if you had read more carefully. Regards, Peter Hurley ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/5] enhance DMA CMA on x86
On Tue, 30 Sep 2014, Peter Hurley wrote: > I read the UFS Unified Memory Extension v1.0 (JESD220-1) specification and > it is not clear to me that using DMA mapping is the right approach to > supporting UM, at least on x86. > > And without a mainline user, the merits of this approach are not evident. > I cannot even find a production x86 UFS controller, much less one that > supports UME. > > The only PCI UFS controller I could find (and that mainline supports) is > Samsung's x86 FPGA-based test unit for developing UFS devices in a x86 test > environment, and not a production x86 design. And how is that relevant? That device exists and you have no reason to deny it to be supported just because you are not interested in it. > Unless there's something else I've missed, I don't think these patches > belong in mainline. You missed that there is no reason WHY such a device should not be supported in mainline. > Samsung's own roadmap > (http://www.slideshare.net/linaroorg/next-gen-mobilestorageufs) > mentions nothing about bringing UFS to x86 designs. And that's telling you what? - That we should deny Samsung proper support for their obviously x86 based test card - That we should ignore a JEDEC Standard which is obviously never going to hit x86 land just because you decide it? Your argumentation is just ass backwards. Linux wants to support the full zoo of hardware including this particular PCI card. Period. Whether the proposed patchset is the correct solution to support it is a completely different question. So either you stop this right now and help Akinobu to find the proper solution or you just go back in your uncontaminated x86 cave and STFU. Thanks, tglx ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/5] enhance DMA CMA on x86
2014-09-30 23:34 GMT+09:00 Peter Hurley : > On 09/29/2014 10:32 AM, Akinobu Mita wrote: >> 2014-09-29 21:09 GMT+09:00 Peter Hurley : >>> On 09/27/2014 08:31 PM, Akinobu Mita wrote: 2014-09-27 23:30 GMT+09:00 Peter Hurley : > On 04/15/2014 09:08 AM, Akinobu Mita wrote: >> This patch set enhances the DMA Contiguous Memory Allocator on x86. >>> >>> [...] >>> > What this patchset does is restrict all iommu configurations which can > map all of system memory to one _very_ small physical region, thus > disabling > the whole point of an iommu. > > Now I know why my GPU is causing paging to disk! And why my RAID > controller > stalls for ages when I do a git log at the same time as a kernel build! The solution I have for this is that instead of trying to dma_alloc_from_contiguous() firstly, call alloc_pages() in dma_alloc_coherent(). dma_alloc_from_contiguous() should be called only when alloc_pages() is failed or DMA_ATTR_FORCE_CONTIGUOUS is specified in dma_attr. >>> >>> Why is all this extra complexity being added when there are no X86 users >>> of DMA_ATTR_FORCE_CONTIGUOUS? >> >> I misunderstood DMA_ATTR_FORCE_CONTIGUOUS. It is specified to request >> that underlaying DMA mapping span physically contiguous with IOMMU. >> But current alloc_dma_coherent() for intel-iommu always returns >> physically contiguous memory, so it is ignored on x86. >> > And the apparent goal of this patchset is to enable DMA allocation below > 4GB, which is already supported in the existing page allocator with the > GFP_DMA32 flag?! The goal of this patchset is to enable huge DMA allocation which alloc_pages() can't (> MAX_ORDER) for the devices that require it. >>> >>> What x86 devices need > MAX_ORDER DMA allocation and why can't they allocate >>> directly from dma_alloc_from_contiguous()? >> >> I need this for UFS unified memory extension which is apparently not in >> mainline for now. >> http://www.jedec.org/standards-documents/docs/jesd220-1 >> http://www.jedec.org/sites/default/files/T_Fujisawa_MF_2013.pdf >> >> But there must be some other use cases on x86, too. Because I have >> received several emails privately from developers who care its status. >> >> And allocating directly from dma_alloc_from_contiguous() in the driver >> doesn't work with IOMMU, as it just returns memory regoin and doesn't >> create DMA mapping. > > > I read the UFS Unified Memory Extension v1.0 (JESD220-1) specification and > it is not clear to me that using DMA mapping is the right approach to > supporting UM, at least on x86. Without DMA mapping, there is no way for the devices to access host memory. Unified memory extension requires a single contiguous memory region instead of multiple scattered mapping. > And without a mainline user, the merits of this approach are not evident. > I cannot even find a production x86 UFS controller, much less one that > supports UME. > > The only PCI UFS controller I could find (and that mainline supports) is > Samsung's x86 FPGA-based test unit for developing UFS devices in a x86 test > environment, and not a production x86 design. > > Samsung's own roadmap > (http://www.slideshare.net/linaroorg/next-gen-mobilestorageufs) > mentions nothing about bringing UFS to x86 designs. > > Unless there's something else I've missed, I don't think these patches > belong in mainline. Removing CONFIG_CMA_DMA support from x86_64 will disappoint me, but it's personal opinion. FWIW, MIPS also starts supporting it in linux-next. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 16/17] iommu/omap: Fix bus error on debugfs access of unattached IOMMU
Any debugfs access on an OMAP IOMMU that is not enabled (done during
attach) results in a bus error due to access of registers without
the clock or the reset enabled for the respective IOMMU. So, add a
check to make sure the IOMMU is enabled/attached by a client device.
This gracefully prints a "Operation not permitted" trace when the
corresponding IOMMU is not enabled.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu-debug.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c
index 54a0dc6..e7b8aca 100644
--- a/drivers/iommu/omap-iommu-debug.c
+++ b/drivers/iommu/omap-iommu-debug.c
@@ -24,6 +24,11 @@ static DEFINE_MUTEX(iommu_debug_lock);
static struct dentry *iommu_debug_root;
+static inline bool is_omap_iommu_detached(struct omap_iommu *obj)
+{
+ return !obj->domain;
+}
+
static ssize_t debug_read_regs(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
{
@@ -31,6 +36,9 @@ static ssize_t debug_read_regs(struct file *file, char __user
*userbuf,
char *p, *buf;
ssize_t bytes;
+ if (is_omap_iommu_detached(obj))
+ return -EPERM;
+
buf = kmalloc(count, GFP_KERNEL);
if (!buf)
return -ENOMEM;
@@ -54,6 +62,9 @@ static ssize_t debug_read_tlb(struct file *file, char __user
*userbuf,
char *p, *buf;
ssize_t bytes, rest;
+ if (is_omap_iommu_detached(obj))
+ return -EPERM;
+
buf = kmalloc(count, GFP_KERNEL);
if (!buf)
return -ENOMEM;
@@ -139,6 +150,9 @@ static ssize_t debug_read_pagetable(struct file *file, char
__user *userbuf,
char *p, *buf;
size_t bytes;
+ if (is_omap_iommu_detached(obj))
+ return -EPERM;
+
buf = (char *)__get_free_page(GFP_KERNEL);
if (!buf)
return -ENOMEM;
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 09/17] iommu/omap: Consolidate OMAP IOMMU modules
The OMAP IOMMU driver was originally designed as modules, and split
into a core module and a thin arch-specific module through the OMAP
arch-specific struct iommu_functions, to scale for both OMAP1 and
OMAP2+ IOMMU variants. The driver can only be built for OMAP2+
platforms currently, and also can only be built-in after the
adaptation to generic IOMMU API. The OMAP1 variant was never added
and will most probably be never added (the code for the only potential
user, its parent, DSP processor has already been cleaned up). So,
consolidate the OMAP2 specific omap-iommu2 module into the core OMAP
IOMMU driver - this eliminates the arch-specific ops structure and
simplifies the driver into a single module that only implements the
generic IOMMU API's iommu_ops.
The following are the main changes:
- omap-iommu2 module is completely eliminated, with the common
definitions moved to the internal omap-iommu.h, and the ops
implementations moved into omap-iommu.c
- OMAP arch-specific struct iommu_functions is also eliminated,
with the ops implementations directly absorbed into the calling
functions
- iotlb_alloc_cr() is no longer inlined and defined only when
PREFETCH_IOTLB is defined
- iotlb_dump_cr() is similarly defined only when CONFIG_OMAP_IOMMU_DEBUG
is defined
- Elimination of the OMAP IOMMU exported functions to register the
arch ops, omap_install_iommu_arch() & omap_uninstall_iommu_arch()
- Any stale comments about OMAP1 are also cleaned up
Signed-off-by: Suman Anna
---
drivers/iommu/Makefile | 1 -
drivers/iommu/omap-iommu.c | 263 ++---
drivers/iommu/omap-iommu.h | 61 +
drivers/iommu/omap-iommu2.c | 311
4 files changed, 217 insertions(+), 419 deletions(-)
delete mode 100644 drivers/iommu/omap-iommu2.c
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 16edef7..18fa446 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -11,7 +11,6 @@ obj-$(CONFIG_INTEL_IOMMU) += iova.o intel-iommu.o
obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o
obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o
-obj-$(CONFIG_OMAP_IOMMU) += omap-iommu2.o
obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o
obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o
obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 332bfb9..9ace5557 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -76,53 +76,23 @@ struct iotlb_lock {
short vict;
};
-/* accommodate the difference between omap1 and omap2/3 */
-static const struct iommu_functions *arch_iommu;
-
static struct platform_driver omap_iommu_driver;
static struct kmem_cache *iopte_cachep;
/**
- * omap_install_iommu_arch - Install archtecure specific iommu functions
- * @ops: a pointer to architecture specific iommu functions
- *
- * There are several kind of iommu algorithm(tlb, pagetable) among
- * omap series. This interface installs such an iommu algorighm.
- **/
-int omap_install_iommu_arch(const struct iommu_functions *ops)
-{
- if (arch_iommu)
- return -EBUSY;
-
- arch_iommu = ops;
- return 0;
-}
-EXPORT_SYMBOL_GPL(omap_install_iommu_arch);
-
-/**
- * omap_uninstall_iommu_arch - Uninstall archtecure specific iommu functions
- * @ops: a pointer to architecture specific iommu functions
- *
- * This interface uninstalls the iommu algorighm installed previously.
- **/
-void omap_uninstall_iommu_arch(const struct iommu_functions *ops)
-{
- if (arch_iommu != ops)
- pr_err("%s: not your arch\n", __func__);
-
- arch_iommu = NULL;
-}
-EXPORT_SYMBOL_GPL(omap_uninstall_iommu_arch);
-
-/**
* omap_iommu_save_ctx - Save registers for pm off-mode support
* @dev: client device
**/
void omap_iommu_save_ctx(struct device *dev)
{
struct omap_iommu *obj = dev_to_omap_iommu(dev);
+ u32 *p = obj->ctx;
+ int i;
- arch_iommu->save_ctx(obj);
+ for (i = 0; i < (MMU_REG_SIZE / sizeof(u32)); i++) {
+ p[i] = iommu_read_reg(obj, i * sizeof(u32));
+ dev_dbg(obj->dev, "%s\t[%02d] %08x\n", __func__, i, p[i]);
+ }
}
EXPORT_SYMBOL_GPL(omap_iommu_save_ctx);
@@ -133,20 +103,75 @@ EXPORT_SYMBOL_GPL(omap_iommu_save_ctx);
void omap_iommu_restore_ctx(struct device *dev)
{
struct omap_iommu *obj = dev_to_omap_iommu(dev);
+ u32 *p = obj->ctx;
+ int i;
- arch_iommu->restore_ctx(obj);
+ for (i = 0; i < (MMU_REG_SIZE / sizeof(u32)); i++) {
+ iommu_write_reg(obj, p[i], i * sizeof(u32));
+ dev_dbg(obj->dev, "%s\t[%02d] %08x\n", __func__, i, p[i]);
+ }
}
EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx);
+static void __iommu_set_twl(struct omap_iommu *obj, bool on)
+{
+ u32 l = iommu_read_reg(obj, MMU_CNTL);
+
+ if
[PATCH 02/17] iommu/omap: Remove unused isr_priv field from omap_iommu
The isr_priv field is a left-over from before the IOMMU API
adaptation, this was used to store the callback data. This is
no longer relevant, so remove it.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 5c14000..18a0f3a 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -30,7 +30,6 @@ struct omap_iommu {
const char *name;
void __iomem*regbase;
struct device *dev;
- void*isr_priv;
struct iommu_domain *domain;
spinlock_t iommu_lock; /* global for this whole object */
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 13/17] iommu/omap: Remove couple of unused exported functions
The exported functions omap_foreach_iommu_device() and
omap_iotlb_cr_to_e() have been deleted, as they are no
longer needed.
The function omap_foreach_iommu_device() is not required
after the consolidation of the OMAP IOMMU debug module,
and the function omap_iotlb_cr_to_e() is not required
after making the debugfs entry 'pagetable' read-only.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu.c | 21 -
drivers/iommu/omap-iommu.h | 5 -
2 files changed, 26 deletions(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 717d654..b3bc087 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -203,20 +203,6 @@ static void iommu_disable(struct omap_iommu *obj)
/*
* TLB operations
*/
-void omap_iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e)
-{
- BUG_ON(!cr || !e);
-
- e->da = cr->cam & MMU_CAM_VATAG_MASK;
- e->pa = cr->ram & MMU_RAM_PADDR_MASK;
- e->valid= cr->cam & MMU_CAM_V;
- e->pgsz = cr->cam & MMU_CAM_PGSZ_MASK;
- e->endian = cr->ram & MMU_RAM_ENDIAN_MASK;
- e->elsz = cr->ram & MMU_RAM_ELSZ_MASK;
- e->mixed= cr->ram & MMU_RAM_MIXED;
-}
-EXPORT_SYMBOL_GPL(omap_iotlb_cr_to_e);
-
static inline int iotlb_cr_valid(struct cr_regs *cr)
{
if (!cr)
@@ -593,13 +579,6 @@ size_t omap_dump_tlb_entries(struct omap_iommu *obj, char
*buf, ssize_t bytes)
}
EXPORT_SYMBOL_GPL(omap_dump_tlb_entries);
-int omap_foreach_iommu_device(void *data, int (*fn)(struct device *, void *))
-{
- return driver_for_each_device(&omap_iommu_driver.driver,
- NULL, data, fn);
-}
-EXPORT_SYMBOL_GPL(omap_foreach_iommu_device);
-
/*
* H/W pagetable operations
*/
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 6e9a8d2..2659139 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -190,14 +190,9 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct
device *dev)
/*
* global functions
*/
-extern void omap_iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e);
-
extern int
omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e);
-extern int omap_foreach_iommu_device(void *data,
- int (*fn)(struct device *, void *));
-
extern ssize_t
omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t len);
extern size_t
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 08/17] iommu/omap: Simplify omap2_iommu_fault_isr()
The function omap2_iommu_fault_isr() does an unnecessary
recomputation of the return value. The logic relies on
setting the same bit fields as the MMU fault error status
bits, so simplify this function and remove the unneeded
macros. These macros were originally exported to notify
MMU faults to users prior to the IOMMU framework adaptation,
but are now redundant.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu2.c | 20 +---
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/drivers/iommu/omap-iommu2.c b/drivers/iommu/omap-iommu2.c
index 372141b..ce2fff3 100644
--- a/drivers/iommu/omap-iommu2.c
+++ b/drivers/iommu/omap-iommu2.c
@@ -53,13 +53,6 @@
((pgsz) == MMU_CAM_PGSZ_64K) ? 0x :\
((pgsz) == MMU_CAM_PGSZ_4K) ? 0xf000 : 0)
-/* IOMMU errors */
-#define OMAP_IOMMU_ERR_TLB_MISS(1 << 0)
-#define OMAP_IOMMU_ERR_TRANS_FAULT (1 << 1)
-#define OMAP_IOMMU_ERR_EMU_MISS(1 << 2)
-#define OMAP_IOMMU_ERR_TBLWALK_FAULT (1 << 3)
-#define OMAP_IOMMU_ERR_MULTIHIT_FAULT (1 << 4)
-
static void __iommu_set_twl(struct omap_iommu *obj, bool on)
{
u32 l = iommu_read_reg(obj, MMU_CNTL);
@@ -122,7 +115,6 @@ static void omap2_iommu_set_twl(struct omap_iommu *obj,
bool on)
static u32 omap2_iommu_fault_isr(struct omap_iommu *obj, u32 *ra)
{
u32 stat, da;
- u32 errs = 0;
stat = iommu_read_reg(obj, MMU_IRQSTATUS);
stat &= MMU_IRQ_MASK;
@@ -134,19 +126,9 @@ static u32 omap2_iommu_fault_isr(struct omap_iommu *obj,
u32 *ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- if (stat & MMU_IRQ_TLBMISS)
- errs |= OMAP_IOMMU_ERR_TLB_MISS;
- if (stat & MMU_IRQ_TRANSLATIONFAULT)
- errs |= OMAP_IOMMU_ERR_TRANS_FAULT;
- if (stat & MMU_IRQ_EMUMISS)
- errs |= OMAP_IOMMU_ERR_EMU_MISS;
- if (stat & MMU_IRQ_TABLEWALKFAULT)
- errs |= OMAP_IOMMU_ERR_TBLWALK_FAULT;
- if (stat & MMU_IRQ_MULTIHITFAULT)
- errs |= OMAP_IOMMU_ERR_MULTIHIT_FAULT;
iommu_write_reg(obj, stat, MMU_IRQSTATUS);
- return errs;
+ return stat;
}
static void omap2_tlb_read_cr(struct omap_iommu *obj, struct cr_regs *cr)
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 12/17] iommu/omap: Integrate omap-iommu-debug into omap-iommu
The debugfs support for OMAP IOMMU is currently implemented
as a module, warranting certain OMAP-specific IOMMU API to
be exported. The OMAP IOMMU, when enabled, can only be built-in
into the kernel, so integrate the OMAP IOMMU debug module
into the OMAP IOMMU driver. This helps in eliminating the
need to export most of the current OMAP IOMMU API.
The following are the main changes:
- The OMAP_IOMMU_DEBUG Kconfig option is eliminated, and
the OMAP IOMMU debugfs support is built alongside the OMAP
IOMMU driver, and enabled automatically only when debugfs
is enabled.
- The debugfs directory and entry creation logic is reversed,
the calls are invoked by the OMAP IOMMU driver now.
- The current iffy circular logic of adding IOMMU archdata
to the IOMMU devices itself to get a pointer to the omap_iommu
object in the debugfs support code is replaced by directly
using the omap_iommu structure while creating the debugfs
entries.
- The debugfs root directory is renamed from the generic name
"iommu" to a specific name "omap_iommu".
- Unneeded headers have also been cleaned up while at this.
- There will no longer be a omap-iommu-debug.ko module after
this patch.
Signed-off-by: Suman Anna
---
drivers/iommu/Kconfig| 9
drivers/iommu/Makefile | 3 +-
drivers/iommu/omap-iommu-debug.c | 102 ---
drivers/iommu/omap-iommu.c | 11 +++--
drivers/iommu/omap-iommu.h | 7 +++
5 files changed, 45 insertions(+), 87 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index dd51122..9724d4a 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -143,15 +143,6 @@ config OMAP_IOMMU
depends on ARCH_OMAP2PLUS
select IOMMU_API
-config OMAP_IOMMU_DEBUG
- tristate "Export OMAP IOMMU internals in DebugFS"
- depends on OMAP_IOMMU && DEBUG_FS
- help
- Select this to see extensive information about
- the internal state of OMAP IOMMU in debugfs.
-
- Say N unless you know you need this.
-
config TEGRA_IOMMU_GART
bool "Tegra GART IOMMU Support"
depends on ARCH_TEGRA_2x_SOC
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 18fa446..e0a4fed 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -10,8 +10,7 @@ obj-$(CONFIG_DMAR_TABLE) += dmar.o
obj-$(CONFIG_INTEL_IOMMU) += iova.o intel-iommu.o
obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o
-obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o
-obj-$(CONFIG_OMAP_IOMMU_DEBUG) += omap-iommu-debug.o
+obj-$(CONFIG_OMAP_IOMMU) += omap-iommu.o omap-iommu-debug.o
obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o
obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o
obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c
index 28de657..54a0dc6 100644
--- a/drivers/iommu/omap-iommu-debug.c
+++ b/drivers/iommu/omap-iommu-debug.c
@@ -10,15 +10,11 @@
* published by the Free Software Foundation.
*/
-#include
#include
-#include
#include
#include
#include
-#include
#include
-#include
#include
#include "omap-iopgtable.h"
@@ -31,8 +27,7 @@ static struct dentry *iommu_debug_root;
static ssize_t debug_read_regs(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
{
- struct device *dev = file->private_data;
- struct omap_iommu *obj = dev_to_omap_iommu(dev);
+ struct omap_iommu *obj = file->private_data;
char *p, *buf;
ssize_t bytes;
@@ -55,8 +50,7 @@ static ssize_t debug_read_regs(struct file *file, char __user
*userbuf,
static ssize_t debug_read_tlb(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
{
- struct device *dev = file->private_data;
- struct omap_iommu *obj = dev_to_omap_iommu(dev);
+ struct omap_iommu *obj = file->private_data;
char *p, *buf;
ssize_t bytes, rest;
@@ -141,8 +135,7 @@ out:
static ssize_t debug_read_pagetable(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
{
- struct device *dev = file->private_data;
- struct omap_iommu *obj = dev_to_omap_iommu(dev);
+ struct omap_iommu *obj = file->private_data;
char *p, *buf;
size_t bytes;
@@ -181,93 +174,58 @@ DEBUG_FOPS_RO(pagetable);
#define __DEBUG_ADD_FILE(attr, mode) \
{ \
struct dentry *dent;\
- dent = debugfs_create_file(#attr, mode, parent, \
- dev, &debug_##attr##_fops); \
+ dent = debugfs_create_file(#attr, mode, obj->debug_dir, \
+ obj,
[PATCH 00/17] OMAP IOMMU Cleanup & Consolidation
Hi, The OMAP IOMMU driver is currently designed as three different pieces - a core OMAP IOMMU driver that implements the generic IOMMU API ops, a OMAP arch-specific layer that implements the OMAP specific arch iommu_functions, and an independent debugfs module. The former two are always built-in, while the last one is tristate. The driver supports OMAP2+ SoCs only, and this series performs a whole lot of cleanup and consolidates all the above pieces into a single built-in driver. The OMAP arch-specific layer is eliminated completely (absorbed into the core OMAP driver), and the debugfs portion is still maintained as a separate file, but not a separate module anymore. This cleanup and consolidation eliminates most of the currently exported functions from the OMAP IOMMU driver. The series also includes couple of fixes to the OMAP IOMMU debugfs entries - permissions on one debugfs entry, eliminate bus errors on debugfs access on disabled IOMMUs; and one enhancement to list all valid page table entries instead of listing only a page worth of data. The patches are baselined on 3.17-rc3 and on top of the OMAP IOMMU patches staged for 3.18 [1]; a reference branch with all these patches is available is hosted at [2]. I am posting this series for review with the intention to have them make it to 3.19, so I can repost once rc1 is out if needed. [1] http://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/log/?h=arm/omap [2] https://github.com/sumananna/omap-kernel/commits/iommu/submit/3.17-rc3-cleanup-consolidation regards Suman Suman Anna (17): iommu/omap: Remove refcount field from omap_iommu object iommu/omap: Remove unused isr_priv field from omap_iommu iommu/omap: Remove duplicate declarations iommu/omap: Remove conditional definition of dev_to_omap_iommu() iommu/omap: Remove ver debugfs entry iommu/omap: Remove omap_iommu_arch_version() and version field iommu/omap: Remove bogus version check in context save/restore iommu/omap: Simplify omap2_iommu_fault_isr() iommu/omap: Consolidate OMAP IOMMU modules iommu/omap: Fix the permissions on nr_tlb_entries iommu/omap: Make pagetable debugfs entry read-only iommu/omap: Integrate omap-iommu-debug into omap-iommu iommu/omap: Remove couple of unused exported functions iommu/omap: Do not export unneeded functions iommu/omap: Reset the domain field upon detaching iommu/omap: Fix bus error on debugfs access of unattached IOMMU iommu/omap: Switch pagetable debugfs entry to use seq_file drivers/iommu/Kconfig| 9 -- drivers/iommu/Makefile | 4 +- drivers/iommu/omap-iommu-debug.c | 244 drivers/iommu/omap-iommu.c | 304 ++- drivers/iommu/omap-iommu.h | 90 +-- drivers/iommu/omap-iommu2.c | 337 --- 6 files changed, 298 insertions(+), 690 deletions(-) delete mode 100644 drivers/iommu/omap-iommu2.c -- 2.1.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 05/17] iommu/omap: Remove ver debugfs entry
The debugfs entry 'ver' to read the OMAP IOMMU version is
not much useful for developers, so it has been removed. The
same can be deduced from the register dump, provided by the
debugfs entry 'regs', REVISION register. This also allows us
to remove the omap_iommu_arch_revision() which is currently
returning a fixed value.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu-debug.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c
index 531658d..0fb92aa 100644
--- a/drivers/iommu/omap-iommu-debug.c
+++ b/drivers/iommu/omap-iommu-debug.c
@@ -30,17 +30,6 @@ static DEFINE_MUTEX(iommu_debug_lock);
static struct dentry *iommu_debug_root;
-static ssize_t debug_read_ver(struct file *file, char __user *userbuf,
- size_t count, loff_t *ppos)
-{
- u32 ver = omap_iommu_arch_version();
- char buf[MAXCOLUMN], *p = buf;
-
- p += sprintf(p, "H/W version: %d.%d\n", (ver >> 4) & 0xf , ver & 0xf);
-
- return simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
-}
-
static ssize_t debug_read_regs(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
{
@@ -228,7 +217,6 @@ static ssize_t debug_read_pagetable(struct file *file, char
__user *userbuf,
.llseek = generic_file_llseek, \
};
-DEBUG_FOPS_RO(ver);
DEBUG_FOPS_RO(regs);
DEBUG_FOPS_RO(tlb);
DEBUG_FOPS(pagetable);
@@ -273,7 +261,6 @@ static int iommu_debug_register(struct device *dev, void
*data)
if (!d)
goto nomem;
- DEBUG_ADD_FILE_RO(ver);
DEBUG_ADD_FILE_RO(regs);
DEBUG_ADD_FILE_RO(tlb);
DEBUG_ADD_FILE(pagetable);
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 15/17] iommu/omap: Reset the domain field upon detaching
The .domain field in omap_iommu struct is set properly when the OMAP IOMMU device is attached to, but is never reset properly on detach. Reset this properly so that the OMAP IOMMU debugfs logic can depend on this field before allowing the debugfs operations. Signed-off-by: Suman Anna --- drivers/iommu/omap-iommu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index a140873..3769dc0 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -1202,6 +1202,7 @@ static void _omap_iommu_detach_dev(struct omap_iommu_domain *omap_domain, omap_domain->iommu_dev = arch_data->iommu_dev = NULL; omap_domain->dev = NULL; + oiommu->domain = NULL; } static void omap_iommu_detach_dev(struct iommu_domain *domain, -- 2.1.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 17/17] iommu/omap: Switch pagetable debugfs entry to use seq_file
The debugfs entry 'pagetable' that shows the page table entry
(PTE) data currently outputs only data that can be fit into a
page. Switch the entry to use the seq_file interface so that
it can show all the valid page table entries.
The patch also corrected the output for L2 entries, and prints
the proper L2 PTE instead of the previous L1 page descriptor
pointer.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu-debug.c | 81 ++--
1 file changed, 28 insertions(+), 53 deletions(-)
diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c
index e7b8aca..096947c5 100644
--- a/drivers/iommu/omap-iommu-debug.c
+++ b/drivers/iommu/omap-iommu-debug.c
@@ -85,95 +85,70 @@ static ssize_t debug_read_tlb(struct file *file, char
__user *userbuf,
return bytes;
}
-#define dump_ioptable_entry_one(lv, da, val) \
- ({ \
- int __err = 0; \
- ssize_t bytes; \
- const int maxcol = 22; \
- const char *str = "%d: %08x %08x\n";\
- bytes = snprintf(p, maxcol, str, lv, da, val); \
- p += bytes; \
- len -= bytes; \
- if (len < maxcol) \
- __err = -ENOMEM;\
- __err; \
- })
-
-static ssize_t dump_ioptable(struct omap_iommu *obj, char *buf, ssize_t len)
+static void dump_ioptable(struct seq_file *s)
{
- int i;
- u32 *iopgd;
- char *p = buf;
+ int i, j;
+ u32 da;
+ u32 *iopgd, *iopte;
+ struct omap_iommu *obj = s->private;
spin_lock(&obj->page_table_lock);
iopgd = iopgd_offset(obj, 0);
for (i = 0; i < PTRS_PER_IOPGD; i++, iopgd++) {
- int j, err;
- u32 *iopte;
- u32 da;
-
if (!*iopgd)
continue;
if (!(*iopgd & IOPGD_TABLE)) {
da = i << IOPGD_SHIFT;
-
- err = dump_ioptable_entry_one(1, da, *iopgd);
- if (err)
- goto out;
+ seq_printf(s, "1: 0x%08x 0x%08x\n", da, *iopgd);
continue;
}
iopte = iopte_offset(iopgd, 0);
-
for (j = 0; j < PTRS_PER_IOPTE; j++, iopte++) {
if (!*iopte)
continue;
da = (i << IOPGD_SHIFT) + (j << IOPTE_SHIFT);
- err = dump_ioptable_entry_one(2, da, *iopgd);
- if (err)
- goto out;
+ seq_printf(s, "2: 0x%08x 0x%08x\n", da, *iopte);
}
}
-out:
- spin_unlock(&obj->page_table_lock);
- return p - buf;
+ spin_unlock(&obj->page_table_lock);
}
-static ssize_t debug_read_pagetable(struct file *file, char __user *userbuf,
- size_t count, loff_t *ppos)
+static int debug_read_pagetable(struct seq_file *s, void *data)
{
- struct omap_iommu *obj = file->private_data;
- char *p, *buf;
- size_t bytes;
+ struct omap_iommu *obj = s->private;
if (is_omap_iommu_detached(obj))
return -EPERM;
- buf = (char *)__get_free_page(GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
- p = buf;
-
- p += sprintf(p, "L: %8s %8s\n", "da:", "pa:");
- p += sprintf(p, "-\n");
-
mutex_lock(&iommu_debug_lock);
- bytes = PAGE_SIZE - (p - buf);
- p += dump_ioptable(obj, p, bytes);
-
- bytes = simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
+ seq_printf(s, "L: %8s %8s\n", "da:", "pte:");
+ seq_puts(s, "--\n");
+ dump_ioptable(s);
mutex_unlock(&iommu_debug_lock);
- free_page((unsigned long)buf);
- return bytes;
+ return 0;
}
+#define DEBUG_SEQ_FOPS_RO(name)
\
+ static int debug_open_##name(struct inode *inode, struct file *file) \
+ { \
+ return single_open(file, debug_read_##name, inode->i_private); \
+ } \
+ \
+ static const struct file_operations debug_##name##_fops = {\
+ .open = debug_open_##name,
[PATCH 06/17] iommu/omap: Remove omap_iommu_arch_version() and version field
The function omap_iommu_arch_version() is not used anymore,
and is not required either, so remove it. The .version field
in struct iommu_functions that this function uses is also
removed, as it is not really an ops to retrieve a version and
there won't be any usage for this field either.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu.c | 9 -
drivers/iommu/omap-iommu.h | 4
drivers/iommu/omap-iommu2.c | 2 --
3 files changed, 15 deletions(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index ed26701..332bfb9 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -138,15 +138,6 @@ void omap_iommu_restore_ctx(struct device *dev)
}
EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx);
-/**
- * omap_iommu_arch_version - Return running iommu arch version
- **/
-u32 omap_iommu_arch_version(void)
-{
- return arch_iommu->version;
-}
-EXPORT_SYMBOL_GPL(omap_iommu_arch_version);
-
static int iommu_enable(struct omap_iommu *obj)
{
int err;
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index d7c5132..45fe67d 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -70,8 +70,6 @@ struct cr_regs {
/* architecture specific functions */
struct iommu_functions {
- unsigned long version;
-
int (*enable)(struct omap_iommu *obj);
void (*disable)(struct omap_iommu *obj);
void (*set_twl)(struct omap_iommu *obj, bool on);
@@ -191,8 +189,6 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct
device *dev)
/*
* global functions
*/
-extern u32 omap_iommu_arch_version(void);
-
extern void omap_iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e);
extern int
diff --git a/drivers/iommu/omap-iommu2.c b/drivers/iommu/omap-iommu2.c
index 5e1ea3b..2f6a9f7 100644
--- a/drivers/iommu/omap-iommu2.c
+++ b/drivers/iommu/omap-iommu2.c
@@ -297,8 +297,6 @@ static void omap2_cr_to_e(struct cr_regs *cr, struct
iotlb_entry *e)
}
static const struct iommu_functions omap2_iommu_ops = {
- .version= IOMMU_ARCH_VERSION,
-
.enable = omap2_iommu_enable,
.disable= omap2_iommu_disable,
.set_twl= omap2_iommu_set_twl,
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 14/17] iommu/omap: Do not export unneeded functions
The following functions were exported previously for usage by
the OMAP IOMMU debug module:
omap_iommu_dump_ctx()
omap_dump_tlb_entries()
omap_iopgtable_store_entry()
These functions need not be exported anymore as the OMAP IOMMU
debugfs code is integrated with the OMAP IOMMU driver, and
there won't be external users for these functions. So, remove
the EXPORT_SYMBOL_GPL on these. The omap_iopgtable_store_entry()
is also made internal only, after making the 'pagetable' debugfs
entry read-only.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu.c | 6 ++
drivers/iommu/omap-iommu.h | 3 ---
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index b3bc087..a140873 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -509,7 +509,6 @@ ssize_t omap_iommu_dump_ctx(struct omap_iommu *obj, char
*buf, ssize_t bytes)
return bytes;
}
-EXPORT_SYMBOL_GPL(omap_iommu_dump_ctx);
static int
__dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
@@ -577,7 +576,6 @@ size_t omap_dump_tlb_entries(struct omap_iommu *obj, char
*buf, ssize_t bytes)
return p - buf;
}
-EXPORT_SYMBOL_GPL(omap_dump_tlb_entries);
/*
* H/W pagetable operations
@@ -760,7 +758,8 @@ iopgtable_store_entry_core(struct omap_iommu *obj, struct
iotlb_entry *e)
* @obj: target iommu
* @e: an iommu tlb entry info
**/
-int omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e)
+static int
+omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e)
{
int err;
@@ -770,7 +769,6 @@ int omap_iopgtable_store_entry(struct omap_iommu *obj,
struct iotlb_entry *e)
prefetch_iotlb_entry(obj, e);
return err;
}
-EXPORT_SYMBOL_GPL(omap_iopgtable_store_entry);
/**
* iopgtable_lookup_entry - Lookup an iommu pte entry
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 2659139..f32ddc3 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -190,9 +190,6 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct
device *dev)
/*
* global functions
*/
-extern int
-omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e);
-
extern ssize_t
omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t len);
extern size_t
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 07/17] iommu/omap: Remove bogus version check in context save/restore
The omap2_iommu_save_ctx() and omap2_iommu_restore_ctx() performs a sanity version check against a fixed value that is correct only for OMAP2/OMAP3 IOMMUs. This fixed check does not scale for all OMAP2+ IOMMUs and is not absolutely required, so it has been removed. Signed-off-by: Suman Anna --- drivers/iommu/omap-iommu2.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/iommu/omap-iommu2.c b/drivers/iommu/omap-iommu2.c index 2f6a9f7..372141b 100644 --- a/drivers/iommu/omap-iommu2.c +++ b/drivers/iommu/omap-iommu2.c @@ -26,8 +26,6 @@ /* * omap2 architecture specific register bit definitions */ -#define IOMMU_ARCH_VERSION 0x0011 - /* IRQSTATUS & IRQENABLE */ #define MMU_IRQ_MULTIHITFAULT (1 << 4) #define MMU_IRQ_TABLEWALKFAULT (1 << 3) @@ -268,8 +266,6 @@ static void omap2_iommu_save_ctx(struct omap_iommu *obj) p[i] = iommu_read_reg(obj, i * sizeof(u32)); dev_dbg(obj->dev, "%s\t[%02d] %08x\n", __func__, i, p[i]); } - - BUG_ON(p[0] != IOMMU_ARCH_VERSION); } static void omap2_iommu_restore_ctx(struct omap_iommu *obj) @@ -281,8 +277,6 @@ static void omap2_iommu_restore_ctx(struct omap_iommu *obj) iommu_write_reg(obj, p[i], i * sizeof(u32)); dev_dbg(obj->dev, "%s\t[%02d] %08x\n", __func__, i, p[i]); } - - BUG_ON(p[0] != IOMMU_ARCH_VERSION); } static void omap2_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e) -- 2.1.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 10/17] iommu/omap: Fix the permissions on nr_tlb_entries
The permissions on the debugfs entry "nr_tlb_entries" should
have been octal, not decimal, so fix it.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu-debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c
index 0fb92aa..a520438 100644
--- a/drivers/iommu/omap-iommu-debug.c
+++ b/drivers/iommu/omap-iommu-debug.c
@@ -256,7 +256,7 @@ static int iommu_debug_register(struct device *dev, void
*data)
goto nomem;
parent = d;
- d = debugfs_create_u8("nr_tlb_entries", 400, parent,
+ d = debugfs_create_u8("nr_tlb_entries", 0400, parent,
(u8 *)&obj->nr_tlb_entries);
if (!d)
goto nomem;
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 11/17] iommu/omap: Make pagetable debugfs entry read-only
Remove the writeability on the 'pagetable' debugfs entry,
so that the mapping/unmapping into an OMAP IOMMU is only
limited to actual client devices/drivers at kernel-level.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu-debug.c | 48 ++--
1 file changed, 2 insertions(+), 46 deletions(-)
diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c
index a520438..28de657 100644
--- a/drivers/iommu/omap-iommu-debug.c
+++ b/drivers/iommu/omap-iommu-debug.c
@@ -24,8 +24,6 @@
#include "omap-iopgtable.h"
#include "omap-iommu.h"
-#define MAXCOLUMN 100 /* for short messages */
-
static DEFINE_MUTEX(iommu_debug_lock);
static struct dentry *iommu_debug_root;
@@ -82,39 +80,6 @@ static ssize_t debug_read_tlb(struct file *file, char __user
*userbuf,
return bytes;
}
-static ssize_t debug_write_pagetable(struct file *file,
-const char __user *userbuf, size_t count, loff_t *ppos)
-{
- struct iotlb_entry e;
- struct cr_regs cr;
- int err;
- struct device *dev = file->private_data;
- struct omap_iommu *obj = dev_to_omap_iommu(dev);
- char buf[MAXCOLUMN], *p = buf;
-
- count = min(count, sizeof(buf));
-
- mutex_lock(&iommu_debug_lock);
- if (copy_from_user(p, userbuf, count)) {
- mutex_unlock(&iommu_debug_lock);
- return -EFAULT;
- }
-
- sscanf(p, "%x %x", &cr.cam, &cr.ram);
- if (!cr.cam || !cr.ram) {
- mutex_unlock(&iommu_debug_lock);
- return -EINVAL;
- }
-
- omap_iotlb_cr_to_e(&cr, &e);
- err = omap_iopgtable_store_entry(obj, &e);
- if (err)
- dev_err(obj->dev, "%s: fail to store cr\n", __func__);
-
- mutex_unlock(&iommu_debug_lock);
- return count;
-}
-
#define dump_ioptable_entry_one(lv, da, val) \
({ \
int __err = 0; \
@@ -202,14 +167,6 @@ static ssize_t debug_read_pagetable(struct file *file,
char __user *userbuf,
return bytes;
}
-#define DEBUG_FOPS(name) \
- static const struct file_operations debug_##name##_fops = { \
- .open = simple_open,\
- .read = debug_read_##name, \
- .write = debug_write_##name,\
- .llseek = generic_file_llseek, \
- };
-
#define DEBUG_FOPS_RO(name)\
static const struct file_operations debug_##name##_fops = { \
.open = simple_open,\
@@ -219,7 +176,7 @@ static ssize_t debug_read_pagetable(struct file *file, char
__user *userbuf,
DEBUG_FOPS_RO(regs);
DEBUG_FOPS_RO(tlb);
-DEBUG_FOPS(pagetable);
+DEBUG_FOPS_RO(pagetable);
#define __DEBUG_ADD_FILE(attr, mode) \
{ \
@@ -230,7 +187,6 @@ DEBUG_FOPS(pagetable);
return -ENOMEM; \
}
-#define DEBUG_ADD_FILE(name) __DEBUG_ADD_FILE(name, 0600)
#define DEBUG_ADD_FILE_RO(name) __DEBUG_ADD_FILE(name, 0400)
static int iommu_debug_register(struct device *dev, void *data)
@@ -263,7 +219,7 @@ static int iommu_debug_register(struct device *dev, void
*data)
DEBUG_ADD_FILE_RO(regs);
DEBUG_ADD_FILE_RO(tlb);
- DEBUG_ADD_FILE(pagetable);
+ DEBUG_ADD_FILE_RO(pagetable);
return 0;
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 01/17] iommu/omap: Remove refcount field from omap_iommu object
The refcount field in omap_iommu object is primarily used to check
if an IOMMU device has already been enabled, but this is already
implicit in the omap_iommu_attach_dev() which ensures that only
a single device can attach to an IOMMU. This field is redundant,
and so has been cleaned up.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu.c | 15 +++
drivers/iommu/omap-iommu.h | 1 -
2 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 4b432c4..ed26701 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -819,8 +819,9 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
u32 *iopgd, *iopte;
struct omap_iommu *obj = data;
struct iommu_domain *domain = obj->domain;
+ struct omap_iommu_domain *omap_domain = domain->priv;
- if (!obj->refcount)
+ if (!omap_domain->iommu_dev)
return IRQ_NONE;
errs = iommu_report_fault(obj, &da);
@@ -880,13 +881,6 @@ static struct omap_iommu *omap_iommu_attach(const char
*name, u32 *iopgd)
spin_lock(&obj->iommu_lock);
- /* an iommu device can only be attached once */
- if (++obj->refcount > 1) {
- dev_err(dev, "%s: already attached!\n", obj->name);
- err = -EBUSY;
- goto err_enable;
- }
-
obj->iopgd = iopgd;
err = iommu_enable(obj);
if (err)
@@ -899,7 +893,6 @@ static struct omap_iommu *omap_iommu_attach(const char
*name, u32 *iopgd)
return obj;
err_enable:
- obj->refcount--;
spin_unlock(&obj->iommu_lock);
return ERR_PTR(err);
}
@@ -915,9 +908,7 @@ static void omap_iommu_detach(struct omap_iommu *obj)
spin_lock(&obj->iommu_lock);
- if (--obj->refcount == 0)
- iommu_disable(obj);
-
+ iommu_disable(obj);
obj->iopgd = NULL;
spin_unlock(&obj->iommu_lock);
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 4f1b68c..5c14000 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -33,7 +33,6 @@ struct omap_iommu {
void*isr_priv;
struct iommu_domain *domain;
- unsigned intrefcount;
spinlock_t iommu_lock; /* global for this whole object */
/*
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 04/17] iommu/omap: Remove conditional definition of dev_to_omap_iommu()
The dev_to_omap_iommu() is local to the OMAP IOMMU modules, and
need not be defined conditionally. The CONFIG_IOMMU_API dependency
check was added in the past to fix a compilation issue back when
the header resided in the arch/arm layers, and is no longer
needed.
While at this, fix the header against double inclusion as well.
Signed-off-by: Suman Anna
---
drivers/iommu/omap-iommu.h | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 4fc51c8..d7c5132 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -10,6 +10,9 @@
* published by the Free Software Foundation.
*/
+#ifndef _OMAP_IOMMU_H
+#define _OMAP_IOMMU_H
+
#if defined(CONFIG_ARCH_OMAP1)
#error "iommu for this processor not implemented yet"
#endif
@@ -92,7 +95,6 @@ struct iommu_functions {
ssize_t (*dump_ctx)(struct omap_iommu *obj, char *buf, ssize_t len);
};
-#ifdef CONFIG_IOMMU_API
/**
* dev_to_omap_iommu() - retrieves an omap iommu object from a user device
* @dev: iommu client device
@@ -103,7 +105,6 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct
device *dev)
return arch_data->iommu_dev;
}
-#endif
/*
* MMU Register offsets
@@ -220,3 +221,5 @@ static inline void iommu_write_reg(struct omap_iommu *obj,
u32 val, size_t offs)
{
__raw_writel(val, obj->regbase + offs);
}
+
+#endif /* _OMAP_IOMMU_H */
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 03/17] iommu/omap: Remove duplicate declarations
The omap_iommu_save_ctx() and omap_iommu_restore_ctx() declarations are defined in include/linux/omap-iommu.h and do not belong in the internal drivers/iommu/omap-iommu.h header, so remove them. Signed-off-by: Suman Anna --- drivers/iommu/omap-iommu.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h index 18a0f3a..4fc51c8 100644 --- a/drivers/iommu/omap-iommu.h +++ b/drivers/iommu/omap-iommu.h @@ -197,9 +197,6 @@ extern void omap_iotlb_cr_to_e(struct cr_regs *cr, struct iotlb_entry *e); extern int omap_iopgtable_store_entry(struct omap_iommu *obj, struct iotlb_entry *e); -extern void omap_iommu_save_ctx(struct device *dev); -extern void omap_iommu_restore_ctx(struct device *dev); - extern int omap_foreach_iommu_device(void *data, int (*fn)(struct device *, void *)); -- 2.1.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v3 7/7] arm: dma-mapping: plumb our iommu mapping ops into arch_setup_dma_ops
Hi Thierry, On Thu, Sep 25, 2014 at 07:40:23AM +0100, Thierry Reding wrote: > On Wed, Sep 24, 2014 at 05:33:38PM +0100, Will Deacon wrote: > > On Tue, Sep 23, 2014 at 08:14:01AM +0100, Thierry Reding wrote: > > > On Mon, Sep 22, 2014 at 06:43:37PM +0100, Will Deacon wrote: > > > > Yup. In this case, the iommu_dma_mapping passed to arch_setup_dma_ops > > > > contains a domain and an allocator for each IOMMU instance in the > > > > system. > > > > It would then be up to the architecture how it makes use of those, but > > > > the most obvious thing to do would be to attach devices mastering > > > > through > > > > an IOMMU instance to that per-instance domain. > > > > > > > > The other use-case is isolation (one domain per device), which I guess > > > > matches what the ARM code is doing at the moment. > > > > > > I think there are two cases here. You can have a composite device that > > > wants to manage a single domain (using its own allocator) for a set of > > > hardware devices. At the same time a set of devices (think 2D and 3D > > > engines) could want to use a multiple domains for process separation. > > > In that case I'd expect a logical DRM device to allocate one domain per > > > process and then associate the 2D and 3D engines with that same domain > > > on process switch. > > > > Sure, but that's well outside of what the dma-mapping API is going to setup > > as a default domain. These specialist setups are certainly possible, but I > > think they should be driven by, for example, the DRM code as opposed to > > being in the core dma-mapping code. > > I completely agree that these special cases should be driven by the > drivers that need them. However the problem here is that the current > patch will already attach the device to an IOMMU domain by default. Sure, but that's not an unfixable problem if somebody cares enough to do it. Right now, I see a small handful of callers for the IOMMU API and nearly all of them would work perfectly well with a default domain. The big exception to that is VFIO, but that requires the device to be unbound from the host driver, so we could detach the mapping at that point. > So I think what we're going to need is a way to prevent the default > attachment to DMA/IOMMU. Or alternatively not associate devices with > IOMMU domains by default but let drivers explicitly make the decision. Which drivers and how would they know what to do? I think you might be jumping the gun a bit here, given where mainline is with using the IOMMU for anything at all. > > > What I proposed a while back was to leave it up to the IOMMU driver to > > > choose an allocator for the device. Or rather, choose whether to use a > > > custom allocator or the DMA/IOMMU integration allocator. The way this > > > worked was to keep a list of devices in the IOMMU driver. Devices in > > > this list would be added to domain reserved for DMA/IOMMU integration. > > > Those would typically be devices such as SD/MMC, audio, ... devices that > > > are in-kernel and need no per-process separation. By default devices > > > wouldn't be added to a domain, so devices forming a composite DRM device > > > would be able to manage their own domain. > > > > I'd live to have as little of this as possible in the IOMMU drivers, as we > > should leave those to deal with the IOMMU hardware and not domain > > management. Having subsystems manage their own dma ops is an extension to > > the dma-mapping API. > > It's not an extension, really. It's more that both need to be able to > coexist. For some devices you may want to create an IOMMU domain and > hook it up with the DMA mapping functions, for others you don't and > handle mapping to IOVA space explicitly. I think it's an extension in the sense that mainline doesn't currently do what you want, regardless of this patch series. My motivation is to enable IOMMU-backed DMA-mapping so that I can continue implementing the virtual SMMU work I started a while back. Patches welcome to enable any other use-cases -- I don't think they're mutually exclusive. > There is another issue with the approach you propose. I'm not sure if > Tegra is special in this case (I'd expect not), but what we do is make > an IOMMU domain correspond to an address space. Address spaces are a > pretty limited resource (earlier generations have 4, newer have 128) > and each address space can be up to 4 GiB. So I've always envisioned > that we should be using a single IOMMU domain for devices that don't > expose direct buffer access to userspace (SATA, PCIe, audio, SD/MMC, > USB, ...). All of those would typically need only a small number of > small buffers, so using a separate address space for each seems like a > big waste. I agree here, the ARM DMA-mapping code should really be doing one domain per SMMU instance; all I've done is hook up the existing code which wasn't previously being called. > Doing so would leave a large number of address spaces available for > things like a GPU driver to
Re: [PATCH v3 0/5] enhance DMA CMA on x86
On 09/29/2014 10:32 AM, Akinobu Mita wrote: > 2014-09-29 21:09 GMT+09:00 Peter Hurley : >> On 09/27/2014 08:31 PM, Akinobu Mita wrote: >>> 2014-09-27 23:30 GMT+09:00 Peter Hurley : On 04/15/2014 09:08 AM, Akinobu Mita wrote: > This patch set enhances the DMA Contiguous Memory Allocator on x86. >> >> [...] >> What this patchset does is restrict all iommu configurations which can map all of system memory to one _very_ small physical region, thus disabling the whole point of an iommu. Now I know why my GPU is causing paging to disk! And why my RAID controller stalls for ages when I do a git log at the same time as a kernel build! >>> >>> The solution I have for this is that instead of trying to >>> dma_alloc_from_contiguous() firstly, call alloc_pages() in >>> dma_alloc_coherent(). >>> dma_alloc_from_contiguous() should be called only when alloc_pages() is >>> failed >>> or DMA_ATTR_FORCE_CONTIGUOUS is specified in dma_attr. >> >> Why is all this extra complexity being added when there are no X86 users >> of DMA_ATTR_FORCE_CONTIGUOUS? > > I misunderstood DMA_ATTR_FORCE_CONTIGUOUS. It is specified to request > that underlaying DMA mapping span physically contiguous with IOMMU. > But current alloc_dma_coherent() for intel-iommu always returns > physically contiguous memory, so it is ignored on x86. > And the apparent goal of this patchset is to enable DMA allocation below 4GB, which is already supported in the existing page allocator with the GFP_DMA32 flag?! >>> >>> The goal of this patchset is to enable huge DMA allocation which >>> alloc_pages() can't (> MAX_ORDER) for the devices that require it. >> >> What x86 devices need > MAX_ORDER DMA allocation and why can't they allocate >> directly from dma_alloc_from_contiguous()? > > I need this for UFS unified memory extension which is apparently not in > mainline for now. > http://www.jedec.org/standards-documents/docs/jesd220-1 > http://www.jedec.org/sites/default/files/T_Fujisawa_MF_2013.pdf > > But there must be some other use cases on x86, too. Because I have > received several emails privately from developers who care its status. > > And allocating directly from dma_alloc_from_contiguous() in the driver > doesn't work with IOMMU, as it just returns memory regoin and doesn't > create DMA mapping. I read the UFS Unified Memory Extension v1.0 (JESD220-1) specification and it is not clear to me that using DMA mapping is the right approach to supporting UM, at least on x86. And without a mainline user, the merits of this approach are not evident. I cannot even find a production x86 UFS controller, much less one that supports UME. The only PCI UFS controller I could find (and that mainline supports) is Samsung's x86 FPGA-based test unit for developing UFS devices in a x86 test environment, and not a production x86 design. Samsung's own roadmap (http://www.slideshare.net/linaroorg/next-gen-mobilestorageufs) mentions nothing about bringing UFS to x86 designs. Unless there's something else I've missed, I don't think these patches belong in mainline. Regards, Peter Hurley ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] driver core: Add BUS_NOTIFY_REMOVED_DEVICE event
From: Joerg Roedel This event closes an important gap in the bus notifiers. There is already the BUS_NOTIFY_DEL_DEVICE event, but that is sent when the device is still bound to its device driver. This is too early for the IOMMU code to destroy any mappings for the device, as they might still be in use by the driver. The new BUS_NOTIFY_REMOVED_DEVICE event introduced with this patch closes this gap as it is sent when the device is already unbound from its device driver and almost completly removed from the driver core. With this event the IOMMU code can safely destroy any mappings and other data structures when a device is removed. Signed-off-by: Joerg Roedel --- drivers/base/core.c| 3 +++ include/linux/device.h | 11 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 20da3ad..7b270a2 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1211,6 +1211,9 @@ void device_del(struct device *dev) */ if (platform_notify_remove) platform_notify_remove(dev); + if (dev->bus) + blocking_notifier_call_chain(&dev->bus->p->bus_notifier, +BUS_NOTIFY_REMOVED_DEVICE, dev); kobject_uevent(&dev->kobj, KOBJ_REMOVE); cleanup_device_parent(dev); kobject_del(&dev->kobj); diff --git a/include/linux/device.h b/include/linux/device.h index 43d183a..d0d5c5d 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -181,13 +181,14 @@ extern int bus_unregister_notifier(struct bus_type *bus, * with the device lock held in the core, so be careful. */ #define BUS_NOTIFY_ADD_DEVICE 0x0001 /* device added */ -#define BUS_NOTIFY_DEL_DEVICE 0x0002 /* device removed */ -#define BUS_NOTIFY_BIND_DRIVER 0x0003 /* driver about to be +#define BUS_NOTIFY_DEL_DEVICE 0x0002 /* device to be removed */ +#define BUS_NOTIFY_REMOVED_DEVICE 0x0003 /* device removed */ +#define BUS_NOTIFY_BIND_DRIVER 0x0004 /* driver about to be bound */ -#define BUS_NOTIFY_BOUND_DRIVER0x0004 /* driver bound to device */ -#define BUS_NOTIFY_UNBIND_DRIVER 0x0005 /* driver about to be +#define BUS_NOTIFY_BOUND_DRIVER0x0005 /* driver bound to device */ +#define BUS_NOTIFY_UNBIND_DRIVER 0x0006 /* driver about to be unbound */ -#define BUS_NOTIFY_UNBOUND_DRIVER 0x0006 /* driver is unbound +#define BUS_NOTIFY_UNBOUND_DRIVER 0x0007 /* driver is unbound from the device */ extern struct kset *bus_get_kset(struct bus_type *bus); -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] iommu/vt-d: Only remove domain when device is removed
From: Joerg Roedel This makes sure any RMRR mappings stay in place when the driver is unbound from the device. Signed-off-by: Joerg Roedel --- drivers/iommu/intel-iommu.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 5619f26..eaf825a 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -3865,16 +3865,7 @@ static int device_notifier(struct notifier_block *nb, if (iommu_dummy(dev)) return 0; - if (action != BUS_NOTIFY_UNBOUND_DRIVER && - action != BUS_NOTIFY_DEL_DEVICE) - return 0; - - /* -* If the device is still attached to a device driver we can't -* tear down the domain yet as DMA mappings may still be in use. -* Wait for the BUS_NOTIFY_UNBOUND_DRIVER event to do that. -*/ - if (action == BUS_NOTIFY_DEL_DEVICE && dev->driver != NULL) + if (action != BUS_NOTIFY_REMOVED_DEVICE) return 0; domain = find_domain(dev); -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/2] iommu/vt-d: Keep RMRR mappings around on driver unbind
Hi, here is a patch-set to fix an issue recently discovered when the Intel IOMMU is in use with devices that need RMRR mappings. The problem is that the RMRR mappings are destroyed when the device driver is unbound from the device, causing DMAR faults. To solve this problem a device driver core change is necessary to catch the right point in time for the IOMMU code to destroy any mappings for a device. With this patch-set the RMRR mappings are only destroyed when the device is actually removed from the system. Please review. Thanks, Joerg Joerg Roedel (2): driver core: Add BUS_NOTIFY_REMOVED_DEVICE event iommu/vt-d: Only remove domain when device is removed drivers/base/core.c | 3 +++ drivers/iommu/intel-iommu.c | 11 +-- include/linux/device.h | 11 ++- 3 files changed, 10 insertions(+), 15 deletions(-) -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/2] iommu/arm-smmu: add support for iova_to_phys through ATS1PR
Hi Mitch,
On Sun, Sep 28, 2014 at 04:27:29AM +0100, Mitchel Humpherys wrote:
> Currently, we provide the iommu_ops.iova_to_phys service by doing a
> table walk in software to translate IO virtual addresses to physical
> addresses. On SMMUs that support it, it can be useful to ask the SMMU
> itself to do the translation. This can be used to warm the TLBs for an
> SMMU. It can also be useful for testing and hardware validation.
>
> Since the address translation registers are optional on SMMUv2, only
> enable hardware translations when using SMMUv1 or when SMMU_IDR0.S1TS=1
> and SMMU_IDR0.ATOSNS=0, as described in the ARM SMMU v1-v2 spec.
[...]
> +static phys_addr_t arm_smmu_iova_to_phys_hard(struct iommu_domain *domain,
> + dma_addr_t iova)
> +{
> + struct arm_smmu_domain *smmu_domain = domain->priv;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> + struct device *dev = smmu->dev;
> + void __iomem *cb_base;
> + u32 tmp;
> + u64 phys;
> +
> + cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
> +
> + if (smmu->version == 1) {
> + u32 reg = iova & ~0xFFF;
Cosmetic comment, but hex constants are lowercase everywhere else in the
file.
> + writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
> + } else {
> + u32 reg = iova & ~0xFFF;
> + writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_LO);
> + reg = (iova & ~0xFFF) >> 32;
> + writel_relaxed(reg, cb_base + ARM_SMMU_CB_ATS1PR_HI);
> + }
> +
> + if (readl_poll_timeout(cb_base + ARM_SMMU_CB_ATSR, tmp,
> + !(tmp & ATSR_ACTIVE), 10, ATSR_LOOP_TIMEOUT)) {
> + dev_err(dev,
> + "iova to phys timed out on 0x%pa. Falling back to
> software table walk.\n",
> + &iova);
> + return arm_smmu_iova_to_phys_soft(domain, iova);
> + }
> +
> + phys = readl_relaxed(cb_base + ARM_SMMU_CB_PAR_LO);
> + phys |= ((u64) readl_relaxed(cb_base + ARM_SMMU_CB_PAR_HI)) << 32;
The absence of locking in this function concerns me a bit. For the software
implementation, we're just reading page tables, but here we're writing ATS
registers and I think we need to ensure serialisation against another
iova_to_phys on the same domain.
> + if (phys & CB_PAR_F) {
> + dev_err(dev, "translation fault!\n");
> + dev_err(dev, "PAR = 0x%llx\n", phys);
> + }
> + phys = (phys & 0xFFF000ULL) | (iova & 0x0FFF);
> +
> + return phys;
You can return phys == 0 on failure (at least, the callers in kvm and vfio
treat this as an error).
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
