Re: [RFC PATCH 2/2] iommu: rockchip: Handle system-wide and runtime PM

[Tomasz Figa]

On Tue, Dec 16, 2014 at 4:53 AM, Laurent Pinchart
 wrote:
> Hi Tomasz,
>
> On Monday 15 December 2014 11:39:01 Tomasz Figa wrote:
>> On Sat, Dec 13, 2014 at 5:47 AM, Laurent Pinchart wrote:
>> > On Friday 12 December 2014 13:15:51 Tomasz Figa wrote:
>> >> On Fri, Dec 12, 2014 at 5:48 AM, Rafael J. Wysocki wrote:
>> >>> On Thursday, December 11, 2014 04:51:37 PM Ulf Hansson wrote:
>>  On 11 December 2014 at 16:31, Kevin Hilman  wrote:
>> > [+ Laurent Pinchart]
>> >
>> > Tomasz Figa  writes:
>> >> On Thu, Dec 11, 2014 at 8:58 PM, Ulf Hansson wrote:
>> > [...]
>> >
>>  @@ -988,11 +1107,28 @@ static int rk_iommu_probe(struct
>>  platform_device *pdev)
>>  return -ENXIO;
>>  }
>> 
>>  +   pm_runtime_no_callbacks(dev);
>>  +   pm_runtime_enable(dev);
>>  +
>>  +   /* Synchronize state of the domain with driver data. */
>>  +   pm_runtime_get_sync(dev);
>>  +   iommu->is_powered = true;
>> >>>
>> >>> Doesn't the runtime PM status reflect the value of "is_powered",
>> >>> thus why do you need to have a copy of it? Could it perpahps be
>> >>> that you try to cope with the case when CONFIG_PM is unset?
>> >>
>> >> It's worth noting that this driver fully relies on status of other
>> >> devices in the power domain the IOMMU is in and does not enforce
>> >> the status on its own. So in general, as far as my understanding of
>> >> PM runtime subsystem, the status of the IOMMU device will be always
>> >> suspended, because nobody will call pm_runtime_get() on it (except
>> >> the get and put pair in probe). So is_powered is here to track
>> >> status of the domain, not the device. Feel free to suggest a better
>> >> way, though.
>> >
>> > I still don't like these notifiers.  I think they add ways to bypass
>> > having proper runtime PM implemented for devices/subsystems.
>> 
>>  I do agree, but I haven't found another good solution to the problem.
>> >>>
>> >>> For the record, I'm not liking this mostly because it "fixes" a generic
>> >>> problem in a way that's hidden in the genpd code and very indirect.
>> >>
>> >> Well, that's true. This is indeed a generic problem of PM dependencies
>> >> between devices (other than those represented by parent-child
>> >> relation), which in fact doesn't have much to do with genpd, but
>> >> rather with those devices directly. It is just that genpd is the most
>> >> convenient location to solve this in current code and in a simple way.
>> >> In other words, I see this solution as a reasonable way to get the
>> >> problem solved quickly for now, so that we can start thinking about a
>> >> more elegant solution.
>> >>
>> >> >> > From a high-level, the IOMMU is just another device inside the PM
>> >> >> > domain, so ideally it should be doing it's own _get() and _put()
>> >> >> > calls so the PM domain code would just do the right thing without
>> >> >> > the need for notifiers.
>> >> >>
>> >> >> As I understand it, the IOMMU (or for other similar cases) shouldn't
>> >> >> be doing any get() and put() at all because there are no IO API to
>> >> >> serve request from.
>> >
>> > Speaking purely from an IOMMU point of view that's not entirely tree.
>> > IOMMU drivers expose map and unmap operations, so they can track whether
>> > any memory is mapped. From a bus master point of view the IOMMU map and
>> > unmap operations are hidden by the DMA mapping API. The IOMMU can thus
>> > track the existence of mappings without any IOMMU awareness in the bus
>> > master driver.
>> >
>> > If no mapping exist the IOMMU shouldn't receive any translation request.
>> > An IOMMU driver could thus call pm_runtime_get_sync() in the map handler
>> > when creating the first mapping, and pm_runtime_put() in the unmap handler
>> > when tearing the last mapping down.
>> >
>> > One could argue that the IOMMU would end up being powered more often than
>> > strictly needed, as bus masters drivers, even when written properly, could
>> > keep mappings around at times they don't perform bus access. This is true,
>> > and that's an argument I've raised during the last kernel summit. The
>> > general response (including Linus Torvald's) was that micro-optimizing
>> > power management might not be worth it, and that measurements proving
>> > that the gain is worth it are required before introducing new APIs to
>> > solve the problem. I can't disagree with that argument.
>>
>> This would be a micro optimization if the IOMMU was located in its own
>> power domain. Unfortunately in most cases it is not, so keeping all
>> the devices in the domain powered on, because one of them have a
>> mapping created doesn't sound like a good idea.
>>
>> Moreover, most of the drivers will keep the mapping for much longer
>> than one run cycle. Please take a look at V4L2's videobuf2 subsystem
>> (which I gues

Re: [PATCH 0/3] Add support for APM X-Gene SoC AHBC IOMMU driver.

[Suman Tripathi]

Hi Joerg,

Hi Suman,

On Tue, Dec 16, 2014 at 01:31:58AM +0530, Suman Tripathi wrote:
> On Dec 16, 2014 1:25 AM, "Don Dutile"  wrote:
>
> On 12/15/2014 11:55 AM, Suman Tripathi wrote:
>
> This patch adds the support for the APM X-Gene SoC AHBC IOMMU
driver.
>
> Signed-off-by: Suman Tripathi 
> ---
>
> Suman Tripathi (3):
>xgene-ahbc-iommu: Add support for APM X-Gene SoC AHBC IOMMU
driver.
>arm64: dts: Add the APM X-Gene AHBC IOMMU DTS node.
>Documentation: dt-bindings: Add the binding info for APM
X-Gene AHBC
>  IOMMU driver.
>
>   .../devicetree/bindings/iommu/xgene,ahbc-iommu.txt |  17 ++
>   arch/arm64/boot/dts/apm-storm.dtsi |   5 +
>   drivers/iommu/Kconfig  |  10 +
>   drivers/iommu/Makefile |   1 +
>   drivers/iommu/xgene-ahbc-iommu.c   | 336
> +
>   5 files changed, 369 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iommu/
> xgene,ahbc-iommu.txt
>   create mode 100644 drivers/iommu/xgene-ahbc-iommu.c
What are the differences between the X-Gene IOMMU and the ARM-SMMU?
[suman] : AHBC is IP from XGene for performing 32-bi to 64 bit and vice
versa with AHBC peripherals. ARM-SMMU is IP from ARM which performs
translations of different peripherals other than AHBC .

Thanks,
with regards,
suman

On Tue, Dec 16, 2014 at 1:57 AM, Joerg Roedel  wrote:
>
> Hi Suman,
>
> On Tue, Dec 16, 2014 at 01:31:58AM +0530, Suman Tripathi wrote:
> > On Dec 16, 2014 1:25 AM, "Don Dutile"  wrote:
> >
> > On 12/15/2014 11:55 AM, Suman Tripathi wrote:
> >
> > This patch adds the support for the APM X-Gene SoC AHBC IOMMU
> driver.
> >
> > Signed-off-by: Suman Tripathi 
> > ---
> >
> > Suman Tripathi (3):
> >xgene-ahbc-iommu: Add support for APM X-Gene SoC AHBC IOMMU
> driver.
> >arm64: dts: Add the APM X-Gene AHBC IOMMU DTS node.
> >Documentation: dt-bindings: Add the binding info for APM
> X-Gene AHBC
> >  IOMMU driver.
> >
> >   .../devicetree/bindings/iommu/xgene,ahbc-iommu.txt |  17 ++
> >   arch/arm64/boot/dts/apm-storm.dtsi |   5 +
> >   drivers/iommu/Kconfig  |  10 +
> >   drivers/iommu/Makefile |   1 +
> >   drivers/iommu/xgene-ahbc-iommu.c   | 336
> > +
> >   5 files changed, 369 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/iommu/
> > xgene,ahbc-iommu.txt
> >   create mode 100644 drivers/iommu/xgene-ahbc-iommu.c
>
> What are the differences between the X-Gene IOMMU and the ARM-SMMU?
>
>
> Joerg
>
>

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, 
is for the sole use of the intended recipient(s) and contains information
that is confidential and proprietary to Applied Micro Circuits Corporation or 
its subsidiaries. 
It is to be used solely for the purpose of furthering the parties' business 
relationship. 
All unauthorized review, use, disclosure or distribution is prohibited. 
If you are not the intended recipient, please contact the sender by reply 
e-mail 
and destroy all copies of the original message.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 0/5] Fix Intel IRQ remapping initialization order

[Jiang Liu]

On 2014/12/16 3:54, Thomas Gleixner wrote:
> On Mon, 15 Dec 2014, Jiang Liu wrote:
>> On 2014/12/15 23:13, Joerg Roedel wrote:
>>> Hi,
>>>
>>> here is a patch-set against tip/x86/apic to fix an initialization order
>>> problem with the IRQ remapping code.  The problem is in the ordering of
>>> the irq_remapping_prepare and irq_remapping_supported functions.
>>>
>>> Currently the call-order is irq_remapping_prepare ->
>>> irq_remapping_supported, so that 'prepare' can succeed but 'supported'
>>> fails, so that interrupt remapping gets initialized but not enabled.
>>> This causes a broken interrupt setup on affected systems (machines with
>>> an Intel IOMMU without, or broken, IRQ remapping support). The result
>>> are lost interrupts and a non-bootable system.
>>>
>>> Both functions do checks whether IRQ remapping can be enabled on the
>>> machine.  The reason for this is that some checks rely on
>>> dmar_table_init() and thus have to be done in irq_remapping_prepare().
>>>
>>> This patch-set moves all these checks into the irq_remapping_prepare()
>>> path with the right ordering and removes the irq_remapping_supported()
>>> function and its call-backs. This fixes the initializion order problem
>>> and simplifies the exported API from the IOMMU code.
>>>
>>> Please review.
>> Hi Joerg,
>>  I have posted a patch set for the same purpose at:
>> https://lkml.org/lkml/2014/12/10/20
>>  Seems we need to combine these two patch sets:)
> 
> Actually you want to combine it also with these patches:
> 
> 326c2bb2c526: iommu/vt-d: Convert allocations to GFP_KERNEL
> e9220e591375: iommu/vt-d: Move iommu preparatory allocations to 
> irq_remap_ops.prepare
> e88edbd316ea: iommu, x86: Restructure setup of the irq remapping feature
> dd60143c04f2: x86, smpboot: Remove pointless preempt_disable() in 
> native_smp_prepare_cpus()
> 
> against 3.19 independent of the irqdomain stuff.
> 
> So that would be a clean base to put the rest of the irqdomain and
> hotplug stuff on top.
> 
> So the ordering of the patches for 3.20 would become:
> 
>iommu cleanup (init and allocations)
>acpi cleanup and parser extensions
>ioapic hotplug
>irqdomain conversion
> 
> I will route dd60143c04f2 "x86, smpboot: Remove pointless
> preempt_disable() in native_smp_prepare_cpus()" into -rc1.  I'm going
> to do so for a few other cherry-picks from x86/apic.
> 
> So can you please create a combined series, which just deals with the
> init cleanup and the allocation conversion (ATOMIC -> GFP) based on
> current Linus tree should be fine.
Hi Thomas,
I will work on that, hope we will have something after Christmas 
Holiday:)
Regards!
Gerry
> 
> Thanks,
> 
>   tglx
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Magnus Damm]

Hi Laurent,

On Tue, Dec 16, 2014 at 3:44 AM, Laurent Pinchart
 wrote:
> Hi Geert,
>
> On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote:
>> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote:
>> > Add the seven IPMMU instances found in the r8a7791 to DT with a disabled
>> > status.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > 
>>
>> The addresses and interrupt numbers look OK to me.
>> However, my comment about the "0x800" offset is still valid.
>> Shouldn't we have two register blocks, and let the driver use only the
>> second one?
>>
>> If you ignore, feel free to add my
>> Acked-by: Geert Uytterhoeven 
>
> I don't want to ignore your comment, but I don't know what to do here :-/ The
> datasheet lacks detailed information about secure vs. non-secure mode and how
> the two register sets are supposed to interoperate and be handled by the
> operating system.

I don't know about that either.

But how about differences within R-Car Gen2 series so far? If there
are known differences then perhaps we should use part number in the
compatible string?

Thanks,

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

[PATCH v10] iopoll: Introduce memory-mapped IO polling macros

[Mitchel Humpherys]

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 
Cc: Arnd Bergmann 
Cc: Andrew Morton 
Cc: Robert Elliott 
Signed-off-by: Matt Wagantall 
Signed-off-by: Mitchel Humpherys 
---
v9..10:
  - Actually added the comments mentioned in v8..v9 (doh!)

v8..v9:
  - Added note in comments about max sleep time (Rob Elliott)

v7..v8:
  - sorted helper macros by size (b, w, l, q)
  - removed some of the more esoteric (or flat-out bogus) helper macros

This patch was originally part of a series [1] for adding support for IOMMU
address translations through an ARM SMMU hardware register.  The other
patch in the series (the one that actually uses these macros and implements
said hardware address translations) was Ack'd by the driver maintainer
there (Will Deacon) so I've pulled this patch out to avoid resending an
already Ack'd patch over and over again.

In short, please see [1] for previous discussion and the first user of
these macros.

Will also acked this patch in [2].  I didn't retain his Ack here because I
added to the macro comments.

[1] http://thread.gmane.org/gmane.linux.kernel.iommu/7140
[2] http://thread.gmane.org/gmane.linux.kernel.iommu/7449

---
 include/linux/iopoll.h | 144 +
 1 file changed, 144 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 ..08fd52cdb5a0
--- /dev/null
+++ b/include/linux/iopoll.h
@@ -0,0 +1,144 @@
+/*
+ * 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 
+
+/**
+ * readx_poll_timeout - Periodically poll an address until a condition is met 
or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @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).  Should be less than ~20ms since usleep_range
+ *is used (see Documentation/timers/timers-howto.txt).
+ * @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.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)  \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   might_sleep_if(sleep_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (sleep_us) \
+   usleep_range((sleep_us >> 2) + 1, sleep_us); \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**
+ * readx_poll_timeout_atomic - Periodically poll an address until a condition 
is met or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @delay_us: Time to udelay between reads in us (0 tight-loops).  Should
+ *be less than ~10us since udelay is used (see
+ *Documentation/timers/timers-howto.txt).
+ * @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.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+  

Re: [PATCH v9] iopoll: Introduce memory-mapped IO polling macros

[Mitchel Humpherys]

On Mon, Dec 15 2014 at 03:31:20 PM, Mitchel Humpherys  
wrote:
> 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 
> Cc: Arnd Bergmann 
> Cc: Andrew Morton 
> Cc: Robert Elliott 
> Signed-off-by: Matt Wagantall 
> Signed-off-by: Mitchel Humpherys 
> ---
> v8..v9:
>   - Added note in comments about max sleep time (Rob Elliott)

Sorry, just noticed that I somehow dropped these additional comments
that Rob requested...  Let me send a v10 that actually includes them.
Please ignore this version.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v9] iopoll: Introduce memory-mapped IO polling macros

[Mitchel Humpherys]

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 
Cc: Arnd Bergmann 
Cc: Andrew Morton 
Cc: Robert Elliott 
Signed-off-by: Matt Wagantall 
Signed-off-by: Mitchel Humpherys 
---
v8..v9:
  - Added note in comments about max sleep time (Rob Elliott)

v7..v8:
  - sorted helper macros by size (b, w, l, q)
  - removed some of the more esoteric (or flat-out bogus) helper macros

This patch was originally part of a series [1] for adding support for IOMMU
address translations through an ARM SMMU hardware register.  The other
patch in the series (the one that actually uses these macros and implements
said hardware address translations) was Ack'd by the driver maintainer
there (Will Deacon) so I've pulled this patch out to avoid resending an
already Ack'd patch over and over again.

In short, please see [1] for previous discussion and the first user of
these macros.

Will also acked this patch in [2].  I didn't retain his Ack here because I
added to the macro comments.

[1] http://thread.gmane.org/gmane.linux.kernel.iommu/7140
[2] http://thread.gmane.org/gmane.linux.kernel.iommu/7449

---
 include/linux/iopoll.h | 140 +
 1 file changed, 140 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 ..bd161dae2d40
--- /dev/null
+++ b/include/linux/iopoll.h
@@ -0,0 +1,140 @@
+/*
+ * 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 
+
+/**
+ * readx_poll_timeout - Periodically poll an address until a condition is met 
or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @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.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)  \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   might_sleep_if(sleep_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (sleep_us) \
+   usleep_range((sleep_us >> 2) + 1, sleep_us); \
+   } \
+   (cond) ? 0 : -ETIMEDOUT; \
+})
+
+/**
+ * readx_poll_timeout_atomic - Periodically poll an address until a condition 
is met or a timeout occurs
+ * @op: accessor function (takes @addr as its only argument)
+ * @addr: Address to poll
+ * @val: Variable to read the value into
+ * @cond: Break condition (usually involving @val)
+ * @delay_us: Time to udelay 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.
+ *
+ * When available, you'll probably want to use one of the specialized
+ * macros defined below rather than this macro directly.
+ */
+#define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
+({ \
+   ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+   for (;;) { \
+   (val) = op(addr); \
+   if (cond) \
+   break; \
+   if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+   (val) = op(addr); \
+   break; \
+   } \
+   if (delay_us) \
+   udelay(delay_us);   \
+   } \
+   (cond) ? 0 : -ETIMEDOU

Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Laurent Pinchart]

Hi Magnus,

On Tuesday 16 December 2014 07:44:32 Magnus Damm wrote:
> On Tue, Dec 16, 2014 at 3:44 AM, Laurent Pinchart wrote:
> > On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote:
> >> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote:
> >> > Add the seven IPMMU instances found in the r8a7791 to DT with a
> >> > disabled status.
> >> > 
> >> > Signed-off-by: Laurent Pinchart
> >> > 
> >> 
> >> The addresses and interrupt numbers look OK to me.
> >> However, my comment about the "0x800" offset is still valid.
> >> Shouldn't we have two register blocks, and let the driver use only the
> >> second one?
> >> 
> >> If you ignore, feel free to add my
> >> Acked-by: Geert Uytterhoeven 
> > 
> > I don't want to ignore your comment, but I don't know what to do here :-/
> > The datasheet lacks detailed information about secure vs. non-secure mode
> > and how the two register sets are supposed to interoperate and be handled
> > by the operating system.
> 
> I don't know about that either.
> 
> But how about differences within R-Car Gen2 series so far? If there
> are known differences then perhaps we should use part number in the
> compatible string?

I haven't noticed differences between H2 and M2. V2 and E2 are extended with 
IOMMU performance monitoring registers, so a specific compatible string is 
needed.

"renesas,ipmmu-vmsa" should be the default fallback compatible string. I was 
thinking about playing with the newer SoCs first to see how the IOMMU react 
and then most likely add SoC-specific compatible strings.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 3/3] Documentation: dt-bindings: Add the binding info for APM X-Gene AHBC IOMMU driver.

[Arnd Bergmann]

On Monday 15 December 2014 22:25:23 Suman Tripathi wrote:
> +*APM X-Gene AHBC IOMMU Architecture Implementation
> +
> +APM X-Gene SoC AHBC IOMMU translates the AHBC 32-bit address from
> +dma master to 42-bit AXI address by using a set of AIM(AHB Inbound mapper)
> +register.
> +
> +** AHBC IOMMU required properties:
> +
> +- compatible: Should be "apm,xgene-ahbc-iommu"
> +- reg   : Base address of the AHBC AXI mapper registers
> +
> +Example:
> +
> +   ahbc_iommu: ahbc_iommu@0x1f2a0080 {
> +   compatible = "apm,xgene-ahbc-iommu";
> +   reg = <0x0 0x1f2a0080 0x0 0x90>;
> +   };
> --
> 1.8.2.1
> 

This is incompatible with the iommu binding from 
Documentation/devicetree/bindings/iommu/iommu.txt

Please fix.

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

Re: [PATCH 1/3] xgene-ahbc-iommu: Add support for APM X-Gene SoC AHBC IOMMU driver.

[Arnd Bergmann]

On Monday 15 December 2014 22:25:21 Suman Tripathi wrote:
> +   return 0;
> +}
> +
> +static struct of_device_id xgene_ahbc_mmu_of_match[] = {
> +   { .compatible = "apm,xgene-ahbc-iommu"},
> +   { },
> +};
> +MODULE_DEVICE_TABLE(of, xgene_ahbc_mmu_of_match);
> +
> +static struct platform_driver xgene_ahbc_mmu_driver = {
> +   .probe  = xgene_ahbc_mmu_probe,
> +   .remove = xgene_ahbc_mmu_remove,
> +   .driver = {
> +   .owner  = THIS_MODULE,
> +   .name   = "xgene-ahbc",
> +   .of_match_table = of_match_ptr(xgene_ahbc_mmu_of_match),
> +   },
> +};
> +
> +static int xgene_ahbc_mmu_init(void)
> +{
> +   return platform_driver_register(&xgene_ahbc_mmu_driver);
> +}
> +subsys_initcall(xgene_ahbc_mmu_init);
> +
> +static void __exit xgene_ahbc_mmu_exit(void)
> +{
> +   platform_driver_unregister(&xgene_ahbc_mmu_driver);
> +}
> +module_exit(xgene_ahbc_mmu_exit);
> 

This should use the newly added of_iommu infrastructure that we
added for 3.19. Otherwise you are just duplicating common code.

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

Re: [PATCH 0/3] Add support for APM X-Gene SoC AHBC IOMMU driver.

[Joerg Roedel]

Hi Suman,

On Tue, Dec 16, 2014 at 01:31:58AM +0530, Suman Tripathi wrote:
> On Dec 16, 2014 1:25 AM, "Don Dutile"  wrote:
> 
> On 12/15/2014 11:55 AM, Suman Tripathi wrote:
> 
> This patch adds the support for the APM X-Gene SoC AHBC IOMMU driver.
> 
> Signed-off-by: Suman Tripathi 
> ---
> 
> Suman Tripathi (3):
>    xgene-ahbc-iommu: Add support for APM X-Gene SoC AHBC IOMMU driver.
>    arm64: dts: Add the APM X-Gene AHBC IOMMU DTS node.
>    Documentation: dt-bindings: Add the binding info for APM X-Gene 
> AHBC
>      IOMMU driver.
> 
>   .../devicetree/bindings/iommu/xgene,ahbc-iommu.txt |  17 ++
>   arch/arm64/boot/dts/apm-storm.dtsi                 |   5 +
>   drivers/iommu/Kconfig                              |  10 +
>   drivers/iommu/Makefile                             |   1 +
>   drivers/iommu/xgene-ahbc-iommu.c                   | 336
> +
>   5 files changed, 369 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iommu/
> xgene,ahbc-iommu.txt
>   create mode 100644 drivers/iommu/xgene-ahbc-iommu.c

