How to specify IOMMU'able devices in DT (was: [RFC 0/5] ARM: dma-mapping: New dma_map_ops to control IOVA more precisely)
On Fri, 21 Sep 2012 20:16:00 +0200 Krishna Reddy vdu...@nvidia.com wrote: The device(H/W controller) need to access few special memory blocks(IOVA==PA) and DRAM as well. OK, so only /some/ of the VA space is VA==PA, and some is remapped; that's a little different that what you originally implied above. BTW, which HW module is this; AVP/COP or something else. This sounds like an odd requirement. This is not specific to ARM7. There are protected memory regions on Tegra that can be accessed by some controllers like display, 2D, 3D, VDE, HDA. These are DRAM regions configured as protected by BootRom. These memory regions are not exposed to and not managed by OS page allocator. The H/W controller accesses to these regions still to go through IOMMU. The IOMMU view for all the H/W controllers is not uniform on Tegra. Some Controllers see entire 4GB IOVA space. i.e all accesses go though IOMMU. Some controllers see the IOVA Space that don't overlap with MMIO space. i.e The MMIO address access bypass IOMMU and directly go to MMIO space. Tegra IOMMU can support multiple address spaces as well. To hide controller Specific behavior, the drivers should take care of one to one mapping and remove inaccessible iova spaces in their address space's based platform device info. The above is also related to another issue, how to specify IOMMU'able devices in DT. As mentioned above, some IOVA mapping may be unique to some devices, and the number of IOMMU'able device are quite many nowadays, a few dozen in Tegra30 now. Basically they are seen as just normal platform devices from CPU even if they belong to different busses in H/W. IOW, their IOMMU'ability just depend on a platfrom bus from _S/W_ POV. Doing each registration(create a map attach device) in board files isn't so nice. Currently we register them at platform_device_add() at once with just a HACK(*1), but this could/should be done based on the info passed from DT. For tegra, those parameter could be, ASID and address range(start, size, alignment). For example in DT: deviceA { start size align iommu = 0x1234 0x0040 0x000; # exclusively specify start or align iommu = 0x 0x0040 0x001; iommu = 0x1234 0x0004 0x1238 0x0004; # start, size could be repeated... asid = 3; # if needed or dma_range = 0x1234 0x0040 0x000; # if iommu is considered as one implementation of dma. }; Is there any way to specify each IOMMU'able _platform device_ and specify its map in DT? The above ASID may be specific to Tegra, though. If we can specify the above info in DT and the info is passed to kernel, some platform common code would register them as IOMMU'able device automatically. It would be really covenient if this is done in platform_device/IOMMU common code. If the above attribute is implemented specific to Tegra/platform, we have to call attach_device quite many times somewhere in device initializations. Any comment would be really appreciated. *1: From dd4dd6532d705c7bba0914b54c819d8d735c2fad Mon Sep 17 00:00:00 2001 From: Hiroshi Doyu hd...@nvidia.com Date: Thu, 22 Mar 2012 16:06:27 +0200 Subject: [RFC 1/1] platform: IOMMU'able platform_device w/ PLATFORM_ENABLE_IOMMU Introduced a new kernel config option, PLATFORM_ENABLE_IOMMU. With this, all platform devices can be converted to be IOMMU'able if platform_bus has non-null dma_iommu_map, where H/Ws always see its IO virtual address and virt_to_phys() doesn't work from H/W POV. Signed-off-by: Hiroshi Doyu hd...@nvidia.com --- arch/arm/mm/dma-mapping.c |7 +++ drivers/base/Kconfig |4 drivers/base/platform.c | 17 +++-- drivers/iommu/Kconfig |2 +- include/linux/device.h|5 - 5 files changed, 31 insertions(+), 4 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 242289f..28ca7c2 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1899,6 +1899,13 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size, mapping-order = order; spin_lock_init(mapping-lock); +#ifdef CONFIG_PLATFORM_ENABLE_IOMMU + if (WARN_ON(bus-map)) + goto err3; + + bus-map = mapping; +#endif + mapping-domain = iommu_domain_alloc(bus); if (!mapping-domain) goto err3; diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index 3df339c..0f45311 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -308,4 +308,8 @@ config CMA_AREAS endif +config PLATFORM_ENABLE_IOMMU +bool Make platform devices iommuable + depends on IOMMU_API + endmenu diff --git a/drivers/base/platform.c b/drivers/base/platform.c index a1a7225..9eae3be 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -21,6 +21,8 @@ #include linux/slab.h #include
Re: How to specify IOMMU'able devices in DT (was: [RFC 0/5] ARM: dma-mapping: New dma_map_ops to control IOVA more precisely)
Hi James, On Mon, 24 Sep 2012 11:28:01 +0200 James Bottomley james.bottom...@hansenpartnership.com wrote: On Mon, 2012-09-24 at 12:04 +0300, Hiroshi Doyu wrote: diff --git a/drivers/base/platform.c b/drivers/base/platform.c index a1a7225..9eae3be 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -21,6 +21,8 @@ #include linux/slab.h #include linux/pm_runtime.h +#include asm/dma-iommu.h + #include base.h #define to_platform_driver(drv)(container_of((drv), struct platform_driver, \ @@ -305,8 +307,19 @@ int platform_device_add(struct platform_device *pdev) dev_name(pdev-dev), dev_name(pdev-dev.parent)); ret = device_add(pdev-dev); - if (ret == 0) - return ret; + if (ret) + goto failed; + +#ifdef CONFIG_PLATFORM_ENABLE_IOMMU + if (platform_bus_type.map !pdev-dev.archdata.mapping) { + ret = arm_iommu_attach_device(pdev-dev, + platform_bus_type.map); + if (ret) + goto failed; This is horrible ... you're adding an architecture specific callback into our generic code; that's really a no-no. If the concept of CONFIG_PLATFORM_ENABE_IOMMU is useful to more than just arm, then this could become a generic callback. As mentioned in the original, this is a heck to explain what is needed. I am looking for some generic solution for how to specify IOMMU info for each platform devices. I'm guessing that some other SoC may have the similar requirements on the above. As you mentioned, this solution should be a generic, not arch specific. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: How to specify IOMMU'able devices in DT (was: [RFC 0/5] ARM: dma-mapping: New dma_map_ops to control IOVA more precisely)
Hello, On Monday, September 24, 2012 11:45 AM Hiroshi Doyu wrote: On Mon, 24 Sep 2012 11:28:01 +0200 James Bottomley james.bottom...@hansenpartnership.com wrote: On Mon, 2012-09-24 at 12:04 +0300, Hiroshi Doyu wrote: diff --git a/drivers/base/platform.c b/drivers/base/platform.c index a1a7225..9eae3be 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -21,6 +21,8 @@ #include linux/slab.h #include linux/pm_runtime.h +#include asm/dma-iommu.h + #include base.h #define to_platform_driver(drv)(container_of((drv), struct platform_driver, \ @@ -305,8 +307,19 @@ int platform_device_add(struct platform_device *pdev) dev_name(pdev-dev), dev_name(pdev-dev.parent)); ret = device_add(pdev-dev); - if (ret == 0) - return ret; + if (ret) + goto failed; + +#ifdef CONFIG_PLATFORM_ENABLE_IOMMU + if (platform_bus_type.map !pdev-dev.archdata.mapping) { + ret = arm_iommu_attach_device(pdev-dev, + platform_bus_type.map); + if (ret) + goto failed; This is horrible ... you're adding an architecture specific callback into our generic code; that's really a no-no. If the concept of CONFIG_PLATFORM_ENABE_IOMMU is useful to more than just arm, then this could become a generic callback. As mentioned in the original, this is a heck to explain what is needed. I am looking for some generic solution for how to specify IOMMU info for each platform devices. I'm guessing that some other SoC may have the similar requirements on the above. As you mentioned, this solution should be a generic, not arch specific. Please read more about bus notifiers. IMHO a good example is provided in the following thread: http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg12238.html Best regards -- Marek Szyprowski Samsung Poland RD Center ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: How to specify IOMMU'able devices in DT
Hi Marek, Marek Szyprowski m.szyprow...@samsung.com wrote @ Mon, 24 Sep 2012 13:14:51 +0200: Hello, On Monday, September 24, 2012 11:45 AM Hiroshi Doyu wrote: On Mon, 24 Sep 2012 11:28:01 +0200 James Bottomley james.bottom...@hansenpartnership.com wrote: On Mon, 2012-09-24 at 12:04 +0300, Hiroshi Doyu wrote: diff --git a/drivers/base/platform.c b/drivers/base/platform.c index a1a7225..9eae3be 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -21,6 +21,8 @@ #include linux/slab.h #include linux/pm_runtime.h +#include asm/dma-iommu.h + #include base.h #define to_platform_driver(drv)(container_of((drv), struct platform_driver, \ @@ -305,8 +307,19 @@ int platform_device_add(struct platform_device *pdev) dev_name(pdev-dev), dev_name(pdev-dev.parent)); ret = device_add(pdev-dev); - if (ret == 0) - return ret; + if (ret) + goto failed; + +#ifdef CONFIG_PLATFORM_ENABLE_IOMMU + if (platform_bus_type.map !pdev-dev.archdata.mapping) { + ret = arm_iommu_attach_device(pdev-dev, + platform_bus_type.map); + if (ret) + goto failed; This is horrible ... you're adding an architecture specific callback into our generic code; that's really a no-no. If the concept of CONFIG_PLATFORM_ENABE_IOMMU is useful to more than just arm, then this could become a generic callback. As mentioned in the original, this is a heck to explain what is needed. I am looking for some generic solution for how to specify IOMMU info for each platform devices. I'm guessing that some other SoC may have the similar requirements on the above. As you mentioned, this solution should be a generic, not arch specific. Please read more about bus notifiers. IMHO a good example is provided in the following thread: http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg12238.html This bus notifier seems enough flexible to afford the variation of IOMMU map info, like Tegra ASID, which could be platform-specific, and the other could be common too. There's already iommu_bus_notifier too. I'll try to implement something base on this. Thanks for the good info. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: How to specify IOMMU'able devices in DT (was: [RFC 0/5] ARM: dma-mapping: New dma_map_ops to control IOVA more precisely)
On Mon, 2012-09-24 at 12:04 +0300, Hiroshi Doyu wrote: diff --git a/drivers/base/platform.c b/drivers/base/platform.c index a1a7225..9eae3be 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -21,6 +21,8 @@ #include linux/slab.h #include linux/pm_runtime.h +#include asm/dma-iommu.h + #include base.h #define to_platform_driver(drv)(container_of((drv), struct platform_driver, \ @@ -305,8 +307,19 @@ int platform_device_add(struct platform_device *pdev) dev_name(pdev-dev), dev_name(pdev-dev.parent)); ret = device_add(pdev-dev); - if (ret == 0) - return ret; + if (ret) + goto failed; + +#ifdef CONFIG_PLATFORM_ENABLE_IOMMU + if (platform_bus_type.map !pdev-dev.archdata.mapping) { + ret = arm_iommu_attach_device(pdev-dev, + platform_bus_type.map); + if (ret) + goto failed; This is horrible ... you're adding an architecture specific callback into our generic code; that's really a no-no. If the concept of CONFIG_PLATFORM_ENABE_IOMMU is useful to more than just arm, then this could become a generic callback. James ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: linux-next: Tree for Sept 24 (iommu)
On Mon, 2012-09-24 at 15:02 -0700, Randy Dunlap wrote: On 09/24/2012 07:53 AM, Stephen Rothwell wrote: Hi all, Today was a train wreck, with lots of new conflicts across several trees and a few build failures as well. Changes since 201209021: on i386: drivers/built-in.o: In function `iommu_group_remove_device': (.text+0x74cb10): multiple definition of `iommu_group_remove_device' arch/x86/built-in.o:(.text+0x140d0): first defined here ... Here's a patch to get it past this. It still doesn't fully build, but the rest isn't iommu related. Thanks, Alex commit 6955e1f06cb20ddd25665984c330b945443cce36 Author: Alex Williamson alex.william...@redhat.com Date: Mon Sep 24 21:13:52 2012 -0600 iommu: inline iommu group stub functions Signed-off-by: Alex Williamson alex.william...@redhat.com diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 7e83370..f3b99e1 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -256,72 +256,78 @@ static inline void iommu_set_fault_handler(struct iommu_domain *domain, { } -int iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group) +static inline int iommu_attach_group(struct iommu_domain *domain, +struct iommu_group *group) { return -ENODEV; } -void iommu_detach_group(struct iommu_domain *domain, struct iommu_group *group) +static inline void iommu_detach_group(struct iommu_domain *domain, + struct iommu_group *group) { } -struct iommu_group *iommu_group_alloc(void) +static inline struct iommu_group *iommu_group_alloc(void) { return ERR_PTR(-ENODEV); } -void *iommu_group_get_iommudata(struct iommu_group *group) +static inline void *iommu_group_get_iommudata(struct iommu_group *group) { return NULL; } -void iommu_group_set_iommudata(struct iommu_group *group, void *iommu_data, - void (*release)(void *iommu_data)) +static inline void iommu_group_set_iommudata(struct iommu_group *group, +void *iommu_data, +void (*release)(void *iommu_data)) { } -int iommu_group_set_name(struct iommu_group *group, const char *name) +static inline int iommu_group_set_name(struct iommu_group *group, + const char *name) { return -ENODEV; } -int iommu_group_add_device(struct iommu_group *group, struct device *dev) +static inline int iommu_group_add_device(struct iommu_group *group, +struct device *dev) { return -ENODEV; } -void iommu_group_remove_device(struct device *dev) +static inline void iommu_group_remove_device(struct device *dev) { } -int iommu_group_for_each_dev(struct iommu_group *group, void *data, -int (*fn)(struct device *, void *)) +static inline int iommu_group_for_each_dev(struct iommu_group *group, + void *data, + int (*fn)(struct device *, void *)) { return -ENODEV; } -struct iommu_group *iommu_group_get(struct device *dev) +static inline struct iommu_group *iommu_group_get(struct device *dev) { return NULL; } -void iommu_group_put(struct iommu_group *group) +static inline void iommu_group_put(struct iommu_group *group) { } -int iommu_group_register_notifier(struct iommu_group *group, - struct notifier_block *nb) +static inline int iommu_group_register_notifier(struct iommu_group *group, + struct notifier_block *nb) { return -ENODEV; } -int iommu_group_unregister_notifier(struct iommu_group *group, - struct notifier_block *nb) +static inline int iommu_group_unregister_notifier(struct iommu_group *group, + struct notifier_block *nb) { return 0; } -int iommu_group_id(struct iommu_group *group) +static inline int iommu_group_id(struct iommu_group *group) { return -ENODEV; } ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu