[PATCH v1 1/1] iommu/amd: set iommu for early mapped ioapic/hpet
From: Su Friendy friendy...@sony.com.cn
The early mapped ioapic/hpet specified by kernel boot parameter
ivrs_ioapic[ID]/ivrs_hpet[ID] always override the ioapic/hpet with
same ID reported by ACPI IVRS table. Therefore, the early mapped
should be always controlled by iommu.
Current driver did not set iommu for the early mapped. This causes
ivrs_ioapic[ID]/ivrs_hpet[ID] actually are not controlled by iommu.
This patch fixes this problem by setting iommu for early mapped.
Signed-off-by: Su Friendy friendy...@sony.com.cn
Signed-off-by: Saeki Shusuke shusuke.sa...@jp.sony.com
Signed-off-by: Tamori Masahiro masahiro.tam...@jp.sony.com
---
drivers/iommu/amd_iommu_init.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 3783e0b..148ab61 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -747,7 +747,7 @@ static int __init add_special_device(u8 type, u8 id, u16
devid, bool cmd_line)
return 0;
}
-static int __init add_early_maps(void)
+static int __init add_early_maps(struct amd_iommu *iommu)
{
int i, ret;
@@ -758,6 +758,11 @@ static int __init add_early_maps(void)
early_ioapic_map[i].cmd_line);
if (ret)
return ret;
+ /*
+* early mapped ioapci overrides ACPI IVRS,
+* they should be always controlled by iommu.
+*/
+ set_iommu_for_device(iommu, early_ioapic_map[i].devid);
}
for (i = 0; i early_hpet_map_size; ++i) {
@@ -767,6 +772,11 @@ static int __init add_early_maps(void)
early_hpet_map[i].cmd_line);
if (ret)
return ret;
+ /*
+* early mapped hpet overrides ACPI IVRS,
+* they should be always controlled by iommu.
+*/
+ set_iommu_for_device(iommu, early_hpet_map[i].devid);
}
return 0;
@@ -811,7 +821,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu
*iommu,
int ret;
- ret = add_early_maps();
+ ret = add_early_maps(iommu);
if (ret)
return ret;
--
1.8.2.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 10/29] drivers: add DRIVER_HAS_OWN_IOMMU_MANAGER flag
Hello,
On 2014-09-01 11:38, Arnd Bergmann wrote:
On Monday 01 September 2014 09:53:29 Marek Szyprowski wrote:
On 2014-09-01 09:07, Thierry Reding wrote:
On Mon, Sep 01, 2014 at 07:22:32AM +0200, Marek Szyprowski wrote:
Hi Greg,
On 2014-08-05 12:47, Marek Szyprowski wrote:
This patch adds a new flags for device drivers. This flag instructs
kernel that the device driver does it own management of IOMMU assisted
IO address space translations, so no default dma-mapping structures
should be initialized.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
include/linux/device.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index 5f4ff02..2e62371 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -253,6 +253,8 @@ struct device_driver {
/* disables bind/unbind via sysfs */
#define DRIVER_SUPPRESS_BIND_ATTRS (1 0)
+/* driver uses own methods to manage IO address space */
+#define DRIVER_HAS_OWN_IOMMU_MANAGER (1 1)
extern int __must_check driver_register(struct device_driver *drv);
extern void driver_unregister(struct device_driver *drv);
Could you comment if the approach of using flags in the struct driver
could be accepted? I've converted suppress_bind_attrs entry to flags to
avoid extending the structure, please see patch [PATCH 05/29] drivers:
convert suppress_bind_attrs parameter into flags.
Is this really necessary? What I did as part of an RFC series for Tegra
IOMMU support is keep this knowledge within the IOMMU driver rather than
export it to the driver core.i
The problem with embedding the list of drivers that you would need to update
it everytime when you modify or extend iommu support in the other drivers.
I've tried also other approach, like adding respective notifiers to
individual
drivers to initialize custom iommu support (or disable default iommu
mapping)
before iommu driver gets initialized, but such solution is in my opinion too
complex and hard to understand if one is not familiar will all this stuff.
All in all it turned out that the simplest and most generic way is to simply
add the flag to the driver core. Flags might be also used in the future
to model other kinds of dependencies between device drivers and/or driver
core.
I don't get it yet. I would expect that a driver doing its own management
of the iommu gets to use the linux/iommu.h interfaces, while a driver
using the default iommu setup uses linux/dma-mapping.h.
You are right.
Who do you think needs to set this flag, and who needs to read it?
In the proposed solution Exynos IOMMU driver creates a separate IO
address space
for every client device in a system and binds it to the default
dma-mapping space
for the given device. When drivers are doing its own management of IO
address
space, instead of relying on what is available by default with dma-mapping
interface, this will require releasing of the previously created default
structures and resources. So this flag is set by the driver doing its own
management of io address space. The flags is then checked by Exynos
IOMMU driver
to avoid creating the default dma-mapping address space for devices
which driver
does its own management.
Best regards
--
Marek Szyprowski, PhD
Samsung RD Institute Poland
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 10/29] drivers: add DRIVER_HAS_OWN_IOMMU_MANAGER flag
Hello, On 2014-09-01 13:56, Arnd Bergmann wrote: On Monday 01 September 2014 12:47:08 Marek Szyprowski wrote: Who do you think needs to set this flag, and who needs to read it? In the proposed solution Exynos IOMMU driver creates a separate IO address space for every client device in a system and binds it to the default dma-mapping space for the given device. When drivers are doing its own management of IO address space, instead of relying on what is available by default with dma-mapping interface, this will require releasing of the previously created default structures and resources. So this flag is set by the driver doing its own management of io address space. The flags is then checked by Exynos IOMMU driver to avoid creating the default dma-mapping address space for devices which driver does its own management. I don't completely understand it yet. I would assume the device to be added to the default domain at device creation time (of_platform_populate), way before we know which device driver is going to be used. How can this prevent the iommu driver from doing the association with the domain? of_platform_populate() is too early to do the association, because that time the exynos iommu driver is even not yet probed. The association with default dma-mapping domain is done in IOMMU_GROUP_NOTIFY_BIND_DRIVER notifier, just before binding the driver to the given device. This way iommu driver can check dev-driver-flags and skip creating default dma-mapping domain if driver announces that it wants to handle it by itself. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Linaro-mm-sig] [PATCH 10/29] drivers: add DRIVER_HAS_OWN_IOMMU_MANAGER flag
On Monday 01 September 2014 14:07:34 Marek Szyprowski wrote: On 2014-09-01 13:56, Arnd Bergmann wrote: On Monday 01 September 2014 12:47:08 Marek Szyprowski wrote: Who do you think needs to set this flag, and who needs to read it? In the proposed solution Exynos IOMMU driver creates a separate IO address space for every client device in a system and binds it to the default dma-mapping space for the given device. When drivers are doing its own management of IO address space, instead of relying on what is available by default with dma-mapping interface, this will require releasing of the previously created default structures and resources. So this flag is set by the driver doing its own management of io address space. The flags is then checked by Exynos IOMMU driver to avoid creating the default dma-mapping address space for devices which driver does its own management. I don't completely understand it yet. I would assume the device to be added to the default domain at device creation time (of_platform_populate), way before we know which device driver is going to be used. How can this prevent the iommu driver from doing the association with the domain? of_platform_populate() is too early to do the association, because that time the exynos iommu driver is even not yet probed. Please have a look at the Introduce automatic DMA configuration for IOMMU masters series that Will Deacon sent out the other day. The idea is that we are changing the probe order so that the iommu gets initialized early enough for all IOMMU association to be done there. The association with default dma-mapping domain is done in IOMMU_GROUP_NOTIFY_BIND_DRIVER notifier, just before binding the driver to the given device. This way iommu driver can check dev-driver-flags and skip creating default dma-mapping domain if driver announces that it wants to handle it by itself. I really want to kill off those notifiers. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu: Allow size of stage 1 output to max possible value for sateg 2 bypass
Hi Will,
On Mon, Sep 1, 2014 at 4:42 AM, Will Deacon will.dea...@arm.com wrote:
Hi Tirumalesh,
On Wed, Aug 27, 2014 at 07:02:21PM +0100, c.tirumal...@gmail.com wrote:
From: Tirumalesh Chalamarla tchalama...@cavium.com
This patch modifes output_mask calculation logic for stage 1 and allow
max possible value supported by SMMU implementaions for translations,
where stage 2 is bypassed.
Erlier it is not possible to access full supported PA address with stage 1,
even if it is supported by SMMU and stage 2 is bypass.
I'm trying to understand what you're getting at here. Essentially, you want
to use the full stage-1 output range for a stage-1 only MMU, right?
YES, restrict stage 1 out put to VA_BITS only if stage 2 is needed
(i.e for NESTED_TRANSLATIONS).
The code is currently structured to truncate that to the stage-2 input size
for nested translation. However, I think that's better solved by faking the
ID registers in the virtual SMMU instead of posing these restrictions on the
host as well.
This sounds good, the only problem is, it is too much bound to virtual
SMMU. There is no harm in changing/checking the
stage 1 output mask, if stage 2 present, in host also.
Assuming I understand the problem correctly, why not simply remove the
truncation from the existing code (untested patch below)? Does that not
work for you?
This will not restrict stage 1 out put to VA_BITS, even for two level
translations. this results in non debuggable problems
if we configure incorrectly. there is no harm in checking the
condition for nested translations, as i did in my patch.
Regards,
Tirumalesh.
Will
---8
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 2b1271658bfa..a02d05793a73 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1917,21 +1917,7 @@ static int arm_smmu_device_cfg_probe(struct
arm_smmu_device *smmu)
/* ID2 */
id = readl_relaxed(gr0_base + ARM_SMMU_GR0_ID2);
size = arm_smmu_id_size_to_bits((id ID2_IAS_SHIFT) ID2_IAS_MASK);
-
- /*
-* Stage-1 output limited by stage-2 input size due to pgd
-* allocation (PTRS_PER_PGD).
-*/
- if (smmu-features ARM_SMMU_FEAT_TRANS_NESTED) {
-#ifdef CONFIG_64BIT
- smmu-s1_output_size = min_t(unsigned long, VA_BITS, size);
-#else
- smmu-s1_output_size = min(32UL, size);
-#endif
- } else {
- smmu-s1_output_size = min_t(unsigned long, PHYS_MASK_SHIFT,
-size);
- }
+ smmu-s1_output_size = min_t(unsigned long, PHYS_MASK_SHIFT, size);
/* The stage-2 output mask is also applied for bypass */
size = arm_smmu_id_size_to_bits((id ID2_OAS_SHIFT) ID2_OAS_MASK);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/7] dma-mapping: replace set_arch_dma_coherent_ops with arch_setup_dma_ops
On Friday 29 August 2014 16:54:25 Will Deacon wrote:
set_arch_dma_coherent_ops is called from of_dma_configure in order to
swizzle the architectural dma-mapping functions over to a cache-coherent
implementation. This is currently implemented only for ARM.
In anticipation of re-using this mechanism for IOMMU-backed dma-mapping
ops too, this patch replaces the function with a broader
arch_setup_dma_ops callback which is also responsible for setting the
DMA mask and offset as well as selecting the correct mapping functions.
A further advantage of this split is that it nicely isolates the
of-specific code from the dma-mapping code, allowing potential reuse by
other buses (e.g. PCI) in the future.
Signed-off-by: Will Deacon will.dea...@arm.com
Looks good, just one tiny comment
---
arch/arm/include/asm/dma-mapping.h | 20 ++
drivers/of/platform.c | 42
++
include/linux/dma-mapping.h| 8 +++-
3 files changed, 30 insertions(+), 40 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index c45b61a4b4a5..936125ef3f3f 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -121,12 +121,24 @@ static inline unsigned long dma_max_pfn(struct device
*dev)
}
#define dma_max_pfn(dev) dma_max_pfn(dev)
-static inline int set_arch_dma_coherent_ops(struct device *dev)
+static inline void arch_setup_dma_ops(struct device *dev, u64 mask,
+ unsigned long offset, bool coherent)
{
- set_dma_ops(dev, arm_coherent_dma_ops);
- return 0;
+ dev-coherent_dma_mask = mask;
+
+ /*
+* Set dma_mask to coherent_dma_mask by default if the architecture
+* code has not set it.
+*/
+ if (!dev-dma_mask)
+ dev-dma_mask = dev-coherent_dma_mask;
+
We are in architecture specific code here, and we know that this
architecture has not set up the dev-dma_mask pointer, so we can
remove the 'if (!dev-dma_mask)'
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/7] iommu: provide early initialisation hook for IOMMU drivers
On Monday 01 September 2014 09:52:19 Thierry Reding wrote: I don't think this is the right direction. We've been preaching that using initcall ordering is a bad thing and people should be using deferred probe instead. Now we have a bunch of these OF tables that do the exact opposite. Note that these are nothing more than a variant of initcalls that get called at platform-specific points in time. There are also ongoing discussions about how to change the device probe order to use dependencies (such as phandles from device tree) to prevent excessive deferred probing. With that in place this would be solved in a much more generic way. We are not creating an ABI here, so it can always be changed later. For now, I think iommus are clearly in the same category as irqchips, timers, clock controllers and smp operations: we really want them to be available before we set up any other devices. I don't mind finding a more generic solution in the long run, but for now, this gets the problem out of the way, and I can't think of any realistic corner cases that would prevent it from working. Do you think that an IOMMU driver may have a dependency on another device driver that is probed later? Do you have an example of such hardware? Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/7] iommu/arm-smmu: fixes for 3.17
Hi Will,
On Fri, Aug 29, 2014 at 03:47:38PM +0100, Will Deacon wrote:
Hans Wennborg (1):
iommu/arm-smmu: fix decimal printf format specifiers prefixed with 0x
Mitchel Humpherys (1):
iommu/arm-smmu: avoid calling request_irq in atomic context
Olav Haugan (2):
iommu/arm-smmu: Do not access non-existing S2CR registers
iommu/arm-smmu: fix programming of SMMU_CBn_TCR for stage 1
Vladimir Murzin (1):
iommu/arm-smmu: remove pgtable_page_{c,d}tor()
Will Deacon (2):
iommu/arm-smmu: fix s2cr and smr teardown on device detach from domain
iommu/arm-smmu: fix corner cases in address size calculations
drivers/iommu/arm-smmu.c | 127
+++
1 file changed, 73 insertions(+), 54 deletions(-)
Please don't forget to add stable-tags to these, if necessary.
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 3/7] iommu: add new iommu_ops callback for adding a device with a set of IDs
On Monday 01 September 2014 10:13:22 Thierry Reding wrote:
On Fri, Aug 29, 2014 at 04:54:26PM +0100, Will Deacon wrote:
This patch adds a new function to the iommu_ops structure to allow a
device to be added to a specific IOMMU instance along with a set of
opaque IDs that are used internally by the IOMMU for identifying and
configuring translations.
Signed-off-by: Will Deacon will.dea...@arm.com
---
include/linux/iommu.h | 2 ++
1 file changed, 2 insertions(+)
I don't really see the point of this new callback. As I understand it,
the strength of the current .add_device() callback is that it uses only
the struct device and figures out which exact IOMMU to associate it with
in case there are multiple IOMMUs.
Although that doesn't seem to be the general case either. That's really
been one of the difficulties in dealing with IOMMU. Every driver seems
to do things very differently and the IOMMU subsystem itself is fairly
different from other subsystems too.
The problem with add_device is that it assumes that each bus_type only
has one IOMMU instance, and that we know which one that is. Current
implementations do more or less nasty hacks to work around this where
necessary, and we should try to remove those hacks rather than adding
more of them into future drivers.
A smaller issue with the add_device callback is the point at which it
gets called: the caller is in the bus_notifier when the device gets
added, but at a time when we don't have access to the bus-specific
information.
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 20f9a527922a..3dd1b99c4542 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -114,6 +114,8 @@ struct iommu_ops {
int (*domain_has_cap)(struct iommu_domain *domain,
unsigned long cap);
int (*add_device)(struct device *dev);
+ int (*add_device_master_ids)(struct device *dev, int count, u32 *ids,
+ void *data);
If we want to pass around IOMMU instances I think we should make them
proper objects rather than some loosely specified void *.
Agreed.
Also the generic IOMMU binding doesn't require the IOMMU specifier to
contain master IDs. So this seems to be a callback that would only be
used by a restricted set of IOMMUs.
No, it should be used by all of them, but we might be passing empty
arguments.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
On Monday 01 September 2014 10:29:40 Thierry Reding wrote: I think this could use a bit more formalization. As I said in another reply earlier, there's very little standardization in the IOMMU API. That certainly gives us a lot of flexibility but it also has the downside that it's difficult to handle these abstractions in the core, which is really what the core is all about, isn't it? One method that worked really well for this in the past for other subsystems is to allow drivers to specify an .of_xlate() function that takes the controller device and a struct of_phandle_args. It is that function's responsibility to take the information in an of_phandle_args structure and use that to create some subsystem specific handle that represents this information in a way that it can readily be used. Yes, good idea. So I think it would really be helpful if IOMMU gained support for something similar. We could create a struct iommu to represent an instance of an IOMMU. IOMMU drivers can embed this structure and add device-specific fields that they need. That way we can easily pass around instances and upcast in the driver in a type-safe way. Right. At the same time, a struct iommu_master could provide the basis to represent a single master interface on an IOMMU. Drivers can again embed that in driver-specific structures with additional fields required for the particular IOMMU implementation. .of_xlate() could return such an IOMMU master for the core to use. I'm not convinced it's necessary. Could this just be a 'struct device' instead of 'struct iommu_master'? With such structures in place we should be able to eliminate many of the loops in IOMMU drivers that serve no other purpose than to find the master context from a struct device * and some parameters. It will also allow us to keep a central registry of IOMMUs and masters rather than duplicating that in every driver. Yes, we should be able to identify an iommu context in a generic way, but why do you want to break it down to individual masters within one context? Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 5/7] dma-mapping: detect and configure IOMMU in of_dma_configure
On Friday 29 August 2014 16:54:28 Will Deacon wrote:
static void of_dma_configure(struct platform_device *pdev)
{
- u64 dma_addr, paddr, size;
+ u64 dma_addr, paddr, size, mask;
int ret;
- bool coherent;
+ bool coherent, iommu;
unsigned long offset;
struct device *dev = pdev-dev;
+ /*
+* Set default dma-mask to 32 bit. Drivers are expected to setup
+* the correct supported dma_mask.
+*/
+ mask = DMA_BIT_MASK(32);
+
ret = of_dma_get_range(dev-of_node, dma_addr, paddr, size);
- offset = ret 0 ? 0 : PFN_DOWN(paddr - dma_addr);
- dev_dbg(dev, dma_pfn_offset(%#08lx)\n, dev-dma_pfn_offset);
+ if (ret 0) {
+ dma_addr = offset = 0;
+ size = mask;
+ } else {
+ offset = PFN_DOWN(paddr - dma_addr);
+ dev_dbg(dev, dma_pfn_offset(%#08lx)\n, dev-dma_pfn_offset);
+ }
coherent = of_dma_is_coherent(dev-of_node);
dev_dbg(dev, device is%sdma coherent\n,
coherent ? : not );
- /*
-* Set default dma-mask to 32 bit. Drivers are expected to setup
-* the correct supported dma_mask.
-*/
- arch_setup_dma_ops(dev, DMA_BIT_MASK(32), offset, coherent);
+ iommu = !of_iommu_configure(dev);
+ dev_dbg(dev, device is%sbehind an iommu\n,
+ iommu ? : not );
+
+ arch_setup_dma_ops(dev, mask, dma_addr, size, offset, coherent,
iommu);
}
Hmm, I was actually expecting of_iommu_configure() to be called from inside
arch_setup_dma_ops(), leaving the choice for how it's set up to the
architecture for now. It's probably not a big difference either way.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/6] iommu/arm-smmu: add support for iova_to_phys through ATS1PR
On Tue, Aug 26, 2014 at 02:54:51PM +0100, Will Deacon wrote:
On Tue, Aug 19, 2014 at 07:12:41PM +0100, Mitchel Humpherys wrote:
On Tue, Aug 19 2014 at 05:44:32 AM, Will Deacon will.dea...@arm.com wrote:
@@ -2005,6 +2073,11 @@ int arm_smmu_device_cfg_probe(struct
arm_smmu_device *smmu)
return -ENODEV;
}
+if (smmu-version == 1 || (!(id ID0_ATOSNS) (id
ID0_S1TS))) {
Are you sure about this? The v2 spec says that is ATOSNS is clear then
S1TS
is also clear.
I was looking at Section 4.1.1 of ARM IHI 0062C ID091613 which states:
In SMMUv2, the address translation registers are OPTIONAL. The
address translation registers are implemented only when both:
o The SMMU_IDR0.S1TS bit is set to 1.
o The SMMU_IDR0.ATOSNS bit is set to 0.
I assume you're referring to section 9.6.1 of the same document:
ATOSNS, bit[26]
Address Translation Operations Not Supported. The possible values of
this bit are:
0 Address translation operations are supported. Stage 1
translation is not supported, that is, the S1TS bit is set to 0.
1 Address translation operations are not supported. Stage 1
translation is supported, that is, the S1TS bit is set to 1.
If that really means that S1TS and ATOSNS always have the same value
then Section 4.1.1 doesn't make any sense. Or am I missing something?
I'll get this checked, as those two paragraphs don't make an awful lot of
sense together.
Right, word from above says that ATOS is implemented iff:
IDR0.S1TS == 1 IDR0.ATOSNS == 0
Basically, ATOS only works for stage-1 when it's present, so that explains
the dependency. Looking at the piece of diff at the top of this mail, I
think that means your code is correct
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 2/7] dma-mapping: replace set_arch_dma_coherent_ops with arch_setup_dma_ops
On Mon, Sep 01, 2014 at 03:27:30PM +0100, Arnd Bergmann wrote:
On Friday 29 August 2014 16:54:25 Will Deacon wrote:
set_arch_dma_coherent_ops is called from of_dma_configure in order to
swizzle the architectural dma-mapping functions over to a cache-coherent
implementation. This is currently implemented only for ARM.
In anticipation of re-using this mechanism for IOMMU-backed dma-mapping
ops too, this patch replaces the function with a broader
arch_setup_dma_ops callback which is also responsible for setting the
DMA mask and offset as well as selecting the correct mapping functions.
A further advantage of this split is that it nicely isolates the
of-specific code from the dma-mapping code, allowing potential reuse by
other buses (e.g. PCI) in the future.
Signed-off-by: Will Deacon will.dea...@arm.com
Looks good, just one tiny comment
---
arch/arm/include/asm/dma-mapping.h | 20 ++
drivers/of/platform.c | 42
++
include/linux/dma-mapping.h| 8 +++-
3 files changed, 30 insertions(+), 40 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index c45b61a4b4a5..936125ef3f3f 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -121,12 +121,24 @@ static inline unsigned long dma_max_pfn(struct device
*dev)
}
#define dma_max_pfn(dev) dma_max_pfn(dev)
-static inline int set_arch_dma_coherent_ops(struct device *dev)
+static inline void arch_setup_dma_ops(struct device *dev, u64 mask,
+ unsigned long offset, bool coherent)
{
- set_dma_ops(dev, arm_coherent_dma_ops);
- return 0;
+ dev-coherent_dma_mask = mask;
+
+ /*
+* Set dma_mask to coherent_dma_mask by default if the architecture
+* code has not set it.
+*/
+ if (!dev-dma_mask)
+ dev-dma_mask = dev-coherent_dma_mask;
+
We are in architecture specific code here, and we know that this
architecture has not set up the dev-dma_mask pointer, so we can
remove the 'if (!dev-dma_mask)'
Good point; I'll fix it.
Thanks,
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 3/7] iommu: add new iommu_ops callback for adding a device with a set of IDs
On Mon, Sep 01, 2014 at 03:39:16PM +0100, Arnd Bergmann wrote:
On Monday 01 September 2014 10:13:22 Thierry Reding wrote:
On Fri, Aug 29, 2014 at 04:54:26PM +0100, Will Deacon wrote:
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 20f9a527922a..3dd1b99c4542 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -114,6 +114,8 @@ struct iommu_ops {
int (*domain_has_cap)(struct iommu_domain *domain,
unsigned long cap);
int (*add_device)(struct device *dev);
+ int (*add_device_master_ids)(struct device *dev, int count, u32
*ids,
+ void *data);
If we want to pass around IOMMU instances I think we should make them
proper objects rather than some loosely specified void *.
Agreed.
For OF, this data argument is the data field of the device_node for the
IOMMU. That's private to the corresponding IOMMU driver and I don't see
what we gain by making that a generic structure. It's likely going to
represent some internal driver data structures anyway, so that the IDs can
be recorded in the relevant place and for the relevant group etc.
In other words, I have no idea what a generic data structure would look
like for this.
Also the generic IOMMU binding doesn't require the IOMMU specifier to
contain master IDs. So this seems to be a callback that would only be
used by a restricted set of IOMMUs.
No, it should be used by all of them, but we might be passing empty
arguments.
Yup; I'd hope to remove/replace the add_device callback in the future,
but that's a lot of work right now.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
On Mon, Sep 01, 2014 at 03:46:18PM +0100, Arnd Bergmann wrote: On Monday 01 September 2014 10:29:40 Thierry Reding wrote: I think this could use a bit more formalization. As I said in another reply earlier, there's very little standardization in the IOMMU API. That certainly gives us a lot of flexibility but it also has the downside that it's difficult to handle these abstractions in the core, which is really what the core is all about, isn't it? One method that worked really well for this in the past for other subsystems is to allow drivers to specify an .of_xlate() function that takes the controller device and a struct of_phandle_args. It is that function's responsibility to take the information in an of_phandle_args structure and use that to create some subsystem specific handle that represents this information in a way that it can readily be used. Yes, good idea. Hmm, how does this work for PCI devices? The current RFC takes care to ensure that the core changes work just as well for OF devices as PCI devices, and the of-specific functions and data structures are not part of it. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 3/7] iommu: add new iommu_ops callback for adding a device with a set of IDs
On Monday 01 September 2014 17:34:00 Will Deacon wrote:
On Mon, Sep 01, 2014 at 03:39:16PM +0100, Arnd Bergmann wrote:
On Monday 01 September 2014 10:13:22 Thierry Reding wrote:
On Fri, Aug 29, 2014 at 04:54:26PM +0100, Will Deacon wrote:
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 20f9a527922a..3dd1b99c4542 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -114,6 +114,8 @@ struct iommu_ops {
int (*domain_has_cap)(struct iommu_domain *domain,
unsigned long cap);
int (*add_device)(struct device *dev);
+ int (*add_device_master_ids)(struct device *dev, int count, u32
*ids,
+ void *data);
If we want to pass around IOMMU instances I think we should make them
proper objects rather than some loosely specified void *.
Agreed.
For OF, this data argument is the data field of the device_node for the
IOMMU. That's private to the corresponding IOMMU driver and I don't see
what we gain by making that a generic structure. It's likely going to
represent some internal driver data structures anyway, so that the IDs can
be recorded in the relevant place and for the relevant group etc.
In other words, I have no idea what a generic data structure would look
like for this.
Something like
struct iommu {
struct device *dev;
const struct iommu_ops *ops;
struct list_head domains;
void *private;
};
There are probably a few more fields we will need in the long run.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
On Monday 01 September 2014 17:40:00 Will Deacon wrote: On Mon, Sep 01, 2014 at 03:46:18PM +0100, Arnd Bergmann wrote: On Monday 01 September 2014 10:29:40 Thierry Reding wrote: I think this could use a bit more formalization. As I said in another reply earlier, there's very little standardization in the IOMMU API. That certainly gives us a lot of flexibility but it also has the downside that it's difficult to handle these abstractions in the core, which is really what the core is all about, isn't it? One method that worked really well for this in the past for other subsystems is to allow drivers to specify an .of_xlate() function that takes the controller device and a struct of_phandle_args. It is that function's responsibility to take the information in an of_phandle_args structure and use that to create some subsystem specific handle that represents this information in a way that it can readily be used. Yes, good idea. Hmm, how does this work for PCI devices? The current RFC takes care to ensure that the core changes work just as well for OF devices as PCI devices, and the of-specific functions and data structures are not part of it. I don't mind handling PCI devices separately. They are different in a number of ways already, in particular the way that they don't normally have an of_node attached to them but actually have a PCI bus/dev/function number. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