What are the differences between the X-Gene IOMMU and the ARM-SMMU?


Joerg

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

Re: [PATCH 0/3] Add support for APM X-Gene SoC AHBC IOMMU driver.

[Suman Tripathi]

No not yet
On Dec 16, 2014 1:25 AM, "Don Dutile"  wrote:

> On 12/15/2014 11:55 AM, Suman Tripathi wrote:
>
>> This patch adds the support for the APM X-Gene SoC AHBC IOMMU driver.
>>
>> Signed-off-by: Suman Tripathi 
>> ---
>>
>> Suman Tripathi (3):
>>xgene-ahbc-iommu: Add support for APM X-Gene SoC AHBC IOMMU driver.
>>arm64: dts: Add the APM X-Gene AHBC IOMMU DTS node.
>>Documentation: dt-bindings: Add the binding info for APM X-Gene AHBC
>>  IOMMU driver.
>>
>>   .../devicetree/bindings/iommu/xgene,ahbc-iommu.txt |  17 ++
>>   arch/arm64/boot/dts/apm-storm.dtsi |   5 +
>>   drivers/iommu/Kconfig  |  10 +
>>   drivers/iommu/Makefile |   1 +
>>   drivers/iommu/xgene-ahbc-iommu.c   | 336
>> +
>>   5 files changed, 369 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/iommu/xgene,ahbc-
>> iommu.txt
>>   create mode 100644 drivers/iommu/xgene-ahbc-iommu.c
>>
>> --
>> 1.8.2.1
>>
>>
> Have you tested this with a kernel that uses 64K page size ?
>
>

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, 
is for the sole use of the intended recipient(s) and contains information
that is confidential and proprietary to Applied Micro Circuits Corporation or 
its subsidiaries. 
It is to be used solely for the purpose of furthering the parties' business 
relationship. 
All unauthorized review, use, disclosure or distribution is prohibited. 
If you are not the intended recipient, please contact the sender by reply 
e-mail 
and destroy all copies of the original message.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 0/3] Add support for APM X-Gene SoC AHBC IOMMU driver.

[Don Dutile]

On 12/15/2014 11:55 AM, Suman Tripathi wrote:

This patch adds the support for the APM X-Gene SoC AHBC IOMMU driver.

Signed-off-by: Suman Tripathi 
---

Suman Tripathi (3):
   xgene-ahbc-iommu: Add support for APM X-Gene SoC AHBC IOMMU driver.
   arm64: dts: Add the APM X-Gene AHBC IOMMU DTS node.
   Documentation: dt-bindings: Add the binding info for APM X-Gene AHBC
 IOMMU driver.

  .../devicetree/bindings/iommu/xgene,ahbc-iommu.txt |  17 ++
  arch/arm64/boot/dts/apm-storm.dtsi |   5 +
  drivers/iommu/Kconfig  |  10 +
  drivers/iommu/Makefile |   1 +
  drivers/iommu/xgene-ahbc-iommu.c   | 336 +
  5 files changed, 369 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/iommu/xgene,ahbc-iommu.txt
  create mode 100644 drivers/iommu/xgene-ahbc-iommu.c

--
1.8.2.1



Have you tested this with a kernel that uses 64K page size ?

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

Re: [PATCH 0/5] Fix Intel IRQ remapping initialization order

[Thomas Gleixner]

On Mon, 15 Dec 2014, Jiang Liu wrote:
> On 2014/12/15 23:13, Joerg Roedel wrote:
> > Hi,
> > 
> > here is a patch-set against tip/x86/apic to fix an initialization order
> > problem with the IRQ remapping code.  The problem is in the ordering of
> > the irq_remapping_prepare and irq_remapping_supported functions.
> > 
> > Currently the call-order is irq_remapping_prepare ->
> > irq_remapping_supported, so that 'prepare' can succeed but 'supported'
> > fails, so that interrupt remapping gets initialized but not enabled.
> > This causes a broken interrupt setup on affected systems (machines with
> > an Intel IOMMU without, or broken, IRQ remapping support). The result
> > are lost interrupts and a non-bootable system.
> > 
> > Both functions do checks whether IRQ remapping can be enabled on the
> > machine.  The reason for this is that some checks rely on
> > dmar_table_init() and thus have to be done in irq_remapping_prepare().
> > 
> > This patch-set moves all these checks into the irq_remapping_prepare()
> > path with the right ordering and removes the irq_remapping_supported()
> > function and its call-backs. This fixes the initializion order problem
> > and simplifies the exported API from the IOMMU code.
> > 
> > Please review.
> Hi Joerg,
>   I have posted a patch set for the same purpose at:
> https://lkml.org/lkml/2014/12/10/20
>   Seems we need to combine these two patch sets:)

Actually you want to combine it also with these patches:

326c2bb2c526: iommu/vt-d: Convert allocations to GFP_KERNEL
e9220e591375: iommu/vt-d: Move iommu preparatory allocations to 
irq_remap_ops.prepare
e88edbd316ea: iommu, x86: Restructure setup of the irq remapping feature
dd60143c04f2: x86, smpboot: Remove pointless preempt_disable() in 
native_smp_prepare_cpus()

against 3.19 independent of the irqdomain stuff.

So that would be a clean base to put the rest of the irqdomain and
hotplug stuff on top.

So the ordering of the patches for 3.20 would become:

   iommu cleanup (init and allocations)
   acpi cleanup and parser extensions
   ioapic hotplug
   irqdomain conversion

I will route dd60143c04f2 "x86, smpboot: Remove pointless
preempt_disable() in native_smp_prepare_cpus()" into -rc1.  I'm going
to do so for a few other cherry-picks from x86/apic.

So can you please create a combined series, which just deals with the
init cleanup and the allocation conversion (ATOMIC -> GFP) based on
current Linus tree should be fine.

Thanks,

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

Re: [RFC PATCH 2/2] iommu: rockchip: Handle system-wide and runtime PM

[Laurent Pinchart]

Hi Tomasz,

On Monday 15 December 2014 11:39:01 Tomasz Figa wrote:
> On Sat, Dec 13, 2014 at 5:47 AM, Laurent Pinchart wrote:
> > On Friday 12 December 2014 13:15:51 Tomasz Figa wrote:
> >> On Fri, Dec 12, 2014 at 5:48 AM, Rafael J. Wysocki wrote:
> >>> On Thursday, December 11, 2014 04:51:37 PM Ulf Hansson wrote:
>  On 11 December 2014 at 16:31, Kevin Hilman  wrote:
> > [+ Laurent Pinchart]
> > 
> > Tomasz Figa  writes:
> >> On Thu, Dec 11, 2014 at 8:58 PM, Ulf Hansson wrote:
> > [...]
> > 
>  @@ -988,11 +1107,28 @@ static int rk_iommu_probe(struct
>  platform_device *pdev)
>  return -ENXIO;
>  }
>  
>  +   pm_runtime_no_callbacks(dev);
>  +   pm_runtime_enable(dev);
>  +
>  +   /* Synchronize state of the domain with driver data. */
>  +   pm_runtime_get_sync(dev);
>  +   iommu->is_powered = true;
> >>> 
> >>> Doesn't the runtime PM status reflect the value of "is_powered",
> >>> thus why do you need to have a copy of it? Could it perpahps be
> >>> that you try to cope with the case when CONFIG_PM is unset?
> >> 
> >> It's worth noting that this driver fully relies on status of other
> >> devices in the power domain the IOMMU is in and does not enforce
> >> the status on its own. So in general, as far as my understanding of
> >> PM runtime subsystem, the status of the IOMMU device will be always
> >> suspended, because nobody will call pm_runtime_get() on it (except
> >> the get and put pair in probe). So is_powered is here to track
> >> status of the domain, not the device. Feel free to suggest a better
> >> way, though.
> > 
> > I still don't like these notifiers.  I think they add ways to bypass
> > having proper runtime PM implemented for devices/subsystems.
>  
>  I do agree, but I haven't found another good solution to the problem.
> >>> 
> >>> For the record, I'm not liking this mostly because it "fixes" a generic
> >>> problem in a way that's hidden in the genpd code and very indirect.
> >> 
> >> Well, that's true. This is indeed a generic problem of PM dependencies
> >> between devices (other than those represented by parent-child
> >> relation), which in fact doesn't have much to do with genpd, but
> >> rather with those devices directly. It is just that genpd is the most
> >> convenient location to solve this in current code and in a simple way.
> >> In other words, I see this solution as a reasonable way to get the
> >> problem solved quickly for now, so that we can start thinking about a
> >> more elegant solution.
> >> 
> >> >> > From a high-level, the IOMMU is just another device inside the PM
> >> >> > domain, so ideally it should be doing it's own _get() and _put()
> >> >> > calls so the PM domain code would just do the right thing without
> >> >> > the need for notifiers.
> >> >> 
> >> >> As I understand it, the IOMMU (or for other similar cases) shouldn't
> >> >> be doing any get() and put() at all because there are no IO API to
> >> >> serve request from.
> > 
> > Speaking purely from an IOMMU point of view that's not entirely tree.
> > IOMMU drivers expose map and unmap operations, so they can track whether
> > any memory is mapped. From a bus master point of view the IOMMU map and
> > unmap operations are hidden by the DMA mapping API. The IOMMU can thus
> > track the existence of mappings without any IOMMU awareness in the bus
> > master driver.
> > 
> > If no mapping exist the IOMMU shouldn't receive any translation request.
> > An IOMMU driver could thus call pm_runtime_get_sync() in the map handler
> > when creating the first mapping, and pm_runtime_put() in the unmap handler
> > when tearing the last mapping down.
> > 
> > One could argue that the IOMMU would end up being powered more often than
> > strictly needed, as bus masters drivers, even when written properly, could
> > keep mappings around at times they don't perform bus access. This is true,
> > and that's an argument I've raised during the last kernel summit. The
> > general response (including Linus Torvald's) was that micro-optimizing
> > power management might not be worth it, and that measurements proving
> > that the gain is worth it are required before introducing new APIs to
> > solve the problem. I can't disagree with that argument.
> 
> This would be a micro optimization if the IOMMU was located in its own
> power domain. Unfortunately in most cases it is not, so keeping all
> the devices in the domain powered on, because one of them have a
> mapping created doesn't sound like a good idea.
> 
> Moreover, most of the drivers will keep the mapping for much longer
> than one run cycle. Please take a look at V4L2's videobuf2 subsystem
> (which I guess you are more familiar with than me;)), which will keep
> MMAP buffers mapped in IOMMU address space for their whole lifetime. I
> believ

Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Laurent Pinchart]

Hi Geert,

On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote:
> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote:
> > Add the seven IPMMU instances found in the r8a7791 to DT with a disabled
> > status.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> The addresses and interrupt numbers look OK to me.
> However, my comment about the "0x800" offset is still valid.
> Shouldn't we have two register blocks, and let the driver use only the
> second one?
> 
> If you ignore, feel free to add my
> Acked-by: Geert Uytterhoeven 

I don't want to ignore your comment, but I don't know what to do here :-/ The 
datasheet lacks detailed information about secure vs. non-secure mode and how 
the two register sets are supposed to interoperate and be handled by the 
operating system.

-- 
Regards,

Laurent Pinchart

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

[PATCH FOR v3.19 - v2] iommu/ipmmu-vmsa: change IOMMU_EXEC to IOMMU_NOEXEC

[Laurent Pinchart]

From: Laurent Pinchart 

Commit a720b41c41f5a7e4 ("iommu/arm-smmu: change IOMMU_EXEC to
IOMMU_NOEXEC") has inverted and replaced the IOMMU_EXEC flag with
IOMMU_NOEXEC. Update the driver accordingly.

Signed-off-by: Laurent Pinchart 
---
 drivers/iommu/ipmmu-vmsa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Changes since v1:

- Rewrote the commit message

Joerg, this is a compilation breakage fix for v3.19. Could you please take it
in your tree and submit it early during the -rc cycle ?

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 68dfb0f..7486931 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -558,7 +558,7 @@ static pmd_t *ipmmu_alloc_pmd(struct ipmmu_vmsa_device 
*mmu, pgd_t *pgd,
 
 static u64 ipmmu_page_prot(unsigned int prot, u64 type)
 {
-   u64 pgprot = ARM_VMSA_PTE_XN | ARM_VMSA_PTE_nG | ARM_VMSA_PTE_AF
+   u64 pgprot = ARM_VMSA_PTE_nG | ARM_VMSA_PTE_AF
   | ARM_VMSA_PTE_SH_IS | ARM_VMSA_PTE_AP_UNPRIV
   | ARM_VMSA_PTE_NS | type;
 
@@ -568,8 +568,8 @@ static u64 ipmmu_page_prot(unsigned int prot, u64 type)
if (prot & IOMMU_CACHE)
pgprot |= IMMAIR_ATTR_IDX_WBRWA << ARM_VMSA_PTE_ATTRINDX_SHIFT;
 
-   if (prot & IOMMU_EXEC)
-   pgprot &= ~ARM_VMSA_PTE_XN;
+   if (prot & IOMMU_NOEXEC)
+   pgprot |= ARM_VMSA_PTE_XN;
else if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
/* If no access create a faulting entry to avoid TLB fills. */
pgprot &= ~ARM_VMSA_PTE_PAGE;
-- 
Regards,

Laurent Pinchart

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

Re: [PATCH] iommu/ipmmu-vmsa: change IOMMU_EXEC to IOMMU_NOEXEC

[Laurent Pinchart]

Hi Geert,

On Monday 15 December 2014 11:45:27 Geert Uytterhoeven wrote:
> On Sun, Dec 14, 2014 at 1:55 AM, Laurent Pinchart wrote:
> > The IOMMU_EXEC flag has been renamed to IOMMU_NOEXEC by commit
> 
> ... inverted and replaced by ...
> 
> At first I hadn't noticed this, and was wondering about the seemingly
> unrelated change in the first hunk.

Good point. I'll submit a v2.

> > a720b41c41f5a7e4 ("iommu/arm-smmu: change IOMMU_EXEC to IOMMU_NOEXEC").
> > Update the driver accordingly.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/iommu/ipmmu-vmsa.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > This fixes a build breakage in Linus' master branch introduce by the
> > commit mentioned above.
> > 
> > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > index e5ed0216929f..7a1bcf6627b6 100644
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -558,7 +558,7 @@ static pmd_t *ipmmu_alloc_pmd(struct ipmmu_vmsa_device
> > *mmu, pgd_t *pgd,
> >  static u64 ipmmu_page_prot(unsigned int prot, u64 type)
> >  {
> > -   u64 pgprot = ARM_VMSA_PTE_XN | ARM_VMSA_PTE_nG | ARM_VMSA_PTE_AF
> > +   u64 pgprot = ARM_VMSA_PTE_nG | ARM_VMSA_PTE_AF
> >| ARM_VMSA_PTE_SH_IS | ARM_VMSA_PTE_AP_UNPRIV
> >| ARM_VMSA_PTE_NS | type;
> > @@ -568,8 +568,8 @@ static u64 ipmmu_page_prot(unsigned int prot, u64
> > type)
> > if (prot & IOMMU_CACHE)
> > pgprot |= IMMAIR_ATTR_IDX_WBRWA <<
> > ARM_VMSA_PTE_ATTRINDX_SHIFT;
> > -   if (prot & IOMMU_EXEC)
> > -   pgprot &= ~ARM_VMSA_PTE_XN;
> > +   if (prot & IOMMU_NOEXEC)
> > +   pgprot |= ARM_VMSA_PTE_XN;
> > else if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
> > /* If no access create a faulting entry to avoid TLB
> > fills. */
> > pgprot &= ~ARM_VMSA_PTE_PAGE;

-- 
Regards,

Laurent Pinchart

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

Re: [Bug 89751] New: Allocating domain for fallback device failed

[Bjorn Helgaas]

[+to David, +cc iommu list, Joerg]

On Sun, Dec 14, 2014 at 7:44 AM,   wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=89751
>
> Bug ID: 89751
>Summary: Allocating domain for fallback device failed
>Product: Drivers
>Version: 2.5
> Kernel Version: 3.18
>   Hardware: x86-64
> OS: Linux
>   Tree: Mainline
> Status: NEW
>   Severity: normal
>   Priority: P1
>  Component: PCI
>   Assignee: drivers_...@kernel-bugs.osdl.org
>   Reporter: rwar...@gmx.de
> Regression: No
>
> Created attachment 160531
>   --> https://bugzilla.kernel.org/attachment.cgi?id=160531&action=edit
> dmesg
>
> Since enabling INTEL_IOMMU_DEFAULT_ON I see "Allocating domain for fallback
> device failed" in my logs.

Ronald, do you know whether this is a regression?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

[Laurent Pinchart]

On Monday 15 December 2014 18:13:23 Will Deacon wrote:
> On Mon, Dec 15, 2014 at 05:53:48PM +, Laurent Pinchart wrote:
> > Hi Will,
> 
> Hello again :)
> 
> > On Monday 15 December 2014 17:43:02 Will Deacon wrote:
> > > On Mon, Dec 15, 2014 at 05:27:30PM +, Laurent Pinchart wrote:
> > > > On Monday 15 December 2014 17:17:00 Will Deacon wrote:
> > > > > Creating the platform device manually for the IOMMU is indeed
> > > > > grotty, but I don't really understand why it's needed. Interrupt
> > > > > controllers, for example, seem to get by without one.
> > > > 
> > > > There's several reasons, one of the most compelling ones I can think
> > > > of at the moment is runtime PM. IRQ controllers close to the CPU use
> > > > CPU PM notifiers instead. Note that IRQ controllers that are further
> > > > away from the CPU (such as GPIO-based IRQ controllers) are real
> > > > platform devices and use runtime PM.
> > > 
> > > Ok, that's a good point, but the IOMMU will still probe later anyway,
> > > right?
> >
> > That depends on the driver implementation, using OF node match an IOMMU
> > driver doesn't have to register a struct driver. Assuming we require
> > IOMMU drivers to register a struct driver, a platform device should then
> > be probed at a later time.
> > 
> > However, if we wait until the IOMMU gets probed to initialize runtime PM
> > and make it functional, we'll be back in square one if the IOMMU gets
> > probed after the bus master, as the bus master could start issuing bus
> > requests at probe time with the IOMMU not powered yet.
> 
> True, but couldn't the early init code do enough to get the thing
> functional? That said, I'm showing my ignorance here as I'm not familiar
> with the PM code (and the software models I have for the SMMU clearly don't
> implement anything in this regard).

We're reaching the limits of my knowledge as well. If the IOMMU is in a power 
domain different than the bus masters the driver would at least need to ensure 
that the power domain is turned on, which might be a bit hackish without 
runtime PM.

> > > > IOMMUs are not as low-level as system interrupt controllers or system
> > > > clocks. I'm beginning to agree with Thierry that they should be
> > > > treated as normal platform devices as they're not required earlier
> > > > than probe time of their bus master devices.
> > > 
> > > Well, I think you'd have to propose patches for discussion since I'm
> > > certainly not wed to the current approach; I just want something that
> > > allows of_{dma,iommu}_configure to run with the information it needs.
> > 
> > Do we need of_dma_configure() to run when the device is created, or could
> > we postpone it to just before probe time ?
> 
> I'm not sure I can answer that one... Arnd?

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

[Will Deacon]

On Mon, Dec 15, 2014 at 05:53:48PM +, Laurent Pinchart wrote:
> Hi Will,

Hello again :)

> On Monday 15 December 2014 17:43:02 Will Deacon wrote:
> > On Mon, Dec 15, 2014 at 05:27:30PM +, Laurent Pinchart wrote:
> > > On Monday 15 December 2014 17:17:00 Will Deacon wrote:
> > > > Creating the platform device manually for the IOMMU is indeed grotty,
> > > > but I don't really understand why it's needed. Interrupt controllers,
> > > > for example, seem to get by without one.
> > > 
> > > There's several reasons, one of the most compelling ones I can think of at
> > > the moment is runtime PM. IRQ controllers close to the CPU use CPU PM
> > > notifiers instead. Note that IRQ controllers that are further away from
> > > the CPU (such as GPIO-based IRQ controllers) are real platform devices
> > > and use runtime PM.
> >
> > Ok, that's a good point, but the IOMMU will still probe later anyway, right?
> 
> That depends on the driver implementation, using OF node match an IOMMU 
> driver 
> doesn't have to register a struct driver. Assuming we require IOMMU drivers 
> to 
> register a struct driver, a platform device should then be probed at a later 
> time.
> 
> However, if we wait until the IOMMU gets probed to initialize runtime PM and 
> make it functional, we'll be back in square one if the IOMMU gets probed 
> after 
> the bus master, as the bus master could start issuing bus requests at probe 
> time with the IOMMU not powered yet.

True, but couldn't the early init code do enough to get the thing
functional? That said, I'm showing my ignorance here as I'm not familiar
with the PM code (and the software models I have for the SMMU clearly don't
implement anything in this regard).

> > > IOMMUs are not as low-level as system interrupt controllers or system
> > > clocks. I'm beginning to agree with Thierry that they should be treated
> > > as normal platform devices as they're not required earlier than probe
> > > time of their bus master devices.
> > 
> > Well, I think you'd have to propose patches for discussion since I'm
> > certainly not wed to the current approach; I just want something that
> > allows of_{dma,iommu}_configure to run with the information it needs.
> 
> Do we need of_dma_configure() to run when the device is created, or could we 
> postpone it to just before probe time ?

I'm not sure I can answer that one... Arnd?

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

Re: [PATCH v6 6/8] dma-mapping: detect and configure IOMMU in of_dma_configure

[Will Deacon]

On Mon, Dec 15, 2014 at 05:16:50PM +, Laurent Pinchart wrote:
> On Monday 15 December 2014 16:40:41 Will Deacon wrote:
> > On Sun, Dec 14, 2014 at 03:49:34PM +, Laurent Pinchart wrote:
> > > On Wednesday 10 December 2014 15:08:53 Will Deacon wrote:
> > > > On Wed, Dec 10, 2014 at 02:52:56PM +, Rob Clark wrote:
> > > > > so, what is the way for a driver that explicitly wants to manage it's
> > > > > own device virtual address space to opt out of this?  I suspect that
> > > > > won't be the common case, but for a gpu, if dma layer all of a sudden
> > > > > thinks it is in control of the gpu's virtual address space, things are
> > > > > going to end in tears..
> > > > 
> > > > I think you'll need to detach from the DMA domain, then have the driver
> > > > manage everything itself. As you say, it's not the common case, so you
> > > > may need to add some hooks for detaching from the default domain and
> > > > swizzling your DMA ops.
> > > 
> > > I'm wondering if it's such an exotic case after all. I can see two reasons
> > > not to use the default domain. In addition to special requirements coming
> > > from the bus master side, the IOMMU itself might not support one domain
> > > per bus master (I'm of course raising the issue from a very selfish
> > > Renesas IPMMU point of view).
> > 
> > Do you mean that certain masters must be grouped into the same domain, or
> > that the IOMMU can fail with -ENOSPC?
> 
> My IOMMU has hardware supports for 4 domains, and serves N masters (where N 
> is 
> dependent on the SoC but is > 4). In its current form the driver supports a 
> single domain and thus detaches devices from the default domain in the 
> add_device callback:

Hmm, ok. Ideally, you wouldn't need to do any of that in the driver, but I
can understand why you decided to go down that route.

>   /*
>* Detach the device from the default ARM VA mapping and attach it to
>* our private mapping.
>*/
>   arm_iommu_detach_device(dev);
>   ret = arm_iommu_attach_device(dev, mmu->mapping);
>   if (ret < 0) {
>   dev_err(dev, "Failed to attach device to VA mapping\n");
>   return ret;
>   }
> 
> I would have implemented that in the of_xlate callback, but that's too early 
> as the ARM default domain isn't created yet at that point.

Yup, the mythical ->get_default_domain might be the right place instead.

> Using a single domain is a bit of a waste of resources in my case, so an 
> evolution would be to create four domains and assign devices to them based on 
> a policy. The policy could be fixed (round-robin for instance), or 
> configurable (possibly through DT, although it's really a policy, not a 
> hardware description).

I think having one default domain, which is home to all of the masters that
don't have any DMA restrictions is a good use of the hardware. That then
leaves you with three domains to cover VFIO, devices with DMA limitations
and potentially device isolation (if we had a way to describe that).

> > For the former, we need a way to represent IOMMU groups for the platform
> > bus.
> 
> To be honest I'm not entirely sure how IOMMU groups are supposed to be used. 
> I 
> understand they can be used by VFIO to group several masters that will be 
> able 
> to see each other's memory through the same page table, and also that a page 
> table could be shared between multiple groups. When it comes to group 
> creation, though, things get fuzzy. I started with creating one group per 
> master in my driver, which is probably not the thing to do. The Exynos IOMMU 
> driver used to do the same, until Marek's patch series converting it to DT-
> based instantiation (on top of your patch set) has removed groups altogether. 
> Groups seem to be more or less optional, except in a couple of places (for 
> instance the remove_device callback will not be called by the 
> BUS_NOTIFY_DEL_DEVICE notifier if the device isn't part of an iommu group).
> 
> I'd appreciate if someone could clarify this to help me make an informed 
> opinion on the topic.

Ok, an iommu_group is the minimum granularity for which a specific IOMMU
can offer address translation. So, if your IPMMU can isolate an arbitrary
device (assuming there is a domain available), then each device is in its
own iommu_group. This isn't always the case, for example if two masters are
behind some sort of bridge that makes them indistinguishable to the IOMMU
(perhaps they appear to have the same master ID), then they would have to
be in the same iommu_group. Essentially, iommu_groups are a property of
the hardware and should be instantiated by the bus. PCI does this, but
we don't yet have anything for the platform bus.

VFIO puts multiple groups (now called vfio_groups) into a container. The
container is synoymous to an iommu_domain (i.e. a shared address space).

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfou

Re: [RFC PATCH 2/2] iommu: rockchip: Handle system-wide and runtime PM

[Kevin Hilman]

Geert Uytterhoeven  writes:

> On Fri, Dec 12, 2014 at 9:04 PM, Kevin Hilman  wrote:
>> An even more exciting problem exists when a CPU is in the same domain as
>> other peripherals, those peripherals are all idle and the power domain
>> is gated. :)
>
> We do have pm_genpd_attach_cpuidle() and pm_genpd_name_attach_cpuidle()?

Exactly.  

And I think to solve this problem, we need to generalize the attaching
of dependent devices to a PM domain.

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

Re: [RFC PATCH v4 0/8] Introduce automatic DMA configuration for IOMMU masters

[Laurent Pinchart]

Hi Will,

On Monday 15 December 2014 17:34:16 Will Deacon wrote:
> On Mon, Dec 15, 2014 at 05:21:16PM +, Laurent Pinchart wrote:
> > On Wednesday 19 November 2014 11:41:50 Will Deacon wrote:
> > > +static void __remove_iommu_mapping_entry(struct kref *kref)
> > > +{
> > > + struct dma_iommu_mapping_entry *entry;
> > > +
> > > + entry = container_of(kref, struct dma_iommu_mapping_entry, kref);
> > > + list_del(&entry->list);
> > > +}
> > > +
> > > +static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base,
> > > u64
> > > size,
> > > + struct iommu_ops *iommu)
> > > +{
> > > + struct iommu_domain *domain;
> > > + struct dma_iommu_mapping_entry *entry = NULL;
> > > +
> > > + if (!iommu->get_default_domain)
> > > + return false;
> > > +
> > > + domain = iommu->get_default_domain(dev);
> > > + if (!domain)
> > > + return false;
> > > +
> > > + spin_lock(&dma_iommu_mapping_lock);
> > > +
> > > + list_for_each_entry(entry, &dma_iommu_mapping_table, list) {
> > > + if (entry->domain == domain)
> > > + break;
> > > + }
> > 
> > That might be a stupid question (fighting my impostor syndrome again
> > here), but is there a fundamental reason why we can't store the VA
> > allocation data (probably using iova) in the domain instead of having to
> > go through hoops and loops here to associate that data to the domain ?
> 
> Well, this was very much a work-in-progress that I ended up abandoning :)
> 
> Ultimately, I'd really like to see the default domain management moved into
> core code, at which point extending struct iommu_domain seems perfectly
> plausible to me (not sure if we'd have a new structure for that).
> 
> Then we'd have a fairly clean device -> group -> domain -> allocator
> abstraction for each master.

I like that. Thank you for confirming that some of my questions are sensible 
:-)

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

[Laurent Pinchart]

Hi Will,

On Monday 15 December 2014 17:43:02 Will Deacon wrote:
> On Mon, Dec 15, 2014 at 05:27:30PM +, Laurent Pinchart wrote:
> > On Monday 15 December 2014 17:17:00 Will Deacon wrote:
> > > On Sun, Dec 14, 2014 at 12:45:36PM +, Laurent Pinchart wrote:
> > > > On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
> > > > > +static int __init exynos_iommu_of_setup(struct device_node *np)
> > > > > +{
> > > > > + struct platform_device *pdev;
> > > > > +
> > > > > + if (!init_done)
> > > > > + exynos_iommu_init();
> > > > > +
> > > > > + pdev = of_platform_device_create(np, NULL,
> > > > > platform_bus_type.dev_root);
> > > > > + if (IS_ERR(pdev))
> > > > > + return PTR_ERR(pdev);
> > > > 
> > > > If we end up having to create the IOMMU platform devices from within
> > > > the drivers, the introduction of IOMMU_OF_DECLARE starts to feel like
> > > > a workaround to me. I wonder whether it wouldn't then be better to let
> > > > the driver core instantiate the IOMMU platform device from DT as for
> > > > all other devices, and use device notifiers to defer probe of the bus
> > > > masters until the required IOMMU(s) are registered.
> > > > 
> > > > Will, what's your opinion on that ?
> > > 
> > > Creating the platform device manually for the IOMMU is indeed grotty,
> > > but I don't really understand why it's needed. Interrupt controllers,
> > > for example, seem to get by without one.
> > 
> > There's several reasons, one of the most compelling ones I can think of at
> > the moment is runtime PM. IRQ controllers close to the CPU use CPU PM
> > notifiers instead. Note that IRQ controllers that are further away from
> > the CPU (such as GPIO-based IRQ controllers) are real platform devices
> > and use runtime PM.
>
> Ok, that's a good point, but the IOMMU will still probe later anyway, right?

That depends on the driver implementation, using OF node match an IOMMU driver 
doesn't have to register a struct driver. Assuming we require IOMMU drivers to 
register a struct driver, a platform device should then be probed at a later 
time.

However, if we wait until the IOMMU gets probed to initialize runtime PM and 
make it functional, we'll be back in square one if the IOMMU gets probed after 
the bus master, as the bus master could start issuing bus requests at probe 
time with the IOMMU not powered yet.

> > IOMMUs are not as low-level as system interrupt controllers or system
> > clocks. I'm beginning to agree with Thierry that they should be treated
> > as normal platform devices as they're not required earlier than probe
> > time of their bus master devices.
> 
> Well, I think you'd have to propose patches for discussion since I'm
> certainly not wed to the current approach; I just want something that
> allows of_{dma,iommu}_configure to run with the information it needs.

Do we need of_dma_configure() to run when the device is created, or could we 
postpone it to just before probe time ?

> The usual answer is `we should model buses properly', but that's really
> not practical afaict.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 0/4] Generic IOMMU page table framework

[Laurent Pinchart]

Hi Will,

On Monday 15 December 2014 17:39:11 Will Deacon wrote:
> On Mon, Dec 15, 2014 at 05:33:32PM +, Laurent Pinchart wrote:
> > On Monday 15 December 2014 16:10:52 Will Deacon wrote:
> > > On Sun, Dec 14, 2014 at 11:49:30PM +, Laurent Pinchart wrote:
> > > > diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
> > > > index c1cff3d045db..a41a15d30596 100644
> > > > --- a/drivers/iommu/io-pgtable.h
> > > > +++ b/drivers/iommu/io-pgtable.h
> > > > @@ -24,6 +24,9 @@ struct iommu_gather_ops {
> > > > 
> > > > void (*flush_pgtable)(void *ptr, size_t size, void *cookie);
> > > >  
> > > >  };
> > > > 
> > > > +/* Set the Non-Secure bit in the PTEs */
> > > > +#define IO_PGTABLE_QUIRK_NON_SECURE(1 << 0)
> > > 
> > > I think I'd stick an _ARM_ somewhere in here, so maybe
> > > IO_PGTABLE_QUIRK_ARM_NS?
> > 
> > I'm fine with that.
> > 
> > By the way, I'm only familiar with the Renesas implementation of the VMSA
> > IOMMU, could you double-check whether setting the NSTABLE and NS bits on
> > all levels make sense to you ? It seems to be required by my hardware,
> > even though the ARM spec mentions that setting the NSTABLE bit causes
> > non-secure accesses to page tables for all lower levels regardless of
> > their NSTABLE/NS bits.
>
> The ARM ARM is very clear that subsequent levels of lookup must ignore the
> NSTABLE/NS bits since otherwise you potentially have a security violation
> where you can use the table walker to access secure memory from
> non-secure...
>
> So, you might want to check up on that, but given that this is a quirk I'm
> happy for it to do whatever you need.

Given that the ARM ARM states that subsequent levels must consider the 
NSTABLE/NS bits to be set, I think it's harmless to actually set them. I just 
wanted to double-check with you, as we agree let's proceed with the proposed 
patch.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

[Will Deacon]

On Mon, Dec 15, 2014 at 05:27:30PM +, Laurent Pinchart wrote:
> On Monday 15 December 2014 17:17:00 Will Deacon wrote:
> > On Sun, Dec 14, 2014 at 12:45:36PM +, Laurent Pinchart wrote:
> > > On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
> > > > +static int __init exynos_iommu_of_setup(struct device_node *np)
> > > > +{
> > > > +   struct platform_device *pdev;
> > > > +
> > > > +   if (!init_done)
> > > > +   exynos_iommu_init();
> > > > +
> > > > +   pdev = of_platform_device_create(np, NULL,
> > > > platform_bus_type.dev_root);
> > > > +   if (IS_ERR(pdev))
> > > > +   return PTR_ERR(pdev);
> > > 
> > > If we end up having to create the IOMMU platform devices from within the
> > > drivers, the introduction of IOMMU_OF_DECLARE starts to feel like a
> > > workaround to me. I wonder whether it wouldn't then be better to let the
> > > driver core instantiate the IOMMU platform device from DT as for all
> > > other devices, and use device notifiers to defer probe of the bus masters
> > > until the required IOMMU(s) are registered.
> > > 
> > > Will, what's your opinion on that ?
> > 
> > Creating the platform device manually for the IOMMU is indeed grotty, but I
> > don't really understand why it's needed. Interrupt controllers, for example,
> > seem to get by without one.
> 
> There's several reasons, one of the most compelling ones I can think of at 
> the 
> moment is runtime PM. IRQ controllers close to the CPU use CPU PM notifiers 
> instead. Note that IRQ controllers that are further away from the CPU (such 
> as 
> GPIO-based IRQ controllers) are real platform devices and use runtime PM.

Ok, that's a good point, but the IOMMU will still probe later anyway, right?

> IOMMUs are not as low-level as system interrupt controllers or system clocks. 
> I'm beginning to agree with Thierry that they should be treated as normal 
> platform devices as they're not required earlier than probe time of their bus 
> master devices.

Well, I think you'd have to propose patches for discussion since I'm
certainly not wed to the current approach; I just want something that
allows of_{dma,iommu}_configure to run with the information it needs.

The usual answer is `we should model buses properly', but that's really
not practical afaict.

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

Re: [PATCH 0/4] Generic IOMMU page table framework

[Will Deacon]

On Mon, Dec 15, 2014 at 05:33:32PM +, Laurent Pinchart wrote:
> On Monday 15 December 2014 16:10:52 Will Deacon wrote:
> > On Sun, Dec 14, 2014 at 11:49:30PM +, Laurent Pinchart wrote:
> > > diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
> > > index c1cff3d045db..a41a15d30596 100644
> > > --- a/drivers/iommu/io-pgtable.h
> > > +++ b/drivers/iommu/io-pgtable.h
> > > @@ -24,6 +24,9 @@ struct iommu_gather_ops {
> > >   void (*flush_pgtable)(void *ptr, size_t size, void *cookie);
> > >  };
> > > 
> > > +/* Set the Non-Secure bit in the PTEs */
> > > +#define IO_PGTABLE_QUIRK_NON_SECURE  (1 << 0)
> > 
> > I think I'd stick an _ARM_ somewhere in here, so maybe
> > IO_PGTABLE_QUIRK_ARM_NS?
> 
> I'm fine with that.
> 
> By the way, I'm only familiar with the Renesas implementation of the VMSA 
> IOMMU, could you double-check whether setting the NSTABLE and NS bits on all 
> levels make sense to you ? It seems to be required by my hardware, even 
> though 
> the ARM spec mentions that setting the NSTABLE bit causes non-secure accesses 
> to page tables for all lower levels regardless of their NSTABLE/NS bits.

The ARM ARM is very clear that subsequent levels of lookup must ignore the
NSTABLE/NS bits since otherwise you potentially have a security violation
where you can use the table walker to access secure memory from
non-secure...

So, you might want to check up on that, but given that this is a quirk I'm
happy for it to do whatever you need.

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

Re: [RFC PATCH v4 0/8] Introduce automatic DMA configuration for IOMMU masters

[Will Deacon]

On Mon, Dec 15, 2014 at 05:21:16PM +, Laurent Pinchart wrote:
> Hi Will,

Hi Laurent,

> On Wednesday 19 November 2014 11:41:50 Will Deacon wrote:
> > +static void __remove_iommu_mapping_entry(struct kref *kref)
> > +{
> > +   struct dma_iommu_mapping_entry *entry;
> > +
> > +   entry = container_of(kref, struct dma_iommu_mapping_entry, kref);
> > +   list_del(&entry->list);
> > +}
> > +
> > +static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64
> > size,
> > +   struct iommu_ops *iommu)
> > +{
> > +   struct iommu_domain *domain;
> > +   struct dma_iommu_mapping_entry *entry = NULL;
> > +
> > +   if (!iommu->get_default_domain)
> > +   return false;
> > +
> > +   domain = iommu->get_default_domain(dev);
> > +   if (!domain)
> > +   return false;
> > +
> > +   spin_lock(&dma_iommu_mapping_lock);
> > +
> > +   list_for_each_entry(entry, &dma_iommu_mapping_table, list) {
> > +   if (entry->domain == domain)
> > +   break;
> > +   }
> 
> That might be a stupid question (fighting my impostor syndrome again here), 
> but is there a fundamental reason why we can't store the VA allocation data 
> (probably using iova) in the domain instead of having to go through hoops and 
> loops here to associate that data to the domain ?

Well, this was very much a work-in-progress that I ended up abandoning :)

Ultimately, I'd really like to see the default domain management moved into
core code, at which point extending struct iommu_domain seems perfectly
plausible to me (not sure if we'd have a new structure for that).

Then we'd have a fairly clean device -> group -> domain -> allocator
abstraction for each master.

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

Re: [PATCH 0/4] Generic IOMMU page table framework

[Laurent Pinchart]

Hi Will,

On Monday 15 December 2014 16:10:52 Will Deacon wrote:
> On Sun, Dec 14, 2014 at 11:49:30PM +, Laurent Pinchart wrote:
> > On Thursday 27 November 2014 11:51:14 Will Deacon wrote:
> > > This series introduces a generic IOMMU page table allocation framework,
> > > implements support for ARM long-descriptors and then ports the arm-smmu
> > > driver over to the new code.
> 
> [...]
> 
> > > All feedback welcome.
> > 
> > I've successfully tested the patch set with the Renesas IPMMU-VMSA driver
> > with the following extension to the allocator.
> > 
> > Tested-by: Laurent Pinchart 
> 
> Wahey, that's really cool, thanks! I have a few minor comments on your patch
> below. If you don't object, then I can make them locally and include your
> patch on top of my v2 series?

Sure. Please see my reply below.

> > From 4bebb7f3a5a48541d4c89ce7c61e6ff66686c3a9 Mon Sep 17 00:00:00 2001
> > From: Laurent Pinchart 
> > Date: Sun, 14 Dec 2014 23:34:50 +0200
> > Subject: [PATCH] iommu: io-pgtable-arm: Add Non-Secure quirk
> > 
> > The quirk causes the Non-Secure bit to be set in all page table entries.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >  drivers/iommu/io-pgtable-arm.c | 7 +++
> >  drivers/iommu/io-pgtable.h | 3 +++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/drivers/iommu/io-pgtable-arm.c
> > b/drivers/iommu/io-pgtable-arm.c index 669e322a83a4..b6910e142734 100644
> > --- a/drivers/iommu/io-pgtable-arm.c
> > +++ b/drivers/iommu/io-pgtable-arm.c
> > @@ -80,11 +80,13 @@
> > 
> >  #define ARM_LPAE_PTE_TYPE_TABLE3
> >  #define ARM_LPAE_PTE_TYPE_PAGE 3
> > +#define ARM_LPAE_PTE_NSTABLE   (((arm_lpae_iopte)1) << 63)
> >  #define ARM_LPAE_PTE_XN(((arm_lpae_iopte)3) << 53)
> >  #define ARM_LPAE_PTE_AF(((arm_lpae_iopte)1) << 10)
> >  #define ARM_LPAE_PTE_SH_NS (((arm_lpae_iopte)0) << 8)
> >  #define ARM_LPAE_PTE_SH_OS (((arm_lpae_iopte)2) << 8)
> >  #define ARM_LPAE_PTE_SH_IS (((arm_lpae_iopte)3) << 8)
> > +#define ARM_LPAE_PTE_NS(((arm_lpae_iopte)1) << 5)
> >  #define ARM_LPAE_PTE_VALID (((arm_lpae_iopte)1) << 0)
> >  #define ARM_LPAE_PTE_ATTR_LO_MASK  (((arm_lpae_iopte)0x3ff) << 2)
> > 
> > @@ -201,6 +203,9 @@ static int arm_lpae_init_pte(struct
> > arm_lpae_io_pgtable *data,> 
> > if (iopte_leaf(*ptep, lvl))
> > return -EEXIST;
> > 
> > +   if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_NON_SECURE)
> > +   pte |= ARM_LPAE_PTE_NS;
> > +
> > if (lvl == ARM_LPAE_MAX_LEVELS - 1)
> > pte |= ARM_LPAE_PTE_TYPE_PAGE;
> > else
> > @@ -244,6 +249,8 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable
> > *data, unsigned long iova,> 
> > data->iop.cfg.tlb->flush_pgtable(cptep, 1UL << data->pg_shift,
> >  cookie);
> > pte = __pa(cptep) | ARM_LPAE_PTE_TYPE_TABLE;
> > +   if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_NON_SECURE)
> > +   pte |= ARM_LPAE_PTE_NSTABLE;
> > *ptep = pte;
> > data->iop.cfg.tlb->flush_pgtable(ptep, sizeof(*ptep), cookie);
> > } else {
> > diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
> > index c1cff3d045db..a41a15d30596 100644
> > --- a/drivers/iommu/io-pgtable.h
> > +++ b/drivers/iommu/io-pgtable.h
> > @@ -24,6 +24,9 @@ struct iommu_gather_ops {
> > void (*flush_pgtable)(void *ptr, size_t size, void *cookie);
> >  };
> > 
> > +/* Set the Non-Secure bit in the PTEs */
> > +#define IO_PGTABLE_QUIRK_NON_SECURE(1 << 0)
> 
> I think I'd stick an _ARM_ somewhere in here, so maybe
> IO_PGTABLE_QUIRK_ARM_NS?

I'm fine with that.

By the way, I'm only familiar with the Renesas implementation of the VMSA 
IOMMU, could you double-check whether setting the NSTABLE and NS bits on all 
levels make sense to you ? It seems to be required by my hardware, even though 
the ARM spec mentions that setting the NSTABLE bit causes non-secure accesses 
to page tables for all lower levels regardless of their NSTABLE/NS bits.

> > +
> > 
> >  struct io_pgtable_cfg {
> 
> and I'd put the #define here, next to the member.

They're right before the structure so I don't think they're too far away, but 
if you prefer that coding style that's fine with me.

> > int quirks; /* IO_PGTABLE_QUIRK_* */
> > unsigned long   pgsize_bitmap;

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

[Laurent Pinchart]

Hi Will,

On Monday 15 December 2014 17:17:00 Will Deacon wrote:
> On Sun, Dec 14, 2014 at 12:45:36PM +, Laurent Pinchart wrote:
> > On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
> > > This patch introduces IOMMU_OF_DECLARE-based initialization to the
> > > driver, which replaces subsys_initcall-based procedure.
> > > exynos_iommu_of_setup ensures that each sysmmu controller is probed
> > > before its master device.
> > > 
> > > Signed-off-by: Marek Szyprowski 
> > > ---
> > > 
> > >  drivers/iommu/exynos-iommu.c | 28 +++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > > index cd28dc09db39..88f9afe641a0 100644
> > > --- a/drivers/iommu/exynos-iommu.c
> > > +++ b/drivers/iommu/exynos-iommu.c
> > 
> > [snip]
> > 
> > > @@ -1125,4 +1134,21 @@ err_reg_driver:
> > >   kmem_cache_destroy(lv2table_kmem_cache);
> > >   return ret;
> > >  
> > >  }
> > > 
> > > -subsys_initcall(exynos_iommu_init);
> > > +
> > > +static int __init exynos_iommu_of_setup(struct device_node *np)
> > > +{
> > > + struct platform_device *pdev;
> > > +
> > > + if (!init_done)
> > > + exynos_iommu_init();
> > > +
> > > + pdev = of_platform_device_create(np, NULL,
> > > platform_bus_type.dev_root);
> > > + if (IS_ERR(pdev))
> > > + return PTR_ERR(pdev);
> > 
> > If we end up having to create the IOMMU platform devices from within the
> > drivers, the introduction of IOMMU_OF_DECLARE starts to feel like a
> > workaround to me. I wonder whether it wouldn't then be better to let the
> > driver core instantiate the IOMMU platform device from DT as for all
> > other devices, and use device notifiers to defer probe of the bus masters
> > until the required IOMMU(s) are registered.
> > 
> > Will, what's your opinion on that ?
> 
> Creating the platform device manually for the IOMMU is indeed grotty, but I
> don't really understand why it's needed. Interrupt controllers, for example,
> seem to get by without one.

There's several reasons, one of the most compelling ones I can think of at the 
moment is runtime PM. IRQ controllers close to the CPU use CPU PM notifiers 
instead. Note that IRQ controllers that are further away from the CPU (such as 
GPIO-based IRQ controllers) are real platform devices and use runtime PM.

IOMMUs are not as low-level as system interrupt controllers or system clocks. 
I'm beginning to agree with Thierry that they should be treated as normal 
platform devices as they're not required earlier than probe time of their bus 
master devices.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator

[Will Deacon]

On Mon, Dec 15, 2014 at 04:35:12PM +, Varun Sethi wrote:
> -Original Message-
> From: Will Deacon [mailto:will.dea...@arm.com] 
> Sent: Monday, December 15, 2014 9:13 PM
> To: Sethi Varun-B16395
> Cc: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org; 
> prem.malla...@broadcom.com; Robin Murphy; lau...@codeaurora.org; 
> mitch...@codeaurora.org; laurent.pinch...@ideasonboard.com; j...@8bytes.org; 
> m.szyprow...@samsung.com
> Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator
> 
> On Mon, Dec 15, 2014 at 01:30:20PM +, Will Deacon wrote:
> > On Sun, Dec 14, 2014 at 05:45:49PM +, Varun Sethi wrote:
> > > [varun] ok, but you could potentially end up splitting mapping to 
> > > the least possible page size e.g. 4K. You, don't seem to take in to 
> > > account the possibility of using the block size at the next level. 
> > > For example, take a case where we have a huge page mapping using 1G 
> > > page size and we have an unmap request for 4K. We could still split 
> > > maximum part of the mapping using 2M pages at the next level. The 
> > > entry where we need to unmap the 4K region would potentially go to the 
> > > next level.
> > 
> > Aha, I see what you mean here, thanks. I'll take a look...
> 
> Scratch that, I think the code is fine as it is. For the case you
> highlight, we iterate over the 1GB region remapping it using 4k pages, but
> skipping the one we want to unmap, so I don't think there's a problem
> (__arm_lpae_map will create the relevant table entries).
> 
> 
> [[varun]]  But you can split 1G in 2M mappings and then split up the
> unmapped region using 4K pages. In this case you split up the entire
> region using 4K pages.

True, I miss an optimisation opportunity there, but I don't know that it's
common enough to care (in the same way that we don't recreate a 1G mapping
if you remapped that 4k page back like it was).

You could add this by making arm_lpae_split_blk_unmap recursive, if you
wanted to.

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

Re: [RFC PATCH v4 0/8] Introduce automatic DMA configuration for IOMMU masters

[Laurent Pinchart]

Hi Will,

On Wednesday 19 November 2014 11:41:50 Will Deacon wrote:
> On Wed, Nov 19, 2014 at 11:21:26AM +, Marek Szyprowski wrote:
> > On 2014-11-14 19:56, Will Deacon wrote:
> > > Hello everybody,
> > > 
> > > Here is the fourth iteration of the RFC I've previously posted here:
> > >RFCv1:
> > >http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/28
> > >3023.html RFCv2:
> > >http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September
> > >/283752.html RFCv3:
> > >http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September
> > >/287031.html> > 
> > > Changes since RFCv3 include:
> > >- Drastic simplification of the data structures, so that we no longer
> > >  pass around lists of domains. Instead, dma-mapping is expected to
> > >  allocate the domain (Joerg talked about adding a get_default_domain
> > >  operation to iommu_ops).
> > >
> > >- iommu_ops is used to hold the per-instance IOMMU data
> > >
> > >- Configuration of DMA segments added to of_dma_configure
> > > 
> > > All feedback welcome.
> > 
> > I've rebased my Exynos SYSMMU patches on top of this patchset and it
> > works fine,
> > You can find them in the "[PATCH v3 00/19] Exynos SYSMMU (IOMMU)
> > integration with DT and DMA-mapping subsystem" thread.
> 
> I just saw that and it looks great, thanks! FWIW, I'll take the first 3
> patches you have into my series in some shape or another.
> 
> > You can add to all your patches:
> > Acked-by: Marek Szyprowski 
> 
> Cheers.
> 
> > I'm also interested in adding get_default_domain() callback, but I
> > assume that this
> > can be done once the basic patchset get merged. Do you plan to work on
> > it, do you want
> > me to implement it?
> 
> If Joerg isn't working on it already (I don't think he is), then please
> do have a go if you have time. You'll probably want to avoid adding devices
> with addressing restrictions (i.e. non-zero dma_pfn_offset, weird dma masks)
> to the default domain, otherwise you'll run into issues initialising the
> iova allocator.
> 
> I had a go at getting ARM dma-mapping to use a hypothetical
> get_default_domain function, so I've included the diff I ended up with
> below, in case it's at all useful.
> 
> Will
> 
> --->8
> 
> diff --git a/arch/arm/include/asm/dma-mapping.h
> b/arch/arm/include/asm/dma-mapping.h index f3c0d953f6a2..5071553bf6b8
> 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -121,14 +121,9 @@ static inline unsigned long dma_max_pfn(struct device
> *dev) }
>  #define dma_max_pfn(dev) dma_max_pfn(dev)
> 
> -static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base,
> -   u64 size, struct iommu_ops *iommu,
> -   bool coherent)
> -{
> - if (coherent)
> - set_dma_ops(dev, &arm_coherent_dma_ops);
> -}
>  #define arch_setup_dma_ops arch_setup_dma_ops
> +extern void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> +struct iommu_ops *iommu, bool coherent);
> 
>  static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
> {
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index c245d903927f..da2c2667bbb1 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1849,7 +1849,8 @@ struct dma_map_ops iommu_coherent_ops = {
>   * arm_iommu_attach_device function.
>   */
>  struct dma_iommu_mapping *
> -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t
> size) +__arm_iommu_create_mapping(struct iommu_domain *domain, dma_addr_t
> base, +  size_t size)
>  {
>   unsigned int bits = size >> PAGE_SHIFT;
>   unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
> @@ -1883,17 +1884,12 @@ arm_iommu_create_mapping(struct bus_type *bus,
> dma_addr_t base, size_t size) mapping->extensions = extensions;
>   mapping->base = base;
>   mapping->bits = BITS_PER_BYTE * bitmap_size;
> + mapping->domain = domain;
> 
>   spin_lock_init(&mapping->lock);
> 
> - mapping->domain = iommu_domain_alloc(bus);
> - if (!mapping->domain)
> - goto err4;
> -
>   kref_init(&mapping->kref);
>   return mapping;
> -err4:
> - kfree(mapping->bitmaps[0]);
>  err3:
>   kfree(mapping->bitmaps);
>  err2:
> @@ -1901,6 +1897,23 @@ err2:
>  err:
>   return ERR_PTR(err);
>  }
> +
> +struct dma_iommu_mapping *
> +arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t
> size) +{
> + struct dma_iommu_mapping *mapping;
> + struct iommu_domain *domain;
> +
> + domain = iommu_domain_alloc(bus);
> + if (!domain)
> + return ERR_PTR(-ENOMEM);
> +
> + mapping = __arm_iommu_create_mapping(domain, base, size);
> + if (IS_ERR(mapping))
> + iommu_domain_free(domain);
> +
> + return mapping;
> +}
>  EXPORT_S

Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator

[Will Deacon]

On Mon, Dec 15, 2014 at 04:43:59PM +, Varun Sethi wrote:
> > Sure we can support 48-bit VAs with 64k pages. Why do you think we can't?
> > 
> > [varun]  My concern was with respect to the bits per level, which is 
> > uneven for the 64 K page sizes. Just wondering how would things work 
> > with 64K pages when we do a 3 level page lookup.
> 
> Well, it's uneven (9) for the 4k case too. Do you actually see an issue here?
> 
> 48-bit VA with 64k pages gives us:
> 
>   va_bits = (48 - 16) = 32
>   bits_per_level = (16 - 3) = 13
>   levels = ceil(32/13) = 3
> 
> so the starting level is 1, which resolves 32-(13*2) = 6 bits.
> 
> Does that make sense?
> 
> [[varun]]Yes, but what I meant was, is that in case of 4K pages you have 9
> bits per level, but for 64K pages you have 6 bits for the first  level and
> 13 each for second and third. So, bits per level would not work in case of
> 64 K pages?

The current code takes this into account.

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

[PATCH 3/3] Documentation: dt-bindings: Add the binding info for APM X-Gene AHBC IOMMU driver.

[Suman Tripathi]

This patch adds the device tree binding information for X-Gene
AHBC IOMMU driver.

Signed-off-by: Suman Tripathi 
---
 .../devicetree/bindings/iommu/xgene,ahbc-iommu.txt  | 17 +
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/xgene,ahbc-iommu.txt

diff --git a/Documentation/devicetree/bindings/iommu/xgene,ahbc-iommu.txt 
b/Documentation/devicetree/bindings/iommu/xgene,ahbc-iommu.txt
new file mode 100644
index 000..d46af54
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/xgene,ahbc-iommu.txt
@@ -0,0 +1,17 @@
+*APM X-Gene AHBC IOMMU Architecture Implementation
+
+APM X-Gene SoC AHBC IOMMU translates the AHBC 32-bit address from
+dma master to 42-bit AXI address by using a set of AIM(AHB Inbound mapper)
+register.
+
+** AHBC IOMMU required properties:
+
+- compatible: Should be "apm,xgene-ahbc-iommu"
+- reg   : Base address of the AHBC AXI mapper registers
+
+Example:
+
+   ahbc_iommu: ahbc_iommu@0x1f2a0080 {
+   compatible = "apm,xgene-ahbc-iommu";
+   reg = <0x0 0x1f2a0080 0x0 0x90>;
+   };
--
1.8.2.1

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

[PATCH 0/3] Add support for APM X-Gene SoC AHBC IOMMU driver.

[Suman Tripathi]

This patch adds the support for the APM X-Gene SoC AHBC IOMMU driver.

Signed-off-by: Suman Tripathi 
---

Suman Tripathi (3):
  xgene-ahbc-iommu: Add support for APM X-Gene SoC AHBC IOMMU driver.
  arm64: dts: Add the APM X-Gene AHBC IOMMU DTS node.
  Documentation: dt-bindings: Add the binding info for APM X-Gene AHBC
IOMMU driver.

 .../devicetree/bindings/iommu/xgene,ahbc-iommu.txt |  17 ++
 arch/arm64/boot/dts/apm-storm.dtsi |   5 +
 drivers/iommu/Kconfig  |  10 +
 drivers/iommu/Makefile |   1 +
 drivers/iommu/xgene-ahbc-iommu.c   | 336 +
 5 files changed, 369 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/xgene,ahbc-iommu.txt
 create mode 100644 drivers/iommu/xgene-ahbc-iommu.c

--
1.8.2.1

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

[PATCH 2/3] arm64: dts: Add the APM X-Gene AHBC IOMMU DTS node.

[Suman Tripathi]

This patch adds the device tree node for APM X-Gene AHBC IOMMU.

Signed-off-by: Suman Tripathi 
---
 arch/arm64/boot/dts/apm-storm.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/apm-storm.dtsi 
b/arch/arm64/boot/dts/apm-storm.dtsi
index c0aceef..f0a6b71 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -389,6 +389,11 @@
phy-names = "sata-phy";
};

+   ahbc_iommu: ahbc_iommu@0x1f2a0080 {
+   compatible = "apm,xgene-ahbc-iommu";
+   reg = <0x0 0x1f2a0080 0x0 0x90>;
+   };
+
rtc: rtc@1051 {
compatible = "apm,xgene-rtc";
reg = <0x0 0x1051 0x0 0x400>;
--
1.8.2.1

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

[PATCH 1/3] xgene-ahbc-iommu: Add support for APM X-Gene SoC AHBC IOMMU driver.

[Suman Tripathi]

This patch adds the support for the APM X-Gene SoC AHBC IOMMU driver.
This driver translates the 32-bit AHB address from the dma master to
42-bit AXI address with the help of a set of AHBC inbound mapper (AIM)
registers. The AHB dma master for slaves, eg: sdhci etc, will use this
driver to do a dma transfer operation.

Signed-off-by: Suman Tripathi 
---
 drivers/iommu/Kconfig|  10 ++
 drivers/iommu/Makefile   |   1 +
 drivers/iommu/xgene-ahbc-iommu.c | 336 +++
 3 files changed, 347 insertions(+)
 create mode 100644 drivers/iommu/xgene-ahbc-iommu.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index dd51122..c0f5f23 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -302,4 +302,14 @@ config ARM_SMMU
  Say Y here if your SoC includes an IOMMU device implementing
  the ARM SMMU architecture.

+config XGENE_AHBC_IOMMU
+   bool "X-Gene AHBC IOMMU Support"
+   default y if ARCH_XGENE
+   select IOMMU_API
+   help
+ Support for AHBC translation driver for X-Gene. This driver
+ translates the 32-bit AHB address from dma master to 42-bit
+ AXI address with the help of some set of AHB inbound mapping
+ registers.
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 16edef7..ae3b663 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
 obj-$(CONFIG_SHMOBILE_IOMMU) += shmobile-iommu.o
 obj-$(CONFIG_SHMOBILE_IPMMU) += shmobile-ipmmu.o
 obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
+obj-$(CONFIG_XGENE_AHBC_IOMMU) += xgene-ahbc-iommu.o
diff --git a/drivers/iommu/xgene-ahbc-iommu.c b/drivers/iommu/xgene-ahbc-iommu.c
new file mode 100644
index 000..7e3701e
--- /dev/null
+++ b/drivers/iommu/xgene-ahbc-iommu.c
@@ -0,0 +1,336 @@
+/*
+ * APM X-Gene SoC AHBC(IOMMU) Translation Driver
+ *
+ * Copyright (c) 2014 Applied Micro Circuits Corporation.
+ * Author: Suman Tripathi 
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define AHB_PAGE_SIZE  (4*1024)
+#define AHB_MAP_COUNT  8
+
+/* APM X-Gene SoC AHB bridge registers */
+#define AIM0   0x
+#define AIM0_SIZE_CTL  0x0004
+#define AIM0_AXI_LO0x0008
+#define AIM0_AXI_HI0x0010
+
+#define AIMX   0x0014
+#define AIM_AXI_ADDRESS_HI_N_WR(src)   (((u32) (src) << 20) & 0xfff0)
+#define AIMX_SIZE_CTL  0x0018
+#define AIM_EN_N_WR(src)   (((u32) (src) << 31) & 0x8000)
+#define AIM_EN_N_RD(src)   (((u32) (src) & 0x8000) >> 31)
+#define ARSB_WR(src)   (((u32) (src) << 24) & 0x0f00)
+#define AWSB_WR(src)   (((u32) (src) << 20) & 0x00f0)
+#define AIM_MASK_N_WR(src) (((u32) (src)) & 0x000f)
+#define AIMX_AXI_LO0x001c
+#define AIMX_AXI_HI0x0020
+#define AIMX_STRIDE0x0010
+
+#define ENABLE_AIM_TRANSLATION AIM_EN_N_WR(1) | ARSB_WR(1) | \
+   AWSB_WR(1) | \
+   AIM_MASK_N_WR(0)
+
+#define DISABLE_AIM_TRANSLATIONAIM_EN_N_WR(0) | ARSB_WR(0) | \
+   AWSB_WR(0) | \
+   AIM_MASK_N_WR(0)
+
+struct xgene_ahbc_mmu_device {
+   struct device *dev;
+   void __iomem *ahb_csr;
+};
+
+struct xgene_ahbc_mmu_domain {
+   struct xgene_ahbc_mmu_device *leaf_ahbc;
+   spinlock_t lock;
+   int slot_used;
+};
+
+struct xgene_ahbc_mmu_device *ahbc_mmu;
+
+static int xgene_ahbc_mmu_find_entry(struct xgene_ahbc_mmu_device *ctx)
+{
+   int i;
+   u32 val;
+
+   /*
+* Find free slot by checking the EN-bit
+* of AIMX register.
+*
+* AIMX_SIZE_CTL[31]
+* 0 : Register available for reuse.
+* 1 : Translation going on using the register.
+*/
+   for (i = 0; i < AHB_MAP_COUNT; i++) {
+   /*
+* The AIM0_LO register offset for slot 0 is different from
+* other slots. So explicitly 

Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

[Will Deacon]

On Sun, Dec 14, 2014 at 12:45:36PM +, Laurent Pinchart wrote:
> On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
> > This patch introduces IOMMU_OF_DECLARE-based initialization to the
> > driver, which replaces subsys_initcall-based procedure.
> > exynos_iommu_of_setup ensures that each sysmmu controller is probed
> > before its master device.
> > 
> > Signed-off-by: Marek Szyprowski 
> > ---
> >  drivers/iommu/exynos-iommu.c | 28 +++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > index cd28dc09db39..88f9afe641a0 100644
> > --- a/drivers/iommu/exynos-iommu.c
> > +++ b/drivers/iommu/exynos-iommu.c
> 
> [snip]
> 
> > @@ -1125,4 +1134,21 @@ err_reg_driver:
> > kmem_cache_destroy(lv2table_kmem_cache);
> > return ret;
> >  }
> > -subsys_initcall(exynos_iommu_init);
> > +
> > +static int __init exynos_iommu_of_setup(struct device_node *np)
> > +{
> > +   struct platform_device *pdev;
> > +
> > +   if (!init_done)
> > +   exynos_iommu_init();
> > +
> > +   pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> > +   if (IS_ERR(pdev))
> > +   return PTR_ERR(pdev);
> 
> If we end up having to create the IOMMU platform devices from within the 
> drivers, the introduction of IOMMU_OF_DECLARE starts to feel like a 
> workaround 
> to me. I wonder whether it wouldn't then be better to let the driver core 
> instantiate the IOMMU platform device from DT as for all other devices, and 
> use device notifiers to defer probe of the bus masters until the required 
> IOMMU(s) are registered.
> 
> Will, what's your opinion on that ?

Creating the platform device manually for the IOMMU is indeed grotty, but I
don't really understand why it's needed. Interrupt controllers, for example,
seem to get by without one.

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

Re: [PATCH v6 6/8] dma-mapping: detect and configure IOMMU in of_dma_configure

[Laurent Pinchart]

Hi Will,

On Monday 15 December 2014 16:40:41 Will Deacon wrote:
> On Sun, Dec 14, 2014 at 03:49:34PM +, Laurent Pinchart wrote:
> > On Wednesday 10 December 2014 15:08:53 Will Deacon wrote:
> > > On Wed, Dec 10, 2014 at 02:52:56PM +, Rob Clark wrote:
> > > > so, what is the way for a driver that explicitly wants to manage it's
> > > > own device virtual address space to opt out of this?  I suspect that
> > > > won't be the common case, but for a gpu, if dma layer all of a sudden
> > > > thinks it is in control of the gpu's virtual address space, things are
> > > > going to end in tears..
> > > 
> > > I think you'll need to detach from the DMA domain, then have the driver
> > > manage everything itself. As you say, it's not the common case, so you
> > > may need to add some hooks for detaching from the default domain and
> > > swizzling your DMA ops.
> > 
> > I'm wondering if it's such an exotic case after all. I can see two reasons
> > not to use the default domain. In addition to special requirements coming
> > from the bus master side, the IOMMU itself might not support one domain
> > per bus master (I'm of course raising the issue from a very selfish
> > Renesas IPMMU point of view).
> 
> Do you mean that certain masters must be grouped into the same domain, or
> that the IOMMU can fail with -ENOSPC?

My IOMMU has hardware supports for 4 domains, and serves N masters (where N is 
dependent on the SoC but is > 4). In its current form the driver supports a 
single domain and thus detaches devices from the default domain in the 
add_device callback:

http://git.linuxtv.org/cgit.cgi/pinchartl/fbdev.git/tree/drivers/iommu/ipmmu-vmsa.c?h=iommu/devel/arm-lpae

/*
 * Detach the device from the default ARM VA mapping and attach it to
 * our private mapping.
 */
arm_iommu_detach_device(dev);
ret = arm_iommu_attach_device(dev, mmu->mapping);
if (ret < 0) {
dev_err(dev, "Failed to attach device to VA mapping\n");
return ret;
}

I would have implemented that in the of_xlate callback, but that's too early 
as the ARM default domain isn't created yet at that point.

Using a single domain is a bit of a waste of resources in my case, so an 
evolution would be to create four domains and assign devices to them based on 
a policy. The policy could be fixed (round-robin for instance), or 
configurable (possibly through DT, although it's really a policy, not a 
hardware description).

> For the former, we need a way to represent IOMMU groups for the platform
> bus.

To be honest I'm not entirely sure how IOMMU groups are supposed to be used. I 
understand they can be used by VFIO to group several masters that will be able 
to see each other's memory through the same page table, and also that a page 
table could be shared between multiple groups. When it comes to group 
creation, though, things get fuzzy. I started with creating one group per 
master in my driver, which is probably not the thing to do. The Exynos IOMMU 
driver used to do the same, until Marek's patch series converting it to DT-
based instantiation (on top of your patch set) has removed groups altogether. 
Groups seem to be more or less optional, except in a couple of places (for 
instance the remove_device callback will not be called by the 
BUS_NOTIFY_DEL_DEVICE notifier if the device isn't part of an iommu group).

I'd appreciate if someone could clarify this to help me make an informed 
opinion on the topic.

> For the latter, we should have a per-IOMMU default domain instead of
> creating one per master as we currently do for ARM.
> 
> Joerg has talked about adding a ->get_default_domain callback to the IOMMU
> layer, but I've not seen any code and my attempt at using it also got
> pretty complicated:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/304076.
> html

Thank you for the pointer. I'll reply to the patch.

> Marek also said he might be taking a look.

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v6 6/8] dma-mapping: detect and configure IOMMU in of_dma_configure

[Will Deacon]

On Sun, Dec 14, 2014 at 03:59:38PM +, Laurent Pinchart wrote:
> On Sunday 14 December 2014 17:49:34 Laurent Pinchart wrote:
> > On Wednesday 10 December 2014 15:08:53 Will Deacon wrote:
> > > On Wed, Dec 10, 2014 at 02:52:56PM +, Rob Clark wrote:
> > > > so, what is the way for a driver that explicitly wants to manage it's
> > > > own device virtual address space to opt out of this?  I suspect that
> > > > won't be the common case, but for a gpu, if dma layer all of a sudden
> > > > thinks it is in control of the gpu's virtual address space, things are
> > > > going to end in tears..
> > > 
> > > I think you'll need to detach from the DMA domain, then have the driver
> > > manage everything itself. As you say, it's not the common case, so you
> > > may need to add some hooks for detaching from the default domain and
> > > swizzling your DMA ops.
> > 
> > I'm wondering if it's such an exotic case after all. I can see two reasons
> > not to use the default domain. In addition to special requirements coming
> > from the bus master side, the IOMMU itself might not support one domain per
> > bus master (I'm of course raising the issue from a very selfish Renesas
> > IPMMU point of view).
> > 
> > In that case the IOMMU driver can still detach the device from the default
> > domain in the .of_xlate() callback and attach it to its private domain.
> 
> I actually spoke too fast, the .of_xlate() callback is called before 
> arch_setup_dma_ops(). The operation could be performed in the .add_device() 
> callback. I'm having trouble understanding how you envision .add_device, 
> .remove_device and .of_xlate working together in the DT case. Could you 
> please 
> detail what you think each callback should be responsible of exactly ?

I think the sequence goes something like:


1: of_iommu_init: early initialisiation of IOMMU driver, stashes its private
  data via of_iommu_set_ops

2: ->of_xlate: called for each master upstream of the IOMMU before the master
   has probed. Passes back the DT info, which can be used to
   get hold of the private data from earlier. Per-device data
   can be stored in archdata.iommu (but I'd like to change this,
   see below).

3. ->domain_init: called from dma-mapping to create default domain (this
  should be replaced by ->get_default_domain in future).

3. ->attach_device: called by dma-mapping to attach device to the default
domain

4. ->add_device: called just before the master is probed


Now, the thing missing here is the instantiation of iommu_groups. They
require a fully initialised struct device because of some sysfs magic
that goes on in iommu_group_add_device. The questions I have are:

  - How should we describe/create iommu_groups for platform devices?

  - When do the groups actually need to be initialised (currently likely
to be in add_device).

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

RE: [PATCH 2/4] iommu: add ARM LPAE page table allocator

[Varun Sethi]

Hi Will,

-Original Message-
From: Will Deacon [mailto:will.dea...@arm.com] 
Sent: Monday, December 15, 2014 9:13 PM
To: Sethi Varun-B16395
Cc: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org; 
prem.malla...@broadcom.com; Robin Murphy; lau...@codeaurora.org; 
mitch...@codeaurora.org; laurent.pinch...@ideasonboard.com; j...@8bytes.org; 
m.szyprow...@samsung.com
Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator

On Mon, Dec 15, 2014 at 01:30:20PM +, Will Deacon wrote:
> On Sun, Dec 14, 2014 at 05:45:49PM +, Varun Sethi wrote:
> > [varun] ok, but you could potentially end up splitting mapping to 
> > the least possible page size e.g. 4K. You, don't seem to take in to 
> > account the possibility of using the block size at the next level. 
> > For example, take a case where we have a huge page mapping using 1G 
> > page size and we have an unmap request for 4K. We could still split 
> > maximum part of the mapping using 2M pages at the next level. The 
> > entry where we need to unmap the 4K region would potentially go to the next 
> > level.
> 
> Aha, I see what you mean here, thanks. I'll take a look...

Scratch that, I think the code is fine as it is. For the case you highlight, we 
iterate over the 1GB region remapping it using 4k pages, but skipping the one 
we want to unmap, so I don't think there's a problem (__arm_lpae_map will 
create the relevant table entries).


[[varun]]  But you can split 1G in 2M mappings and then split up the unmapped 
region using 4K pages. In this case you split up the entire region using 4K 
pages.

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

RE: [PATCH 2/4] iommu: add ARM LPAE page table allocator

[Varun Sethi]

-Original Message-
From: Will Deacon [mailto:will.dea...@arm.com] 
Sent: Monday, December 15, 2014 7:00 PM
To: Sethi Varun-B16395
Cc: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org; 
prem.malla...@broadcom.com; Robin Murphy; lau...@codeaurora.org; 
mitch...@codeaurora.org; laurent.pinch...@ideasonboard.com; j...@8bytes.org; 
m.szyprow...@samsung.com
Subject: Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator

On Sun, Dec 14, 2014 at 05:45:49PM +, Varun Sethi wrote:
> Please find my response inline. Search for "varun".

This is getting fiddly now that you've already replied once. Any chance you 
could sort your mail client out, please?


[[varun]]  Yes I need to do that, this is painful.

> > +   if (!data)
> > +   return NULL;
> > +
> > +   data->pages_per_pgd = 1;
> > +   data->pg_shift = __ffs(cfg->pgsize_bitmap);
> > +   data->bits_per_level = data->pg_shift - 
> > + ilog2(sizeof(arm_lpae_iopte));
> > +
> > +   va_bits = cfg->ias - data->pg_shift;
> > +   data->levels = DIV_ROUND_UP(va_bits, data->bits_per_level);
> >
> > [[varun]]  Not related to the patch, but this would be applicable to 
> > the CPU tables as well i.e, we can't support 48bit VA with 64 KB 
> > page tables, right? The AR64 memory maps shows possibility of using 
> > 6 bits for the first level page table.
> 
> Sure we can support 48-bit VAs with 64k pages. Why do you think we can't?
> 
> [varun]  My concern was with respect to the bits per level, which is 
> uneven for the 64 K page sizes. Just wondering how would things work 
> with 64K pages when we do a 3 level page lookup.

Well, it's uneven (9) for the 4k case too. Do you actually see an issue here?

48-bit VA with 64k pages gives us:

  va_bits = (48 - 16) = 32
  bits_per_level = (16 - 3) = 13
  levels = ceil(32/13) = 3

so the starting level is 1, which resolves 32-(13*2) = 6 bits.

Does that make sense?

[[varun]]Yes, but what I meant was, is that in case of 4K pages you have 9 bits 
per level, but for 64K pages you have 6 bits for the first  level and 13 each 
for second and third. So, bits per level would not work in case of 64 K pages?

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

Re: [PATCH v6 6/8] dma-mapping: detect and configure IOMMU in of_dma_configure

[Will Deacon]

On Sun, Dec 14, 2014 at 03:49:34PM +, Laurent Pinchart wrote:
> Hi Will,

Hi Laurent,

> On Wednesday 10 December 2014 15:08:53 Will Deacon wrote:
> > On Wed, Dec 10, 2014 at 02:52:56PM +, Rob Clark wrote:
> > > so, what is the way for a driver that explicitly wants to manage it's
> > > own device virtual address space to opt out of this?  I suspect that
> > > won't be the common case, but for a gpu, if dma layer all of a sudden
> > > thinks it is in control of the gpu's virtual address space, things are
> > > going to end in tears..
> > 
> > I think you'll need to detach from the DMA domain, then have the driver
> > manage everything itself. As you say, it's not the common case, so you
> > may need to add some hooks for detaching from the default domain and
> > swizzling your DMA ops.
> 
> I'm wondering if it's such an exotic case after all. I can see two reasons 
> not 
> to use the default domain. In addition to special requirements coming from 
> the 
> bus master side, the IOMMU itself might not support one domain per bus master 
> (I'm of course raising the issue from a very selfish Renesas IPMMU point of 
> view).

Do you mean that certain masters must be grouped into the same domain, or
that the IOMMU can fail with -ENOSPC?

For the former, we need a way to represent IOMMU groups for the platform
bus. For the latter, we should have a per-IOMMU default domain instead of
creating one per master as we currently do for ARM.

Joerg has talked about adding a ->get_default_domain callback to the IOMMU
layer, but I've not seen any code and my attempt at using it also got
pretty complicated:

  
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/304076.html

Marek also said he might be taking a look.

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

Re: [PATCH 0/4] Generic IOMMU page table framework

[Will Deacon]

On Sun, Dec 14, 2014 at 11:49:30PM +, Laurent Pinchart wrote:
> Hi Will,

Hi Laurent,

> On Thursday 27 November 2014 11:51:14 Will Deacon wrote:
> > This series introduces a generic IOMMU page table allocation framework,
> > implements support for ARM long-descriptors and then ports the arm-smmu
> > driver over to the new code.

[...]

> > All feedback welcome.
> 
> I've successfully tested the patch set with the Renesas IPMMU-VMSA driver with
> the following extension to the allocator.
> 
> Tested-by: Laurent Pinchart 

Wahey, that's really cool, thanks! I have a few minor comments on your patch
below. If you don't object, then I can make them locally and include your
patch on top of my v2 series?

> From 4bebb7f3a5a48541d4c89ce7c61e6ff66686c3a9 Mon Sep 17 00:00:00 2001
> From: Laurent Pinchart 
> Date: Sun, 14 Dec 2014 23:34:50 +0200
> Subject: [PATCH] iommu: io-pgtable-arm: Add Non-Secure quirk
> 
> The quirk causes the Non-Secure bit to be set in all page table entries.
> 
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/iommu/io-pgtable-arm.c | 7 +++
>  drivers/iommu/io-pgtable.h | 3 +++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 669e322a83a4..b6910e142734 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -80,11 +80,13 @@
>  #define ARM_LPAE_PTE_TYPE_TABLE  3
>  #define ARM_LPAE_PTE_TYPE_PAGE   3
>  
> +#define ARM_LPAE_PTE_NSTABLE (((arm_lpae_iopte)1) << 63)
>  #define ARM_LPAE_PTE_XN  (((arm_lpae_iopte)3) << 53)
>  #define ARM_LPAE_PTE_AF  (((arm_lpae_iopte)1) << 10)
>  #define ARM_LPAE_PTE_SH_NS   (((arm_lpae_iopte)0) << 8)
>  #define ARM_LPAE_PTE_SH_OS   (((arm_lpae_iopte)2) << 8)
>  #define ARM_LPAE_PTE_SH_IS   (((arm_lpae_iopte)3) << 8)
> +#define ARM_LPAE_PTE_NS  (((arm_lpae_iopte)1) << 5)
>  #define ARM_LPAE_PTE_VALID   (((arm_lpae_iopte)1) << 0)
>  
>  #define ARM_LPAE_PTE_ATTR_LO_MASK(((arm_lpae_iopte)0x3ff) << 2)
> @@ -201,6 +203,9 @@ static int arm_lpae_init_pte(struct arm_lpae_io_pgtable 
> *data,
>   if (iopte_leaf(*ptep, lvl))
>   return -EEXIST;
>  
> + if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_NON_SECURE)
> + pte |= ARM_LPAE_PTE_NS;
> +
>   if (lvl == ARM_LPAE_MAX_LEVELS - 1)
>   pte |= ARM_LPAE_PTE_TYPE_PAGE;
>   else
> @@ -244,6 +249,8 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable 
> *data, unsigned long iova,
>   data->iop.cfg.tlb->flush_pgtable(cptep, 1UL << data->pg_shift,
>cookie);
>   pte = __pa(cptep) | ARM_LPAE_PTE_TYPE_TABLE;
> + if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_NON_SECURE)
> + pte |= ARM_LPAE_PTE_NSTABLE;
>   *ptep = pte;
>   data->iop.cfg.tlb->flush_pgtable(ptep, sizeof(*ptep), cookie);
>   } else {
> diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
> index c1cff3d045db..a41a15d30596 100644
> --- a/drivers/iommu/io-pgtable.h
> +++ b/drivers/iommu/io-pgtable.h
> @@ -24,6 +24,9 @@ struct iommu_gather_ops {
>   void (*flush_pgtable)(void *ptr, size_t size, void *cookie);
>  };
>  
> +/* Set the Non-Secure bit in the PTEs */
> +#define IO_PGTABLE_QUIRK_NON_SECURE  (1 << 0)

I think I'd stick an _ARM_ somewhere in here, so maybe
IO_PGTABLE_QUIRK_ARM_NS?

> +
>  struct io_pgtable_cfg {

and I'd put the #define here, next to the member.

>   int quirks; /* IO_PGTABLE_QUIRK_* */
>   unsigned long   pgsize_bitmap;
> -- 

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

Re: [PATCH] iommu/exynos: Fix arm64 allmodconfig build

[Mark Brown]

On Mon, Dec 15, 2014 at 03:38:04PM +, Will Deacon wrote:
> On Mon, Dec 15, 2014 at 03:35:29PM +, Mark Brown wrote:

> > I don't mind either way, it just seemed to be easier to report the bug
> > with a patch.  If Jeorg is busy perhaps it can go via the arm64 tree,
> > the issue is triggered by the addition of the platform there?

> Can I pass the buck to arm-soc, as they're handling arm64 platform code?

> It seems sensible to merge the fix via the same tree that introduced the
> breakage.

Sure, just resending to them...


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

[PATCH] iommu/exynos: Fix arm64 allmodconfig build

[Mark Brown]

The Exynos IOMMU driver uses the ARM specific dmac_flush_range() and
outer_flush_range() functions. This breaks the build on arm64 allmodconfig
in -next since support has been merged for some Exynos ARMv8 SoCs. Add a
dependency on ARM to keep things building until either the driver has the
ARM dependencies removed or the ARMv8 architecture code implements these
ARM specific APIs.

Signed-off-by: Mark Brown 
---

Resending to the arm-soc people since the addition of the Exynos
platform for ARMv8 went via them, Krzysztof also sent a fix for this
earlier but it there's been no response.

 drivers/iommu/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 01e8bfae569b..325188eef1c1 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -187,7 +187,7 @@ config TEGRA_IOMMU_SMMU
 
 config EXYNOS_IOMMU
bool "Exynos IOMMU Support"
-   depends on ARCH_EXYNOS
+   depends on ARCH_EXYNOS && ARM
select IOMMU_API
select ARM_DMA_USE_IOMMU
help
-- 
2.1.3

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

Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator

[Will Deacon]

On Mon, Dec 15, 2014 at 01:30:20PM +, Will Deacon wrote:
> On Sun, Dec 14, 2014 at 05:45:49PM +, Varun Sethi wrote:
> > [varun] ok, but you could potentially end up splitting mapping to the
> > least possible page size e.g. 4K. You, don't seem to take in to account
> > the possibility of using the block size at the next level. For example,
> > take a case where we have a huge page mapping using 1G page size and we
> > have an unmap request for 4K. We could still split maximum part of the
> > mapping using 2M pages at the next level. The entry where we need to unmap
> > the 4K region would potentially go to the next level.
> 
> Aha, I see what you mean here, thanks. I'll take a look...

Scratch that, I think the code is fine as it is. For the case you highlight,
we iterate over the 1GB region remapping it using 4k pages, but skipping
the one we want to unmap, so I don't think there's a problem
(__arm_lpae_map will create the relevant table entries).

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

Re: [PATCH] iommu/exynos: Fix arm64 allmodconfig build

[Will Deacon]

On Mon, Dec 15, 2014 at 03:35:29PM +, Mark Brown wrote:
> On Mon, Dec 15, 2014 at 02:10:37PM +0100, Krzysztof Kozłowski wrote:
> > Few days ago I posted similar patch:
> > https://lkml.org/lkml/2014/12/5/268
> > but no one have picked it up.
> 
> > Anyway the fix of yours seems fine to me.
> 
> I don't mind either way, it just seemed to be easier to report the bug
> with a patch.  If Jeorg is busy perhaps it can go via the arm64 tree,
> the issue is triggered by the addition of the platform there?

Can I pass the buck to arm-soc, as they're handling arm64 platform code?

It seems sensible to merge the fix via the same tree that introduced the
breakage.

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

Re: [PATCH] iommu/exynos: Fix arm64 allmodconfig build

[Mark Brown]

On Mon, Dec 15, 2014 at 02:10:37PM +0100, Krzysztof Kozłowski wrote:

> Hi Mark,

> Few days ago I posted similar patch:
> https://lkml.org/lkml/2014/12/5/268
> but no one have picked it up.

> Anyway the fix of yours seems fine to me.

I don't mind either way, it just seemed to be easier to report the bug
with a patch.  If Jeorg is busy perhaps it can go via the arm64 tree,
the issue is triggered by the addition of the platform there?


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

Re: [PATCH 0/5] Fix Intel IRQ remapping initialization order

[Jiang Liu]

On 2014/12/15 23:13, Joerg Roedel wrote:
> Hi,
> 
> here is a patch-set against tip/x86/apic to fix an initialization order
> problem with the IRQ remapping code.  The problem is in the ordering of
> the irq_remapping_prepare and irq_remapping_supported functions.
> 
> Currently the call-order is irq_remapping_prepare ->
> irq_remapping_supported, so that 'prepare' can succeed but 'supported'
> fails, so that interrupt remapping gets initialized but not enabled.
> This causes a broken interrupt setup on affected systems (machines with
> an Intel IOMMU without, or broken, IRQ remapping support). The result
> are lost interrupts and a non-bootable system.
> 
> Both functions do checks whether IRQ remapping can be enabled on the
> machine.  The reason for this is that some checks rely on
> dmar_table_init() and thus have to be done in irq_remapping_prepare().
> 
> This patch-set moves all these checks into the irq_remapping_prepare()
> path with the right ordering and removes the irq_remapping_supported()
> function and its call-backs. This fixes the initializion order problem
> and simplifies the exported API from the IOMMU code.
> 
> Please review.
Hi Joerg,
I have posted a patch set for the same purpose at:
https://lkml.org/lkml/2014/12/10/20
Seems we need to combine these two patch sets:)
Regards!
Gerry


> 
> Thanks,
> 
>   Joerg
> 
> Joerg Roedel (5):
>   iommu/vt-d: Allocate IRQ remapping data structures only for all IOMMUs
>   iommu/vt-d: Don't check for ecap_ir_support in
> intel_irq_remapping_supported
>   iommu/vt-d: Move supported-checks to intel_prepare_irq_remapping()
>   iommu/amd: Check for irq-remap support amd_iommu_prepare()
>   iommu, x86, apic: Remove irq_remapping_supported()
> 
>  arch/x86/kernel/apic/apic.c |  5 ---
>  drivers/iommu/amd_iommu.c   |  1 -
>  drivers/iommu/amd_iommu_init.c  |  8 ++---
>  drivers/iommu/amd_iommu_proto.h |  1 -
>  drivers/iommu/intel_irq_remapping.c | 62 
> +
>  drivers/iommu/irq_remapping.c   | 11 ---
>  drivers/iommu/irq_remapping.h   |  3 --
>  7 files changed, 31 insertions(+), 60 deletions(-)
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/5] iommu/vt-d: Don't check for ecap_ir_support in intel_irq_remapping_supported

[Joerg Roedel]

From: Joerg Roedel 

This check needs an initialized dmar-table and is already
done in intel_prepare_irq_remapping().

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/intel_irq_remapping.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c 
b/drivers/iommu/intel_irq_remapping.c
index 3ef63b8..45223ea 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -512,9 +512,6 @@ static int __init dmar_x2apic_optout(void)
 
 static int __init intel_irq_remapping_supported(void)
 {
-   struct dmar_drhd_unit *drhd;
-   struct intel_iommu *iommu;
-
if (disable_irq_remap)
return 0;
if (irq_remap_broken) {
@@ -532,10 +529,6 @@ static int __init intel_irq_remapping_supported(void)
if (!dmar_ir_support())
return 0;
 
-   for_each_iommu(iommu, drhd)
-   if (!ecap_ir_support(iommu->ecap))
-   return 0;
-
return 1;
 }
 
-- 
1.9.1

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

[PATCH 5/5] iommu, x86, apic: Remove irq_remapping_supported()

[Joerg Roedel]

From: Joerg Roedel 

The functionality has been moved to irq_remapping_prepare,
so remove this function and all its call-backs.

Signed-off-by: Joerg Roedel 
---
 arch/x86/kernel/apic/apic.c |  5 -
 drivers/iommu/amd_iommu.c   |  1 -
 drivers/iommu/amd_iommu_init.c  |  5 -
 drivers/iommu/amd_iommu_proto.h |  1 -
 drivers/iommu/intel_irq_remapping.c |  6 --
 drivers/iommu/irq_remapping.c   | 11 ---
 drivers/iommu/irq_remapping.h   |  3 ---
 7 files changed, 32 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 141f103..4484483 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1575,11 +1575,6 @@ void enable_x2apic(void)
 int __init enable_IR(void)
 {
 #ifdef CONFIG_IRQ_REMAP
-   if (!irq_remapping_supported()) {
-   pr_debug("intr-remapping not supported\n");
-   return -1;
-   }
-
if (!x2apic_preenabled && skip_ioapic_setup) {
pr_info("Skipped enabling intr-remap because of skipping "
"io-apic setup\n");
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index a5c59b4..05ffb7c 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -4114,7 +4114,6 @@ static struct irq_domain *get_irq_domain(struct 
irq_alloc_info *info)
 }
 
 struct irq_remap_ops amd_iommu_irq_ops = {
-   .supported  = amd_iommu_supported,
.prepare= amd_iommu_prepare,
.enable = amd_iommu_enable,
.disable= amd_iommu_disable,
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index fa63016..b9ce87e 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2133,11 +2133,6 @@ int __init amd_iommu_prepare(void)
return iommu_go_to_state(IOMMU_ACPI_FINISHED);
 }
 
-int __init amd_iommu_supported(void)
-{
-   return amd_iommu_irq_remap ? 1 : 0;
-}
-
 int __init amd_iommu_enable(void)
 {
int ret;
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 612a221..fcf61ca 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -33,7 +33,6 @@ extern void amd_iommu_init_notifier(void);
 extern void amd_iommu_init_api(void);
 
 /* Needed for interrupt remapping */
-extern int amd_iommu_supported(void);
 extern int amd_iommu_prepare(void);
 extern int amd_iommu_enable(void);
 extern void amd_iommu_disable(void);
diff --git a/drivers/iommu/intel_irq_remapping.c 
b/drivers/iommu/intel_irq_remapping.c
index 0c9226f..8918f47 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -510,11 +510,6 @@ static int __init dmar_x2apic_optout(void)
return dmar->flags & DMAR_X2APIC_OPT_OUT;
 }
 
-static int __init intel_irq_remapping_supported(void)
-{
-   return 1;
-}
-
 static void __init intel_cleanup_irq_remapping(void)
 {
struct dmar_drhd_unit *drhd;
@@ -970,7 +965,6 @@ static struct irq_domain *intel_get_irq_domain(struct 
irq_alloc_info *info)
 }
 
 struct irq_remap_ops intel_irq_remap_ops = {
-   .supported  = intel_irq_remapping_supported,
.prepare= intel_prepare_irq_remapping,
.enable = intel_enable_irq_remapping,
.disable= disable_irq_remapping,
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 66517e7..6e7acd1 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -80,17 +80,6 @@ void set_irq_remapping_broken(void)
irq_remap_broken = 1;
 }
 
-int irq_remapping_supported(void)
-{
-   if (disable_irq_remap)
-   return 0;
-
-   if (!remap_ops || !remap_ops->supported)
-   return 0;
-
-   return remap_ops->supported();
-}
-
 int __init irq_remapping_prepare(void)
 {
remap_ops = &intel_irq_remap_ops;
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index 4bd791d..961dc49 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -36,9 +36,6 @@ extern int no_x2apic_optout;
 extern int irq_remapping_enabled;
 
 struct irq_remap_ops {
-   /* Check whether Interrupt Remapping is supported */
-   int (*supported)(void);
-
/* Initializes hardware and makes it ready for remapping interrupts */
int  (*prepare)(void);
 
-- 
1.9.1

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

[PATCH 1/5] iommu/vt-d: Allocate IRQ remapping data structures only for all IOMMUs

[Joerg Roedel]

From: Joerg Roedel 

IRQ remapping is only supported when all IOMMUs in the
system support it. So check if all IOMMUs in the system
support IRQ remapping before doing the allocations.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/intel_irq_remapping.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c 
b/drivers/iommu/intel_irq_remapping.c
index ff35b03..3ef63b8 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -568,14 +568,16 @@ static int __init intel_prepare_irq_remapping(void)
goto error;
}
 
-   for_each_iommu(iommu, drhd) {
+   /* First make sure all IOMMUs support IRQ remapping */
+   for_each_iommu(iommu, drhd)
if (!ecap_ir_support(iommu->ecap))
-   continue;
+   goto error;
 
-   /* Do the allocations early */
+   /* Do the allocations early */
+   for_each_iommu(iommu, drhd)
if (intel_setup_irq_remapping(iommu))
goto error;
-   }
+
return 0;
 error:
intel_cleanup_irq_remapping();
-- 
1.9.1

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

[PATCH 4/5] iommu/amd: Check for irq-remap support amd_iommu_prepare()

[Joerg Roedel]

From: Joerg Roedel 

This allows to get rid of the irq_remapping_supported()
function and all its call-backs into the Intel and AMD
IOMMU drivers.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/amd_iommu_init.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index de3390a..fa63016 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2127,6 +2127,9 @@ static int __init iommu_go_to_state(enum iommu_init_state 
state)
 #ifdef CONFIG_IRQ_REMAP
 int __init amd_iommu_prepare(void)
 {
+   if (!amd_iommu_irq_remap)
+   return -1;
+
return iommu_go_to_state(IOMMU_ACPI_FINISHED);
 }
 
-- 
1.9.1

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

[PATCH 0/5] Fix Intel IRQ remapping initialization order

[Joerg Roedel]

Hi,

here is a patch-set against tip/x86/apic to fix an initialization order
problem with the IRQ remapping code.  The problem is in the ordering of
the irq_remapping_prepare and irq_remapping_supported functions.

Currently the call-order is irq_remapping_prepare ->
irq_remapping_supported, so that 'prepare' can succeed but 'supported'
fails, so that interrupt remapping gets initialized but not enabled.
This causes a broken interrupt setup on affected systems (machines with
an Intel IOMMU without, or broken, IRQ remapping support). The result
are lost interrupts and a non-bootable system.

Both functions do checks whether IRQ remapping can be enabled on the
machine.  The reason for this is that some checks rely on
dmar_table_init() and thus have to be done in irq_remapping_prepare().

This patch-set moves all these checks into the irq_remapping_prepare()
path with the right ordering and removes the irq_remapping_supported()
function and its call-backs. This fixes the initializion order problem
and simplifies the exported API from the IOMMU code.

Please review.

Thanks,

Joerg

Joerg Roedel (5):
  iommu/vt-d: Allocate IRQ remapping data structures only for all IOMMUs
  iommu/vt-d: Don't check for ecap_ir_support in
intel_irq_remapping_supported
  iommu/vt-d: Move supported-checks to intel_prepare_irq_remapping()
  iommu/amd: Check for irq-remap support amd_iommu_prepare()
  iommu, x86, apic: Remove irq_remapping_supported()

 arch/x86/kernel/apic/apic.c |  5 ---
 drivers/iommu/amd_iommu.c   |  1 -
 drivers/iommu/amd_iommu_init.c  |  8 ++---
 drivers/iommu/amd_iommu_proto.h |  1 -
 drivers/iommu/intel_irq_remapping.c | 62 +
 drivers/iommu/irq_remapping.c   | 11 ---
 drivers/iommu/irq_remapping.h   |  3 --
 7 files changed, 31 insertions(+), 60 deletions(-)

-- 
1.9.1

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

[PATCH 3/5] iommu/vt-d: Move supported-checks to intel_prepare_irq_remapping()

[Joerg Roedel]

From: Joerg Roedel 

The checks for IRQ remapping support rely on ordering with
dmar_table_init(), which is called in
intel_prepare_irq_remapping(). Move the checks for IRQ
remapping support to this function to ensure the right
ordering.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/intel_irq_remapping.c | 39 +
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c 
b/drivers/iommu/intel_irq_remapping.c
index 45223ea..0c9226f 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -512,23 +512,6 @@ static int __init dmar_x2apic_optout(void)
 
 static int __init intel_irq_remapping_supported(void)
 {
-   if (disable_irq_remap)
-   return 0;
-   if (irq_remap_broken) {
-   printk(KERN_WARNING
-   "This system BIOS has enabled interrupt remapping\n"
-   "on a chipset that contains an erratum making that\n"
-   "feature unstable.  To maintain system stability\n"
-   "interrupt remapping is being disabled.  Please\n"
-   "contact your BIOS vendor for an update\n");
-   add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
-   disable_irq_remap = 1;
-   return 0;
-   }
-
-   if (!dmar_ir_support())
-   return 0;
-
return 1;
 }
 
@@ -553,6 +536,24 @@ static int __init intel_prepare_irq_remapping(void)
struct dmar_drhd_unit *drhd;
struct intel_iommu *iommu;
 
+   /* First check whether IRQ remapping should be enabled */
+   if (disable_irq_remap)
+   return -1;
+
+   /* Are we running on sane hardware? */
+   if (irq_remap_broken) {
+   printk(KERN_WARNING
+   "This system BIOS has enabled interrupt remapping\n"
+   "on a chipset that contains an erratum making that\n"
+   "feature unstable.  To maintain system stability\n"
+   "interrupt remapping is being disabled.  Please\n"
+   "contact your BIOS vendor for an update\n");
+   add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
+   disable_irq_remap = 1;
+   return -1;
+   }
+
+   /* Everthing seems fine so far, parse the DMAR table */
if (dmar_table_init() < 0)
return -1;
 
@@ -561,6 +562,10 @@ static int __init intel_prepare_irq_remapping(void)
goto error;
}
 
+   /* Is interrupt remapping supported by hardware? */
+   if (!dmar_ir_support())
+   return 0;
+
/* First make sure all IOMMUs support IRQ remapping */
for_each_iommu(iommu, drhd)
if (!ecap_ir_support(iommu->ecap))
-- 
1.9.1

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

Re: [PATCH 2/4] iommu: add ARM LPAE page table allocator

[Will Deacon]

On Sun, Dec 14, 2014 at 05:45:49PM +, Varun Sethi wrote:
> Please find my response inline. Search for "varun".

This is getting fiddly now that you've already replied once. Any chance you
could sort your mail client out, please?

> > +static int arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,
> > +unsigned long iova, phys_addr_t paddr,
> > +arm_lpae_iopte prot, int lvl,
> > +arm_lpae_iopte *ptep) {
> > +   arm_lpae_iopte pte = prot;
> > +
> > +   /* We require an unmap first */
> > +   if (iopte_leaf(*ptep, lvl))
> > +   return -EEXIST;
> > [varun] Instead of returning an error, how about displaying a warning
> > and replacing the entry?
> 
> I'd be ok with displaying a warning, but I don't think we should just
> continue. It indicates a misuse of the IOMMU API and probably a missing
> TLBI.
> 
> [[varun]] May not apply now, but what if we are dealing with a case where
> memory is not pinned? It may be possible to hookup (without an unmap) an
> iova to a different physical address. Offcourse, tlb invalidation would be
> required. Could this scenario be relevant in case of stall mode?

If we wanted to support that, then we'd need some new functions for grabbing
hold of the entry and manipulating it in a similar way to the CPU side (e.g.
pte_mkdirty etc).

> > +   if (__arm_lpae_map(data, blk_start, blk_paddr, size, prot, 
> > lvl,
> > +  tablep) < 0) {
> >
> >
> > [[varun]] Again not clear how are we unmapping the range. Index at the
> > current level should point to a page table (with contiguous block
> > mappings). Unmap would applied to the mappings at the next level.
> > Unmap can happen anywhere in the contiguous range. It seems that you
> > are just creating a subset of the block mapping.
> 
> We will be unmapping a single entry at the next level, so we basically
> create a table, then map everything at the next level apart from the part
> we need to unmap.
> 
> 
> [varun] ok, but you could potentially end up splitting mapping to the
> least possible page size e.g. 4K. You, don't seem to take in to account
> the possibility of using the block size at the next level. For example,
> take a case where we have a huge page mapping using 1G page size and we
> have an unmap request for 4K. We could still split maximum part of the
> mapping using 2M pages at the next level. The entry where we need to unmap
> the 4K region would potentially go to the next level.

Aha, I see what you mean here, thanks. I'll take a look...

> > +static int __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
> > +   unsigned long iova, size_t size, int lvl,
> > +   arm_lpae_iopte *ptep) {
> > +   arm_lpae_iopte pte;
> > +   struct iommu_gather_ops *tlb = data->iop.cfg.tlb;
> > +   void *cookie = data->iop.cookie;
> > +   size_t blk_size = ARM_LPAE_BLOCK_SIZE(lvl, data);
> > +
> > +   ptep += ARM_LPAE_LVL_IDX(iova, lvl, data);
> > +   pte = *ptep;
> > +
> > +   /* Something went horribly wrong and we ran out of page table */
> > +   if (WARN_ON(!pte || (lvl == ARM_LPAE_MAX_LEVELS)))
> > +   return 0;
> > +
> > +   /* If the size matches this level, we're in the right place */
> > +   if (size == blk_size) {
> > +   *ptep = 0;
> > +   tlb->flush_pgtable(ptep, sizeof(*ptep), cookie);
> > +
> > +   if (!iopte_leaf(pte, lvl)) {
> > +   /* Also flush any partial walks */
> > +   tlb->tlb_add_flush(iova, size, false, cookie);
> > +   tlb->tlb_sync(data->iop.cookie);
> > +   ptep = iopte_deref(pte, data);
> > +   __arm_lpae_free_pgtable(data, lvl + 1, ptep);
> > +   } else {
> > +   tlb->tlb_add_flush(iova, size, true, cookie);
> > +   }
> > +
> > +   return size;
> > +   } else if (iopte_leaf(pte, lvl)) {
> > +   /*
> > +* Insert a table at the next level to map the old region,
> > +* minus the part we want to unmap
> > +*/
> > [[varun]]  Minus could be somwhere in between the contiguous chunk? We
> > should first break the entire block mapping in to a next level page
> > mapping and then unmap a chunk.
> 
> The amount to unmap will match exactly one entry at the next level --
> that's enforced by the IOMMU API (and it will also be aligned as such).
> 
> > +static struct arm_lpae_io_pgtable *
> > +arm_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg) {
> > +   unsigned long va_bits;
> > +   struct arm_lpae_io_pgtable *data;
> > +
> > +   arm_lpae_restrict_pgsizes(cfg);
> > +
> > +   if (!(cfg->pgsize_bitmap & (SZ_4K | SZ_16K | SZ_64K)))
> > +   return NULL;
> > +
> > +   if (cfg->ias > ARM_LPAE_MAX_ADDR

Re: [PATCH] iommu/exynos: Fix arm64 allmodconfig build

[Krzysztof Kozłowski]

On 15.12.2014 14:00, Mark Brown wrote:
> The Exynos IOMMU driver uses the ARM specific dmac_flush_range() and
> outer_flush_range() functions. This breaks the build on arm64 allmodconfig
> in -next since support has been merged for some Exynos ARMv8 SoCs. Add a
> dependency on ARM to keep things building until either the driver has the
> ARM dependencies removed or the ARMv8 architecture code implements these
> ARM specific APIs.

Hi Mark,

Few days ago I posted similar patch:
https://lkml.org/lkml/2014/12/5/268
but no one have picked it up.

Anyway the fix of yours seems fine to me.

Best regards,
Krzysztof

> 
> Signed-off-by: Mark Brown 
> ---
>  drivers/iommu/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 01e8bfae569b..325188eef1c1 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -187,7 +187,7 @@ config TEGRA_IOMMU_SMMU
>  
>  config EXYNOS_IOMMU
>   bool "Exynos IOMMU Support"
> - depends on ARCH_EXYNOS
> + depends on ARCH_EXYNOS && ARM
>   select IOMMU_API
>   select ARM_DMA_USE_IOMMU
>   help
> 

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

Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Geert Uytterhoeven]

On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart
 wrote:
> Add the seven IPMMU instances found in the r8a7791 to DT with a disabled
> status.
>
> Signed-off-by: Laurent Pinchart 

The addresses and interrupt numbers look OK to me.
However, my comment about the "0x800" offset is still valid.
Shouldn't we have two register blocks, and let the driver use only the
second one?

If you ignore, feel free to add my
Acked-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] iommu/exynos: Fix arm64 allmodconfig build

[Mark Brown]

The Exynos IOMMU driver uses the ARM specific dmac_flush_range() and
outer_flush_range() functions. This breaks the build on arm64 allmodconfig
in -next since support has been merged for some Exynos ARMv8 SoCs. Add a
dependency on ARM to keep things building until either the driver has the
ARM dependencies removed or the ARMv8 architecture code implements these
ARM specific APIs.

Signed-off-by: Mark Brown 
---
 drivers/iommu/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 01e8bfae569b..325188eef1c1 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -187,7 +187,7 @@ config TEGRA_IOMMU_SMMU
 
 config EXYNOS_IOMMU
bool "Exynos IOMMU Support"
-   depends on ARCH_EXYNOS
+   depends on ARCH_EXYNOS && ARM
select IOMMU_API
select ARM_DMA_USE_IOMMU
help
-- 
2.1.3

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

Re: [PATCH v6 6/8] dma-mapping: detect and configure IOMMU in of_dma_configure

[Will Deacon]

On Sun, Dec 14, 2014 at 03:51:13PM +, Laurent Pinchart wrote:
> On Monday 01 December 2014 16:57:12 Will Deacon wrote:
> > This patch extends of_dma_configure so that it sets up the IOMMU for a
> > device, as well as the coherent/non-coherent DMA mapping ops.
> > 
> > Acked-by: Arnd Bergmann 
> > Acked-by: Marek Szyprowski 
> > Tested-by: Robin Murphy 
> > Signed-off-by: Will Deacon 

[...]

> > +static void of_dma_deconfigure(struct device *dev)
> > +{
> > +   arch_teardown_dma_ops(dev);
> >  }
> > 
> >  /**
> > @@ -223,16 +234,12 @@ static struct platform_device
> > *of_platform_device_create_pdata( if (!dev)
> > goto err_clear_flag;
> > 
> > -   of_dma_configure(&dev->dev);
> > dev->dev.bus = &platform_bus_type;
> > dev->dev.platform_data = platform_data;
> > -
> > -   /* We do not fill the DMA ops for platform devices by default.
> > -* This is currently the responsibility of the platform code
> > -* to do such, possibly using a device notifier
> > -*/
> > +   of_dma_configure(&dev->dev);
> > 
> > if (of_device_add(dev) != 0) {
> > +   of_dma_deconfigure(&dev->dev);
> 
> Don't you also need to call of_dma_deconfigure() when the device is destroyed 
> ? Otherwise the default domain created by arch_setup_dma_ops() will be leaked.

Something like below?

Will

--->8

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index b89caf8c7586..ec29c25b4fce 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -525,6 +525,7 @@ static int of_platform_device_destroy(struct device *dev, 
void *data)
amba_device_unregister(to_amba_device(dev));
 #endif
 
+   of_dma_deconfigure(dev);
of_node_clear_flag(dev->of_node, OF_POPULATED);
of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
return 0;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/ipmmu-vmsa: change IOMMU_EXEC to IOMMU_NOEXEC

[Geert Uytterhoeven]

Hi Laurent,

On Sun, Dec 14, 2014 at 1:55 AM, Laurent Pinchart
 wrote:
> The IOMMU_EXEC flag has been renamed to IOMMU_NOEXEC by commit

... inverted and replaced by ...

At first I hadn't noticed this, and was wondering about the seemingly unrelated
change in the first hunk.

> a720b41c41f5a7e4 ("iommu/arm-smmu: change IOMMU_EXEC to IOMMU_NOEXEC").
> Update the driver accordingly.
>
> Signed-off-by: Laurent Pinchart 
> ---
>  drivers/iommu/ipmmu-vmsa.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> This fixes a build breakage in Linus' master branch introduce by the commit
> mentioned above.
>
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index e5ed0216929f..7a1bcf6627b6 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -558,7 +558,7 @@ static pmd_t *ipmmu_alloc_pmd(struct ipmmu_vmsa_device 
> *mmu, pgd_t *pgd,
>
>  static u64 ipmmu_page_prot(unsigned int prot, u64 type)
>  {
> -   u64 pgprot = ARM_VMSA_PTE_XN | ARM_VMSA_PTE_nG | ARM_VMSA_PTE_AF
> +   u64 pgprot = ARM_VMSA_PTE_nG | ARM_VMSA_PTE_AF
>| ARM_VMSA_PTE_SH_IS | ARM_VMSA_PTE_AP_UNPRIV
>| ARM_VMSA_PTE_NS | type;
>
> @@ -568,8 +568,8 @@ static u64 ipmmu_page_prot(unsigned int prot, u64 type)
> if (prot & IOMMU_CACHE)
> pgprot |= IMMAIR_ATTR_IDX_WBRWA << 
> ARM_VMSA_PTE_ATTRINDX_SHIFT;
>
> -   if (prot & IOMMU_EXEC)
> -   pgprot &= ~ARM_VMSA_PTE_XN;
> +   if (prot & IOMMU_NOEXEC)
> +   pgprot |= ARM_VMSA_PTE_XN;
> else if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
> /* If no access create a faulting entry to avoid TLB fills. */
> pgprot &= ~ARM_VMSA_PTE_PAGE;

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 10/10] iommu/vt-d: Use old irte in kdump kernel

[Li, Zhen-Hua]

Fix the intr-remapping fault.

[1.594890] dmar: DRHD: handling fault status reg 2
[1.594894] dmar: INTR-REMAP: Request device [[41:00.0] fault index 4d
[1.594894] INTR-REMAP:[fault reason 34] Present field in the IRTE entry
is clear

Use old irte in kdump kernel, do not disable and re-enable interrupt
remapping.

Signed-off-by: Li, Zhen-Hua 
---
 drivers/iommu/intel_irq_remapping.c | 42 -
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/intel_irq_remapping.c 
b/drivers/iommu/intel_irq_remapping.c
index 090ba32..fee7d66 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -198,6 +198,11 @@ static int modify_irte(int irq, struct irte *irte_modified)
 
set_64bit(&irte->low, irte_modified->low);
set_64bit(&irte->high, irte_modified->high);
+
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel())
+   __iommu_update_old_irte(iommu, index);
+#endif
__iommu_flush_cache(iommu, irte, sizeof(*irte));
 
rc = qi_flush_iec(iommu, index, 0);
@@ -259,6 +264,11 @@ static int clear_entries(struct irq_2_iommu *irq_iommu)
bitmap_release_region(iommu->ir_table->bitmap, index,
  irq_iommu->irte_mask);
 
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel())
+   __iommu_update_old_irte(iommu, -1);
+#endif
+
return qi_flush_iec(iommu, index, irq_iommu->irte_mask);
 }
 
@@ -640,11 +650,20 @@ static int __init intel_enable_irq_remapping(void)
 */
dmar_fault(-1, iommu);
 
-   /*
-* Disable intr remapping and queued invalidation, if already
-* enabled prior to OS handover.
-*/
-   iommu_disable_irq_remapping(iommu);
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel()) {
+   /* Do notdisable irq and then re-enable again. */
+   } else {
+#endif
+   /*
+* Disable intr remapping and queued invalidation,
+* if already enabled prior to OS handover.
+*/
+   iommu_disable_irq_remapping(iommu);
+
+#ifdef CONFIG_CRASH_DUMP
+   }
+#endif
 
dmar_disable_qi(iommu);
}
@@ -687,7 +706,20 @@ static int __init intel_enable_irq_remapping(void)
if (intel_setup_irq_remapping(iommu))
goto error;
 
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel()) {
+   unsigned long long q;
+
+   q = dmar_readq(iommu->reg + DMAR_IRTA_REG);
+   iommu->ir_table->base_old_phys = q & VTD_PAGE_MASK;
+   iommu->ir_table->base_old_virt = ioremap_cache(
+   iommu->ir_table->base_old_phys,
+   INTR_REMAP_TABLE_ENTRIES*sizeof(struct irte));
+   __iommu_load_old_irte(iommu);
+   } else
+#endif
iommu_set_irq_remapping(iommu, eim);
+
setup = 1;
}
 
-- 
2.0.0-rc0

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

[PATCH 07/10] iommu/vt-d: enable kdump support in iommu module

[Li, Zhen-Hua]

Modify the operation of the following functions when called during crash dump:
 device_to_domain_id
 get_domain_for_dev
 init_dmars
 intel_iommu_init

Bill Sumner:
Original version.

Zhenhua:
Minor change, change some function name, add spin_lock_irqsave.

Signed-off-by: Bill Sumner 
Signed-off-by: Li, Zhen-Hua 
---
 drivers/iommu/intel-iommu.c | 142 ++--
 1 file changed, 125 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 9b2f725..4837ce5 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -907,6 +907,11 @@ static struct context_entry * 
device_to_context_entry(struct intel_iommu *iommu,
set_root_value(root, phy_addr);
set_root_present(root);
__iommu_flush_cache(iommu, root, sizeof(*root));
+
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel())
+   __iommu_update_old_root_entry(iommu, bus);
+#endif
}
spin_unlock_irqrestore(&iommu->lock, flags);
return &context[devfn];
@@ -958,7 +963,8 @@ static void free_context_table(struct intel_iommu *iommu)
 
spin_lock_irqsave(&iommu->lock, flags);
if (!iommu->root_entry) {
-   goto out;
+   spin_unlock_irqrestore(&iommu->lock, flags);
+   return;
}
for (i = 0; i < ROOT_ENTRY_NR; i++) {
root = &iommu->root_entry[i];
@@ -966,10 +972,26 @@ static void free_context_table(struct intel_iommu *iommu)
if (context)
free_pgtable_page(context);
}
-   free_pgtable_page(iommu->root_entry);
-   iommu->root_entry = NULL;
-out:
+
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel()) {
+   iommu->root_entry_old_phys = 0;
+   root = iommu->root_entry;
+   iommu->root_entry = NULL;
+   } else {
+#endif
+   free_pgtable_page(iommu->root_entry);
+   iommu->root_entry = NULL;
+#ifdef CONFIG_CRASH_DUMP
+   }
+#endif
+
spin_unlock_irqrestore(&iommu->lock, flags);
+
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel())
+   iounmap(root);
+#endif
 }
 
 static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
@@ -2381,6 +2403,9 @@ static struct dmar_domain *get_domain_for_dev(struct 
device *dev, int gaw)
unsigned long flags;
u8 bus, devfn;
int did = -1;   /* Default to "no domain_id supplied" */
+#ifdef CONFIG_CRASH_DUMP
+   struct domain_values_entry *dve = NULL;
+#endif /* CONFIG_CRASH_DUMP */
 
domain = find_domain(dev);
if (domain)
@@ -2414,6 +2439,24 @@ static struct dmar_domain *get_domain_for_dev(struct 
device *dev, int gaw)
domain = alloc_domain(0);
if (!domain)
return NULL;
+
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel()) {
+   /*
+* if this device had a did in the old kernel
+* use its values instead of generating new ones
+*/
+   did = device_to_domain_id(iommu, bus, devfn);
+   if (did > 0 || (did == 0 && !cap_caching_mode(iommu->cap)))
+   dve = intel_iommu_did_to_domain_values_entry(did,
+   iommu);
+   if (dve)
+   gaw = dve->gaw;
+   else
+   did = -1;
+   }
+#endif /* CONFIG_CRASH_DUMP */
+
domain->id = iommu_attach_domain(domain, iommu, did);
if (domain->id < 0) {
free_domain_mem(domain);
@@ -2425,6 +2468,18 @@ static struct dmar_domain *get_domain_for_dev(struct 
device *dev, int gaw)
return NULL;
}
 
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel() && dve) {
+
+   if (domain->pgd)
+   free_pgtable_page(domain->pgd);
+
+   domain->pgd = dve->pgd;
+
+   copy_reserved_iova(&dve->iovad, &domain->iovad);
+   }
+#endif /* CONFIG_CRASH_DUMP */
+
/* register PCI DMA alias device */
if (dev_is_pci(dev)) {
tmp = dmar_insert_dev_info(iommu, PCI_BUS_NUM(dma_alias),
@@ -2948,14 +3003,35 @@ static int __init init_dmars(void)
if (ret)
goto free_iommu;
 
-   /*
-* TBD:
-* we could share the same root & context tables
-* among all IOMMU's. Need to Split it later.
-*/
-   ret = iommu_alloc_root_entry(iommu);
-   if (ret)
-   goto free_iommu;
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel()) {
+   pr_info("IOMMU Copying translate tables from panicked 
kernel\n");
+   ret = intel_iommu_copy_translation_tables(drhd,
+

[PATCH 09/10] iommu/vt-d: Copy functions for irte

[Li, Zhen-Hua]

Functions to copy the irte data from the old kernel into the kdump kernel.

Signed-off-by: Li, Zhen-Hua 
---
 drivers/iommu/intel_irq_remapping.c | 57 +
 include/linux/intel-iommu.h |  4 +++
 2 files changed, 61 insertions(+)

diff --git a/drivers/iommu/intel_irq_remapping.c 
b/drivers/iommu/intel_irq_remapping.c
index 27541d4..090ba32 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -17,6 +18,11 @@
 
 #include "irq_remapping.h"
 
+#ifdef CONFIG_CRASH_DUMP
+static int __iommu_load_old_irte(struct intel_iommu *iommu);
+static int __iommu_update_old_irte(struct intel_iommu *iommu, int index);
+#endif /* CONFIG_CRASH_DUMP */
+
 struct ioapic_scope {
struct intel_iommu *iommu;
unsigned int id;
@@ -1296,3 +1302,54 @@ int dmar_ir_hotplug(struct dmar_drhd_unit *dmaru, bool 
insert)
 
return ret;
 }
+
+#ifdef CONFIG_CRASH_DUMP
+
+static int __iommu_load_old_irte(struct intel_iommu *iommu)
+{
+   if ((!iommu)
+   || (!iommu->ir_table)
+   || (!iommu->ir_table->base)
+   || (!iommu->ir_table->base_old_phys)
+   || (!iommu->ir_table->base_old_virt))
+   return -1;
+
+   memcpy(iommu->ir_table->base,
+   iommu->ir_table->base_old_virt,
+   INTR_REMAP_TABLE_ENTRIES*sizeof(struct irte));
+
+   return 0;
+}
+
+static int __iommu_update_old_irte(struct intel_iommu *iommu, int index)
+{
+   int start;
+   unsigned long size;
+   void __iomem *to;
+   void *from;
+
+   if ((!iommu)
+   || (!iommu->ir_table)
+   || (!iommu->ir_table->base)
+   || (!iommu->ir_table->base_old_phys)
+   || (!iommu->ir_table->base_old_virt))
+   return -1;
+
+   if (index < -1 || index >= INTR_REMAP_TABLE_ENTRIES)
+   return -1;
+
+   if (index == -1) {
+   start = 0;
+   size = INTR_REMAP_TABLE_ENTRIES * sizeof(struct irte);
+   } else {
+   start = index * sizeof(struct irte);
+   size = sizeof(struct irte);
+   }
+
+   to = iommu->ir_table->base_old_virt;
+   from = iommu->ir_table->base;
+   memcpy(to + start, from + start, size);
+
+   return 0;
+}
+#endif /* CONFIG_CRASH_DUMP */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 8e29b97..76c6ea5 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -290,6 +290,10 @@ struct q_inval {
 struct ir_table {
struct irte *base;
unsigned long *bitmap;
+#ifdef CONFIG_CRASH_DUMP
+   void __iomem *base_old_virt;
+   unsigned long base_old_phys;
+#endif
 };
 #endif
 
-- 
2.0.0-rc0

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

[PATCH 06/10] iommu/vt-d: datatypes and functions used for kdump

[Li, Zhen-Hua]

Populate it with support functions to copy iommu translation tables from
from the panicked kernel into the kdump kernel in the event of a crash.

Functions:
malloc new context table and copy old context table to the new one.
malloc new page table and copy old page table to the new one.

Bill Sumner:
Original version, the creation of the data types and functions.

Li, Zhenhua:
Minor change:
Update the usage of context_get_* and context_put*, use context_*
and context_set_* for replacement.
Update the name of the function that copies root entry table.
Use new function to copy old context entry tables and page tables.

Signed-off-by: Bill Sumner 
Signed-off-by: Li, Zhen-Hua 
---
 drivers/iommu/intel-iommu.c | 542 
 1 file changed, 542 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 126294db..9b2f725 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -399,6 +399,62 @@ struct iommu_remapped_entry {
 static LIST_HEAD(__iommu_remapped_mem);
 static DEFINE_MUTEX(__iommu_mem_list_lock);
 
+/* 
+ * Copy iommu translation tables from old kernel into new  kernel.
+ * Entry to this set of functions is: intel_iommu_copy_translation_tables()
+ * 
+ */
+
+/*
+ * Lists of domain_values_entry to hold domain values found during the copy.
+ * One list for each iommu in g_number_of_iommus.
+ */
+static struct list_head *domain_values_list;
+
+
+#define RET_BADCOPY -1 /* Return-code: Cannot copy translate tables */
+
+/*
+ * Struct copy_page_addr_parms is used to allow copy_page_addr()
+ * to accumulate values across multiple calls and returns.
+ */
+struct copy_page_addr_parms {
+   u32 first;  /* flag: first-time  */
+   u32 last;   /* flag: last-time */
+   u32 bus;/* last bus number we saw */
+   u32 devfn;  /* last devfn we saw */
+   u32 shift;  /* last shift we saw */
+   u64 pte;/* Page Table Entry */
+   u64 next_addr;  /* next-expected page_addr */
+
+   u64 page_addr;  /* page_addr accumulating size */
+   u64 page_size;  /* page_size accumulated */
+
+   struct domain_values_entry *dve;/* to accumulate iova ranges */
+};
+
+enum returns_from_copy_context_entry {
+RET_CCE_NOT_PRESENT = 1,
+RET_CCE_NEW_PAGE_TABLES,
+RET_CCE_PASS_THROUGH_1,
+RET_CCE_PASS_THROUGH_2,
+RET_CCE_RESERVED_VALUE,
+RET_CCE_PREVIOUS_DID
+};
+
+static int copy_context_entry(struct intel_iommu *iommu, u32 bus, u32 devfn,
+ void *ppap, struct context_entry *ce);
+
+static int copy_context_entry_table(struct intel_iommu *iommu,
+   u32 bus, void *ppap,
+   struct context_entry **context_new_p,
+   unsigned long context_old_phys);
+
+static int copy_root_entry_table(struct intel_iommu *iommu, void *ppap);
+
+static int intel_iommu_copy_translation_tables(struct dmar_drhd_unit *drhd,
+   int g_num_of_iommus);
+
 #endif /* CONFIG_CRASH_DUMP */
 
 /*
@@ -5039,4 +5095,490 @@ static void __iommu_update_old_root_entry(struct 
intel_iommu *iommu, int index)
memcpy(to + start, from + start, size);
 }
 
+/*
+ * constant for initializing instances of copy_page_addr_parms properly.
+ */
+static struct copy_page_addr_parms copy_page_addr_parms_init = {1, 0};
+
+
+
+/*
+ * Lowest-level function in the 'Copy Page Tables' set
+ * Called once for each page_addr present in an iommu page-address table.
+ *
+ * Because of the depth-first traversal of the page-tables by the
+ * higher-level functions that call 'copy_page_addr', all pages
+ * of a domain will be presented in ascending order of IO Virtual Address.
+ *
+ * This function accumulates each contiguous range of these IOVAs and
+ * reserves it within the proper domain in the crashdump kernel when a
+ * non-contiguous range is detected, as determined by any of the following:
+ * 1. a change in the bus or device owning the presented page
+ * 2. a change in the page-size of the presented page (parameter shift)
+ * 3. a change in the page-table entry of the presented page
+ * 4. a presented IOVA that does not match the expected next-page address
+ * 5. the 'last' flag is set, indicating that all IOVAs have been seen.
+ */
+static int copy_page_addr(u64 page_addr, u32 shift, u32 bus, u32 devfn,
+   u64 pte, struct domain_values_entry *dve,
+   void *parms)
+{
+   struct copy_page_addr_parms *ppap = parms;
+
+   u64 page_size = ((u64)1 << shift);  /* page_size */
+   u64 pfn_lo; /* For reserving IOVA range */
+   u64 pfn_hi; /* For reserving IOVA range */
+   struct iova

[PATCH 08/10] iommu/vtd: assign new page table for dma_map

[Li, Zhen-Hua]

When a device driver issues the first dma_map command for a
device, we assign a new and empty page-table, thus removing all
mappings from the old kernel for the device.

Signed-off-by: Li, Zhen-Hua 
---
 drivers/iommu/intel-iommu.c | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 4837ce5..f11a9b1 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3200,15 +3200,28 @@ static struct dmar_domain 
*__get_valid_domain_for_dev(struct device *dev)
}
 
/* make sure context mapping is ok */
-   if (unlikely(!domain_context_mapped(dev))) {
-   ret = domain_context_mapping(domain, dev, 
CONTEXT_TT_MULTI_LEVEL);
-   if (ret) {
-   printk(KERN_ERR "Domain context map for %s failed",
-  dev_name(dev));
-   return NULL;
-   }
+   if (likely(domain_context_mapped(dev))) {
+#ifdef CONFIG_CRASH_DUMP
+   if (is_kdump_kernel()) {
+   domain_exit(domain);
+   domain = get_domain_for_dev(dev,
+   DEFAULT_DOMAIN_ADDRESS_WIDTH);
+   if (!domain) {
+   pr_err("Allocating domain for %s failed",
+  dev_name(dev));
+   return NULL;
+   }
+   } else
+#endif
+   return domain;
}
 
+   ret = domain_context_mapping(domain, dev, CONTEXT_TT_MULTI_LEVEL);
+   if (ret) {
+   pr_err("Domain context map for %s failed",
+  dev_name(dev));
+   return NULL;
+   }
return domain;
 }
 
-- 
2.0.0-rc0

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

[PATCH 03/10] iommu/vt-d: Add domain-id functions

[Li, Zhen-Hua]

Interfaces for when a new domain in the crashdump kernel needs some
values from the panicked kernel's context entries.

Signed-off-by: Bill Sumner 
---
 drivers/iommu/intel-iommu.c | 62 +
 1 file changed, 62 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 5ce2850..c0bebd6 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -381,6 +381,13 @@ struct domain_values_entry {
   2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
 };
 
+static struct domain_values_entry *intel_iommu_did_to_domain_values_entry(
+   int did, struct intel_iommu *iommu);
+
+static int intel_iommu_get_dids_from_old_kernel(struct intel_iommu *iommu);
+
+static int device_to_domain_id(struct intel_iommu *iommu, u8 bus, u8 devfn);
+
 #endif /* CONFIG_CRASH_DUMP */
 
 /*
@@ -4832,3 +4839,58 @@ static void __init check_tylersburg_isoch(void)
printk(KERN_WARNING "DMAR: Recommended TLB entries for ISOCH unit is 
16; your BIOS set %d\n",
   vtisochctrl);
 }
+
+#ifdef CONFIG_CRASH_DUMP
+
+/*
+ * Interfaces for when a new domain in the crashdump kernel needs some
+ * values from the panicked kernel's context entries
+ *
+ */
+static struct domain_values_entry *intel_iommu_did_to_domain_values_entry(
+   int did, struct intel_iommu *iommu)
+{
+   struct domain_values_entry *dve;/* iterator */
+
+   list_for_each_entry(dve, &domain_values_list[iommu->seq_id], link)
+   if (dve->did == did)
+   return dve;
+   return NULL;
+}
+
+/* Mark domain-id's from old kernel as in-use on this iommu so that a new
+ * domain-id is allocated in the case where there is a device in the new kernel
+ * that was not in the old kernel -- and therefore a new domain-id is needed.
+ */
+static int intel_iommu_get_dids_from_old_kernel(struct intel_iommu *iommu)
+{
+   struct domain_values_entry *dve;/* iterator */
+
+   pr_info("IOMMU:%d Domain ids from panicked kernel:\n", iommu->seq_id);
+
+   list_for_each_entry(dve, &domain_values_list[iommu->seq_id], link) {
+   set_bit(dve->did, iommu->domain_ids);
+   pr_info("DID did:%d(0x%4.4x)\n", dve->did, dve->did);
+   }
+
+   pr_info("\n");
+   return 0;
+}
+
+static int device_to_domain_id(struct intel_iommu *iommu, u8 bus, u8 devfn)
+{
+   int did = -1;   /* domain-id returned */
+   struct root_entry *root;
+   struct context_entry *context;
+   unsigned long flags;
+
+   spin_lock_irqsave(&iommu->lock, flags);
+   root = &iommu->root_entry[bus];
+   context = get_context_addr_from_root(root);
+   if (context && context_present(context+devfn))
+   did = context_domain_id(context+devfn);
+   spin_unlock_irqrestore(&iommu->lock, flags);
+   return did;
+}
+
+#endif /* CONFIG_CRASH_DUMP */
-- 
2.0.0-rc0

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

[PATCH 04/10] iommu/vt-d: functions to copy data from old mem

[Li, Zhen-Hua]

Add some functions to copy the data from old kernel.
These functions are used to copy context tables and page tables.

To avoid calling iounmap between spin_lock_irqsave and spin_unlock_irqrestore,
use a link here, store the pointers , and then use iounmap to free them in
another place.

Signed-off-by: Li, Zhen-Hua 
---
 drivers/iommu/intel-iommu.c | 97 +
 include/linux/intel-iommu.h |  9 +
 2 files changed, 106 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c0bebd6..8a7ad72 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -388,6 +388,13 @@ static int intel_iommu_get_dids_from_old_kernel(struct 
intel_iommu *iommu);
 
 static int device_to_domain_id(struct intel_iommu *iommu, u8 bus, u8 devfn);
 
+struct iommu_remapped_entry {
+   struct list_head list;
+   void __iomem *mem;
+};
+static LIST_HEAD(__iommu_remapped_mem);
+static DEFINE_MUTEX(__iommu_mem_list_lock);
+
 #endif /* CONFIG_CRASH_DUMP */
 
 /*
@@ -4843,6 +4850,96 @@ static void __init check_tylersburg_isoch(void)
 #ifdef CONFIG_CRASH_DUMP
 
 /*
+ * Copy memory from a physically-addressed area into a virtually-addressed area
+ */
+int __iommu_load_from_oldmem(void *to, unsigned long from, unsigned long size)
+{
+   unsigned long pfn;  /* Page Frame Number */
+   size_t csize = (size_t)size;/* Num(bytes to copy) */
+   unsigned long offset;   /* Lower 12 bits of to */
+   void __iomem *virt_mem;
+   struct iommu_remapped_entry *mapped;
+
+   pfn = from >> VTD_PAGE_SHIFT;
+   offset = from & (~VTD_PAGE_MASK);
+
+   if (page_is_ram(pfn)) {
+   memcpy(to, pfn_to_kaddr(pfn) + offset, csize);
+   } else{
+
+   mapped = kzalloc(sizeof(struct iommu_remapped_entry),
+   GFP_KERNEL);
+   if (!mapped)
+   return -ENOMEM;
+
+   virt_mem = ioremap_cache((unsigned long)from, size);
+   if (!virt_mem) {
+   kfree(mapped);
+   return -ENOMEM;
+   }
+   memcpy(to, virt_mem, size);
+
+   mutex_lock(&__iommu_mem_list_lock);
+   mapped->mem = virt_mem;
+   list_add_tail(&mapped->list, &__iommu_remapped_mem);
+   mutex_unlock(&__iommu_mem_list_lock);
+   }
+   return size;
+}
+
+/*
+ * Copy memory from a virtually-addressed area into a physically-addressed area
+ */
+int __iommu_save_to_oldmem(unsigned long to, void *from, unsigned long size)
+{
+   unsigned long pfn;  /* Page Frame Number */
+   size_t csize = (size_t)size;/* Num(bytes to copy) */
+   unsigned long offset;   /* Lower 12 bits of to */
+   void __iomem *virt_mem;
+   struct iommu_remapped_entry *mapped;
+
+   pfn = to >> VTD_PAGE_SHIFT;
+   offset = to & (~VTD_PAGE_MASK);
+
+   if (page_is_ram(pfn)) {
+   memcpy(pfn_to_kaddr(pfn) + offset, from, csize);
+   } else{
+   mapped = kzalloc(sizeof(struct iommu_remapped_entry),
+   GFP_KERNEL);
+   if (!mapped)
+   return -ENOMEM;
+
+   virt_mem = ioremap_cache((unsigned long)to, size);
+   if (!virt_mem) {
+   kfree(mapped);
+   return -ENOMEM;
+   }
+   memcpy(virt_mem, from, size);
+   mutex_lock(&__iommu_mem_list_lock);
+   mapped->mem = virt_mem;
+   list_add_tail(&mapped->list, &__iommu_remapped_mem);
+   mutex_unlock(&__iommu_mem_list_lock);
+   }
+   return size;
+}
+
+/*
+ * Free the mapped memory for ioremap;
+ */
+int __iommu_free_mapped_mem(void)
+{
+   struct iommu_remapped_entry *mem_entry, *tmp;
+
+   mutex_lock(&__iommu_mem_list_lock);
+   list_for_each_entry_safe(mem_entry, tmp, &__iommu_remapped_mem, list) {
+   iounmap(mem_entry->mem);
+   list_del(&mem_entry->list);
+   kfree(mem_entry);
+   }
+   mutex_unlock(&__iommu_mem_list_lock);
+   return 0;
+}
+/*
  * Interfaces for when a new domain in the crashdump kernel needs some
  * values from the panicked kernel's context entries
  *
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index a65208a..8ffa523 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -368,4 +369,12 @@ extern int dmar_ir_support(void);
 
 extern const struct attribute_group *intel_iommu_groups[];
 
+#ifdef CONFIG_CRASH_DUMP
+extern int __iommu_load_from_oldmem(void *to, unsigned long from,
+   unsigned long size);
+extern int __iommu_save_to_oldmem(unsigned long to, void *from,
+  

[PATCH 01/10] iommu/vt-d: Update iommu_attach_domain() and its callers

[Li, Zhen-Hua]

Allow specification of the domain-id for the new domain.
This patch only adds the 'did' parameter to iommu_attach_domain()
and modifies all of its callers to specify the default value of -1
which says "no did specified, allocate a new one".

This is no functional change from current behaviour -- just enables
a functional change to be made in a later patch.

Bill Sumner:
Original version.

Li, Zhenhua:
Minor change, add change to function __iommu_attach_domain.

Signed-off-by: Bill Sumner 
Signed-off-by: Li, Zhen-Hua 
---
 drivers/iommu/intel-iommu.c | 34 --
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 1232336..2dc6250 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1534,31 +1534,36 @@ static struct dmar_domain *alloc_domain(int flags)
 }
 
 static int __iommu_attach_domain(struct dmar_domain *domain,
-struct intel_iommu *iommu)
+struct intel_iommu *iommu,
+int domain_number)
 {
int num;
unsigned long ndomains;
 
ndomains = cap_ndoms(iommu->cap);
-   num = find_first_zero_bit(iommu->domain_ids, ndomains);
-   if (num < ndomains) {
-   set_bit(num, iommu->domain_ids);
-   iommu->domains[num] = domain;
-   } else {
-   num = -ENOSPC;
-   }
+   if (domain_number < 0) {
+   num = find_first_zero_bit(iommu->domain_ids, ndomains);
+   if (num < ndomains) {
+   set_bit(num, iommu->domain_ids);
+   iommu->domains[num] = domain;
+   } else {
+   num = -ENOSPC;
+   }
+   } else
+   num = domain_number;
 
return num;
 }
 
 static int iommu_attach_domain(struct dmar_domain *domain,
-  struct intel_iommu *iommu)
+  struct intel_iommu *iommu,
+  int domain_number)
 {
int num;
unsigned long flags;
 
spin_lock_irqsave(&iommu->lock, flags);
-   num = __iommu_attach_domain(domain, iommu);
+   num = __iommu_attach_domain(domain, iommu, domain_number);
spin_unlock_irqrestore(&iommu->lock, flags);
if (num < 0)
pr_err("IOMMU: no free domain ids\n");
@@ -1577,7 +1582,7 @@ static int iommu_attach_vm_domain(struct dmar_domain 
*domain,
if (iommu->domains[num] == domain)
return num;
 
-   return __iommu_attach_domain(domain, iommu);
+   return __iommu_attach_domain(domain, iommu, -1);
 }
 
 static void iommu_detach_domain(struct dmar_domain *domain,
@@ -2231,6 +2236,7 @@ static struct dmar_domain *get_domain_for_dev(struct 
device *dev, int gaw)
u16 dma_alias;
unsigned long flags;
u8 bus, devfn;
+   int did = -1;   /* Default to "no domain_id supplied" */
 
domain = find_domain(dev);
if (domain)
@@ -2264,7 +2270,7 @@ static struct dmar_domain *get_domain_for_dev(struct 
device *dev, int gaw)
domain = alloc_domain(0);
if (!domain)
return NULL;
-   domain->id = iommu_attach_domain(domain, iommu);
+   domain->id = iommu_attach_domain(domain, iommu, did);
if (domain->id < 0) {
free_domain_mem(domain);
return NULL;
@@ -2442,7 +2448,7 @@ static int __init si_domain_init(int hw)
return -EFAULT;
 
for_each_active_iommu(iommu, drhd) {
-   ret = iommu_attach_domain(si_domain, iommu);
+   ret = iommu_attach_domain(si_domain, iommu, -1);
if (ret < 0) {
domain_exit(si_domain);
return -EFAULT;
@@ -3866,7 +3872,7 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
iommu_enable_translation(iommu);
 
if (si_domain) {
-   ret = iommu_attach_domain(si_domain, iommu);
+   ret = iommu_attach_domain(si_domain, iommu, -1);
if (ret < 0 || si_domain->id != ret)
goto disable_iommu;
domain_attach_iommu(si_domain, iommu);
-- 
2.0.0-rc0

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

[PATCH 05/10] iommu/vt-d: Add functions to load and save old re

[Li, Zhen-Hua]

Add functions to load root entry table from old kernel, and to save updated
root entry table.
Add two member in struct intel_iommu, to store the RTA in old kernel, and
the mapped virt address of it.

We use the old RTA in dump kernel, and when the iommu->root_entry is used as
a cache in kdump kernel, its phys address will not be save to RTA register,
but when its data is changed, we will save the new data to old root entry table.

Signed-off-by: Li, Zhen-Hua 
---
 drivers/iommu/intel-iommu.c | 49 +
 include/linux/intel-iommu.h |  5 +
 2 files changed, 54 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 8a7ad72..126294db 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -388,6 +388,10 @@ static int intel_iommu_get_dids_from_old_kernel(struct 
intel_iommu *iommu);
 
 static int device_to_domain_id(struct intel_iommu *iommu, u8 bus, u8 devfn);
 
+static void __iommu_load_old_root_entry(struct intel_iommu *iommu);
+
+static void __iommu_update_old_root_entry(struct intel_iommu *iommu, int 
index);
+
 struct iommu_remapped_entry {
struct list_head list;
void __iomem *mem;
@@ -4990,4 +4994,49 @@ static int device_to_domain_id(struct intel_iommu 
*iommu, u8 bus, u8 devfn)
return did;
 }
 
+/*
+ * Load the old root entry table to new root entry table.
+ */
+static void __iommu_load_old_root_entry(struct intel_iommu *iommu)
+{
+   if ((!iommu)
+   || (!iommu->root_entry)
+   || (!iommu->root_entry_old_virt)
+   || (!iommu->root_entry_old_phys))
+   return;
+   memcpy(iommu->root_entry, iommu->root_entry_old_virt, PAGE_SIZE);
+}
+
+/*
+ * When the data in new root entry table is changed, this function
+ * must be called to save the updated data to old root entry table.
+ */
+static void __iommu_update_old_root_entry(struct intel_iommu *iommu, int index)
+{
+   u8 start;
+   unsigned long size;
+   void __iomem *to;
+   void *from;
+
+   if ((!iommu)
+   || (!iommu->root_entry)
+   || (!iommu->root_entry_old_virt)
+   || (!iommu->root_entry_old_phys))
+   return;
+
+   if (index < -1 || index >= ROOT_ENTRY_NR)
+   return;
+
+   if (index == -1) {
+   start = 0;
+   size = ROOT_ENTRY_NR * sizeof(struct root_entry);
+   } else {
+   start = index * sizeof(struct root_entry);
+   size = sizeof(struct root_entry);
+   }
+   to = iommu->root_entry_old_virt;
+   from = iommu->root_entry;
+   memcpy(to + start, from + start, size);
+}
+
 #endif /* CONFIG_CRASH_DUMP */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 8ffa523..8e29b97 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -329,6 +329,11 @@ struct intel_iommu {
spinlock_t  lock; /* protect context, domain ids */
struct root_entry *root_entry; /* virtual address */
 
+#ifdef CONFIG_CRASH_DUMP
+   void __iomem *root_entry_old_virt; /* mapped from old root entry */
+   unsigned long root_entry_old_phys; /* root entry in old kernel */
+#endif
+
struct iommu_flush flush;
 #endif
struct q_inval  *qi;/* Queued invalidation info */
-- 
2.0.0-rc0

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

[PATCH 0/10] iommu/vt-d: Fix intel vt-d faults in kdump kernel

[Li, Zhen-Hua]

This patchset is an update of Bill Sumner's patchset, implements a fix for:
If a kernel boots with intel_iommu=on on a system that supports intel vt-d, 
when a panic happens, the kdump kernel will boot with these faults:

dmar: DRHD: handling fault status reg 102
dmar: DMAR:[DMA Read] Request device [01:00.0] fault addr fff8
DMAR:[fault reason 01] Present bit in root entry is clear

dmar: DRHD: handling fault status reg 2
dmar: INTR-REMAP: Request device [[61:00.0] fault index 42
INTR-REMAP:[fault reason 34] Present field in the IRTE entry is clear

On some system, the interrupt remapping fault will also happen even if the 
intel_iommu is not set to on, because the interrupt remapping will be enabled 
when x2apic is needed by the system.

The cause of the DMA fault is described in Bill's original version, and the 
INTR-Remap fault is caused by a similar reason. In short, the initialization 
of vt-d drivers causes the in-flight DMA and interrupt requests get wrong 
response.

To fix this problem, we modifies the behaviors of the intel vt-d in the 
crashdump kernel:

For DMA Remapping:
1. To accept the vt-d hardware in an active state,
2. Do not disable and re-enable the translation, keep it enabled.
3. Use the old root entry table, do not rewrite the RTA register.
4. Malloc and use new context entry table and page table, copy data from the 
   old ones that used by the old kernel.
5. to use different portions of the iova address ranges for the device drivers
   in the crashdump kernel than the iova ranges that were in-use at the time
   of the panic.  
6. After device driver is loaded, when it issues the first dma_map command, 
   free the dmar_domain structure for this device, and generate a new one, so 
   that the device can be assigned a new and empty page table. 
7. When a new context entry table is generated, we also save its address to 
   the old root entry table.

For Interrupt Remapping:
1. To accept the vt-d hardware in an active state,
2. Do not disable and re-enable the interrupt remapping, keep it enabled.
3. Use the old interrupt remapping table, do not rewrite the IRTA register.
4. When ioapic entry is setup, the interrupt remapping table is changed, and 
   the updated data will be stored to the old interrupt remapping table.

Advantages of this approach:
1. All manipulation of the IO-device is done by the Linux device-driver
   for that device.
2. This approach behaves in a manner very similar to operation without an
   active iommu.
3. Any activity between the IO-device and its RMRR areas is handled by the
   device-driver in the same manner as during a non-kdump boot.
4. If an IO-device has no driver in the kdump kernel, it is simply left alone.
   This supports the practice of creating a special kdump kernel without
   drivers for any devices that are not required for taking a crashdump. 
5. Minimal code-changes among the existing mainline intel vt-d code.

Summary of changes in this patch set:
1. Added some useful function for root entry table in code intel-iommu.c
2. Added new members to struct root_entry and struct irte;
3. Functions to load old root entry table to iommu->root_entry from the memory 
   of old kernel.
4. Functions to malloc new context entry table and page table and copy the data
   from the old ones to the malloced new ones.
5. Functions to enable support for DMA remapping in kdump kernel.
6. Functions to load old irte data from the old kernel to the kdump kernel.
7. Some code changes that support other behaviours that have been listed.
8. In the new functions, use physical address as "unsigned long" type, not 
   pointers.

Original version by Bill Sumner:
https://lkml.org/lkml/2014/1/10/518
https://lkml.org/lkml/2014/4/15/716
https://lkml.org/lkml/2014/4/24/836

Zhenhua's last of Bill's patchset:
https://lkml.org/lkml/2014/10/21/134

Changed in this version:
1. Do not disable and re-enable traslation and interrupt remapping. 
2. Use old root entry table.
3. Use old interrupt remapping table.

This patchset should be applied with this one together:
https://lkml.org/lkml/2014/11/5/43
x86/iommu: fix incorrect bit operations in setting values

Bill Sumner (5):
  iommu/vt-d: Update iommu_attach_domain() and its callers
  iommu/vt-d: Items required for kdump
  iommu/vt-d: data types and functions used for kdump
  iommu/vt-d: Add domain-id functions
  iommu/vt-d: enable kdump support in iommu module

Li, Zhen-Hua (10):
  iommu/vt-d: Update iommu_attach_domain() and its callers
  iommu/vt-d: Items required for kdump
  iommu/vt-d: Add domain-id functions
  iommu/vt-d: functions to copy data from old mem
  iommu/vt-d: Add functions to load and save old re
  iommu/vt-d: datatypes and functions used for kdump
  iommu/vt-d: enable kdump support in iommu module
  iommu/vtd: assign new page table for dma_map
  iommu/vt-d: Copy functions for irte
  iommu/vt-d: Use old irte in kdump kernel

 drivers/iommu/i

[PATCH 02/10] iommu/vt-d: Items required for kdump

[Li, Zhen-Hua]

Add structure type domain_values_entry used for kdump;
Add context entry functions needed for kdump.

Bill Sumner:
Original version;

Li, Zhenhua:
Changed the name of new functions, make them consistent with current
context get/set functions.

Signed-off-by: Bill Sumner 
Signed-off-by: Li, Zhen-Hua 
---
 drivers/iommu/intel-iommu.c | 70 +
 1 file changed, 70 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 2dc6250..5ce2850 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -208,6 +209,12 @@ get_context_addr_from_root(struct root_entry *root)
NULL);
 }
 
+static inline unsigned long
+get_context_phys_from_root(struct root_entry *root)
+{
+   return  root_present(root) ? (root->val & VTD_PAGE_MASK) : 0;
+}
+
 /*
  * low 64 bits:
  * 0: present
@@ -228,6 +235,32 @@ static inline bool context_present(struct context_entry 
*context)
 {
return (context->lo & 1);
 }
+
+static inline int context_fault_enable(struct context_entry *c)
+{
+   return((c->lo >> 1) & 0x1);
+}
+
+static inline int context_translation_type(struct context_entry *c)
+{
+   return((c->lo >> 2) & 0x3);
+}
+
+static inline u64 context_address_root(struct context_entry *c)
+{
+   return((c->lo >> VTD_PAGE_SHIFT));
+}
+
+static inline int context_address_width(struct context_entry *c)
+{
+   return((c->hi >> 0) & 0x7);
+}
+
+static inline int context_domain_id(struct context_entry *c)
+{
+   return((c->hi >> 8) & 0x);
+}
+
 static inline void context_set_present(struct context_entry *context)
 {
context->lo |= 1;
@@ -313,6 +346,43 @@ static inline int first_pte_in_page(struct dma_pte *pte)
return !((unsigned long)pte & ~VTD_PAGE_MASK);
 }
 
+
+#ifdef CONFIG_CRASH_DUMP
+
+/*
+ * Fix Crashdump failure caused by leftover DMA through a hardware IOMMU
+ *
+ * Fixes the crashdump kernel to deal with an active iommu and legacy
+ * DMA from the (old) panicked kernel in a manner similar to how legacy
+ * DMA is handled when no hardware iommu was in use by the old kernel --
+ * allow the legacy DMA to continue into its current buffers.
+ *
+ * In the crashdump kernel, this code:
+ * 1. skips disabling the IOMMU's translating of IO Virtual Addresses (IOVA).
+ * 2. Do not re-enable IOMMU's translating.
+ * 3. In kdump kernel, use the old root entry table.
+ * 4. Leaves the current translations in-place so that legacy DMA will
+ *continue to use its current buffers.
+ * 5. Allocates to the device drivers in the crashdump kernel
+ *portions of the iova address ranges that are different
+ *from the iova address ranges that were being used by the old kernel
+ *at the time of the panic.
+ *
+ */
+
+struct domain_values_entry {
+   struct list_head link;  /* link entries into a list */
+   struct iova_domain iovad;   /* iova's that belong to this domain */
+   struct dma_pte  *pgd;   /* virtual address */
+   intdid; /* domain id */
+   intgaw; /* max guest address width */
+   intiommu_superpage; /* Level of superpages supported:
+  0 == 4KiB (no superpages), 1 == 2MiB,
+  2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
+};
+
+#endif /* CONFIG_CRASH_DUMP */
+
 /*
  * This domain is a statically identity mapping domain.
  * 1. This domain creats a static 1:1 mapping to all usable memory.
-- 
2.0.0-rc0

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

Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

[Thierry Reding]

On Sun, Dec 14, 2014 at 02:45:36PM +0200, Laurent Pinchart wrote:
> Hi Marek,
> 
> Thank you for the patch.
> 
> On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
> > This patch introduces IOMMU_OF_DECLARE-based initialization to the
> > driver, which replaces subsys_initcall-based procedure.
> > exynos_iommu_of_setup ensures that each sysmmu controller is probed
> > before its master device.
> > 
> > Signed-off-by: Marek Szyprowski 
> > ---
> >  drivers/iommu/exynos-iommu.c | 28 +++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > index cd28dc09db39..88f9afe641a0 100644
> > --- a/drivers/iommu/exynos-iommu.c
> > +++ b/drivers/iommu/exynos-iommu.c
> 
> [snip]
> 
> > @@ -1125,4 +1134,21 @@ err_reg_driver:
> > kmem_cache_destroy(lv2table_kmem_cache);
> > return ret;
> >  }
> > -subsys_initcall(exynos_iommu_init);
> > +
> > +static int __init exynos_iommu_of_setup(struct device_node *np)
> > +{
> > +   struct platform_device *pdev;
> > +
> > +   if (!init_done)
> > +   exynos_iommu_init();
> > +
> > +   pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> > +   if (IS_ERR(pdev))
> > +   return PTR_ERR(pdev);
> 
> If we end up having to create the IOMMU platform devices from within the 
> drivers, the introduction of IOMMU_OF_DECLARE starts to feel like a 
> workaround 
> to me. I wonder whether it wouldn't then be better to let the driver core 
> instantiate the IOMMU platform device from DT as for all other devices, and 
> use device notifiers to defer probe of the bus masters until the required 
> IOMMU(s) are registered.

Notifiers don't work very well for this. Notifier blocks are supposed to
return a very limited number of values, so sneaking in a -EPROBE_DEFER
isn't likely to work out very well.

This was in fact one of Hiroshi's proposals over a year ago and got
refused because of those reasons. The next solution was to introduce a
function, not very much unlike the of_iommu_configure() that would be
called in the core prior to calling into the driver's ->probe() callback
so that it could handle this at probe time (as opposed to device
creation time). That way the core can easily defer probe if the IOMMU is
not there yet. At the same time it can simply use the driver model
without requiring per-architecture hacks or workarounds.

Note that there is really no need for any of this configuration or
initialization to happen at device creation time. Drivers won't be able
to use the IOMMU or DMA APIs until their .probe(), so handling this any
earlier is completely unnecessary.

Thierry


pgpRaNlupPFLR.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/4] iommu: introduce generic page table allocation framework

[Will Deacon]

On Sun, Dec 14, 2014 at 11:46:30PM +, Laurent Pinchart wrote:
> > +struct io_pgtable_cfg {
> > +   int quirks; /* IO_PGTABLE_QUIRK_* */
> > +   unsigned long   pgsize_bitmap;
> > +   unsigned intias;
> > +   unsigned intoas;
> > +   struct iommu_gather_ops *tlb;
> 
> Could you make this pointer const ?

Sure. I'll post a v2 at -rc1 assuming I'm not fishing/drinking (I'm off
work from Wednesday this week).

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

Re: [PATCH 0/5] iommu/vt-d: Fix crash dump failure caused by legacy DMA/IO

[Baoquan He]

On 12/12/14 at 10:25am, Li, ZhenHua wrote:
> Sorry I have no plan yet.
> Could you send me your logs on your AMD system?

Sure, please check the attachment. AMD iommu seems a little different on
action. On the machine I reserved for testing, it always hang the system
bootup. As Joerg said, we can just know this issue, vt-d is still the
first thing.

[root@hp-sl4545g7-01 ~]# echo c >/proc/sysrq-trigger 
[ 4178.349174] SysRq : Trigger a crash
[ 4178.367887] BUG: unable to handle kernel NULL pointer dereference at 
  (null)
[ 4178.413933] IP: [] sysrq_handle_crash+0x16/0x20
[ 4178.449637] PGD 8219eb067 PUD 824b4f067 PMD 0 
[ 4178.475360] Oops: 0002 [#1] SMP 
[ 4178.494112] Modules linked in: cfg80211 sg rfkill kvm_amd kvm nfsd igb 
crct10dif_pclmul crc32_pclmul ptp crc32c_intel ghash_clmulni_intel auth_rpcgss 
aesni_intel pps_core dca ipmi_si hpwdt sp5100_tco lrw nfs_acl gf128mul hpilo 
pcspkr ipmi_msghandler serio_raw fam15h_power amd64_edac_mod glue_helper 
i2c_piix4 edac_mce_amd k10temp lockd edac_core ablk_helper cryptd shpchp sunrpc 
xfs libcrc32c radeon i2c_algo_bit drm_kms_helper ttm sd_mod ata_generic 
crc_t10dif pata_acpi ahci drm crct10dif_common libahci pata_atiixp libata hpsa 
i2c_core dm_mirror dm_region_hash dm_log dm_mod
[ 4178.785689] CPU: 1 PID: 1872 Comm: bash Not tainted 3.14.0+ #44
[ 4178.819575] Hardware name: HP ProLiant SL4545 G7/, BIOS A31 12/08/2012
[ 4178.856427] task: 88081ab9b680 ti: 88082036 task.ti: 
88082036
[ 4178.899369] RIP: 0010:[]  [] 
sysrq_handle_crash+0x16/0x20
[ 4178.947941] RSP: 0018:880820361e80  EFLAGS: 00010046
[ 4178.977106] RAX: 000f RBX: 81a0b6c0 RCX: 
[ 4179.018848] RDX:  RSI: 88083ea0e708 RDI: 0063
[ 4179.059550] RBP: 880820361e80 R08: 0092 R09: 051b
[ 4179.099603] R10: 051a R11: 0003 R12: 0063
[ 4179.140142] R13: 0246 R14: 0007 R15: 
[ 4179.179475] FS:  7f4edd746740() GS:88083ea0() 
knlGS:
[ 4179.225437] CS:  0010 DS:  ES:  CR0: 80050033
[ 4179.256725] CR2:  CR3: 000821b75000 CR4: 000406e0
[ 4179.297414] Stack:
[ 4179.308744]  880820361eb8 813a8882 0002 
7f4edd744000
[ 4179.349142]  880820361f48 0002  
880820361ed0
[ 4179.390914]  813a8d8f 88082405fa40 880820361ef0 
81236d5d
[ 4179.440766] Call Trace:
[ 4179.453918]  [] __handle_sysrq+0xa2/0x170
[ 4179.485923]  [] write_sysrq_trigger+0x2f/0x40
[ 4179.518602]  [] proc_reg_write+0x3d/0x80
[ 4179.548870]  [] vfs_write+0xba/0x1e0
[ 4179.578130]  [] SyS_write+0x55/0xd0
[ 4179.606615]  [] system_call_fastpath+0x16/0x1b
[ 4179.640318] Code: 65 34 75 e5 4c 89 ef e8 d9 f7 ff ff eb db 0f 1f 80 00 00 
00 00 66 66 66 66 90 55 c7 05 a0 d5 5b 00 01 00 00 00 48 89 e5 0f ae f8  04 
25 00 00 00 00 01 5d c3 66 66 66 66 90 55 31 c0 c7 05 1e 
[ 4179.748149] RIP  [] sysrq_handle_crash+0x16/0x20
[ 4179.783683]  RSP 
[ 4179.802762] CR2: 
[0.00] Initializing cgroup subsys cpuset
[0.00] Initializing cgroup subsys cpu
[0.00] Initializing cgroup subsys cpuacct
[0.00] Linux version 3.14.0+ (b...@ibm-x3755m3-02.lab.bos.redhat.com) 
(gcc version 4.8.2 20140120 (Red Hat 4.8.2-13) (GCC) ) #44 SMP Thu Apr 10 
22:28:02 EDT 2014
[0.00] Command line: BOOT_IMAGE=/vmlinuz-3.14.0+ 
root=/dev/mapper/rhel_hp--sl4545g7--01-root ro console=ttyS1,115200n81 
rd.lvm.lv=rhel_hp-sl4545g7-01/root rd.lvm.lv=rhel_hp-sl4545g7-01/swap 
vconsole.font=latarcyrheb-sun16 vconsole.keymap=us LANG=en_US.UTF-8 irqpoll 
nr_cpus=1 reset_devices cgroup_disable=memory mce=off numa=off 
udev.children-max=2 panic=10 rootflags=nofail acpi_no_memhotplug 
disable_cpu_apicid=0 memmap=exactmap memmap=592K@4K memmap=523672K@311296K 
elfcorehdr=834968K memmap=192K#3141752K
[0.00] e820: BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x0100-0x00094fff] usable
[0.00] BIOS-e820: [mem 0x00095000-0x00095bff] reserved
[0.00] BIOS-e820: [mem 0x00098000-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000f-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0xbfc1dfff] usable
[ [mem 0xbfc1e000-0xbfc4dfff] ACPI data
[0.00] BIOS-e820: [mem 0xbfc4e000-0xbfc4efff] usable
[0.00] BIOS-e820: [mem 0xbfc4f000-0xcfff] reserved
[0.00] BIOS-e820: [mem 0xfec0-0xfee0] reserved
[0.00] BIOS-e820: [mem 0xff80-0x] reserved
[0.00] BIOS-e820: [mem 0x0001-0x00083effefff] usable
[0.00] e820: last_pfn = 0x83efff max_arch_pfn = 0x4
[0.00] NX (Execute

Re: [PATCH 0/5] iommu/vt-d: Fix crash dump failure caused by legacy DMA/IO

[Baoquan He]

On 12/12/14 at 05:11pm, Joerg Roedel wrote:
> On Fri, Dec 12, 2014 at 10:25:31AM +0800, Li, ZhenHua wrote:
> > Sorry I have no plan yet.
> > Could you send me your logs on your AMD system?
> 
> > On 12/10/2014 04:46 PM, Baoquan He wrote:
> > >This issue happens on AMD iommu too, do you have any plans or
> > >thoughts on that?
> 
> I think the best approach for now is to get a prove-of-concept on the
> VT-d driver. If it works there the way we expect, we can implement the
> same handling in the AMD driver. But I see no reason to hold back the
> VT-d patches until it is also fixed for AMD systems.

Yes, I agree with you. Just raise this issue to upstream. 

Thanks, Joerg.

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

Re: [RFC PATCH 2/2] iommu: rockchip: Handle system-wide and runtime PM

[Geert Uytterhoeven]

On Fri, Dec 12, 2014 at 9:04 PM, Kevin Hilman  wrote:
> An even more exciting problem exists when a CPU is in the same domain as
> other peripherals, those peripherals are all idle and the power domain
> is gated. :)

We do have pm_genpd_attach_cpuidle() and pm_genpd_name_attach_cpuidle()?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu