[PATCH 01/11] iommu: exynos: rework iommu group initialization
This patch replaces custom code in add_device implementation with iommu_group_get_for_dev() call and provides the needed callback. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 27 --- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 97c41b8..4fc0790 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1114,28 +1114,32 @@ static phys_addr_t exynos_iommu_iova_to_phys(struct iommu_domain *iommu_domain, return phys; } +static struct iommu_group *get_device_iommu_group(struct device *dev) +{ + struct iommu_group *group; + + group = iommu_group_get(dev); + if (!group) + group = iommu_group_alloc(); + + return group; +} + static int exynos_iommu_add_device(struct device *dev) { struct iommu_group *group; - int ret; if (!has_sysmmu(dev)) return -ENODEV; - group = iommu_group_get(dev); + group = iommu_group_get_for_dev(dev); - if (!group) { - group = iommu_group_alloc(); - if (IS_ERR(group)) { - dev_err(dev, "Failed to allocate IOMMU group\n"); - return PTR_ERR(group); - } - } + if (IS_ERR(group)) + return PTR_ERR(group); - ret = iommu_group_add_device(group, dev); iommu_group_put(group); - return ret; + return 0; } static void exynos_iommu_remove_device(struct device *dev) @@ -1182,6 +1186,7 @@ static struct iommu_ops exynos_iommu_ops = { .unmap = exynos_iommu_unmap, .map_sg = default_iommu_map_sg, .iova_to_phys = exynos_iommu_iova_to_phys, + .device_group = get_device_iommu_group, .add_device = exynos_iommu_add_device, .remove_device = exynos_iommu_remove_device, .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE, -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 10/11] iommu: exynos: add support for v5 SYSMMU
This patch adds support for v5 of SYSMMU controller, found in Samsung Exynos 5433 SoCs. The main difference of v5 is support for 36-bit physical address space and some changes in register layout and core clocks hanging. This patch also adds support for ARM64 architecture, which is used by Exynos 5433 SoCs. Signed-off-by: Marek Szyprowski --- .../devicetree/bindings/iommu/samsung,sysmmu.txt | 5 +- drivers/iommu/Kconfig | 2 +- drivers/iommu/exynos-iommu.c | 195 +++-- 3 files changed, 150 insertions(+), 52 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt index f61ca25..85f0688 100644 --- a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt +++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt @@ -35,9 +35,10 @@ Required properties: - interrupts: An interrupt specifier for interrupt signal of System MMU, according to the format defined by a particular interrupt controller. -- clock-names: Should be "sysmmu" if the System MMU is needed to gate its clock. +- clock-names: Should be "sysmmu" or a pair of "aclk" and "pclk" to gate + SYSMMU core clocks. Optional "master" if the clock to the System MMU is gated by - another gate clock other than "sysmmu" (usually main gate clock + another gate clock other core (usually main gate clock of peripheral device this SYSMMU belongs to). - clocks: Phandles for respective clocks described by clock-names. - power-domains: Required if the System MMU is needed to gate its power. diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index a1e75cb..1674de1 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -243,7 +243,7 @@ config TEGRA_IOMMU_SMMU config EXYNOS_IOMMU bool "Exynos IOMMU Support" - depends on ARCH_EXYNOS && ARM && MMU + depends on ARCH_EXYNOS && MMU select IOMMU_API select ARM_DMA_USE_IOMMU help diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index e42a76c..f00378a 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1,6 +1,5 @@ -/* linux/drivers/iommu/exynos_iommu.c - * - * Copyright (c) 2011 Samsung Electronics Co., Ltd. +/* + * Copyright (c) 2011,2016 Samsung Electronics Co., Ltd. * http://www.samsung.com * * This program is free software; you can redistribute it and/or modify @@ -55,17 +54,25 @@ typedef u32 sysmmu_pte_t; #define lv2ent_small(pent) ((*(pent) & 2) == 2) #define lv2ent_large(pent) ((*(pent) & 3) == 1) -static u32 sysmmu_page_offset(sysmmu_iova_t iova, u32 size) -{ - return iova & (size - 1); -} - -#define section_phys(sent) (*(sent) & SECT_MASK) -#define section_offs(iova) sysmmu_page_offset((iova), SECT_SIZE) -#define lpage_phys(pent) (*(pent) & LPAGE_MASK) -#define lpage_offs(iova) sysmmu_page_offset((iova), LPAGE_SIZE) -#define spage_phys(pent) (*(pent) & SPAGE_MASK) -#define spage_offs(iova) sysmmu_page_offset((iova), SPAGE_SIZE) +/* + * v1.x - v3.x SYSMMU supports 32bit physical and 32bit virtual address spaces + * v5.0 introduced support for 36bit physical address space by shifting + * all page entry values by 4 bits. + * All SYSMMU controllers in the system support the address spaces of the same + * size, so PG_ENT_SHIFT can be initialized on first SYSMMU probe to proper + * value (0 or 4). + */ +static short PG_ENT_SHIFT = -1; +#define SYSMMU_PG_ENT_SHIFT 0 +#define SYSMMU_V5_PG_ENT_SHIFT 4 + +#define sect_to_phys(ent) (((phys_addr_t) ent) << PG_ENT_SHIFT) +#define section_phys(sent) (sect_to_phys(*(sent)) & SECT_MASK) +#define section_offs(iova) (iova & (SECT_SIZE - 1)) +#define lpage_phys(pent) (sect_to_phys(*(pent)) & LPAGE_MASK) +#define lpage_offs(iova) (iova & (LPAGE_SIZE - 1)) +#define spage_phys(pent) (sect_to_phys(*(pent)) & SPAGE_MASK) +#define spage_offs(iova) (iova & (SPAGE_SIZE - 1)) #define NUM_LV1ENTRIES 4096 #define NUM_LV2ENTRIES (SECT_SIZE / SPAGE_SIZE) @@ -84,13 +91,12 @@ static u32 lv2ent_offset(sysmmu_iova_t iova) #define LV2TABLE_SIZE (NUM_LV2ENTRIES * sizeof(sysmmu_pte_t)) #define SPAGES_PER_LPAGE (LPAGE_SIZE / SPAGE_SIZE) +#define lv2table_base(sent) (sect_to_phys(*(sent) & 0xFFC0)) -#define lv2table_base(sent) (*(sent) & 0xFC00) - -#define mk_lv1ent_sect(pa) ((pa) | 2) -#define mk_lv1ent_page(pa) ((pa) | 1) -#define mk_lv2ent_lpage(pa) ((pa) | 1) -#define mk_lv2ent_spage(pa) ((pa) | 2) +#define mk_lv1ent_sect(pa) ((pa >> PG_ENT_SHIFT) | 2) +#define mk_lv1ent_page(pa) ((pa >> PG_ENT_SHIFT) | 1) +#define mk_lv2ent_lpage(pa) ((pa >> PG_ENT_SHIFT) | 1) +#define mk_lv2ent_spage
[PATCH 06/11] iommu: exynos: refactor fault handling code
This patch provides a new implementation for page fault handing code. The new implementation is ready for future extensions. No functional changes have been made. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 109 --- 1 file changed, 41 insertions(+), 68 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 4275222..3a577a4 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -148,40 +148,25 @@ static sysmmu_pte_t *page_entry(sysmmu_pte_t *sent, sysmmu_iova_t iova) lv2table_base(sent)) + lv2ent_offset(iova); } -enum exynos_sysmmu_inttype { - SYSMMU_PAGEFAULT, - SYSMMU_AR_MULTIHIT, - SYSMMU_AW_MULTIHIT, - SYSMMU_BUSERROR, - SYSMMU_AR_SECURITY, - SYSMMU_AR_ACCESS, - SYSMMU_AW_SECURITY, - SYSMMU_AW_PROTECTION, /* 7 */ - SYSMMU_FAULT_UNKNOWN, - SYSMMU_FAULTS_NUM -}; - -static unsigned short fault_reg_offset[SYSMMU_FAULTS_NUM] = { - REG_PAGE_FAULT_ADDR, - REG_AR_FAULT_ADDR, - REG_AW_FAULT_ADDR, - REG_DEFAULT_SLAVE_ADDR, - REG_AR_FAULT_ADDR, - REG_AR_FAULT_ADDR, - REG_AW_FAULT_ADDR, - REG_AW_FAULT_ADDR +/* + * IOMMU fault information register + */ +struct sysmmu_fault_info { + unsigned int bit; /* bit number in STATUS register */ + unsigned short addr_reg; /* register to read VA fault address */ + const char *name; /* human readable fault name */ + unsigned int type; /* fault type for report_iommu_fault */ }; -static char *sysmmu_fault_name[SYSMMU_FAULTS_NUM] = { - "PAGE FAULT", - "AR MULTI-HIT FAULT", - "AW MULTI-HIT FAULT", - "BUS ERROR", - "AR SECURITY PROTECTION FAULT", - "AR ACCESS PROTECTION FAULT", - "AW SECURITY PROTECTION FAULT", - "AW ACCESS PROTECTION FAULT", - "UNKNOWN FAULT" +static const struct sysmmu_fault_info sysmmu_faults[] = { + { 0, REG_PAGE_FAULT_ADDR, "PAGE", IOMMU_FAULT_READ }, + { 1, REG_AR_FAULT_ADDR, "AR MULTI-HIT", IOMMU_FAULT_READ }, + { 2, REG_AW_FAULT_ADDR, "AW MULTI-HIT", IOMMU_FAULT_WRITE }, + { 3, REG_DEFAULT_SLAVE_ADDR, "BUS ERROR", IOMMU_FAULT_READ }, + { 4, REG_AR_FAULT_ADDR, "AR SECURITY PROTECTION", IOMMU_FAULT_READ }, + { 5, REG_AR_FAULT_ADDR, "AR ACCESS PROTECTION", IOMMU_FAULT_READ }, + { 6, REG_AW_FAULT_ADDR, "AW SECURITY PROTECTION", IOMMU_FAULT_WRITE }, + { 7, REG_AW_FAULT_ADDR, "AW ACCESS PROTECTION", IOMMU_FAULT_WRITE }, }; /* @@ -299,24 +284,19 @@ static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd) __sysmmu_tlb_invalidate(data); } -static void show_fault_information(const char *name, - enum exynos_sysmmu_inttype itype, - phys_addr_t pgtable_base, sysmmu_iova_t fault_addr) +static void show_fault_information(struct sysmmu_drvdata *data, + const struct sysmmu_fault_info *finfo, + sysmmu_iova_t fault_addr) { sysmmu_pte_t *ent; - if ((itype >= SYSMMU_FAULTS_NUM) || (itype < SYSMMU_PAGEFAULT)) - itype = SYSMMU_FAULT_UNKNOWN; - - pr_err("%s occurred at %#x by %s(Page table base: %pa)\n", - sysmmu_fault_name[itype], fault_addr, name, &pgtable_base); - - ent = section_entry(phys_to_virt(pgtable_base), fault_addr); - pr_err("\tLv1 entry: %#x\n", *ent); - + dev_err(data->sysmmu, "%s FAULT occurred at %#x (page table base: %pa)\n", + finfo->name, fault_addr, &data->pgtable); + ent = section_entry(phys_to_virt(data->pgtable), fault_addr); + dev_err(data->sysmmu, "\tLv1 entry: %#x\n", *ent); if (lv1ent_page(ent)) { ent = page_entry(ent, fault_addr); - pr_err("\t Lv2 entry: %#x\n", *ent); + dev_err(data->sysmmu, "\t Lv2 entry: %#x\n", *ent); } } @@ -324,8 +304,10 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) { /* SYSMMU is in blocked state when interrupt occurred. */ struct sysmmu_drvdata *data = dev_id; - enum exynos_sysmmu_inttype itype; - sysmmu_iova_t addr = -1; + const struct sysmmu_fault_info *finfo = sysmmu_faults; + int i, n = ARRAY_SIZE(sysmmu_faults); + unsigned int itype; + sysmmu_iova_t fault_addr = -1; int ret = -ENOSYS; WARN_ON(!is_sysmmu_active(data)); @@ -334,29 +316,20 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) clk_enable(data->clk_master); - itype = (enum exyno
Re: [PATCH 1/2] iommu: call detach also for default_domain before attaching to new one
Hello, On 2016-02-16 16:59, Joerg Roedel wrote: On Tue, Feb 16, 2016 at 03:40:31PM +0100, Marek Szyprowski wrote: This patch ensures that devices attached to the default_domain will be first detached from it before attaching to new domain. To avoid forward declaration, __iommu_attach_group() function has been moved to new place in the source code. Actually it was intentional to not invoke the detach_device call-back in the attach_device path. The reason is that detaching first and than attaching again leaves the device without a domain for a short period of time, until it is attached to the new domain. The attach_device call-back is supposed to handle this situation and just silently overwrite any other domain->device binding it finds for the device. This allows to do re-attachment with less iommu flushes and to get rid of the detach_device call-back at some point. Huh, I wasn't aware of this change in the iommu drivers api. For some drivers attach/detach callbacks does something more than just programming page table base register, like for example in case of exynos iommu it is enabling runtime power management and clocks. The code is really much simpler if those calls are balanced, but if the goal is to allow multiple unballanced attach calls, I will try to fix this in our driver. Maybe it should be documented somewhere, that attach calls can be unbalanced? Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu: call detach also for default_domain before attaching to new one
Hello, On 2016-02-17 12:14, Joerg Roedel wrote: On Wed, Feb 17, 2016 at 08:35:10AM +0100, Marek Szyprowski wrote: Huh, I wasn't aware of this change in the iommu drivers api. For some drivers attach/detach callbacks does something more than just programming page table base register, like for example in case of exynos iommu it is enabling runtime power management and clocks. The code is really much simpler if those calls are balanced, but if the goal is to allow multiple unballanced attach calls, I will try to fix this in our driver. Maybe it should be documented somewhere, that attach calls can be unbalanced? Well, when your driver uses default-domains, the detach_dev call-back is not used anymore. The attach_dev call-back is supposed to just overwrite any existing binding that may exist for the device. So the calls are not unbalanced, the detach_dev calls just don't happen anymore. From driver perspective the default_domains don't really differ from the 'other' domains. They are just allocated from the IOMMU core and used by the IOMMU/DMA-mapping glue code. That's what I got from reading the code. There should be also a way to detach the driver even from the default domain to implement the arch_tear_down_dma_ops function. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 03/13] iommu: exynos: remove ARM-specific cache flush interface
This patch replaces custom ARM-specific code for performing CPU cache flush operations with generic code based on DMA-mapping. Domain managing code is independent of particular SYSMMU device, so the first registered SYSMMU device is used for DMA-mapping calls. This simplification works fine because all SYSMMU controllers are in the same address space (where DMA address equals physical address) and the DMA-mapping calls are done mainly to flush CPU cache to make changes visible to SYSMMU controllers. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 74 +--- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 595e0da55db4..8c8a7f7968d1 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -27,9 +27,6 @@ #include #include -#include -#include - typedef u32 sysmmu_iova_t; typedef u32 sysmmu_pte_t; @@ -83,6 +80,7 @@ static u32 lv2ent_offset(sysmmu_iova_t iova) return (iova >> SPAGE_ORDER) & (NUM_LV2ENTRIES - 1); } +#define LV1TABLE_SIZE (NUM_LV1ENTRIES * sizeof(sysmmu_pte_t)) #define LV2TABLE_SIZE (NUM_LV2ENTRIES * sizeof(sysmmu_pte_t)) #define SPAGES_PER_LPAGE (LPAGE_SIZE / SPAGE_SIZE) @@ -134,6 +132,7 @@ static u32 lv2ent_offset(sysmmu_iova_t iova) #define has_sysmmu(dev)(dev->archdata.iommu != NULL) +static struct device *dma_dev; static struct kmem_cache *lv2table_kmem_cache; static sysmmu_pte_t *zero_lv2_table; #define ZERO_LV2LINK mk_lv1ent_page(virt_to_phys(zero_lv2_table)) @@ -650,16 +649,19 @@ static struct platform_driver exynos_sysmmu_driver __refdata = { } }; -static inline void pgtable_flush(void *vastart, void *vaend) +static inline void update_pte(sysmmu_pte_t *ent, sysmmu_pte_t val) { - dmac_flush_range(vastart, vaend); - outer_flush_range(virt_to_phys(vastart), - virt_to_phys(vaend)); + dma_sync_single_for_cpu(dma_dev, virt_to_phys(ent), sizeof(*ent), + DMA_TO_DEVICE); + *ent = val; + dma_sync_single_for_device(dma_dev, virt_to_phys(ent), sizeof(*ent), + DMA_TO_DEVICE); } static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type) { struct exynos_iommu_domain *domain; + dma_addr_t handle; int i; @@ -694,7 +696,10 @@ static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type) domain->pgtable[i + 7] = ZERO_LV2LINK; } - pgtable_flush(domain->pgtable, domain->pgtable + NUM_LV1ENTRIES); + handle = dma_map_single(dma_dev, domain->pgtable, LV1TABLE_SIZE, + DMA_TO_DEVICE); + /* For mapping page table entries we rely on dma == phys */ + BUG_ON(handle != virt_to_phys(domain->pgtable)); spin_lock_init(&domain->lock); spin_lock_init(&domain->pgtablelock); @@ -738,10 +743,18 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain) if (iommu_domain->type == IOMMU_DOMAIN_DMA) iommu_put_dma_cookie(iommu_domain); + dma_unmap_single(dma_dev, virt_to_phys(domain->pgtable), LV1TABLE_SIZE, +DMA_TO_DEVICE); + for (i = 0; i < NUM_LV1ENTRIES; i++) - if (lv1ent_page(domain->pgtable + i)) + if (lv1ent_page(domain->pgtable + i)) { + phys_addr_t base = lv2table_base(domain->pgtable + i); + + dma_unmap_single(dma_dev, base, LV2TABLE_SIZE, +DMA_TO_DEVICE); kmem_cache_free(lv2table_kmem_cache, - phys_to_virt(lv2table_base(domain->pgtable + i))); + phys_to_virt(base)); + } free_pages((unsigned long)domain->pgtable, 2); free_pages((unsigned long)domain->lv2entcnt, 1); @@ -834,11 +847,10 @@ static sysmmu_pte_t *alloc_lv2entry(struct exynos_iommu_domain *domain, if (!pent) return ERR_PTR(-ENOMEM); - *sent = mk_lv1ent_page(virt_to_phys(pent)); + update_pte(sent, mk_lv1ent_page(virt_to_phys(pent))); kmemleak_ignore(pent); *pgcounter = NUM_LV2ENTRIES; - pgtable_flush(pent, pent + NUM_LV2ENTRIES); - pgtable_flush(sent, sent + 1); + dma_map_single(dma_dev, pent, LV2TABLE_SIZE, DMA_TO_DEVICE); /* * If pre-fetched SLPD is a faulty SLPD in zero_l2_table, @@ -891,9 +903,7 @@ static int lv1set_section(struct exynos_iommu_domain *domain, *pgcnt = 0; } - *sent = mk_lv1ent_sect(paddr); - - pgtable_flush(sent, sent + 1); + update_pte(sent, m
[PATCH v2 05/13] iommu: exynos: refactor code (no direct register access)
This patch changes some internal functions to have access to the state of sysmmu device instead of having only it's registers. This will make the code ready for future extensions. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 40 +++- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index bf6b826b1d8b..4275222cead3 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -254,50 +254,49 @@ static bool is_sysmmu_active(struct sysmmu_drvdata *data) return data->activations > 0; } -static void sysmmu_unblock(void __iomem *sfrbase) +static void sysmmu_unblock(struct sysmmu_drvdata *data) { - __raw_writel(CTRL_ENABLE, sfrbase + REG_MMU_CTRL); + __raw_writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); } -static bool sysmmu_block(void __iomem *sfrbase) +static bool sysmmu_block(struct sysmmu_drvdata *data) { int i = 120; - __raw_writel(CTRL_BLOCK, sfrbase + REG_MMU_CTRL); - while ((i > 0) && !(__raw_readl(sfrbase + REG_MMU_STATUS) & 1)) + __raw_writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL); + while ((i > 0) && !(__raw_readl(data->sfrbase + REG_MMU_STATUS) & 1)) --i; - if (!(__raw_readl(sfrbase + REG_MMU_STATUS) & 1)) { - sysmmu_unblock(sfrbase); + if (!(__raw_readl(data->sfrbase + REG_MMU_STATUS) & 1)) { + sysmmu_unblock(data); return false; } return true; } -static void __sysmmu_tlb_invalidate(void __iomem *sfrbase) +static void __sysmmu_tlb_invalidate(struct sysmmu_drvdata *data) { - __raw_writel(0x1, sfrbase + REG_MMU_FLUSH); + __raw_writel(0x1, data->sfrbase + REG_MMU_FLUSH); } -static void __sysmmu_tlb_invalidate_entry(void __iomem *sfrbase, +static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, sysmmu_iova_t iova, unsigned int num_inv) { unsigned int i; for (i = 0; i < num_inv; i++) { __raw_writel((iova & SPAGE_MASK) | 1, - sfrbase + REG_MMU_FLUSH_ENTRY); + data->sfrbase + REG_MMU_FLUSH_ENTRY); iova += SPAGE_SIZE; } } -static void __sysmmu_set_ptbase(void __iomem *sfrbase, - phys_addr_t pgd) +static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd) { - __raw_writel(pgd, sfrbase + REG_PT_BASE_ADDR); + __raw_writel(pgd, data->sfrbase + REG_PT_BASE_ADDR); - __sysmmu_tlb_invalidate(sfrbase); + __sysmmu_tlb_invalidate(data); } static void show_fault_information(const char *name, @@ -363,7 +362,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) __raw_writel(1 << itype, data->sfrbase + REG_INT_CLEAR); - sysmmu_unblock(data->sfrbase); + sysmmu_unblock(data); clk_disable(data->clk_master); @@ -440,7 +439,7 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) __sysmmu_init_config(data); - __sysmmu_set_ptbase(data->sfrbase, data->pgtable); + __sysmmu_set_ptbase(data, data->pgtable); __raw_writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); @@ -521,10 +520,9 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, if (MMU_MAJ_VER(data->version) == 2) num_inv = min_t(unsigned int, size / PAGE_SIZE, 64); - if (sysmmu_block(data->sfrbase)) { - __sysmmu_tlb_invalidate_entry( - data->sfrbase, iova, num_inv); - sysmmu_unblock(data->sfrbase); + if (sysmmu_block(data)) { + __sysmmu_tlb_invalidate_entry(data, iova, num_inv); + sysmmu_unblock(data); } clk_disable(data->clk_master); } else { -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 00/13] SYSMMU driver update and support for Exynos 5433
Hello, This patchset updates Exynos SYSMMU (IOMMU) driver to make use of the new features in the IOMMU core (support for IOMMU_DOMAIN_DMA) and adds support for SYSMMU v5 controllers, which are available in Samsung Exynos 5433 SoCs. The driver has been also updated to compile and work on ARM64 architecture. Best regards Marek Szyprowski Samsung R&D Institute Poland Changelog: v2: - added support for multiple calls of device_attach (without detach), needed for default domain handling in iommu core (patch no 13), more information in the following thread: https://lists.linaro.org/pipermail/linaro-mm-sig/2016-February/004625.html - fixed support for SYSMMU controllers with bogus version register value (patch no 9) v1: http://www.spinics.net/lists/arm-kernel/msg483531.html - initial version Patch summary: Marek Szyprowski (13): iommu: exynos: rework iommu group initialization iommu: exynos: add support for IOMMU_DOMAIN_DMA domain type iommu: exynos: remove ARM-specific cache flush interface iommu: exynos: simplify master clock operations iommu: exynos: refactor code (no direct register access) iommu: exynos: refactor fault handling code iommu: exynos: refactor init config code iommu: exynos: unify code for fldp cache invalidation iommu: exynos: add support for SYSMMU controller with bogus version reg iommu: exynos: update device tree documentation iommu: exynos: add support for v5 SYSMMU iommu: exynos: add Maintainers entry for Exynos SYSMMU driver iommu: exynos: support multiple attach_device calls .../devicetree/bindings/iommu/samsung,sysmmu.txt | 22 +- MAINTAINERS| 6 + drivers/iommu/Kconfig | 2 +- drivers/iommu/exynos-iommu.c | 598 - 4 files changed, 372 insertions(+), 256 deletions(-) -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 08/13] iommu: exynos: unify code for fldp cache invalidation
This patch simplifies the code for handling of flpdcache invalidation. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 15787a177a16..e42a76cc9674 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -440,13 +440,6 @@ static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable, return ret; } -static void __sysmmu_tlb_invalidate_flpdcache(struct sysmmu_drvdata *data, - sysmmu_iova_t iova) -{ - if (data->version == MAKE_MMU_VER(3, 3)) - __raw_writel(iova | 0x1, data->sfrbase + REG_MMU_FLUSH_ENTRY); -} - static void sysmmu_tlb_invalidate_flpdcache(struct sysmmu_drvdata *data, sysmmu_iova_t iova) { @@ -455,8 +448,10 @@ static void sysmmu_tlb_invalidate_flpdcache(struct sysmmu_drvdata *data, clk_enable(data->clk_master); spin_lock_irqsave(&data->lock, flags); - if (is_sysmmu_active(data)) - __sysmmu_tlb_invalidate_flpdcache(data, iova); + if (is_sysmmu_active(data)) { + if (data->version >= MAKE_MMU_VER(3, 3)) + __sysmmu_tlb_invalidate_entry(data, iova, 1); + } spin_unlock_irqrestore(&data->lock, flags); clk_disable(data->clk_master); -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 01/13] iommu: exynos: rework iommu group initialization
This patch replaces custom code in add_device implementation with iommu_group_get_for_dev() call and provides the needed callback. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 27 --- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 97c41b8ab5d9..4fc079073c86 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1114,28 +1114,32 @@ static phys_addr_t exynos_iommu_iova_to_phys(struct iommu_domain *iommu_domain, return phys; } +static struct iommu_group *get_device_iommu_group(struct device *dev) +{ + struct iommu_group *group; + + group = iommu_group_get(dev); + if (!group) + group = iommu_group_alloc(); + + return group; +} + static int exynos_iommu_add_device(struct device *dev) { struct iommu_group *group; - int ret; if (!has_sysmmu(dev)) return -ENODEV; - group = iommu_group_get(dev); + group = iommu_group_get_for_dev(dev); - if (!group) { - group = iommu_group_alloc(); - if (IS_ERR(group)) { - dev_err(dev, "Failed to allocate IOMMU group\n"); - return PTR_ERR(group); - } - } + if (IS_ERR(group)) + return PTR_ERR(group); - ret = iommu_group_add_device(group, dev); iommu_group_put(group); - return ret; + return 0; } static void exynos_iommu_remove_device(struct device *dev) @@ -1182,6 +1186,7 @@ static struct iommu_ops exynos_iommu_ops = { .unmap = exynos_iommu_unmap, .map_sg = default_iommu_map_sg, .iova_to_phys = exynos_iommu_iova_to_phys, + .device_group = get_device_iommu_group, .add_device = exynos_iommu_add_device, .remove_device = exynos_iommu_remove_device, .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE, -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 09/13] iommu: exynos: add support for SYSMMU controller with bogus version reg
SYSMMU on some SoCs reports bogus values in VERSION register. Force hardware version to 1.0 for such controllers. This patch also moves reading version register to driver's probe() function. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index e42a76cc9674..8e289e2a05fb 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -284,6 +284,28 @@ static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd) __sysmmu_tlb_invalidate(data); } +static void __sysmmu_get_version(struct sysmmu_drvdata *data) +{ + u32 ver; + + clk_enable(data->clk_master); + clk_enable(data->clk); + + ver = __raw_readl(data->sfrbase + REG_MMU_VERSION); + + /* controllers on some SoCs don't report proper version */ + if (ver == 0x8001u) + data->version = MAKE_MMU_VER(1, 0); + else + data->version = MMU_RAW_VER(ver); + + dev_dbg(data->sysmmu, "hardware version: %d.%d\n", + MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version)); + + clk_disable(data->clk); + clk_disable(data->clk_master); +} + static void show_fault_information(struct sysmmu_drvdata *data, const struct sysmmu_fault_info *finfo, sysmmu_iova_t fault_addr) @@ -385,7 +407,6 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data) { unsigned int cfg; - data->version = MMU_RAW_VER(__raw_readl(data->sfrbase + REG_MMU_VERSION)); if (data->version <= MAKE_MMU_VER(3, 1)) cfg = CFG_LRU | CFG_QOS(15); else if (data->version <= MAKE_MMU_VER(3, 2)) @@ -551,6 +572,7 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) platform_set_drvdata(pdev, data); + __sysmmu_get_version(data); pm_runtime_enable(dev); return 0; -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 04/13] iommu: exynos: simplify master clock operations
All clock API function can be called on NULL clock, so simplify code avoid checking of master clock presence. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 32 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 8c8a7f7968d1..bf6b826b1d8b 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -333,8 +333,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) spin_lock(&data->lock); - if (!IS_ERR(data->clk_master)) - clk_enable(data->clk_master); + clk_enable(data->clk_master); itype = (enum exynos_sysmmu_inttype) __ffs(__raw_readl(data->sfrbase + REG_INT_STATUS)); @@ -366,8 +365,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) sysmmu_unblock(data->sfrbase); - if (!IS_ERR(data->clk_master)) - clk_disable(data->clk_master); + clk_disable(data->clk_master); spin_unlock(&data->lock); @@ -376,15 +374,13 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data) { - if (!IS_ERR(data->clk_master)) - clk_enable(data->clk_master); + clk_enable(data->clk_master); __raw_writel(CTRL_DISABLE, data->sfrbase + REG_MMU_CTRL); __raw_writel(0, data->sfrbase + REG_MMU_CFG); clk_disable(data->clk); - if (!IS_ERR(data->clk_master)) - clk_disable(data->clk_master); + clk_disable(data->clk_master); } static bool __sysmmu_disable(struct sysmmu_drvdata *data) @@ -437,8 +433,7 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data) static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) { - if (!IS_ERR(data->clk_master)) - clk_enable(data->clk_master); + clk_enable(data->clk_master); clk_enable(data->clk); __raw_writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL); @@ -449,8 +444,7 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) __raw_writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); - if (!IS_ERR(data->clk_master)) - clk_disable(data->clk_master); + clk_disable(data->clk_master); } static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable, @@ -493,16 +487,14 @@ static void sysmmu_tlb_invalidate_flpdcache(struct sysmmu_drvdata *data, { unsigned long flags; - if (!IS_ERR(data->clk_master)) - clk_enable(data->clk_master); + clk_enable(data->clk_master); spin_lock_irqsave(&data->lock, flags); if (is_sysmmu_active(data)) __sysmmu_tlb_invalidate_flpdcache(data, iova); spin_unlock_irqrestore(&data->lock, flags); - if (!IS_ERR(data->clk_master)) - clk_disable(data->clk_master); + clk_disable(data->clk_master); } static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, @@ -514,8 +506,7 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, if (is_sysmmu_active(data)) { unsigned int num_inv = 1; - if (!IS_ERR(data->clk_master)) - clk_enable(data->clk_master); + clk_enable(data->clk_master); /* * L2TLB invalidation required @@ -535,8 +526,7 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, data->sfrbase, iova, num_inv); sysmmu_unblock(data->sfrbase); } - if (!IS_ERR(data->clk_master)) - clk_disable(data->clk_master); + clk_disable(data->clk_master); } else { dev_dbg(data->master, "disabled. Skipping TLB invalidation @ %#x\n", iova); @@ -593,6 +583,8 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) dev_err(dev, "Failed to prepare master's clk\n"); return ret; } + } else { + data->clk_master = NULL; } data->sysmmu = dev; -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 07/13] iommu: exynos: refactor init config code
This patch rewrites sysmmu_init_config function to make it easier to read and understand. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 25 + 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 3a577a473f3c..15787a177a16 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -383,24 +383,17 @@ static bool __sysmmu_disable(struct sysmmu_drvdata *data) static void __sysmmu_init_config(struct sysmmu_drvdata *data) { - unsigned int cfg = CFG_LRU | CFG_QOS(15); - unsigned int ver; - - ver = MMU_RAW_VER(__raw_readl(data->sfrbase + REG_MMU_VERSION)); - if (MMU_MAJ_VER(ver) == 3) { - if (MMU_MIN_VER(ver) >= 2) { - cfg |= CFG_FLPDCACHE; - if (MMU_MIN_VER(ver) == 3) { - cfg |= CFG_ACGEN; - cfg &= ~CFG_LRU; - } else { - cfg |= CFG_SYSSEL; - } - } - } + unsigned int cfg; + + data->version = MMU_RAW_VER(__raw_readl(data->sfrbase + REG_MMU_VERSION)); + if (data->version <= MAKE_MMU_VER(3, 1)) + cfg = CFG_LRU | CFG_QOS(15); + else if (data->version <= MAKE_MMU_VER(3, 2)) + cfg = CFG_LRU | CFG_QOS(15) | CFG_FLPDCACHE | CFG_SYSSEL; + else + cfg = CFG_QOS(15) | CFG_FLPDCACHE | CFG_ACGEN; __raw_writel(cfg, data->sfrbase + REG_MMU_CFG); - data->version = ver; } static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 02/13] iommu: exynos: add support for IOMMU_DOMAIN_DMA domain type
This patch adds support for DMA domain type. Such domain have DMA cookie prepared and can be used by generic DMA-IOMMU glue layer. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 4fc079073c86..595e0da55db4 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -25,9 +25,9 @@ #include #include #include +#include #include -#include #include typedef u32 sysmmu_iova_t; @@ -662,16 +662,21 @@ static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type) struct exynos_iommu_domain *domain; int i; - if (type != IOMMU_DOMAIN_UNMANAGED) - return NULL; domain = kzalloc(sizeof(*domain), GFP_KERNEL); if (!domain) return NULL; + if (type == IOMMU_DOMAIN_DMA) { + if (iommu_get_dma_cookie(&domain->domain) != 0) + goto err_pgtable; + } else if (type != IOMMU_DOMAIN_UNMANAGED) { + goto err_pgtable; + } + domain->pgtable = (sysmmu_pte_t *)__get_free_pages(GFP_KERNEL, 2); if (!domain->pgtable) - goto err_pgtable; + goto err_dma_cookie; domain->lv2entcnt = (short *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1); if (!domain->lv2entcnt) @@ -703,6 +708,9 @@ static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type) err_counter: free_pages((unsigned long)domain->pgtable, 2); +err_dma_cookie: + if (type == IOMMU_DOMAIN_DMA) + iommu_put_dma_cookie(&domain->domain); err_pgtable: kfree(domain); return NULL; @@ -727,6 +735,9 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain) spin_unlock_irqrestore(&domain->lock, flags); + if (iommu_domain->type == IOMMU_DOMAIN_DMA) + iommu_put_dma_cookie(iommu_domain); + for (i = 0; i < NUM_LV1ENTRIES; i++) if (lv1ent_page(domain->pgtable + i)) kmem_cache_free(lv2table_kmem_cache, -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 11/13] iommu: exynos: add support for v5 SYSMMU
This patch adds support for v5 of SYSMMU controller, found in Samsung Exynos 5433 SoCs. The main difference of v5 is support for 36-bit physical address space and some changes in register layout and core clocks hanging. This patch also adds support for ARM64 architecture, which is used by Exynos 5433 SoCs. Signed-off-by: Marek Szyprowski --- .../devicetree/bindings/iommu/samsung,sysmmu.txt | 5 +- drivers/iommu/Kconfig | 2 +- drivers/iommu/exynos-iommu.c | 187 +++-- 3 files changed, 143 insertions(+), 51 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt index f61ca25ca136..85f068805dd8 100644 --- a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt +++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt @@ -35,9 +35,10 @@ Required properties: - interrupts: An interrupt specifier for interrupt signal of System MMU, according to the format defined by a particular interrupt controller. -- clock-names: Should be "sysmmu" if the System MMU is needed to gate its clock. +- clock-names: Should be "sysmmu" or a pair of "aclk" and "pclk" to gate + SYSMMU core clocks. Optional "master" if the clock to the System MMU is gated by - another gate clock other than "sysmmu" (usually main gate clock + another gate clock other core (usually main gate clock of peripheral device this SYSMMU belongs to). - clocks: Phandles for respective clocks described by clock-names. - power-domains: Required if the System MMU is needed to gate its power. diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index a1e75cba18e0..1674de1cfed0 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -243,7 +243,7 @@ config TEGRA_IOMMU_SMMU config EXYNOS_IOMMU bool "Exynos IOMMU Support" - depends on ARCH_EXYNOS && ARM && MMU + depends on ARCH_EXYNOS && MMU select IOMMU_API select ARM_DMA_USE_IOMMU help diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 8e289e2a05fb..9c8ce951158d 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1,6 +1,5 @@ -/* linux/drivers/iommu/exynos_iommu.c - * - * Copyright (c) 2011 Samsung Electronics Co., Ltd. +/* + * Copyright (c) 2011,2016 Samsung Electronics Co., Ltd. * http://www.samsung.com * * This program is free software; you can redistribute it and/or modify @@ -55,17 +54,25 @@ typedef u32 sysmmu_pte_t; #define lv2ent_small(pent) ((*(pent) & 2) == 2) #define lv2ent_large(pent) ((*(pent) & 3) == 1) -static u32 sysmmu_page_offset(sysmmu_iova_t iova, u32 size) -{ - return iova & (size - 1); -} - -#define section_phys(sent) (*(sent) & SECT_MASK) -#define section_offs(iova) sysmmu_page_offset((iova), SECT_SIZE) -#define lpage_phys(pent) (*(pent) & LPAGE_MASK) -#define lpage_offs(iova) sysmmu_page_offset((iova), LPAGE_SIZE) -#define spage_phys(pent) (*(pent) & SPAGE_MASK) -#define spage_offs(iova) sysmmu_page_offset((iova), SPAGE_SIZE) +/* + * v1.x - v3.x SYSMMU supports 32bit physical and 32bit virtual address spaces + * v5.0 introduced support for 36bit physical address space by shifting + * all page entry values by 4 bits. + * All SYSMMU controllers in the system support the address spaces of the same + * size, so PG_ENT_SHIFT can be initialized on first SYSMMU probe to proper + * value (0 or 4). + */ +static short PG_ENT_SHIFT = -1; +#define SYSMMU_PG_ENT_SHIFT 0 +#define SYSMMU_V5_PG_ENT_SHIFT 4 + +#define sect_to_phys(ent) (((phys_addr_t) ent) << PG_ENT_SHIFT) +#define section_phys(sent) (sect_to_phys(*(sent)) & SECT_MASK) +#define section_offs(iova) (iova & (SECT_SIZE - 1)) +#define lpage_phys(pent) (sect_to_phys(*(pent)) & LPAGE_MASK) +#define lpage_offs(iova) (iova & (LPAGE_SIZE - 1)) +#define spage_phys(pent) (sect_to_phys(*(pent)) & SPAGE_MASK) +#define spage_offs(iova) (iova & (SPAGE_SIZE - 1)) #define NUM_LV1ENTRIES 4096 #define NUM_LV2ENTRIES (SECT_SIZE / SPAGE_SIZE) @@ -84,13 +91,12 @@ static u32 lv2ent_offset(sysmmu_iova_t iova) #define LV2TABLE_SIZE (NUM_LV2ENTRIES * sizeof(sysmmu_pte_t)) #define SPAGES_PER_LPAGE (LPAGE_SIZE / SPAGE_SIZE) +#define lv2table_base(sent) (sect_to_phys(*(sent) & 0xFFC0)) -#define lv2table_base(sent) (*(sent) & 0xFC00) - -#define mk_lv1ent_sect(pa) ((pa) | 2) -#define mk_lv1ent_page(pa) ((pa) | 1) -#define mk_lv2ent_lpage(pa) ((pa) | 1) -#define mk_lv2ent_spage(pa) ((pa) | 2) +#define mk_lv1ent_sect(pa) ((pa >> PG_ENT_SHIFT) | 2) +#define mk_lv1ent_page(pa) ((pa >> PG_ENT_SHIFT) | 1) +#define mk_lv2ent_lpage(pa) ((pa >> PG_ENT_SHIFT) | 1) +#de
[PATCH v2 13/13] iommu: exynos: support multiple attach_device calls
IOMMU core calls attach_device callback without detaching device from the previous domain. This patch adds support for such unballanced calls. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 72 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 9c8ce951158d..b0665042bf29 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -201,6 +201,7 @@ static const struct sysmmu_fault_info sysmmu_v5_faults[] = { */ struct exynos_iommu_owner { struct list_head controllers; /* list of sysmmu_drvdata.owner_node */ + struct iommu_domain *domain;/* domain this device is attached */ }; /* @@ -825,6 +826,41 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain) kfree(domain); } +static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, + struct device *dev) +{ + struct exynos_iommu_owner *owner = dev->archdata.iommu; + struct exynos_iommu_domain *domain = to_exynos_domain(iommu_domain); + phys_addr_t pagetable = virt_to_phys(domain->pgtable); + struct sysmmu_drvdata *data, *next; + unsigned long flags; + bool found = false; + + if (!has_sysmmu(dev) || owner->domain != iommu_domain) + return; + + spin_lock_irqsave(&domain->lock, flags); + list_for_each_entry_safe(data, next, &domain->clients, domain_node) { + if (data->master == dev) { + if (__sysmmu_disable(data)) { + data->master = NULL; + list_del_init(&data->domain_node); + } + pm_runtime_put(data->sysmmu); + found = true; + } + } + spin_unlock_irqrestore(&domain->lock, flags); + + owner->domain = NULL; + + if (found) + dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", + __func__, &pagetable); + else + dev_err(dev, "%s: No IOMMU is attached\n", __func__); +} + static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, struct device *dev) { @@ -838,6 +874,9 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, if (!has_sysmmu(dev)) return -ENODEV; + if (owner->domain) + exynos_iommu_detach_device(owner->domain, dev); + list_for_each_entry(data, &owner->controllers, owner_node) { pm_runtime_get_sync(data->sysmmu); ret = __sysmmu_enable(data, pagetable, domain); @@ -856,44 +895,13 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, return ret; } + owner->domain = iommu_domain; dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa %s\n", __func__, &pagetable, (ret == 0) ? "" : ", again"); return ret; } -static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, - struct device *dev) -{ - struct exynos_iommu_domain *domain = to_exynos_domain(iommu_domain); - phys_addr_t pagetable = virt_to_phys(domain->pgtable); - struct sysmmu_drvdata *data, *next; - unsigned long flags; - bool found = false; - - if (!has_sysmmu(dev)) - return; - - spin_lock_irqsave(&domain->lock, flags); - list_for_each_entry_safe(data, next, &domain->clients, domain_node) { - if (data->master == dev) { - if (__sysmmu_disable(data)) { - data->master = NULL; - list_del_init(&data->domain_node); - } - pm_runtime_put(data->sysmmu); - found = true; - } - } - spin_unlock_irqrestore(&domain->lock, flags); - - if (found) - dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", - __func__, &pagetable); - else - dev_err(dev, "%s: No IOMMU is attached\n", __func__); -} - static sysmmu_pte_t *alloc_lv2entry(struct exynos_iommu_domain *domain, sysmmu_pte_t *sent, sysmmu_iova_t iova, short *pgcounter) { -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 06/13] iommu: exynos: refactor fault handling code
This patch provides a new implementation for page fault handing code. The new implementation is ready for future extensions. No functional changes have been made. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 109 --- 1 file changed, 41 insertions(+), 68 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 4275222cead3..3a577a473f3c 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -148,40 +148,25 @@ static sysmmu_pte_t *page_entry(sysmmu_pte_t *sent, sysmmu_iova_t iova) lv2table_base(sent)) + lv2ent_offset(iova); } -enum exynos_sysmmu_inttype { - SYSMMU_PAGEFAULT, - SYSMMU_AR_MULTIHIT, - SYSMMU_AW_MULTIHIT, - SYSMMU_BUSERROR, - SYSMMU_AR_SECURITY, - SYSMMU_AR_ACCESS, - SYSMMU_AW_SECURITY, - SYSMMU_AW_PROTECTION, /* 7 */ - SYSMMU_FAULT_UNKNOWN, - SYSMMU_FAULTS_NUM -}; - -static unsigned short fault_reg_offset[SYSMMU_FAULTS_NUM] = { - REG_PAGE_FAULT_ADDR, - REG_AR_FAULT_ADDR, - REG_AW_FAULT_ADDR, - REG_DEFAULT_SLAVE_ADDR, - REG_AR_FAULT_ADDR, - REG_AR_FAULT_ADDR, - REG_AW_FAULT_ADDR, - REG_AW_FAULT_ADDR +/* + * IOMMU fault information register + */ +struct sysmmu_fault_info { + unsigned int bit; /* bit number in STATUS register */ + unsigned short addr_reg; /* register to read VA fault address */ + const char *name; /* human readable fault name */ + unsigned int type; /* fault type for report_iommu_fault */ }; -static char *sysmmu_fault_name[SYSMMU_FAULTS_NUM] = { - "PAGE FAULT", - "AR MULTI-HIT FAULT", - "AW MULTI-HIT FAULT", - "BUS ERROR", - "AR SECURITY PROTECTION FAULT", - "AR ACCESS PROTECTION FAULT", - "AW SECURITY PROTECTION FAULT", - "AW ACCESS PROTECTION FAULT", - "UNKNOWN FAULT" +static const struct sysmmu_fault_info sysmmu_faults[] = { + { 0, REG_PAGE_FAULT_ADDR, "PAGE", IOMMU_FAULT_READ }, + { 1, REG_AR_FAULT_ADDR, "AR MULTI-HIT", IOMMU_FAULT_READ }, + { 2, REG_AW_FAULT_ADDR, "AW MULTI-HIT", IOMMU_FAULT_WRITE }, + { 3, REG_DEFAULT_SLAVE_ADDR, "BUS ERROR", IOMMU_FAULT_READ }, + { 4, REG_AR_FAULT_ADDR, "AR SECURITY PROTECTION", IOMMU_FAULT_READ }, + { 5, REG_AR_FAULT_ADDR, "AR ACCESS PROTECTION", IOMMU_FAULT_READ }, + { 6, REG_AW_FAULT_ADDR, "AW SECURITY PROTECTION", IOMMU_FAULT_WRITE }, + { 7, REG_AW_FAULT_ADDR, "AW ACCESS PROTECTION", IOMMU_FAULT_WRITE }, }; /* @@ -299,24 +284,19 @@ static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd) __sysmmu_tlb_invalidate(data); } -static void show_fault_information(const char *name, - enum exynos_sysmmu_inttype itype, - phys_addr_t pgtable_base, sysmmu_iova_t fault_addr) +static void show_fault_information(struct sysmmu_drvdata *data, + const struct sysmmu_fault_info *finfo, + sysmmu_iova_t fault_addr) { sysmmu_pte_t *ent; - if ((itype >= SYSMMU_FAULTS_NUM) || (itype < SYSMMU_PAGEFAULT)) - itype = SYSMMU_FAULT_UNKNOWN; - - pr_err("%s occurred at %#x by %s(Page table base: %pa)\n", - sysmmu_fault_name[itype], fault_addr, name, &pgtable_base); - - ent = section_entry(phys_to_virt(pgtable_base), fault_addr); - pr_err("\tLv1 entry: %#x\n", *ent); - + dev_err(data->sysmmu, "%s FAULT occurred at %#x (page table base: %pa)\n", + finfo->name, fault_addr, &data->pgtable); + ent = section_entry(phys_to_virt(data->pgtable), fault_addr); + dev_err(data->sysmmu, "\tLv1 entry: %#x\n", *ent); if (lv1ent_page(ent)) { ent = page_entry(ent, fault_addr); - pr_err("\t Lv2 entry: %#x\n", *ent); + dev_err(data->sysmmu, "\t Lv2 entry: %#x\n", *ent); } } @@ -324,8 +304,10 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) { /* SYSMMU is in blocked state when interrupt occurred. */ struct sysmmu_drvdata *data = dev_id; - enum exynos_sysmmu_inttype itype; - sysmmu_iova_t addr = -1; + const struct sysmmu_fault_info *finfo = sysmmu_faults; + int i, n = ARRAY_SIZE(sysmmu_faults); + unsigned int itype; + sysmmu_iova_t fault_addr = -1; int ret = -ENOSYS; WARN_ON(!is_sysmmu_active(data)); @@ -334,29 +316,20 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) clk_enable(data->clk_master); - itype = (enum exyno
[PATCH v2 10/13] iommu: exynos: update device tree documentation
Exynos SYSMMU bindings documentation was merged before generic IOMMU binding have been introduced. This patch updates documentation to match current state. Signed-off-by: Marek Szyprowski --- .../devicetree/bindings/iommu/samsung,sysmmu.txt | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt index bc620fe32a70..f61ca25ca136 100644 --- a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt +++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt @@ -23,28 +23,23 @@ MMUs. for window 1, 2 and 3. * M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and the other System MMU on the write channel. -The drivers must consider how to handle those System MMUs. One of the idea is -to implement child devices or sub-devices which are the client devices of the -System MMU. -Note: -The current DT binding for the Exynos System MMU is incomplete. -The following properties can be removed or changed, if found incompatible with -the "Generic IOMMU Binding" support for attaching devices to the IOMMU. +For information on assigning System MMU controller to its peripheral devices, +see generic IOMMU bindings. Required properties: - compatible: Should be "samsung,exynos-sysmmu" - reg: A tuple of base address and size of System MMU registers. +- #iommu-cells: Should be <0>. - interrupt-parent: The phandle of the interrupt controller of System MMU - interrupts: An interrupt specifier for interrupt signal of System MMU, according to the format defined by a particular interrupt controller. - clock-names: Should be "sysmmu" if the System MMU is needed to gate its clock. Optional "master" if the clock to the System MMU is gated by - another gate clock other than "sysmmu". - Exynos4 SoCs, there needs no "master" clock. - Exynos5 SoCs, some System MMUs must have "master" clocks. -- clocks: Required if the System MMU is needed to gate its clock. + another gate clock other than "sysmmu" (usually main gate clock + of peripheral device this SYSMMU belongs to). +- clocks: Phandles for respective clocks described by clock-names. - power-domains: Required if the System MMU is needed to gate its power. Please refer to the following document: Documentation/devicetree/bindings/power/pd-samsung.txt @@ -57,6 +52,7 @@ Examples: power-domains = <&pd_gsc>; clocks = <&clock CLK_GSCL0>; clock-names = "gscl"; + iommus = <&sysmmu_gsc0>; }; sysmmu_gsc0: sysmmu@13E8 { @@ -67,4 +63,5 @@ Examples: clock-names = "sysmmu", "master"; clocks = <&clock CLK_SMMU_GSCL0>, <&clock CLK_GSCL0>; power-domains = <&pd_gsc>; + #iommu-cells = <0>; }; -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 12/13] iommu: exynos: add Maintainers entry for Exynos SYSMMU driver
Add Marek Szyprowski as maintainer for Exynos IOMMU driver. Signed-off-by: Marek Szyprowski Acked-by: Krzysztof Kozlowski --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index cc2f753cb357..4afc061e878a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4305,6 +4305,12 @@ L: dri-de...@lists.freedesktop.org S: Maintained F: drivers/gpu/drm/exynos/exynos_dp* +EXYNOS SYSMMU (IOMMU) driver +M: Marek Szyprowski +L: iommu@lists.linux-foundation.org +S: Maintained +F: drivers/iommu/exynos-iommu.c + EXYNOS MIPI DISPLAY DRIVERS M: Inki Dae M: Donghwa Lee -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC 1/3] drm/exynos: rewrite IOMMU support code
This patch replaces usage of ARM-specific IOMMU/DMA-mapping related calls with new generic code for managing DMA-IOMMU integration layer. It also removes all the hacks, which were needed to configure common DMA/IO address space on the virtual exynos-drm device. Since moving Exynos GEM code to use on of real devices for DMA-mapping operations, such hacks are no longer needed. The only requirement is to have all the devices, which build Exynos DRM, attached to the same IOMMU domain (to share IO address space). Signed-off-by: Marek Szyprowski --- drivers/gpu/drm/exynos/Kconfig| 2 +- drivers/gpu/drm/exynos/exynos_drm_drv.c | 7 +-- drivers/gpu/drm/exynos/exynos_drm_drv.h | 2 +- drivers/gpu/drm/exynos/exynos_drm_iommu.c | 91 +++ drivers/gpu/drm/exynos/exynos_drm_iommu.h | 2 +- 5 files changed, 59 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig index 83efca941388..b0d0aaa7fea5 100644 --- a/drivers/gpu/drm/exynos/Kconfig +++ b/drivers/gpu/drm/exynos/Kconfig @@ -15,7 +15,7 @@ if DRM_EXYNOS config DRM_EXYNOS_IOMMU bool - depends on EXYNOS_IOMMU && ARM_DMA_USE_IOMMU + depends on EXYNOS_IOMMU && IOMMU_DMA default y comment "CRTCs" diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index c7fce7ffeef5..45aa480f1890 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -159,12 +159,7 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags) DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n", dev_name(private->dma_dev)); - /* -* create mapping to manage iommu table and set a pointer to iommu -* mapping structure to iommu_mapping of private data. -* also this iommu_mapping can be used to check if iommu is supported -* or not. -*/ + /* create common IOMMU mapping for all devices attached to Exynos DRM */ ret = drm_create_iommu_mapping(dev); if (ret < 0) { DRM_ERROR("failed to create iommu mapping.\n"); diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index 303056311c0c..b107f77d0897 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -222,7 +222,7 @@ struct exynos_drm_private { struct device *dma_dev; unsigned long da_start; unsigned long da_space_size; - void *mapping; + struct iommu_domain *domain; unsigned int pipe; diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c index 146ac88078ae..89e51ed6499d 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c +++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c @@ -14,13 +14,28 @@ #include #include -#include - -#include +#include #include "exynos_drm_drv.h" #include "exynos_drm_iommu.h" +static inline int configure_dma_max_seg_size(struct device *dev) +{ + if (!dev->dma_parms) + dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL); + if (!dev->dma_parms) + return -ENOMEM; + + dma_set_max_seg_size(dev, DMA_BIT_MASK(32)); + return 0; +} + +static inline void clear_dma_max_seg_size(struct device *dev) +{ + kfree(dev->dma_parms); + dev->dma_parms = NULL; +} + /* * drm_create_iommu_mapping - create a mapping structure * @@ -28,38 +43,48 @@ */ int drm_create_iommu_mapping(struct drm_device *drm_dev) { - struct dma_iommu_mapping *mapping = NULL; struct exynos_drm_private *priv = drm_dev->dev_private; + struct device *dev = to_dma_dev(drm_dev); + int ret; if (!priv->da_start) priv->da_start = EXYNOS_DEV_ADDR_START; if (!priv->da_space_size) priv->da_space_size = EXYNOS_DEV_ADDR_SIZE; - mapping = arm_iommu_create_mapping(&platform_bus_type, priv->da_start, - priv->da_space_size); + priv->domain = iommu_domain_alloc(dev->bus); + if (!priv->domain) + return -ENOMEM; - if (IS_ERR(mapping)) - return PTR_ERR(mapping); + ret = iommu_get_dma_cookie(priv->domain); + if (ret) + goto free_domain; - priv->mapping = mapping; + ret = iommu_dma_init_domain(priv->domain, priv->da_start, + priv->da_space_size); + if (ret) + goto put_cookie; return 0; + +put_cookie: + iommu_put_dma_cookie(priv->domain); +free_domain: + iommu_domain_free(priv->domain); + return ret; } /* * drm_release_iommu_m
[RFC 0/3] Unify IOMMU-based DMA-mapping code for ARM and ARM64
Dear All, This is an initial RFC on the unification of IOMMU-based DMA-mapping code for ARM and ARM64 architectures. Right now ARM architecture still use my old code for IOMMU-based DMA-mapping glue, initially merged in commit 4ce63fcd919c32d22528e54dcd89506962933719 ("ARM: dma-mapping: add support for IOMMU mapper"). In meantime ARM64 got a new, slightly improved implementation provided by Robin Murphy in commit 13b8629f651164d71f4d38b821925f93ba4236c8 ("arm64: Add IOMMU dma_ops"). Both implementations are very similar thus their unification is desired to avoid duplicating future works and simplify code, which uses this layer on both architectures. In this patchset I've selected the new implementation (from ARM64 architecture) as a base. This means that ARM-specific, old interface (arm_iommu_* functions) for configuring IOMMU domains will be no longer available and its users have to be converted to new API. Besides lack of old interface, the second difference is additional requirements for IOMMU drivers. New code relies on the support for IOMMU_DOMAIN_DMA and default IOMMU domain, which is automatically attached by the IOMMU core. The new code also assumes that the IOMMU-based DMA-mapping ops are mainly configured from arch_setup_dma_ops() function, which means that the IOMMU driver should provide needed of_xlate callbacks and initialize IOMMU ops for device nodes. However it should be also possible to initialize IOMMU-based DMA-mapping ops for client devices directly from IOMMU drivers by calling common_iommu_setup_dma_ops() (some drivers used such approach). IOMMU drivers should be also aware of the fact that the default domain is attached via device_attach and then device_attach callback can be called once again with different domain without previous detach from default domain. For more information on this issue, see the following thread: https://lists.linaro.org/pipermail/linaro-mm-sig/2016-February/004625.html Currently there are 4 users of the old arm_iommu_* interface: 1. Exynos DRM driver 2. Rockchip DRM driver 3. OMAP3 ISP camera driver 4. Renesas VMSA-compatible IPMMU driver In this patchset I've converted Exynos DRM driver for the new API (patch 1). This required some changes in the memory management model inside the driver and removal of some hacks, which were used to setup IOMMU-based DMA-mapping ops on the 'exynos-drm' virtual device and common IOMMU domain for all Exynos DRM sub-devices, those changes have been posted separately here: http://www.spinics.net/lists/dri-devel/msg100861.html Rockchip DRM driver requires similar conversion. Converting OMAP3 ISP camera driver to new API requires adding support for IOMMU groups to OMAP IOMMU driver, because the new DMA/IOMMU code used IOMMU_DOMAIN_DMA type domains and default groups. Renesas IPMMU driver needs also to be extended with IOMMU_DOMAIN_DMA domain type support. It can also be prepared for IOMMU_OF_DECLARE and of_xlate callback-based initialization to let core to automatically setup of IOMMU-based DMA mapping implementation. Patch 2 moves existing code from arch/arm64 to drivers/iommu and introduces some minor changes in function names - mainly adding arch_ prefix to some dma-mapping internal functions, which stay in arch/arm64/ (functions of similar names are present in arch/arm). Patch 3 adapts ARM architecture for the common code. I would like to get your comments on the proposed approach. There is still some work that need to be done to convert remaining users of the old API and updating IOMMU drivers to the new API requirements. This change need to be tested on the all affected ARM sub-architectures. Right now patches were tested on only Exynos based boards: ARM 32bit: Exynos4412 and Exynos5422 boards and ARM 64 bit Exnyos 5433 (with some out-of-tree DTS). To ease testing I've prepared a branch with all the patches needed (there are all needed patches for Exynos subarch, which have been posted as separate patchsets): https://git.linaro.org/people/marek.szyprowski/linux-srpol.git v4.5-dma-iommu-unification Patches are based on Linux v4.5-rc4 vanilla tree. Best regards Marek Szyprowski Samsung R&D Institute Poland Patch summary: Marek Szyprowski (3): drm/exynos: rewrite IOMMU support code iommu: dma-iommu: move IOMMU/DMA-mapping code from ARM64 arch to drivers iommu: dma-iommu: use common implementation also on ARM architecture arch/arm/Kconfig | 22 +- arch/arm/include/asm/device.h |9 - arch/arm/include/asm/dma-iommu.h | 37 - arch/arm/include/asm/dma-mapping.h| 59 +- arch/arm/mm/dma-mapping.c | 1158 + arch/arm64/include/asm/dma-mapping.h | 39 +- arch/arm64/mm/dma-mapping.c | 491 +--- drivers/gpu/drm/exynos/Kconfig|2 +- drivers/gpu/drm/exynos/exynos_drm_drv.c |7 +- drivers/gpu/drm/exynos/exy
[RFC 2/3] iommu: dma-iommu: move IOMMU/DMA-mapping code from ARM64 arch to drivers
This patch moves all the IOMMU-based DMA-mapping code from arch/arm64/mm to drivers/iommu/dma-iommu-ops.c. This way it can be easily shared with ARM architecture, which will also use them. Signed-off-by: Marek Szyprowski --- arch/arm64/include/asm/dma-mapping.h | 39 ++- arch/arm64/mm/dma-mapping.c | 491 ++- drivers/iommu/Makefile | 2 +- drivers/iommu/dma-iommu-ops.c| 471 + include/linux/dma-iommu.h| 14 + 5 files changed, 538 insertions(+), 479 deletions(-) create mode 100644 drivers/iommu/dma-iommu-ops.c diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h index ba437f090a74..3a582d820717 100644 --- a/arch/arm64/include/asm/dma-mapping.h +++ b/arch/arm64/include/asm/dma-mapping.h @@ -22,6 +22,7 @@ #include #include +#include #include #define DMA_ERROR_CODE (~(dma_addr_t)0) @@ -47,14 +48,17 @@ static inline struct dma_map_ops *get_dma_ops(struct device *dev) return __generic_dma_ops(dev); } +static inline void arch_set_dma_ops(struct device *dev, struct dma_map_ops *ops) +{ + dev->archdata.dma_ops = ops; +} + void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, struct iommu_ops *iommu, bool coherent); #define arch_setup_dma_ops arch_setup_dma_ops -#ifdef CONFIG_IOMMU_DMA void arch_teardown_dma_ops(struct device *dev); #define arch_teardown_dma_ops arch_teardown_dma_ops -#endif /* do not use this function in a driver */ static inline bool is_device_dma_coherent(struct device *dev) @@ -86,5 +90,36 @@ static inline void dma_mark_clean(void *addr, size_t size) { } +static inline void arch_flush_page(struct device *dev, const void *virt, + phys_addr_t phys) +{ + __dma_flush_range(virt, virt + PAGE_SIZE); +} + +static inline void arch_dma_map_area(phys_addr_t phys, size_t size, +enum dma_data_direction dir) +{ + __dma_map_area(phys_to_virt(phys), size, dir); +} + +static inline void arch_dma_unmap_area(phys_addr_t phys, size_t size, + enum dma_data_direction dir) +{ + __dma_unmap_area(phys_to_virt(phys), size, dir); +} + +static inline pgprot_t arch_get_dma_pgprot(struct dma_attrs *attrs, + pgprot_t prot, bool coherent) +{ + if (!coherent || dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs)) + return pgprot_writecombine(prot); + return prot; +} + +extern void *arch_alloc_from_atomic_pool(size_t size, struct page **ret_page, +gfp_t flags); +extern bool arch_in_atomic_pool(void *start, size_t size); +extern int arch_free_from_atomic_pool(void *start, size_t size); + #endif /* __KERNEL__ */ #endif /* __ASM_DMA_MAPPING_H */ diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index a6e757cbab77..d8cb8552bbff 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -24,19 +24,12 @@ #include #include #include +#include #include #include #include -static pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot, -bool coherent) -{ - if (!coherent || dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs)) - return pgprot_writecombine(prot); - return prot; -} - static struct gen_pool *atomic_pool; #define DEFAULT_DMA_COHERENT_POOL_SIZE SZ_256K @@ -49,7 +42,7 @@ static int __init early_coherent_pool(char *p) } early_param("coherent_pool", early_coherent_pool); -static void *__alloc_from_pool(size_t size, struct page **ret_page, gfp_t flags) +void *arch_alloc_from_atomic_pool(size_t size, struct page **ret_page, gfp_t flags) { unsigned long val; void *ptr = NULL; @@ -71,14 +64,14 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page, gfp_t flags) return ptr; } -static bool __in_atomic_pool(void *start, size_t size) +bool arch_in_atomic_pool(void *start, size_t size) { return addr_in_gen_pool(atomic_pool, (unsigned long)start, size); } -static int __free_from_pool(void *start, size_t size) +int arch_free_from_atomic_pool(void *start, size_t size) { - if (!__in_atomic_pool(start, size)) + if (!arch_in_atomic_pool(start, size)) return 0; gen_pool_free(atomic_pool, (unsigned long)start, size); @@ -142,13 +135,13 @@ static void *__dma_alloc(struct device *dev, size_t size, struct page *page; void *ptr, *coherent_ptr; bool coherent = is_device_dma_coherent(dev); - pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, false); + pgprot_t prot = arch_get_dma_pgprot(attrs, PAGE_KERNEL, false); size = PAGE_ALIGN(size); if (!coherent && !gfpflags_allow_blocking(flags)) {
[RFC 3/3] iommu: dma-iommu: use common implementation also on ARM architecture
This patch replaces ARM-specific IOMMU-based DMA-mapping implementation with generic IOMMU DMA-mapping code shared with ARM64 architecture. The side-effect of this change is a switch from bitmap-based IO address space management to tree-based code. There should be no functional changes for drivers, which rely on initialization from generic arch_setup_dna_ops() interface. Code, which used old arm_iommu_* functions must be updated to new interface. Signed-off-by: Marek Szyprowski --- arch/arm/Kconfig | 22 +- arch/arm/include/asm/device.h |9 - arch/arm/include/asm/dma-iommu.h | 37 -- arch/arm/include/asm/dma-mapping.h | 59 +- arch/arm/mm/dma-mapping.c | 1158 +--- drivers/gpu/drm/rockchip/Kconfig |1 + drivers/iommu/Kconfig |1 + drivers/media/platform/Kconfig |1 + 8 files changed, 82 insertions(+), 1206 deletions(-) delete mode 100644 arch/arm/include/asm/dma-iommu.h diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 4f799e567fc8..ed45f0d63cee 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -107,27 +107,7 @@ config ARM_DMA_USE_IOMMU bool select ARM_HAS_SG_CHAIN select NEED_SG_DMA_LENGTH - -if ARM_DMA_USE_IOMMU - -config ARM_DMA_IOMMU_ALIGNMENT - int "Maximum PAGE_SIZE order of alignment for DMA IOMMU buffers" - range 4 9 - default 8 - help - DMA mapping framework by default aligns all buffers to the smallest - PAGE_SIZE order which is greater than or equal to the requested buffer - size. This works well for buffers up to a few hundreds kilobytes, but - for larger buffers it just a waste of address space. Drivers which has - relatively small addressing window (like 64Mib) might run out of - virtual space with just a few allocations. - - With this parameter you can specify the maximum PAGE_SIZE order for - DMA IOMMU buffers. Larger buffers will be aligned only to this - specified order. The order is expressed as a power of two multiplied - by the PAGE_SIZE. - -endif + select IOMMU_DMA config MIGHT_HAVE_PCI bool diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h index 4111592f0130..6ea939c39cd4 100644 --- a/arch/arm/include/asm/device.h +++ b/arch/arm/include/asm/device.h @@ -14,9 +14,6 @@ struct dev_archdata { #ifdef CONFIG_IOMMU_API void *iommu; /* private IOMMU data */ #endif -#ifdef CONFIG_ARM_DMA_USE_IOMMU - struct dma_iommu_mapping*mapping; -#endif bool dma_coherent; }; @@ -28,10 +25,4 @@ struct pdev_archdata { #endif }; -#ifdef CONFIG_ARM_DMA_USE_IOMMU -#define to_dma_iommu_mapping(dev) ((dev)->archdata.mapping) -#else -#define to_dma_iommu_mapping(dev) NULL -#endif - #endif diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h deleted file mode 100644 index 2ef282f96651.. --- a/arch/arm/include/asm/dma-iommu.h +++ /dev/null @@ -1,37 +0,0 @@ -#ifndef ASMARM_DMA_IOMMU_H -#define ASMARM_DMA_IOMMU_H - -#ifdef __KERNEL__ - -#include -#include -#include -#include -#include - -struct dma_iommu_mapping { - /* iommu specific data */ - struct iommu_domain *domain; - - unsigned long **bitmaps; /* array of bitmaps */ - unsigned intnr_bitmaps; /* nr of elements in array */ - unsigned intextensions; - size_t bitmap_size;/* size of a single bitmap */ - size_t bits; /* per bitmap */ - dma_addr_t base; - - spinlock_t lock; - struct kref kref; -}; - -struct dma_iommu_mapping * -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64 size); - -void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping); - -int arm_iommu_attach_device(struct device *dev, - struct dma_iommu_mapping *mapping); -void arm_iommu_detach_device(struct device *dev); - -#endif /* __KERNEL__ */ -#endif diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h index 6ad1ceda62a5..08bedb0c02c6 100644 --- a/arch/arm/include/asm/dma-mapping.h +++ b/arch/arm/include/asm/dma-mapping.h @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -32,7 +33,7 @@ static inline struct dma_map_ops *get_dma_ops(struct device *dev) return __generic_dma_ops(dev); } -static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops) +static inline void arch_set_dma_ops(struct device *dev, struct dma_map_ops *ops) { BUG_ON(!dev); dev->archdata.dma_ops = ops; @@ -275,5 +276,61 @@ extern int arm_dma_get_sgtable(struct device *dev, struct sg_table *sgt, void *cpu_addr, dma_addr_t dma_addr, size_t size, struct
Re: [RFC 3/3] iommu: dma-iommu: use common implementation also on ARM architecture
Hello, On 2016-02-19 11:30, Arnd Bergmann wrote: On Friday 19 February 2016 09:22:44 Marek Szyprowski wrote: This patch replaces ARM-specific IOMMU-based DMA-mapping implementation with generic IOMMU DMA-mapping code shared with ARM64 architecture. The side-effect of this change is a switch from bitmap-based IO address space management to tree-based code. There should be no functional changes for drivers, which rely on initialization from generic arch_setup_dna_ops() interface. Code, which used old arm_iommu_* functions must be updated to new interface. Signed-off-by: Marek Szyprowski I like the overall idea. However, this interface from the iommu subsystem into architecture specific code: +/* + * The DMA API is built upon the notion of "buffer ownership". A buffer + * is either exclusively owned by the CPU (and therefore may be accessed + * by it) or exclusively owned by the DMA device. These helper functions + * represent the transitions between these two ownership states. + * + * Note, however, that on later ARMs, this notion does not work due to + * speculative prefetches. We model our approach on the assumption that + * the CPU does do speculative prefetches, which means we clean caches + * before transfers and delay cache invalidation until transfer completion. + * + */ +extern void __dma_page_cpu_to_dev(struct page *, unsigned long, size_t, + enum dma_data_direction); +extern void __dma_page_dev_to_cpu(struct page *, unsigned long, size_t, + enum dma_data_direction); + +static inline void arch_flush_page(struct device *dev, const void *virt, + phys_addr_t phys) +{ + dmac_flush_range(virt, virt + PAGE_SIZE); + outer_flush_range(phys, phys + PAGE_SIZE); +} + +static inline void arch_dma_map_area(phys_addr_t phys, size_t size, +enum dma_data_direction dir) +{ + unsigned int offset = phys & ~PAGE_MASK; + __dma_page_cpu_to_dev(phys_to_page(phys & PAGE_MASK), offset, size, dir); +} + +static inline void arch_dma_unmap_area(phys_addr_t phys, size_t size, + enum dma_data_direction dir) +{ + unsigned int offset = phys & ~PAGE_MASK; + __dma_page_dev_to_cpu(phys_to_page(phys & PAGE_MASK), offset, size, dir); +} + +static inline pgprot_t arch_get_dma_pgprot(struct dma_attrs *attrs, + pgprot_t prot, bool coherent) +{ + if (coherent) + return prot; + + prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ? + pgprot_writecombine(prot) : + pgprot_dmacoherent(prot); + return prot; +} + +extern void *arch_alloc_from_atomic_pool(size_t size, struct page **ret_page, +gfp_t flags); +extern bool arch_in_atomic_pool(void *start, size_t size); +extern int arch_free_from_atomic_pool(void *start, size_t size); + + doesn't feel completely right yet. In particular the arch_flush_page() interface is probably still too specific to ARM/ARM64 and won't work that way on other architectures. I think it would be better to do this either more generic, or less generic: a) leave the iommu_dma_map_ops definition in the architecture specific code, but make it call helper functions in the drivers/iommu to do all of the really generic parts. b) clarify that this is only applicable to arch/arm and arch/arm64, and unify things further between these two, as they have very similar requirements in the CPU architecture. Some really generic parts are already in iommu/dma-iommu.c and one can build it's own, non-ARM CPU architecture based IOMMU/DMA-mapping code. Initially I also wanted to use that generic code on both ARM and ARM64, but it turned out that both archs, ARM and ARM64 will duplicate 99% of code, which use this 'generic' functions. This was the reason why I dedided to move all that common code from arch/{arm,arm64}/mm/dma-mapping.c to drivers/iommu/dma-iommu-ops.c I'm not sure if I can design all the changes that need to be made to drivers/iommu/dma-iommu-ops.c to make it more generic. Maybe when one will try to use that code with other, non-ARM architecture based arch glue code, a better abstraction can be developed. For now I would like to keep all this code in a common place so both arm and arm64 will benefit from improvements done there. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: exynos: pointers are nto physical addresses
Hi Arnd, On 2016-02-29 09:45, Arnd Bergmann wrote: The exynos iommu driver changed an incorrect cast from pointer to 'unsigned int' to an equally incorrect cast to a 'phys_addr_t', which results in an obvious compile-time error when phys_addr_t is wider than pointers are: drivers/iommu/exynos-iommu.c: In function 'alloc_lv2entry': drivers/iommu/exynos-iommu.c:918:32: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] The code does not actually want the physical address (which would involve using virt_to_phys()), but just checks the alignment, so we can change it to use a cast to uintptr_t instead. Signed-off-by: Arnd Bergmann Fixes: 740a01eee9ad ("iommu/exynos: Add support for v5 SYSMMU") Thanks for this fix. --- I also see that some incorrect __raw_writel() calls have crept in around the same time, which breaks running big-endian kernels when this driver is loaded. Please fix and that that as well. Okay, so in the driver code all __raw_writel should be replaced by writel(), right? Those __raw_writel() calls were there from the beginning and I didn't know that they should not be used in the driver code. drivers/iommu/exynos-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index b0665042bf29..484b3b37631f 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -915,7 +915,7 @@ static sysmmu_pte_t *alloc_lv2entry(struct exynos_iommu_domain *domain, bool need_flush_flpd_cache = lv1ent_zero(sent); pent = kmem_cache_zalloc(lv2table_kmem_cache, GFP_ATOMIC); - BUG_ON((phys_addr_t)pent & (LV2TABLE_SIZE - 1)); + BUG_ON((uintptr_t)pent & (LV2TABLE_SIZE - 1)); if (!pent) return ERR_PTR(-ENOMEM); Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/exynos: Use proper readl/writel register interface
Drivers should use generic readl/writel calls to access HW registers, so replace all __raw_readl/writel with generic version. Suggested-by: Arnd Bergmann Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 38 +++--- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index b066504..20fe443 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -268,18 +268,18 @@ static bool is_sysmmu_active(struct sysmmu_drvdata *data) static void sysmmu_unblock(struct sysmmu_drvdata *data) { - __raw_writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); + writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); } static bool sysmmu_block(struct sysmmu_drvdata *data) { int i = 120; - __raw_writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL); - while ((i > 0) && !(__raw_readl(data->sfrbase + REG_MMU_STATUS) & 1)) + writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL); + while ((i > 0) && !(readl(data->sfrbase + REG_MMU_STATUS) & 1)) --i; - if (!(__raw_readl(data->sfrbase + REG_MMU_STATUS) & 1)) { + if (!(readl(data->sfrbase + REG_MMU_STATUS) & 1)) { sysmmu_unblock(data); return false; } @@ -290,9 +290,9 @@ static bool sysmmu_block(struct sysmmu_drvdata *data) static void __sysmmu_tlb_invalidate(struct sysmmu_drvdata *data) { if (MMU_MAJ_VER(data->version) < 5) - __raw_writel(0x1, data->sfrbase + REG_MMU_FLUSH); + writel(0x1, data->sfrbase + REG_MMU_FLUSH); else - __raw_writel(0x1, data->sfrbase + REG_V5_MMU_FLUSH_ALL); + writel(0x1, data->sfrbase + REG_V5_MMU_FLUSH_ALL); } static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, @@ -302,10 +302,10 @@ static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, for (i = 0; i < num_inv; i++) { if (MMU_MAJ_VER(data->version) < 5) - __raw_writel((iova & SPAGE_MASK) | 1, + writel((iova & SPAGE_MASK) | 1, data->sfrbase + REG_MMU_FLUSH_ENTRY); else - __raw_writel((iova & SPAGE_MASK) | 1, + writel((iova & SPAGE_MASK) | 1, data->sfrbase + REG_V5_MMU_FLUSH_ENTRY); iova += SPAGE_SIZE; } @@ -314,9 +314,9 @@ static void __sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd) { if (MMU_MAJ_VER(data->version) < 5) - __raw_writel(pgd, data->sfrbase + REG_PT_BASE_ADDR); + writel(pgd, data->sfrbase + REG_PT_BASE_ADDR); else - __raw_writel(pgd >> PAGE_SHIFT, + writel(pgd >> PAGE_SHIFT, data->sfrbase + REG_V5_PT_BASE_PFN); __sysmmu_tlb_invalidate(data); @@ -331,7 +331,7 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data) clk_enable(data->pclk); clk_enable(data->aclk); - ver = __raw_readl(data->sfrbase + REG_MMU_VERSION); + ver = readl(data->sfrbase + REG_MMU_VERSION); /* controllers on some SoCs don't report proper version */ if (ver == 0x8001u) @@ -392,7 +392,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) clk_enable(data->clk_master); - itype = __ffs(__raw_readl(data->sfrbase + reg_status)); + itype = __ffs(readl(data->sfrbase + reg_status)); for (i = 0; i < n; i++, finfo++) if (finfo->bit == itype) break; @@ -400,7 +400,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) BUG_ON(i == n); /* print debug message */ - fault_addr = __raw_readl(data->sfrbase + finfo->addr_reg); + fault_addr = readl(data->sfrbase + finfo->addr_reg); show_fault_information(data, finfo, fault_addr); if (data->domain) @@ -409,7 +409,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) /* fault is not recovered by fault handler */ BUG_ON(ret != 0); - __raw_writel(1 << itype, data->sfrbase + reg_clear); + writel(1 << itype, data->sfrbase + reg_clear); sysmmu_unblock(data); @@ -424,8 +424,8 @@ static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data) { clk_enable(data->clk_master); - __raw_writel(CTRL_DISABLE, data->sfrbase + REG_MMU_CTRL); - __raw_writel(0, data->sfrbase + REG_MMU_CFG); + writel(CTRL_DISABLE, data->sfrbase + REG_MMU_CTR
Re: [patch] iommu/exynos: checking for IS_ERR() instead of NULL
Hello, On 2016-03-02 11:10, Dan Carpenter wrote: of_platform_device_create() returns NULL on error, it never returns error pointers. Fixes: 8ed55c812fa8 ('iommu/exynos: Init from dt-specific callback instead of initcall') Signed-off-by: Dan Carpenter Acked-by: Marek Szyprowski diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index b066504..cb57bda 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1347,8 +1347,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np) exynos_iommu_init(); pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root); - if (IS_ERR(pdev)) - return PTR_ERR(pdev); + if (!pdev) + return -ENOMEM; /* * use the first registered sysmmu device for performing Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 3/3] iommu: dma-iommu: use common implementation also on ARM architecture
Hello, On 2016-03-15 12:18, Magnus Damm wrote: Hi Marek, On Fri, Feb 19, 2016 at 5:22 PM, Marek Szyprowski wrote: This patch replaces ARM-specific IOMMU-based DMA-mapping implementation with generic IOMMU DMA-mapping code shared with ARM64 architecture. The side-effect of this change is a switch from bitmap-based IO address space management to tree-based code. There should be no functional changes for drivers, which rely on initialization from generic arch_setup_dna_ops() interface. Code, which used old arm_iommu_* functions must be updated to new interface. Signed-off-by: Marek Szyprowski --- Thanks for your efforts and my apologies for late comments. Just FYI I'll try your patch (and this series) with the ipmmu-vmsa.c driver on 32-bit ARM and see how it goes. Nice not to have to support multiple interfaces depending on architecture! Thanks for testing! One question that comes to mind is how to handle features. For instance, the 32-bit ARM code supports DMA_ATTR_FORCE_CONTIGUOUS while the shared code in drivers/iommu/dma-iommu.c does not. I assume existing users may rely on such features so from my point of view it probably makes sense to carry over features from the 32-bit ARM code into the shared code before pulling the plug. Right, this has to be added to common code before merging. I also wonder if it is possible to do a step-by-step migration and support both old and new interfaces in the same binary? That may make things easier for multiplatform enablement. So far I've managed to make one IOMMU driver support both 32-bit ARM and 64-bit ARM with some ugly magic, so adjusting 32-bit ARM dma-mapping code to coexist with the shared code in drivers/iommu/dma-iommu.c may also be possible. And probably involving even more ugly magic. =) Having one IOMMU driver for both 32-bit and 64-bit ARM archs is quite easy IF you rely on the iommu core to setup everything. See exynos-iommu driver - after my last patches it now works fine on both archs (using arch specific interfaces). Most of the magic is done automatically by arch_setup_dma_ops(). The real problem is the fact that there are drivers (like DRM) which rely on specific dma-mapping functions from ARM architecture, which need to be rewritten. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic
Hello, On 2016-04-14 19:25, Lorenzo Pieralisi wrote: On systems booting with ACPI, the IOMMU drivers require the same kind of id mapping carried out with a DT tree through the of_xlate() API in order to map devices identifiers to IOMMU ones. On ACPI systems, since DT nodes are not present (ie struct device.of_node == NULL), to identify the device requiring the translation the struct device_node (and the structure used to pass translation information - struct of_phandle_args - that contains a struct device_node) cannot be used, so a generic translation structure to be used for IOMMU mapping should be defined, based on the firmware agnostic fwnode_handle type. This patch mechanically refactors/renames the of_xlate API to make it DT agnostic, by declaring a new type (struct iommu_fwspec), that allows the kernel to pass a device identifier (fwnode - which can represent either a DT node or an IOMMU FW node) and by changing the of_xlate signature so that it does not take anymore the DT specific of_phandle_args argument and replaces it with the DT agnostic iommu_fwspec one. Signed-off-by: Lorenzo Pieralisi Cc: Matthias Brugger Cc: Will Deacon Cc: Hanjun Guo Cc: Rob Herring Cc: Krzysztof Kozlowski Cc: Robin Murphy Cc: Tomasz Nowicki Cc: Joerg Roedel Cc: Marek Szyprowski I'm not sure if this is the right approach, although I have not enough knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the new "fwspec" based interface? Right now of_xlate is rather common interface used by various subsystems. Maybe it will be much easier to just add acpi_xlate callback and plumb it to the generic code? Maybe later when similar code will be in other subsystems and drivers, it can be unified, having much more real use cases? --- drivers/iommu/arm-smmu.c | 12 +++- drivers/iommu/exynos-iommu.c | 11 +++ drivers/iommu/mtk_iommu.c| 13 - drivers/iommu/of_iommu.c | 20 ++-- include/linux/iommu.h| 24 5 files changed, 60 insertions(+), 20 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 6c42770..84bcff7 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1440,18 +1440,20 @@ out_unlock: return ret; } -static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args) +static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args) { struct arm_smmu_device *smmu; - struct platform_device *smmu_pdev; + struct platform_device *smmu_pdev = NULL; + + if (is_of_node(args->fwnode)) + smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode)); - smmu_pdev = of_find_device_by_node(args->np); if (!smmu_pdev) return -ENODEV; smmu = platform_get_drvdata(smmu_pdev); - return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]); + return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]); } static struct iommu_ops arm_smmu_ops = { @@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = { .device_group = arm_smmu_device_group, .domain_get_attr= arm_smmu_domain_get_attr, .domain_set_attr= arm_smmu_domain_set_attr, - .of_xlate = arm_smmu_of_xlate, + .fw_xlate = arm_smmu_fw_xlate, .pgsize_bitmap = -1UL, /* Restricted during device attach */ }; diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 5ecc86c..84ff5bb 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device *dev) iommu_group_remove_device(dev); } -static int exynos_iommu_of_xlate(struct device *dev, -struct of_phandle_args *spec) +static int exynos_iommu_fw_xlate(struct device *dev, +struct iommu_fwspec *args) { struct exynos_iommu_owner *owner = dev->archdata.iommu; - struct platform_device *sysmmu = of_find_device_by_node(spec->np); + struct platform_device *sysmmu = NULL; struct sysmmu_drvdata *data; + if (is_of_node(args->fwnode)) + sysmmu = of_find_device_by_node(to_of_node(args->fwnode)); + if (!sysmmu) return -ENODEV; @@ -1290,7 +1293,7 @@ static struct iommu_ops exynos_iommu_ops = { .add_device = exynos_iommu_add_device, .remove_device = exynos_iommu_remove_device, .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE, - .of_xlate = exynos_iommu_of_xlate, + .fw_xlate = exynos_iommu_fw_xlate, }; static bool init_done; diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index 929a66a..e08dc0a 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -4
Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic
Hi Lorenzo, On 2016-04-19 13:30, Lorenzo Pieralisi wrote: Hi Marek, On Tue, Apr 19, 2016 at 10:28:02AM +0200, Marek Szyprowski wrote: Hello, On 2016-04-14 19:25, Lorenzo Pieralisi wrote: On systems booting with ACPI, the IOMMU drivers require the same kind of id mapping carried out with a DT tree through the of_xlate() API in order to map devices identifiers to IOMMU ones. On ACPI systems, since DT nodes are not present (ie struct device.of_node == NULL), to identify the device requiring the translation the struct device_node (and the structure used to pass translation information - struct of_phandle_args - that contains a struct device_node) cannot be used, so a generic translation structure to be used for IOMMU mapping should be defined, based on the firmware agnostic fwnode_handle type. This patch mechanically refactors/renames the of_xlate API to make it DT agnostic, by declaring a new type (struct iommu_fwspec), that allows the kernel to pass a device identifier (fwnode - which can represent either a DT node or an IOMMU FW node) and by changing the of_xlate signature so that it does not take anymore the DT specific of_phandle_args argument and replaces it with the DT agnostic iommu_fwspec one. Signed-off-by: Lorenzo Pieralisi Cc: Matthias Brugger Cc: Will Deacon Cc: Hanjun Guo Cc: Rob Herring Cc: Krzysztof Kozlowski Cc: Robin Murphy Cc: Tomasz Nowicki Cc: Joerg Roedel Cc: Marek Szyprowski I'm not sure if this is the right approach, although I have not enough knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the new "fwspec" based interface? Right now of_xlate is rather common interface used by various subsystems. Maybe it will be much easier to Yes, that's a valid concern, when you say "it is rather common" though, it seems to me that the of_xlate footprint is still subsystem specific, so this patch should be self-contained anyway (granted, doing this conversion for a specific subsystem is questionable, I guess that what you are asking is, if you do it for IOMMU, why would not you do it for other subsystems ?). I was curious if you want to replace of_xlate() interface in other subsystems like clocks, power domains, regulators, etc. Each of_xlate interface is specific to particular subsystem, but they all more or less follows the same style, what makes it easier to understand the code. It is an RFC for this specific reason. just add acpi_xlate callback and plumb it to the generic code? Maybe later when similar code will be in other subsystems and drivers, it can be unified, having much more real use cases? Yes, that's a possibility, it means adding yet another hook into the IOMMU drivers, probably a simpler change than this one, I posted this code as and RFC to see which direction we want to take so further feedback is welcome we can then choose the best approach. Thanks, Lorenzo --- drivers/iommu/arm-smmu.c | 12 +++- drivers/iommu/exynos-iommu.c | 11 +++ drivers/iommu/mtk_iommu.c| 13 - drivers/iommu/of_iommu.c | 20 ++-- include/linux/iommu.h| 24 5 files changed, 60 insertions(+), 20 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 6c42770..84bcff7 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1440,18 +1440,20 @@ out_unlock: return ret; } -static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args) +static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args) { struct arm_smmu_device *smmu; - struct platform_device *smmu_pdev; + struct platform_device *smmu_pdev = NULL; + + if (is_of_node(args->fwnode)) + smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode)); - smmu_pdev = of_find_device_by_node(args->np); if (!smmu_pdev) return -ENODEV; smmu = platform_get_drvdata(smmu_pdev); - return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]); + return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]); } static struct iommu_ops arm_smmu_ops = { @@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = { .device_group = arm_smmu_device_group, .domain_get_attr= arm_smmu_domain_get_attr, .domain_set_attr= arm_smmu_domain_set_attr, - .of_xlate = arm_smmu_of_xlate, + .fw_xlate = arm_smmu_fw_xlate, .pgsize_bitmap = -1UL, /* Restricted during device attach */ }; diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 5ecc86c..84ff5bb 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device *dev) iommu_group_remove_device(dev); } -s
Re: [RFC 0/9] IOMMU probe deferral support
Hello, On 2016-04-25 17:58, Sricharan R wrote: This is mostly a repost of the probe deferral series from Laurent Pinchart [1]. Added a check to fix boot with ACPI. Adapted arm-smmu driver to work with deferred probing and added a new api for the below reason. This is based on the generic iommu binding series from Robin Murphy . Thanks for this patchset. I'm working on some serious rework in exynos power domains and clocks support code and it turned out that I need this feature to resolve probing order. It works fine on my internal tree, where some iommu controllers cannot get their clocks early enough. Tested-by: Marek Szyprowski Now associating a group with an master has become mandatory and happens when the master is added to the bus using BUS_ADD_DEVICE from iommu core. But the iommu has to be ready before this using early iommu registration and also master should have been added to the iommu using xlate. So, when trying to get rid of the early registration and using late probing for iommu devices to sort out the probing order, the newly added api is meant to be called during the late probing to add the master to the iommu. add_iommu_group could be modified to do the same though. The one issue here is the DRIVER_BIND notifier from iommu core might be called at a wrong point since the addition of a group to the device happens after this. Will be good to know the right direction to proceed on this fully. [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html Laurent Pinchart (6): arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops() of: dma: Move range size workaround to of_dma_get_range() of: dma: Make of_dma_deconfigure() public of: dma: Split of_configure_dma() into mask and ops configuration drivers: platform: Configure dma operations at probe time iommu: of: Handle IOMMU lookup failure with deferred probing or error Sricharan R (3): drivers: iommu: Add a new add device api drivers: of: call iommu_bus_add_dev after iommu_configure_ops drivers: iommu: arm-smmu: Set iommu_ops in probe arch/arm/mm/dma-mapping.c | 9 ++ drivers/base/platform.c | 13 drivers/iommu/arm-smmu.c | 34 ++-- drivers/iommu/iommu.c | 12 +++ drivers/iommu/of_iommu.c | 16 -- drivers/of/address.c | 20 ++-- drivers/of/device.c | 81 +++ drivers/of/platform.c | 16 -- drivers/pci/probe.c | 3 +- include/linux/iommu.h | 1 + include/linux/of_device.h | 14 ++-- 11 files changed, 141 insertions(+), 78 deletions(-) Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/exynos: Suppress unbinding to prevent system failure
Removal of IOMMU driver cannot be done reliably, so Exynos IOMMU driver doesn't support this operation. It is essential for system operation, so it makes sense to prevent unbinding by disabling bind/unbind sysfs feature for SYSMMU controller driver to avoid kernel ops or trashing memory caused by such operation. Signed-off-by: Marek Szyprowski CC: sta...@vger.kernel.org # v4.2+ --- drivers/iommu/exynos-iommu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 558a40d3a6a0..d365c40a7b84 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -681,6 +681,7 @@ static struct platform_driver exynos_sysmmu_driver __refdata = { .name = "exynos-sysmmu", .of_match_table = sysmmu_of_match, .pm = &sysmmu_pm_ops, + .suppress_bind_attrs = true, } }; -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC 8/9] drivers: of: call iommu_bus_add_dev after iommu_configure_ops
Hello, On 2016-04-25 17:58, Sricharan R wrote: Now that the device's iommu ops are configured at probe time, the device has to be added to the iommu late. Signed-off-by: Sricharan R --- drivers/of/device.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/of/device.c b/drivers/of/device.c index 57a5f2d..722115c 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -154,6 +155,9 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np) dev_dbg(dev, "device is%sbehind an iommu\n", iommu ? " " : " not "); + if (iommu) + iommu_bus_add_dev(dev); + This causes build break when IOMMU subsystem is not enabled: drivers/of/device.c: In function 'of_dma_configure_ops': drivers/of/device.c:159:3: error: implicit declaration of function 'iommu_bus_add_dev' [-Werror=implicit-function-declaration] iommu_bus_add_dev(dev); ^ arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent); return 0; Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3] iommu/exynos: Return proper errors from getting clocks
This patch reworks driver probe code to propagate error codes from clk_get() operation. This will allow to properly handle deferred probe in the future. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 56 +++- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index e27e3b7df4e7..989365682d14 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -602,37 +602,31 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) } data->clk = devm_clk_get(dev, "sysmmu"); - if (!IS_ERR(data->clk)) { - ret = clk_prepare(data->clk); - if (ret) { - dev_err(dev, "Failed to prepare clk\n"); - return ret; - } - } else { + if (PTR_ERR(data->clk) == -ENOENT) data->clk = NULL; - } + else if (IS_ERR(data->clk)) + return PTR_ERR(data->clk); + ret = clk_prepare(data->clk); + if (ret) + return ret; data->aclk = devm_clk_get(dev, "aclk"); - if (!IS_ERR(data->aclk)) { - ret = clk_prepare(data->aclk); - if (ret) { - dev_err(dev, "Failed to prepare aclk\n"); - return ret; - } - } else { + if (PTR_ERR(data->aclk) == -ENOENT) data->aclk = NULL; - } + else if (IS_ERR(data->aclk)) + return PTR_ERR(data->aclk); + ret = clk_prepare(data->aclk); + if (ret) + return ret; data->pclk = devm_clk_get(dev, "pclk"); - if (!IS_ERR(data->pclk)) { - ret = clk_prepare(data->pclk); - if (ret) { - dev_err(dev, "Failed to prepare pclk\n"); - return ret; - } - } else { + if (PTR_ERR(data->pclk) == -ENOENT) data->pclk = NULL; - } + else if (IS_ERR(data->pclk)) + return PTR_ERR(data->pclk); + ret = clk_prepare(data->pclk); + if (ret) + return ret; if (!data->clk && (!data->aclk || !data->pclk)) { dev_err(dev, "Failed to get device clock(s)!\n"); @@ -640,15 +634,13 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) } data->clk_master = devm_clk_get(dev, "master"); - if (!IS_ERR(data->clk_master)) { - ret = clk_prepare(data->clk_master); - if (ret) { - dev_err(dev, "Failed to prepare master's clk\n"); - return ret; - } - } else { + if (PTR_ERR(data->clk_master) == -ENOENT) data->clk_master = NULL; - } + else if (IS_ERR(data->clk_master)) + return PTR_ERR(data->clk_master); + ret = clk_prepare(data->clk_master); + if (ret) + return ret; data->sysmmu = dev; spin_lock_init(&data->lock); -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/3] Exynos IOMMU: improve clock management
Hello, This patch series improves clock management in Exynos IOMMU driver, so the driver will be ready for potential deferred probe caused by not-yet-available clocks. Such case doesn't happen with current kernel, but it may happen with the planned rework on clocks and power domain code. Best regards Marek Szyprowski Samsung R&D Institute Poland Marek Szyprowski (3): iommu/exynos: Return proper errors from getting clocks iommu/exynos: Fix master clock management for inactive SYSMMU iommu/exynos: Prepare clocks when needed, not in driver probe drivers/iommu/exynos-iommu.c | 95 1 file changed, 42 insertions(+), 53 deletions(-) -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] iommu/exynos: Fix master clock management for inactive SYSMMU
If SYSMMU controller is not active, there is no point in enabling master's clock just for doing the the of internal state. This patch moves enabling that clock to the block which actually does the register access. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 989365682d14..018bcd5d5edc 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -524,16 +524,15 @@ static void sysmmu_tlb_invalidate_flpdcache(struct sysmmu_drvdata *data, { unsigned long flags; - clk_enable(data->clk_master); spin_lock_irqsave(&data->lock, flags); - if (is_sysmmu_active(data)) { - if (data->version >= MAKE_MMU_VER(3, 3)) - __sysmmu_tlb_invalidate_entry(data, iova, 1); + if (is_sysmmu_active(data) && data->version >= MAKE_MMU_VER(3, 3)) { + clk_enable(data->clk_master); + __sysmmu_tlb_invalidate_entry(data, iova, 1); + clk_disable(data->clk_master); } spin_unlock_irqrestore(&data->lock, flags); - clk_disable(data->clk_master); } static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/3] iommu/exynos: Prepare clocks when needed, not in driver probe
Make clock preparation together with clk_enable(). This way inactive SYSMMU controllers will not keep clocks prepared all the time. This change allows more fine graded power management in the future. All the code assumes that clock management doesn't fail, so guard clock_prepare_enable() it with BUG_ON(). Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 54 +--- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 018bcd5d5edc..e0b834375f17 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -322,14 +322,27 @@ static void __sysmmu_set_ptbase(struct sysmmu_drvdata *data, phys_addr_t pgd) __sysmmu_tlb_invalidate(data); } +static void __sysmmu_enable_clocks(struct sysmmu_drvdata *data) +{ + BUG_ON(clk_prepare_enable(data->clk_master)); + BUG_ON(clk_prepare_enable(data->clk)); + BUG_ON(clk_prepare_enable(data->pclk)); + BUG_ON(clk_prepare_enable(data->aclk)); +} + +static void __sysmmu_disable_clocks(struct sysmmu_drvdata *data) +{ + clk_disable_unprepare(data->aclk); + clk_disable_unprepare(data->pclk); + clk_disable_unprepare(data->clk); + clk_disable_unprepare(data->clk_master); +} + static void __sysmmu_get_version(struct sysmmu_drvdata *data) { u32 ver; - clk_enable(data->clk_master); - clk_enable(data->clk); - clk_enable(data->pclk); - clk_enable(data->aclk); + __sysmmu_enable_clocks(data); ver = readl(data->sfrbase + REG_MMU_VERSION); @@ -342,10 +355,7 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data) dev_dbg(data->sysmmu, "hardware version: %d.%d\n", MMU_MAJ_VER(data->version), MMU_MIN_VER(data->version)); - clk_disable(data->aclk); - clk_disable(data->pclk); - clk_disable(data->clk); - clk_disable(data->clk_master); + __sysmmu_disable_clocks(data); } static void show_fault_information(struct sysmmu_drvdata *data, @@ -427,10 +437,7 @@ static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data) writel(CTRL_DISABLE, data->sfrbase + REG_MMU_CTRL); writel(0, data->sfrbase + REG_MMU_CFG); - clk_disable(data->aclk); - clk_disable(data->pclk); - clk_disable(data->clk); - clk_disable(data->clk_master); + __sysmmu_disable_clocks(data); } static bool __sysmmu_disable(struct sysmmu_drvdata *data) @@ -475,10 +482,7 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data) static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) { - clk_enable(data->clk_master); - clk_enable(data->clk); - clk_enable(data->pclk); - clk_enable(data->aclk); + __sysmmu_enable_clocks(data); writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL); @@ -488,6 +492,12 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); + /* +* SYSMMU driver keeps master's clock enabled only for the short +* time, while accessing the registers. For performing address +* translation during DMA transaction it relies on the client +* driver to enable it. +*/ clk_disable(data->clk_master); } @@ -605,27 +615,18 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) data->clk = NULL; else if (IS_ERR(data->clk)) return PTR_ERR(data->clk); - ret = clk_prepare(data->clk); - if (ret) - return ret; data->aclk = devm_clk_get(dev, "aclk"); if (PTR_ERR(data->aclk) == -ENOENT) data->aclk = NULL; else if (IS_ERR(data->aclk)) return PTR_ERR(data->aclk); - ret = clk_prepare(data->aclk); - if (ret) - return ret; data->pclk = devm_clk_get(dev, "pclk"); if (PTR_ERR(data->pclk) == -ENOENT) data->pclk = NULL; else if (IS_ERR(data->pclk)) return PTR_ERR(data->pclk); - ret = clk_prepare(data->pclk); - if (ret) - return ret; if (!data->clk && (!data->aclk || !data->pclk)) { dev_err(dev, "Failed to get device clock(s)!\n"); @@ -637,9 +638,6 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) data->clk_master = NULL; else if (IS_ERR(data->clk_master)) return PTR_ERR(data->clk_master); - ret = clk_prepare(data->clk_master); - if (ret) - return ret; data->sysmmu = dev; spin_lock_init(&data->lock); -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/3] iommu/exynos: Prepare for deferred probe support
Register iommu_ops at the end of successful probe instead of doing that unconditionally. This makes Exynos IOMMU driver ready for deferred probe caused by not-yet-available clocks. Signed-off-by: Marek Szyprowski --- I'm sorry for the confussing patch number. This patch got lost while rebasing, but it should belong to the "Exynos IOMMU: improve clock management" patchset. Best regards Marek Szyprowski Samsung R&D Institute Poland --- drivers/iommu/exynos-iommu.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index e0b834375f17..633e6d023c0d 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -581,6 +581,8 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, spin_unlock_irqrestore(&data->lock, flags); } +static struct iommu_ops exynos_iommu_ops; + static int __init exynos_sysmmu_probe(struct platform_device *pdev) { int irq, ret; @@ -654,6 +656,8 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) pm_runtime_enable(dev); + of_iommu_set_ops(dev->of_node, &exynos_iommu_ops); + return 0; } @@ -1347,7 +1351,6 @@ static int __init exynos_iommu_of_setup(struct device_node *np) if (!dma_dev) dma_dev = &pdev->dev; - of_iommu_set_ops(np, &exynos_iommu_ops); return 0; } -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] iommu/exynos: Remove excessive, useless debug
Remove excessive, useless debug about skipping TLB invalidation, which is a normal situation when more aggressive power management is enabled. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 633e6d023c0d..9d1a14f88891 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -574,9 +574,6 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, sysmmu_unblock(data); } clk_disable(data->clk_master); - } else { - dev_dbg(data->master, - "disabled. Skipping TLB invalidation @ %#x\n", iova); } spin_unlock_irqrestore(&data->lock, flags); } -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/3] Exynos IOMMU: proper runtime pm support
Hello, This patch series finally implements proper runtime pm support in Exynos IOMMU driver. This has been achieved by adding runtime pm notifiers, which lets SYSMMU controller to follow it's master device (the device which actually performs DMA transaction) runtime pm. The main idea behind this solution is an observation that any DMA activity from master device can be done only when master device is active, thus when master device is suspended SYSMMU controller device can also be suspended. This patchset finally solves the situation that power domains are always enabled, because all SYSMMU controllers (which belongs to those domains) are permanently active (because existing driver was simplified and kept SYSMMU device active all the time after initialization). This patchset requires my previous changes to Exynos IOMMU driver submitted in the "Exynos IOMMU: improve clock management" thread: http://www.spinics.net/lists/arm-kernel/msg505695.html Best regards Marek Szyprowski Samsung R&D Institute Poland Krzysztof Kozlowski (1): PM / Runtime: Add notifiers for device runtime PM events Marek Szyprowski (2): iommu/exynos: Remove excessive, useless debug iommu/exynos: Add proper runtime pm support drivers/base/power/generic_ops.c | 9 ++ drivers/base/power/power.h | 6 ++ drivers/base/power/runtime.c | 34 ++- drivers/iommu/exynos-iommu.c | 214 ++- include/linux/pm.h | 2 + include/linux/pm_runtime.h | 51 ++ 6 files changed, 201 insertions(+), 115 deletions(-) -- 1.9.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/3] iommu/exynos: Add proper runtime pm support
This patch uses recently introduced runtime pm notifiers to track the runtime pm state of the master's device. This way each SYSMMU controller knows when its master's device is active and can save/restore its state instead of being enabled all the time. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not used. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 211 +-- 1 file changed, 101 insertions(+), 110 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 9d1a14f88891..de4126787c41 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -25,6 +25,7 @@ #include #include #include +#include typedef u32 sysmmu_iova_t; typedef u32 sysmmu_pte_t; @@ -233,13 +234,14 @@ struct sysmmu_drvdata { struct clk *aclk; /* SYSMMU's aclk clock */ struct clk *pclk; /* SYSMMU's pclk clock */ struct clk *clk_master; /* master's device clock */ - int activations;/* number of calls to sysmmu_enable */ spinlock_t lock;/* lock for modyfying state */ + int active; /* current status */ struct exynos_iommu_domain *domain; /* domain we belong to */ struct list_head domain_node; /* node for domain clients list */ struct list_head owner_node;/* node for owner controllers list */ phys_addr_t pgtable;/* assigned page table structure */ unsigned int version; /* our version */ + struct notifier_block pm_nb;/* for tracking master's runtime pm */ }; static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom) @@ -247,25 +249,6 @@ static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom) return container_of(dom, struct exynos_iommu_domain, domain); } -static bool set_sysmmu_active(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU was not active previously - and it needs to be initialized */ - return ++data->activations == 1; -} - -static bool set_sysmmu_inactive(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU is needed to be disabled */ - BUG_ON(data->activations < 1); - return --data->activations == 0; -} - -static bool is_sysmmu_active(struct sysmmu_drvdata *data) -{ - return data->activations > 0; -} - static void sysmmu_unblock(struct sysmmu_drvdata *data) { writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); @@ -384,7 +367,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) unsigned short reg_status, reg_clear; int ret = -ENOSYS; - WARN_ON(!is_sysmmu_active(data)); + WARN_ON(!data->active); if (MMU_MAJ_VER(data->version) < 5) { reg_status = REG_INT_STATUS; @@ -440,32 +423,6 @@ static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data) __sysmmu_disable_clocks(data); } -static bool __sysmmu_disable(struct sysmmu_drvdata *data) -{ - bool disabled; - unsigned long flags; - - spin_lock_irqsave(&data->lock, flags); - - disabled = set_sysmmu_inactive(data); - - if (disabled) { - data->pgtable = 0; - data->domain = NULL; - - __sysmmu_disable_nocount(data); - - dev_dbg(data->sysmmu, "Disabled\n"); - } else { - dev_dbg(data->sysmmu, "%d times left to disable\n", - data->activations); - } - - spin_unlock_irqrestore(&data->lock, flags); - - return disabled; -} - static void __sysmmu_init_config(struct sysmmu_drvdata *data) { unsigned int cfg; @@ -501,34 +458,6 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) clk_disable(data->clk_master); } -static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable, - struct exynos_iommu_domain *domain) -{ - int ret = 0; - unsigned long flags; - - spin_lock_irqsave(&data->lock, flags); - if (set_sysmmu_active(data)) { - data->pgtable = pgtable; - data->domain = domain; - - __sysmmu_enable_nocount(data); - - dev_dbg(data->sysmmu, "Enabled\n"); - } else { - ret = (pgtable == data->pgtable) ? 1 : -EBUSY; - - dev_dbg(data->sysmmu, "already enabled\n"); - } - - if (WARN_ON(ret < 0)) - set_sysmmu_inactive(data); /* decrement count */ - - spin_unlock_irqrestore(&data->lock, flags); - - return ret; -} - st
[PATCH 1/3] PM / Runtime: Add notifiers for device runtime PM events
From: Krzysztof Kozlowski Allow drivers registering for certain runtime PM events of other devices. Some drivers in power domain are more or less coupled. When one driver is suspending (thus leading to power domain being also turned off) the other might have to perform some necessary steps. For example Exynos IOMMU has to save its context. Based on previous work of Sylwester Nawrocki . Signed-off-by: Krzysztof Kozlowski Signed-off-by: Marek Szyprowski --- drivers/base/power/generic_ops.c | 9 +++ drivers/base/power/power.h | 6 + drivers/base/power/runtime.c | 34 +-- include/linux/pm.h | 2 ++ include/linux/pm_runtime.h | 51 5 files changed, 100 insertions(+), 2 deletions(-) diff --git a/drivers/base/power/generic_ops.c b/drivers/base/power/generic_ops.c index 07c3c4a9522d..f0838229b781 100644 --- a/drivers/base/power/generic_ops.c +++ b/drivers/base/power/generic_ops.c @@ -10,6 +10,7 @@ #include #include #include +#include "power.h" #ifdef CONFIG_PM /** @@ -25,8 +26,12 @@ int pm_generic_runtime_suspend(struct device *dev) const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; int ret; + pm_runtime_notifier_call(dev, RPM_EVENT_SUSPEND_PRE); + ret = pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : 0; + pm_runtime_notifier_call(dev, RPM_EVENT_SUSPEND_POST); + return ret; } EXPORT_SYMBOL_GPL(pm_generic_runtime_suspend); @@ -44,8 +49,12 @@ int pm_generic_runtime_resume(struct device *dev) const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; int ret; + pm_runtime_notifier_call(dev, RPM_EVENT_RESUME_PRE); + ret = pm && pm->runtime_resume ? pm->runtime_resume(dev) : 0; + pm_runtime_notifier_call(dev, RPM_EVENT_RESUME_POST); + return ret; } EXPORT_SYMBOL_GPL(pm_generic_runtime_resume); diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h index 50e30e7b059d..30b6319ce96c 100644 --- a/drivers/base/power/power.h +++ b/drivers/base/power/power.h @@ -1,4 +1,5 @@ #include +#include static inline void device_pm_init_common(struct device *dev) { @@ -20,6 +21,7 @@ static inline void pm_runtime_early_init(struct device *dev) extern void pm_runtime_init(struct device *dev); extern void pm_runtime_reinit(struct device *dev); extern void pm_runtime_remove(struct device *dev); +extern int pm_runtime_notifier_call(struct device *dev, enum rpm_event event); struct wake_irq { struct device *dev; @@ -87,6 +89,10 @@ static inline void pm_runtime_early_init(struct device *dev) static inline void pm_runtime_init(struct device *dev) {} static inline void pm_runtime_reinit(struct device *dev) {} static inline void pm_runtime_remove(struct device *dev) {} +static inline pm_runtime_notifier_call(struct device *dev, enum rpm_event event) +{ + return 0; +} static inline int dpm_sysfs_add(struct device *dev) { return 0; } static inline void dpm_sysfs_remove(struct device *dev) {} diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index b74690418504..3a5637ca8400 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -227,6 +227,27 @@ void pm_runtime_set_memalloc_noio(struct device *dev, bool enable) } EXPORT_SYMBOL_GPL(pm_runtime_set_memalloc_noio); +int pm_runtime_notifier_call(struct device *dev, enum rpm_event event) +{ + return atomic_notifier_call_chain(&dev->power.runtime_notifier, + event, dev); +} + +int pm_runtime_register_notifier(struct device *dev, struct notifier_block *nb) +{ + return atomic_notifier_chain_register(&dev->power.runtime_notifier, + nb); +} +EXPORT_SYMBOL_GPL(pm_runtime_register_notifier); + +int pm_runtime_unregister_notifier(struct device *dev, + struct notifier_block *nb) +{ + return atomic_notifier_chain_unregister(&dev->power.runtime_notifier, + nb); +} +EXPORT_SYMBOL_GPL(pm_runtime_unregister_notifier); + /** * rpm_check_suspend_allowed - Test whether a device may be suspended. * @dev: Device to test. @@ -1174,6 +1195,7 @@ void __pm_runtime_disable(struct device *dev, bool check_resume) goto out; } + pm_runtime_notifier_call(dev, RPM_EVENT_DISABLE_PRE); /* * Wake up the device if there's a resume request pending, because that * means there probably is some I/O to process and disabling runtime PM @@ -1195,6 +1217,7 @@ void __pm_runtime_disable(struct device *dev, bool check_resume) if (!dev->power.disable_depth++) __pm_runtime_barrier(dev); + pm_runtime_notifier_call(dev, RPM_EVENT_DISABLE_POS
Re: [RFC PATCH 15/15] iommu/exynos: update to use iommu big-endian
Hi On 2016-06-08 20:31, Matthew Leach wrote: From: Ben Dooks Add initial support for big endian by always writing the pte in le32. Note, revisit if hardware capable of doing big endian fetches. Signed-off-by: Ben Dooks Acked-by: Marek Szyprowski Just to keep my curiosity satisfied - what's the reason to use big-endian on ARM? Good luck with fixing the kernel and userspace codes, which usually assume little-endian is the only possible order! --- Cc: Marek Szyprowski Cc: Joerg Roedel Cc: Kukjin Kim Cc: Krzysztof Kozlowski Cc: iommu@lists.linux-foundation.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-samsung-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org (open list) --- drivers/iommu/exynos-iommu.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 5ecc86c..dd8b3b3 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -54,6 +54,10 @@ typedef u32 sysmmu_pte_t; #define lv2ent_small(pent) ((*(pent) & 2) == 2) #define lv2ent_large(pent) ((*(pent) & 3) == 1) +#ifdef CONFIG_BIG_ENDIAN +#warning "revisit driver if we can enable big-endian ptes" +#endif + This warning can be removed. There is no way to force SYSMMU to operate with big-endian PTEs according to the datasheet. /* * v1.x - v3.x SYSMMU supports 32bit physical and 32bit virtual address spaces * v5.0 introduced support for 36bit physical address space by shifting @@ -716,7 +720,7 @@ static inline void update_pte(sysmmu_pte_t *ent, sysmmu_pte_t val) { dma_sync_single_for_cpu(dma_dev, virt_to_phys(ent), sizeof(*ent), DMA_TO_DEVICE); - *ent = val; + *ent = cpu_to_le32(val); dma_sync_single_for_device(dma_dev, virt_to_phys(ent), sizeof(*ent), DMA_TO_DEVICE); } Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] PM / Runtime: Add notifiers for device runtime PM events
Hi Rafael, On 2016-06-08 19:18, Rafael J. Wysocki wrote: On Wed, Jun 8, 2016 at 12:25 PM, Marek Szyprowski wrote: From: Krzysztof Kozlowski Allow drivers registering for certain runtime PM events of other devices. Some drivers in power domain are more or less coupled. When one driver is suspending (thus leading to power domain being also turned off) the other might have to perform some necessary steps. For example Exynos IOMMU has to save its context. Based on previous work of Sylwester Nawrocki. Signed-off-by: Krzysztof Kozlowski Signed-off-by: Marek Szyprowski No, this is not the right way to address this and using notifiers for that is just wrong (because of the potential ordering issues). Also, the problem is not limited to runtime PM, but also to system suspend/resume and initialization/shutdown. I posted a series of device dependencies patches a few months ago that might help to address this problem, but there was almost no interest in it at that time. I spent some time digging for your patches. Sadly no list archives had them all and I finally found them only in the linux-pm patchwork. This may explain why there was almost no interest in them. After some debugging I've managed to get it working for my case. I will include your patches in my v2 patchset together with the fixes needed to get it working. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 01/10] driver core: Add a wrapper around __device_release_driver()
From: "Rafael J. Wysocki" Add an internal wrapper around __device_release_driver() that will acquire device locks and do the necessary checks before calling it. The next patch will make use of it. Signed-off-by: Rafael J. Wysocki Signed-off-by: Marek Szyprowski --- drivers/base/dd.c | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 16688f50729c..d9e76e9205c7 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -796,6 +796,22 @@ static void __device_release_driver(struct device *dev) } } +static void device_release_driver_internal(struct device *dev, + struct device_driver *drv, + struct device *parent) +{ + if (parent) + device_lock(parent); + + device_lock(dev); + if (!drv || drv == dev->driver) + __device_release_driver(dev); + + device_unlock(dev); + if (parent) + device_unlock(parent); +} + /** * device_release_driver - manually detach device from driver. * @dev: device. @@ -810,9 +826,7 @@ void device_release_driver(struct device *dev) * within their ->remove callback for the same device, they * will deadlock right here. */ - device_lock(dev); - __device_release_driver(dev); - device_unlock(dev); + device_release_driver_internal(dev, NULL, NULL); } EXPORT_SYMBOL_GPL(device_release_driver); @@ -837,15 +851,7 @@ void driver_detach(struct device_driver *drv) dev = dev_prv->device; get_device(dev); spin_unlock(&drv->p->klist_devices.k_lock); - - if (dev->parent)/* Needed for USB */ - device_lock(dev->parent); - device_lock(dev); - if (dev->driver == drv) - __device_release_driver(dev); - device_unlock(dev); - if (dev->parent) - device_unlock(dev->parent); + device_release_driver_internal(dev, drv, dev->parent); put_device(dev); } } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 04/10] PM core: Make runtime PM of devices use device links
From: "Rafael J. Wysocki" Modify the runtime PM framework to use device links to ensure that supplier devices will not be suspended if any of their consumer devices are active. The idea is to reference count suppliers on the consumer's resume and drop references to them on its suspend. The information on whether or not the supplier has been reference counted by the consumer's (runtime) resume is stored in a new field (rpm_active) in the link object for each link. It may be necessary to clean up those references when the supplier is unbinding and that's why the links whose status is DEVICE_LINK_SUPPLIER_UNBIND are skipped by the runtime suspend and resume code. The above means that if the consumer device is probed in the runtime-active state, the supplier has to be resumed and reference counted by device_link_add() so the code works as expected on its (runtime) suspend. There is a new flag, DEVICE_LINK_RPM_ACTIVE, to tell device_link_add() about that (in which case the caller is responsible for making sure that the consumer really will be runtime-active when runtime PM is enabled for it). The other new link flag, DEVICE_LINK_PM_RUNTIME, tells the core whether or not the link should be used for runtime PM at all. Signed-off-by: Rafael J. Wysocki Signed-off-by: Marek Szyprowski --- drivers/base/core.c | 15 +++ drivers/base/dd.c| 1 + drivers/base/power/runtime.c | 93 +--- include/linux/device.h | 5 +++ include/linux/pm_runtime.h | 2 + 5 files changed, 111 insertions(+), 5 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 51a479ed68b5..1fe7cf232e1e 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -88,6 +88,11 @@ struct devlink *device_link_add(struct device *consumer, if (!consumer || !supplier || !flags) return NULL; +#define RPM_ACTIVE_FLAGS (DEVICE_LINK_PM_RUNTIME | DEVICE_LINK_PROBE_TIME) + if ((flags & DEVICE_LINK_RPM_ACTIVE) + && (flags & RPM_ACTIVE_FLAGS) != RPM_ACTIVE_FLAGS) + return NULL; + mutex_lock(&device_links_lock); list_for_each_entry(link, &supplier->supplier_links, s_node) @@ -98,6 +103,16 @@ struct devlink *device_link_add(struct device *consumer, if (!link) goto out; + if (flags & DEVICE_LINK_RPM_ACTIVE) { + if (pm_runtime_get_sync(supplier) < 0) { + pm_runtime_put_noidle(supplier); + kfree(link); + goto out; + } + link->rpm_active = true; + } else { + link->rpm_active = false; + } get_device(supplier); link->supplier = supplier; INIT_LIST_HEAD(&link->s_node); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 7c0abeba89e9..2efec98ddb12 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -791,6 +791,7 @@ static void __device_release_driver(struct device *dev, struct device *parent) } pm_runtime_get_sync(dev); + pm_runtime_clean_up_links(dev); driver_sysfs_remove(dev); diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index b74690418504..ba21c123b16e 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -12,6 +12,8 @@ #include #include #include + +#include "../base.h" #include "power.h" typedef int (*pm_callback_t)(struct device *); @@ -266,19 +268,69 @@ static int rpm_check_suspend_allowed(struct device *dev) static int __rpm_callback(int (*cb)(struct device *), struct device *dev) __releases(&dev->power.lock) __acquires(&dev->power.lock) { - int retval; + struct devlink *link; + int retval, idx; - if (dev->power.irq_safe) + if (dev->power.irq_safe) { spin_unlock(&dev->power.lock); - else + } else { spin_unlock_irq(&dev->power.lock); + /* +* Resume suppliers if necessary. +* +* The device's runtime PM status cannot change until this +* routine returns, so it is safe to read the status outside of +* the lock. +*/ + if (dev->power.runtime_status == RPM_RESUMING) { + idx = device_links_read_lock(); + + list_for_each_entry_rcu(link, &dev->consumer_links, c_node) + if ((link->flags & DEVICE_LINK_PM_RUNTIME) + && link->status != DEVICE_LINK_SUPPLIER_UNBIND + && !link->rpm_active) { + retval = pm_runt
[PATCH v2 08/10] PM core: Fix restoring devices with links during system PM transition
When devices are being runtime resumed during the system PM transition to suspend state, the link suppliers might be already resumed and have runtime pm disabled. This is normal case. This patch adds special support for such case. Simple call to pm_runtime_get_syncreturns error when device has runtime PM disabled, what results in incorrect runtime PM state during system wide PM transition. Signed-off-by: Marek Szyprowski --- drivers/base/power/runtime.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 0ea00d442e0f..d00b079fad88 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -261,6 +261,26 @@ static int rpm_check_suspend_allowed(struct device *dev) } /** + * __pm_runtime_force_resume - force resume of the device + * @dev: Device to resume + * + * This function works like __pm_runtime_resume(dev, RPM_GET_PUT), but + * also handles devices with runtime PM disabled. This allows to properly + * control all devices during preparation for system PM transition. + */ +static int __pm_runtime_force_resume(struct device *dev) +{ + if (!dev->power.disable_depth) + return pm_runtime_get_sync(dev); + + if (dev->power.runtime_status != RPM_ACTIVE || + atomic_inc_not_zero(&dev->power.usage_count) == 0) + return -EINVAL; + + return 0; +} + +/** * __rpm_callback - Run a given runtime PM callback for a given device. * @cb: Runtime PM callback to run. * @dev: Device to run the callback for. @@ -291,7 +311,7 @@ static int __rpm_callback(int (*cb)(struct device *), struct device *dev) if ((link->flags & DEVICE_LINK_PM_RUNTIME) && link->status != DEVICE_LINK_SUPPLIER_UNBIND && !link->rpm_active) { - retval = pm_runtime_get_sync(link->supplier); + retval = __pm_runtime_force_resume(link->supplier); if (retval < 0) { pm_runtime_put_noidle(link->supplier); goto fail; -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 09/10] iommu/exynos: Remove excessive, useless debug
Remove excessive, useless debug about skipping TLB invalidation, which is a normal situation when more aggressive power management is enabled. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 633e6d023c0d..9d1a14f88891 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -574,9 +574,6 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, sysmmu_unblock(data); } clk_disable(data->clk_master); - } else { - dev_dbg(data->master, - "disabled. Skipping TLB invalidation @ %#x\n", iova); } spin_unlock_irqrestore(&data->lock, flags); } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 00/10] Exynos IOMMU: proper runtime PM support (use device dependencies)
Hello, This patch series finally implements proper runtime PM support in Exynos IOMMU driver. This has been achieved by using device links, which lets SYSMMU controller's runtime PM to follow master's device runtime PM (the device which actually performs DMA transaction). The main idea behind this solution is an observation that any DMA activity from master device can be done only when master device is active, thus when master device is suspended SYSMMU controller device can also be suspended. This patchset solves the situation that power domains are always enabled, because all SYSMMU controllers (which belongs to those domains) are permanently active (because existing driver was simplified and kept SYSMMU device active all the time after initialization). Patches 1-5 are resend of the "[RFC][PATCH 0/5] Functional dependencies between devices" patchset: http://thread.gmane.org/gmane.linux.power-management.general/67424/focus=2126379 I've included them here, because it is hard to find them all on mailing list archives. Patches 6-8 are fixes to device dependencies/links code, which were required to use this solution for Exynos IOMMU driver. I'm not PM/runtime PM code expert, so please double check if my changes are really correct. This patchset requires my previous changes to Exynos IOMMU driver submitted in the "Exynos IOMMU: improve clock management" thread: http://www.spinics.net/lists/arm-kernel/msg505695.html Best regards Marek Szyprowski Samsung R&D Institute Poland Changelog: v2: - replaced PM notifiers with generic device dependencies/links developped by Rafael J. Wysocki v1: http://www.spinics.net/lists/arm-kernel/msg509600.html - initial version Patch summary: Marek Szyprowski (5): driver core: Avoid endless recursion if device has more than one link driver core: Add support for links to already probed drivers PM core: Fix restoring devices with links during system PM transition iommu/exynos: Remove excessive, useless debug iommu/exynos: Add proper runtime pm support Rafael J. Wysocki (5): driver core: Add a wrapper around __device_release_driver() driver core: Functional dependencies tracking support PM core: Make async suspend/resume of devices use device links PM core: Make runtime PM of devices use device links PM core: Optimize the use of device links for runtime PM drivers/base/base.h | 13 ++ drivers/base/core.c | 410 +++ drivers/base/dd.c| 65 +-- drivers/base/power/main.c| 68 ++- drivers/base/power/runtime.c | 130 +- drivers/iommu/exynos-iommu.c | 221 +++ include/linux/device.h | 41 + include/linux/pm.h | 1 + include/linux/pm_runtime.h | 6 + 9 files changed, 809 insertions(+), 146 deletions(-) -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 10/10] iommu/exynos: Add proper runtime pm support
This patch uses recently introduced device links to track the runtime pm state of the master's device. This way each SYSMMU controller is runtime active its master's device is active and can save/restore its state instead of being enabled all the time. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not used. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 218 --- 1 file changed, 99 insertions(+), 119 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 9d1a14f88891..80b7f1e7268c 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -233,8 +233,8 @@ struct sysmmu_drvdata { struct clk *aclk; /* SYSMMU's aclk clock */ struct clk *pclk; /* SYSMMU's pclk clock */ struct clk *clk_master; /* master's device clock */ - int activations;/* number of calls to sysmmu_enable */ spinlock_t lock;/* lock for modyfying state */ + int active; /* current status */ struct exynos_iommu_domain *domain; /* domain we belong to */ struct list_head domain_node; /* node for domain clients list */ struct list_head owner_node;/* node for owner controllers list */ @@ -247,25 +247,6 @@ static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom) return container_of(dom, struct exynos_iommu_domain, domain); } -static bool set_sysmmu_active(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU was not active previously - and it needs to be initialized */ - return ++data->activations == 1; -} - -static bool set_sysmmu_inactive(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU is needed to be disabled */ - BUG_ON(data->activations < 1); - return --data->activations == 0; -} - -static bool is_sysmmu_active(struct sysmmu_drvdata *data) -{ - return data->activations > 0; -} - static void sysmmu_unblock(struct sysmmu_drvdata *data) { writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); @@ -384,7 +365,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) unsigned short reg_status, reg_clear; int ret = -ENOSYS; - WARN_ON(!is_sysmmu_active(data)); + WARN_ON(!data->active); if (MMU_MAJ_VER(data->version) < 5) { reg_status = REG_INT_STATUS; @@ -430,7 +411,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) return IRQ_HANDLED; } -static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data) +static void __sysmmu_disable(struct sysmmu_drvdata *data) { clk_enable(data->clk_master); @@ -440,32 +421,6 @@ static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data) __sysmmu_disable_clocks(data); } -static bool __sysmmu_disable(struct sysmmu_drvdata *data) -{ - bool disabled; - unsigned long flags; - - spin_lock_irqsave(&data->lock, flags); - - disabled = set_sysmmu_inactive(data); - - if (disabled) { - data->pgtable = 0; - data->domain = NULL; - - __sysmmu_disable_nocount(data); - - dev_dbg(data->sysmmu, "Disabled\n"); - } else { - dev_dbg(data->sysmmu, "%d times left to disable\n", - data->activations); - } - - spin_unlock_irqrestore(&data->lock, flags); - - return disabled; -} - static void __sysmmu_init_config(struct sysmmu_drvdata *data) { unsigned int cfg; @@ -480,7 +435,7 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data) writel(cfg, data->sfrbase + REG_MMU_CFG); } -static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) +static void __sysmmu_enable(struct sysmmu_drvdata *data) { __sysmmu_enable_clocks(data); @@ -501,34 +456,6 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) clk_disable(data->clk_master); } -static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable, - struct exynos_iommu_domain *domain) -{ - int ret = 0; - unsigned long flags; - - spin_lock_irqsave(&data->lock, flags); - if (set_sysmmu_active(data)) { - data->pgtable = pgtable; - data->domain = domain; - - __sysmmu_enable_nocount(data); - - dev_dbg(data->sysmmu, "Enabled\n"); - } else { - ret = (pgtable == data->pgtable) ? 1 : -EBUSY; - - dev_dbg(data->sysmmu, "already enabled\n"); - } - - if (WARN_ON(ret <
[PATCH v2 03/10] PM core: Make async suspend/resume of devices use device links
From: "Rafael J. Wysocki" Make the device suspend/resume part of the core system suspend/resume code use device links to ensure that supplier and consumer devices will be suspended and resumed in the right order in case of async suspend/resume. The idea, roughly, is to use dpm_wait() to wait for all consumers before a supplier device suspend and to wait for all suppliers before a consumer device resume. Signed-off-by: Rafael J. Wysocki Signed-off-by: Marek Szyprowski --- drivers/base/base.h | 2 ++ drivers/base/core.c | 4 +-- drivers/base/power/main.c | 68 ++- 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index cccb1d211541..123b986eb061 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -163,3 +163,5 @@ extern void device_links_driver_gone(struct device *dev); extern void device_links_no_driver(struct device *dev); extern bool device_links_busy(struct device *dev); extern void device_links_unbind_consumers(struct device *dev); +extern int device_links_read_lock(void); +extern void device_links_read_unlock(int idx); diff --git a/drivers/base/core.c b/drivers/base/core.c index 416341df3268..51a479ed68b5 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -163,12 +163,12 @@ void device_link_del(struct devlink *link) } EXPORT_SYMBOL_GPL(device_link_del); -static int device_links_read_lock(void) +int device_links_read_lock(void) { return srcu_read_lock(&device_links_srcu); } -static void device_links_read_unlock(int idx) +void device_links_read_unlock(int idx) { return srcu_read_unlock(&device_links_srcu, idx); } diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index e44944f4be77..9470ccd87f11 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -244,6 +244,62 @@ static void dpm_wait_for_children(struct device *dev, bool async) device_for_each_child(dev, &async, dpm_wait_fn); } +static void dpm_wait_for_suppliers(struct device *dev, bool async) +{ + struct devlink *link; + int idx; + + idx = device_links_read_lock(); + + /* +* If the supplier goes away right after we've checked the link to it, +* we'll wait for its completion to change the state, but that's fine, +* because the only things that will block as a result are the SRCU +* callbacks freeing the link objects for the links in the list we're +* walking. +*/ + list_for_each_entry_rcu(link, &dev->consumer_links, c_node) + if (link->status != DEVICE_LINK_DORMANT) + dpm_wait(link->supplier, async); + + device_links_read_unlock(idx); +} + +static void dpm_wait_for_superior(struct device *dev, bool async) +{ + dpm_wait(dev->parent, async); + dpm_wait_for_suppliers(dev, async); +} + +static void dpm_wait_for_consumers(struct device *dev, bool async) +{ + struct devlink *link; + int idx; + + idx = device_links_read_lock(); + + /* +* The status of a device link can only be changed from "dormant" by a +* probe, but that cannot happen during system suspend/resume. In +* theory it can change to "dormant" at that time, but then it is +* reasonable to wait for the target device anyway (eg. if it goes +* away, it's better to wait for it to go away completely and then +* continue instead of trying to continue in parallel with its +* unregistration). +*/ + list_for_each_entry_rcu(link, &dev->supplier_links, s_node) + if (link->status != DEVICE_LINK_DORMANT) + dpm_wait(link->consumer, async); + + device_links_read_unlock(idx); +} + +static void dpm_wait_for_subordinate(struct device *dev, bool async) +{ + dpm_wait_for_children(dev, async); + dpm_wait_for_consumers(dev, async); +} + /** * pm_op - Return the PM operation appropriate for given PM event. * @ops: PM operations to choose from. @@ -488,7 +544,7 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn if (!dev->power.is_noirq_suspended) goto Out; - dpm_wait(dev->parent, async); + dpm_wait_for_superior(dev, async); if (dev->pm_domain) { info = "noirq power domain "; @@ -618,7 +674,7 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn if (!dev->power.is_late_suspended) goto Out; - dpm_wait(dev->parent, async); + dpm_wait_for_superior(dev, async); if (dev->pm_domain) { info = "early power domain "; @@ -750,7 +806,7 @@ static int device_resume(struct device *dev, pm_message_t state,
[PATCH v2 02/10] driver core: Functional dependencies tracking support
From: "Rafael J. Wysocki" Currently, there is a problem with handling cases where functional dependencies between devices are involved. What I mean by a "functional dependency" is when the driver of device B needs both device A and its driver to be present and functional to be able to work. This implies that the driver of A needs to be working for B to be probed successfully and it cannot be unbound from the device before the B's driver. This also has certain consequences for power management of these devices (suspend/resume and runtime PM ordering). Add support for representing those functional dependencies between devices to allow the driver core to track them and act on them in certain cases where they matter. The argument for doing that in the driver core is that there are quite a few distinct use cases related to that, they are relatively hard to get right in a driver (if one wants to address all of them properly) and it only gets worse if multiplied by the number of drivers potentially needing to do it. Morever, at least one case (asynchronous system suspend/resume) cannot be handled in a single driver at all, because it requires the driver of A to wait for B to suspend (during system suspend) and the driver of B to wait for A to resume (during system resume). To that end, represent links between devices (or more precisely between device+driver combos) as a struct devlink object containing pointers to the devices in question, a list node for each of them, status information, flags, a lock and an RCU head for synchronization. Also add two new list heads, supplier_links and consumer_links, to struct device to represent the lists of links to the devices that depend on the given one (consumers) and to the devices depended on by it (suppliers), respectively. The entire data structure consisting of all of the lists of link objects for all devices is protected by SRCU (for list walking) and a by mutex (for link object addition/removal). In addition to that, each link object has an internal status field whose value reflects what's happening to the devices pointed to by the link. That status field is protected by an internal spinlock. New links are added by calling device_link_add() which may happen either before the consumer device is probed or when probing it, in which case the caller needs to ensure that the driver of the supplier device is present and functional and the DEVICE_LINK_PROBE_TIME flag should be passed to device_link_add() to reflect that. Link objects are deleted either explicitly, by calling device_link_del() on the link object in question, or automatically, when the consumer device is unbound from its driver or when one of the target devices is deleted, depending on the link type. There are two types of link objects, persistent and non-persistent. The difference between them is that the persistent links stay around until one of the target devices is deleted, which the non-persistent ones are deleted when the consumer driver is unbound from its device (ie. they are assumed to be valid only as long as the consumer device has a driver bound to it). The DEVICE_LINK_PERSISTENT flag has to be passed to device_link_add() so as to create a persistent link. One of the actions carried out by device_link_add() is to reorder the lists used for device shutdown and system suspend/resume to put the consumer device along with all of its children and all of its consumers (and so on, recursively) to the ends of those list in order to ensure the right ordering between the all of the supplier and consumer devices. Signed-off-by: Rafael J. Wysocki Signed-off-by: Marek Szyprowski --- drivers/base/base.h| 11 ++ drivers/base/core.c| 386 + drivers/base/dd.c | 42 +- include/linux/device.h | 36 + 4 files changed, 470 insertions(+), 5 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index e05db388bd1c..cccb1d211541 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -107,6 +107,9 @@ extern void bus_remove_device(struct device *dev); extern int bus_add_driver(struct device_driver *drv); extern void bus_remove_driver(struct device_driver *drv); +extern void device_release_driver_internal(struct device *dev, + struct device_driver *drv, + struct device *parent); extern void driver_detach(struct device_driver *drv); extern int driver_probe_device(struct device_driver *drv, struct device *dev); @@ -152,3 +155,11 @@ extern int devtmpfs_init(void); #else static inline int devtmpfs_init(void) { return 0; } #endif + +/* Device links */ +extern int device_links_check_suppliers(struct device *dev); +extern void device_links_driver_bound(struct device *dev); +extern void device_links_driver_gone(struct device *dev); +extern void device_links_no_driver(struct device *dev); +extern
[PATCH v2 06/10] driver core: Avoid endless recursion if device has more than one link
This patch fixes endless recursion, which happends when device has more than one link. Signed-off-by: Marek Szyprowski --- drivers/base/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 215cd44de761..4e778539b750 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -57,7 +57,8 @@ static int device_reorder_to_tail(struct device *dev, void *not_used) device_pm_move_last(dev); device_for_each_child(dev, NULL, device_reorder_to_tail); list_for_each_entry(link, &dev->consumer_links, c_node) - device_reorder_to_tail(link->consumer, NULL); + if (link->consumer != dev) + device_reorder_to_tail(link->consumer, NULL); return 0; } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 07/10] driver core: Add support for links to already probed drivers
Set proper link state if link is created between already probed supplier device and to be probed consumer device. Signed-off-by: Marek Szyprowski --- drivers/base/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 4e778539b750..d9c5c5542a6b 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -125,7 +125,9 @@ struct devlink *device_link_add(struct device *consumer, link->flags = flags; link->status = (flags & DEVICE_LINK_PROBE_TIME) ? - DEVICE_LINK_CONSUMER_PROBE : DEVICE_LINK_DORMANT; + DEVICE_LINK_CONSUMER_PROBE : + (supplier->driver ? DEVICE_LINK_AVAILABLE : +DEVICE_LINK_DORMANT); spin_lock_init(&link->lock); /* -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 05/10] PM core: Optimize the use of device links for runtime PM
From: "Rafael J. Wysocki" If the device has no links to suppliers that should be used for runtime PM (links with DEVICE_LINK_PM_RUNTIME set), there is no reason to walk the list of suppliers for that device during runtime suspend and resume. Add a simple mechanism to detect that case and possibly avoid the extra unnecessary overhead. Signed-off-by: Rafael J. Wysocki Signed-off-by: Marek Szyprowski --- drivers/base/core.c | 6 ++ drivers/base/power/runtime.c | 23 --- include/linux/pm.h | 1 + include/linux/pm_runtime.h | 4 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 1fe7cf232e1e..215cd44de761 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -119,6 +119,9 @@ struct devlink *device_link_add(struct device *consumer, get_device(consumer); link->consumer = consumer; INIT_LIST_HEAD(&link->c_node); + if (flags & DEVICE_LINK_PM_RUNTIME) + pm_runtime_new_link(consumer); + link->flags = flags; link->status = (flags & DEVICE_LINK_PROBE_TIME) ? DEVICE_LINK_CONSUMER_PROBE : DEVICE_LINK_DORMANT; @@ -161,6 +164,9 @@ static void devlink_del(struct devlink *link) dev_info(link->consumer, "Dropping the link to %s\n", dev_name(link->supplier)); + if (link->flags & DEVICE_LINK_PM_RUNTIME) + pm_runtime_drop_link(link->consumer); + list_del_rcu(&link->s_node); list_del_rcu(&link->c_node); call_srcu(&device_links_srcu, &link->rcu_head, __devlink_free_srcu); diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index ba21c123b16e..0ea00d442e0f 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -270,6 +270,7 @@ static int __rpm_callback(int (*cb)(struct device *), struct device *dev) { struct devlink *link; int retval, idx; + bool use_links = dev->power.links_count > 0; if (dev->power.irq_safe) { spin_unlock(&dev->power.lock); @@ -283,7 +284,7 @@ static int __rpm_callback(int (*cb)(struct device *), struct device *dev) * routine returns, so it is safe to read the status outside of * the lock. */ - if (dev->power.runtime_status == RPM_RESUMING) { + if (use_links && dev->power.runtime_status == RPM_RESUMING) { idx = device_links_read_lock(); list_for_each_entry_rcu(link, &dev->consumer_links, c_node) @@ -314,8 +315,9 @@ static int __rpm_callback(int (*cb)(struct device *), struct device *dev) * * Do that if resume fails too. */ - if ((dev->power.runtime_status == RPM_SUSPENDING && !retval) - || (dev->power.runtime_status == RPM_RESUMING && retval)) { + if (use_links + && ((dev->power.runtime_status == RPM_SUSPENDING && !retval) + || (dev->power.runtime_status == RPM_RESUMING && retval))) { idx = device_links_read_lock(); fail: @@ -1525,6 +1527,21 @@ void pm_runtime_clean_up_links(struct device *dev) device_links_read_unlock(idx); } +void pm_runtime_new_link(struct device *dev) +{ + spin_lock_irq(&dev->power.lock); + dev->power.links_count++; + spin_unlock_irq(&dev->power.lock); +} + +void pm_runtime_drop_link(struct device *dev) +{ + spin_lock_irq(&dev->power.lock); + WARN_ON(dev->power.links_count == 0); + dev->power.links_count--; + spin_unlock_irq(&dev->power.lock); +} + /** * pm_runtime_force_suspend - Force a device into suspend state if needed. * @dev: Device to suspend. diff --git a/include/linux/pm.h b/include/linux/pm.h index 06eb353182ab..49c811378110 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -596,6 +596,7 @@ struct dev_pm_info { unsigned intuse_autosuspend:1; unsigned inttimer_autosuspends:1; unsigned intmemalloc_noio:1; + unsigned intlinks_count; enum rpm_requestrequest; enum rpm_status runtime_status; int runtime_error; diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index c21a21c064e3..bef6203aaf14 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -56,6 +56,8 @@ extern void pm_runtime_update_max_time_suspended(struct device *dev, s64 delta_ns); extern void pm_runtime_set_memalloc_noio(struct device *dev, bool enable); exter
Re: [RFC PATCH v2 03/15] arm64: mm: change IOMMU notifier action to attach DMA ops
Hi Robin, On 2016-06-17 11:27, Robin Murphy wrote: Hi Lorenzo, I think this patch makes sense even independent of the rest of the series, one nit inline notwithstanding. Marek; I'm curious as to whether this could make the workaround in 722ec35f7 obsolete as well, or are all the drivers also bound super-early in the setup you had there? Yes, this will solve that problem too. I will also hide some possible deferred probe issues, because the moment at which IOMMU is activated will be postponed. The only drawback with this approach is the fact that is drivers won't be allowed to do any dma-mapping operations on devices, which they don't own. This should not be a big issue, but this was the reason to setup IOMMU on device add instead of driver bind. While at it, please make sure that the case of failed client driver probe will be handled properly. IOMMU might do some operations while setting up and if the client driver fails to probe (for whatever reason, might be a deferred probe too), those operation has to be undone. However the current code of the driver core won't call any notifier (like BUS_NOTIFY_UNBOUND_DRIVER or whatever else) in such case. Long time ago I used BUS_NOTIFY_BIND_DRIVER based approach for my Exynos IOMMU patches and had to extend bus core with such patch: https://patchwork.kernel.org/patch/4678181/ to properly cleanup after failed client driver probe and avoid leaking resources. Please read the discussion, because some changes were requested to it. Best regards Marek Szyprowski, PhD Samsung R&D Institute Poland On 07/06/16 14:30, Lorenzo Pieralisi wrote: Current bus notifier in ARM64 (__iommu_attach_notifier) attempts to attach dma_ops to a device on BUS_NOTIFY_ADD_DEVICE action notification. This causes issues on ACPI based systems, where PCI devices can be added before the IOMMUs the devices are attached to had a chance to be probed, causing failures on attempts to attach dma_ops in that the domain for the respective IOMMU may not be set-up yet by the time the bus notifier is run. Devices dma_ops do not require to be set-up till the matching device drivers are probed. This means that instead of running the notifier attaching dma_ops to devices (__iommu_attach_notifier) on BUS_NOTIFY_ADD_DEVICE action, it can be run just before the device driver is bound to the device in question (on action BUS_NOTIFY_BIND_DRIVER) so that it is certain that its IOMMU group and domain are set-up accordingly at the time the notifier is triggered. This patch changes the notifier action upon which dma_ops are attached to devices and defer it to driver binding time, so that IOMMU devices have a chance to be probed and to register their bus notifiers before the dma_ops attach sequence for a device is actually carried out. Signed-off-by: Lorenzo Pieralisi Cc: Will Deacon Cc: Catalin Marinas Cc: Robin Murphy --- arch/arm64/mm/dma-mapping.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index c566ec8..79b0882 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -848,7 +848,7 @@ static int __iommu_attach_notifier(struct notifier_block *nb, { struct iommu_dma_notifier_data *master, *tmp; -if (action != BUS_NOTIFY_ADD_DEVICE) +if (action != BUS_NOTIFY_BIND_DRIVER) With this, you can also get rid of the priority setting and big fat explanatory comment in register_iommu_dma_ops_notifier(). Robin. return 0; mutex_lock(&iommu_dma_notifier_lock); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 03/15] arm64: mm: change IOMMU notifier action to attach DMA ops
Hi Lorenzo, On 2016-06-21 18:06, Lorenzo Pieralisi wrote: Hi Marek, On Tue, Jun 21, 2016 at 09:53:20AM +0200, Marek Szyprowski wrote: Hi Robin, On 2016-06-17 11:27, Robin Murphy wrote: Hi Lorenzo, I think this patch makes sense even independent of the rest of the series, one nit inline notwithstanding. Marek; I'm curious as to whether this could make the workaround in 722ec35f7 obsolete as well, or are all the drivers also bound super-early in the setup you had there? Yes, this will solve that problem too. I will also hide some possible deferred probe issues, because the moment at which IOMMU is activated will be postponed. The only drawback with this approach is the fact that is drivers won't be allowed to do any dma-mapping operations on devices, which they don't own. This should not be a big issue, but this was the reason to setup IOMMU on device add instead of driver bind. While at it, please make sure that the case of failed client driver probe will be handled properly. IOMMU might do some operations while setting up and if the client driver fails to probe (for whatever reason, might be a deferred probe too), those operation has to be undone. However the current code of the driver core won't call any notifier (like BUS_NOTIFY_UNBOUND_DRIVER or whatever else) in such case. Isn't Andy's commit 14b6257a5f3d enough ? Is that what you had in mind ? Long time ago I used BUS_NOTIFY_BIND_DRIVER based approach for my Exynos IOMMU patches and had to extend bus core with such patch: https://patchwork.kernel.org/patch/4678181/ to properly cleanup after failed client driver probe and avoid leaking resources. Please read the discussion, because some changes were requested to it. It looks like commit 14b6257a5f3d ("device core: add BUS_NOTIFY_DRIVER_NOT_BOUND notification") does what you are requesting, please let me know if that's enough. Yes, that's exactly the change I needed that time. Nice to see that it finally landed in mainline. I will revert the changes in 722ec35f7 and fold them in the new version along with Robin's suggestions. Okay. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/9] iommu/of: Consolidate device creation workarounds
Hi Robin, On 2016-06-28 17:48, Robin Murphy wrote: So far, all the users of the generic of_xlate configuration mechanism are resorting to explicit platform device creation to ensure the IOMMU device is ready before anything tries to refer to it. As I'm about to convert two more drivers that will need exactly the same thing, let's nip that proliferation in the bud and move it to a single place. A neat solution is to make of_get_iommu_ops() ensure an IOMMU device is instantiated before giving out its associated ops, since it's a fairly safe assumption that the ops aren't going to be much use if the IOMMU can't or won't exist to back them up. CC: Marek Szyprowski CC: Yong Wu Signed-off-by: Robin Murphy Frankly, I would avoid moving this workaround to the iommu core. IMHO the best solution would be to let IOMMU controllers to be instantiated as normal devices and implement proper support in the device core for waiting for the iommu controller. Then the workaround can be removed from exynos and mtk iommu drivers. What's the status of IOMMU deferred probe patches? I've encountered a serious problems with current code (the one which instantiates iommu controller devices from iommu driver) and its integration with power domains, clocks and runtime pm, which were not possible to resolve without iommu deferred probe. --- v3: New. drivers/iommu/exynos-iommu.c | 20 +++- drivers/iommu/mtk_iommu.c| 8 +--- drivers/iommu/of_iommu.c | 6 +- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 5ecc86cb74c8..97380ee56d71 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -665,6 +665,13 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) pm_runtime_enable(dev); + /* +* use the first registered sysmmu device for performing +* dma mapping operations on iommu page tables (cpu cache flush) +*/ + if (!dma_dev) + dma_dev = dev; + return 0; } @@ -1341,22 +1348,9 @@ err_reg_driver: 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); - - /* -* use the first registered sysmmu device for performing -* dma mapping operations on iommu page tables (cpu cache flush) -*/ - if (!dma_dev) - dma_dev = &pdev->dev; - of_iommu_set_ops(np, &exynos_iommu_ops); return 0; } diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index c3043d8754e3..f7ae87abea99 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -724,14 +724,8 @@ static struct platform_driver mtk_iommu_driver = { static int mtk_iommu_init_fn(struct device_node *np) { - int ret; - struct platform_device *pdev; + int ret = platform_driver_register(&mtk_iommu_driver); - pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root); - if (!pdev) - return -ENOMEM; - - ret = platform_driver_register(&mtk_iommu_driver); if (ret) { pr_err("%s: Failed to register driver\n", __func__); return ret; diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index af499aea0a1a..7e6369cffc95 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -22,6 +22,7 @@ #include #include #include +#include #include static const struct of_device_id __iommu_of_table_sentinel @@ -127,7 +128,10 @@ const struct iommu_ops *of_iommu_get_ops(struct device_node *np) spin_lock(&of_iommu_lock); list_for_each_entry(node, &of_iommu_list, list) if (node->np == np) { - ops = node->ops; + if (of_node_check_flag(np, OF_POPULATED) || + of_platform_device_create(np, NULL, + platform_bus_type.dev_root)) + ops = node->ops; break; } spin_unlock(&of_iommu_lock); Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/9] iommu/of: Consolidate device creation workarounds
Hi Robin, On 2016-07-01 13:19, Robin Murphy wrote: Hi Marek, On 01/07/16 11:32, Marek Szyprowski wrote: Hi Robin, On 2016-06-28 17:48, Robin Murphy wrote: So far, all the users of the generic of_xlate configuration mechanism are resorting to explicit platform device creation to ensure the IOMMU device is ready before anything tries to refer to it. As I'm about to convert two more drivers that will need exactly the same thing, let's nip that proliferation in the bud and move it to a single place. A neat solution is to make of_get_iommu_ops() ensure an IOMMU device is instantiated before giving out its associated ops, since it's a fairly safe assumption that the ops aren't going to be much use if the IOMMU can't or won't exist to back them up. CC: Marek Szyprowski CC: Yong Wu Signed-off-by: Robin Murphy Frankly, I would avoid moving this workaround to the iommu core. IMHO the best solution would be to let IOMMU controllers to be instantiated as normal devices and implement proper support in the device core for waiting for the iommu controller. Then the workaround can be removed from exynos and mtk iommu drivers. What's the status of IOMMU deferred probe patches? I think revisiting probe ordering is now second-from-top on my to-do list after this lot. This patch was kind of thinking ahead to get the "touch all the drivers" aspect out of the way before it grows any bigger, and all the development can then happen in the core code alone, but I admit it's not a particularly strong argument. I've encountered a serious problems with current code (the one which instantiates iommu controller devices from iommu driver) and its integration with power domains, clocks and runtime pm, which were not possible to resolve without iommu deferred probe. OK. Do you have any plans to try tweaking the current workaround, or is it really not worth it? I will keep it as is until the core will be ready for initializing Exynos iommu driver simply from the core_initcall (or other), which will just do platform_driver_register(&exynos_sysmmu_driver). FWIW I do have an Exynos 5410 (Odroid-XU) on my desk which I could theoretically test things on, but I suspect it would take a fair amount of work to get the SYSMMUs and relevant media bits up and running on top of Krzysztof's basic support. It should be quite easy to get Exynos DRM with HDMI display working on Odroid XU1, I can take a look in that after getting back from holidays (probably after 17th July). Will: for the time being, the alternative to this patch would be to squash the following change into patch 7/9 (without either, patch 8/9 doesn't really work). Robin. -8<- diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index e4c3916efba9..c002ff06b625 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -2678,6 +2678,14 @@ module_exit(arm_smmu_exit); static int __init arm_smmu_of_init(struct device_node *np) { + static bool registered; + + if (!registered) + registered = !arm_smmu_init(); + + if (!of_platform_device_create(np, NULL, platform_bus_type.dev_root)) + return -ENODEV; + of_iommu_set_ops(np, &arm_smmu_ops); return 0; Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH -next] iommu/exynos: Fix return value check in exynos_iommu_of_setup()
Hello, On 2016-07-06 15:09, Krzysztof Kozlowski wrote: On 07/06/2016 02:15 PM, weiyj...@163.com wrote: From: Wei Yongjun In case of error, the function of_platform_device_create() returns NULL pointer not ERR_PTR(). The IS_ERR() test in the return value check should be replaced with NULL test. Signed-off-by: Wei Yongjun --- drivers/iommu/exynos-iommu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 33dcc29..9b23059 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np) exynos_iommu_init(); pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root); - if (IS_ERR(pdev)) - return PTR_ERR(pdev); + if (!pdev) + return -ENOMEM; /* * use the first registered sysmmu device for performing Reviewed-by: Krzysztof Kozlowski Which in case of first sysmmu device and of_platform_device_create() failure will lead to NULL pointer exception, so I think it is appropriate to: Fixes: 8ed55c812fa8 ("iommu/exynos: Init from dt-specific callback instead of initcall") Cc: Thanks for spotting this issue. Acked-by: Marek Szyprowski Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] Exynos IOMMU: proper runtime PM support (use device dependencies)
Hi Tobias, On 2016-07-15 15:21, Tobias Jakobi wrote: Tobias Jakobi wrote: Hello Marek, I've tested the patchset on 4.7-rc7 and noticed that it breaks reboot on my ODROID-X2. Going to check where exactly things break. Sadly it's the last patch where everything comes together: "iommu/exynos: Add proper runtime pm support" I still have to check if forcing runpm status to 'on' makes a difference. I suspect that the aggressive clock gating might be the reason? Thanks for testing. I will check this issue. Could you send me your .config? Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] Exynos IOMMU: proper runtime PM support (use device dependencies)
Dear Tobias On 2016-07-18 13:00, Tobias Jakobi wrote: Marek Szyprowski wrote: On 2016-07-15 15:21, Tobias Jakobi wrote: Tobias Jakobi wrote: Hello Marek, I've tested the patchset on 4.7-rc7 and noticed that it breaks reboot on my ODROID-X2. Going to check where exactly things break. Sadly it's the last patch where everything comes together: "iommu/exynos: Add proper runtime pm support" I still have to check if forcing runpm status to 'on' makes a difference. I suspect that the aggressive clock gating might be the reason? Thanks for testing. I will check this issue. Could you send me your .config? This is the config I'm currently using: https://github.com/tobiasjakobi/odroid-environment/blob/master/sourcecode/system/vanilla-4.7-debug.conf Do you think checking this with no_console_suspend makes sense? no_console_suspend switch won't provide more information, but I managed to reproduce your issue. I'm really confused how enabling runtime pm can cause problems with usb/smsc95xx ethernet driver (that is the reason for failed reboot). Maybe it is somehow related to the global relations between devices and drivers and the fact that creating the runtime pm links change the order of operations. I will check this again when Rafael send updated patches. Here is the log I got (after waiting some time): # reboot Broadcast message from root@target (ttySAC1) (Mon Jul 18 13:33:38 2016): The system is going down for reboot NOW! INIT: Switching to runlevel: 6 INIT: Sending processes the TERM signal [info] Using makefile-style concurrent boot in runlevel 6. [ ok ] Stopping cgroup management proxy daemon: cgproxy[] Stopping internet superserver: inetd. [] Stopping cgroup management daemon: cgmanagermax77686-rtc max77686-rtc: RTC alarm IRQ: 119 [ ok ] Shutting down ALSA...done. random: nonblocking pool is initialized [info] Saving the system clock. [info] Hardware Clock updated to Mon Jul 18 13:33:40 UTC 2016. [ ok ] Asking all remaining processes to terminate...done. [ ok ] All processes ended within 1 seconds...done. [ ok ] Stopping rpcbind daemon [ ok ] Deconfiguring network interfaces...done. [ ok ] Unmounting temporary filesystems...done. [ ok ] Deactivating swap...done. EXT4-fs (mmcblk1p2): re-mounted. Opts: (null) [info] Will now restart. smsc95xx 1-2:1.0 eth0: Failed to read reg index 0x0114: -110 smsc95xx 1-2:1.0 eth0: Error reading MII_ACCESS smsc95xx 1-2:1.0 eth0: MII is busy in smsc95xx_mdio_read smsc95xx 1-2:1.0 eth0: Failed to read MII_BMSR INFO: task kworker/0:1:410 blocked for more than 120 seconds. Not tainted 4.7.0-rc7-debug+ #155 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. kworker/0:1 D c06850a8 0 410 2 0x Workqueue: events_freezable mmc_rescan Backtrace: [] (__schedule) from [] (schedule+0x44/0xb0) r10: r9:eebe1d88 r8:c0686234 r7: r6:0002 r5:7fff r4:7fff [] (schedule) from [] (schedule_timeout+0x154/0x1b0) [] (schedule_timeout) from [] (wait_for_common+0xe0/0x174) r8:c0686234 r7: r6:0002 r5:eebe1d8c r4:7fff [] (wait_for_common) from [] (wait_for_completion+0x18/0x1c) r10:0001 r9: r8: r7:eebe1d88 r6:eebe1d78 r5:ee26 r4:eebe1de4 [] (wait_for_completion) from [] (mmc_wait_for_req+0xc8/0x15c) [] (mmc_wait_for_req) from [] (mmc_wait_for_cmd+0x6c/0xa4) r8:eef73d00 r7:0003 r6:ee26 r5:c0a02448 r4:eebe1de4 r3: [] (mmc_wait_for_cmd) from [] (mmc_send_status+0x8c/0xb0) r7:eebe1eb0 r6:ee26 r5:ee26 r4: [] (mmc_send_status) from [] (mmc_alive+0x18/0x1c) r4:ee26 [] (mmc_alive) from [] (_mmc_detect_card_removed+0x40/0x90) [] (_mmc_detect_card_removed) from [] (mmc_detect+0x2c/0x78) r5:ee2602bc r4:ee26 [] (mmc_detect) from [] (mmc_rescan+0x1b0/0x324) r5:ee2602bc r4:ee260368 [] (mmc_rescan) from [] (process_one_work+0x194/0x414) r8:eef73d00 r7:eebe1eb0 r6:eef70280 r5:ee260368 r4:eea24080 r3:c04e94fc [] (process_one_work) from [] (worker_thread+0x34/0x4d4) r10:eef70280 r9:c0a02100 r8:0008 r7:eef70280 r6:eea24098 r5:eef702b4 r4:eea24080 [] (worker_thread) from [] (kthread+0xf8/0x11c) r10: r9: r8: r7:c013aad8 r6:eea24080 r5: r4:eea29d00 [] (kthread) from [] (ret_from_fork+0x14/0x24) r7: r6: r5:c0141d20 r4:eea29d00 2 locks held by kworker/0:1/410: #0: ("events_freezable"){.+.+.+}, at: [] process_one_work+0x128/0x414 #1: ((&(&host->detect)->work)){+.+.+.}, at: [] process_one_work+0x128/0x414 Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] Exynos IOMMU: proper runtime PM support (use device dependencies)
Hi Tobias On 2016-07-18 18:43, Tobias Jakobi wrote: Marek Szyprowski wrote: On 2016-07-18 13:00, Tobias Jakobi wrote: Marek Szyprowski wrote: On 2016-07-15 15:21, Tobias Jakobi wrote: Tobias Jakobi wrote: Hello Marek, I've tested the patchset on 4.7-rc7 and noticed that it breaks reboot on my ODROID-X2. Going to check where exactly things break. Sadly it's the last patch where everything comes together: "iommu/exynos: Add proper runtime pm support" I still have to check if forcing runpm status to 'on' makes a difference. I suspect that the aggressive clock gating might be the reason? Thanks for testing. I will check this issue. Could you send me your .config? This is the config I'm currently using: https://github.com/tobiasjakobi/odroid-environment/blob/master/sourcecode/system/vanilla-4.7-debug.conf Do you think checking this with no_console_suspend makes sense? no_console_suspend switch won't provide more information, but I managed to reproduce your issue. I'm really confused how enabling runtime pm can cause problems with usb/smsc95xx ethernet driver (that is the reason for failed reboot). Maybe it is somehow related to the global relations between devices and drivers and the fact that creating the runtime pm links change the order of operations. I will check this again when Rafael send updated patches. Here is the log I got (after waiting some time): thanks for looking into this! I'll try to reproduce this on my board. I have to admit that I didn't wait too long for the hung task message to appear. I wonder if this has something to do with regulator code cutting some supplies too early. Is this on a X2 or a U2/U3? I've reproduced it on U3. I'm not sure if we currently model the regulator setup correctly here (IIRC then buck8 is supplying the LAN/USB block on U2/U3). IMHO it is not really related to regulator operations, but the sequence of shutting down logical devices in the system. For some reasons when pm links are used, something changes the order of operations in system shutdown procedure, what causes smsc95xx to hang. I have no idea why, but I don't have time to investigate it further. I will wait for the next release of Rafael's pm links patches and then check everything again. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 7/8] iommu: of: Handle IOMMU lookup failure with deferred probing or error
Hi Sricharan, On 2016-08-12 17:40, Sricharan wrote: Hi Tomaz, + if (ops->add_device) + ops = ops->add_device(dev) ? ops : NULL; Patch description fails to mention anything about this change. Also it looks slightly incorrect to lose the error condition here. I think we should at least print some error message here and tell the user that we are falling back to non-IOMMU setup. Ok, will have to improve the patch description to add this and will fix the error value as well. Will also add the remove_device during deconfigure as well. of_node_put(np); idx++; @@ -200,7 +213,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, err_put_node: of_node_put(np); - return NULL; + return ops; } void __init of_iommu_init(void) @@ -211,7 +224,7 @@ void __init of_iommu_init(void) for_each_matching_node_and_match(np, matches, &match) { const of_iommu_init_fn init_fn = match->data; - if (init_fn(np)) + if (init_fn && init_fn(np)) When is it possible to have NULL init_fn? ya wrong, should not be needed. init_fn can be NULL if you convert IOMMU driver to use platform device infrastructure based initialization and you don't need to do anything before the driver gets probed, so please keep this check. I used this approach here: https://git.linaro.org/people/marek.szyprowski/linux-srpol.git/commitdiff/a30735973573128b14bb4a25cf4debaa0979a655 (this commit is a part of v4.8-clocks-pm branch) IOMMU_OF_DECLARE() with NULL init_fn is needed to notify IOMMU core that this IOMMU driver is available in the system and core has to defer probing of drivers before the IOMMU driver gets initialized from its controller's platform device. pr_err("Failed to initialise IOMMU %s\n", of_node_full_name(np)); } diff --git a/drivers/of/device.c b/drivers/of/device.c index e1fad50..92e02dc 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -149,6 +149,8 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np) coherent ? " " : " not "); iommu = of_iommu_configure(dev, np); + if (IS_ERR(iommu)) + return PTR_ERR(iommu); nit: Would be good to add a blank line here for improved readability. ok, will add. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/8] arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()
Hi Sricharan and Laurent, On 2016-08-09 00:49, Sricharan R wrote: From: Laurent Pinchart The arch_setup_dma_ops() function is in charge of setting dma_ops with a call to set_dma_ops(). set_dma_ops() is also called from - highbank and mvebu bus notifiers - dmabounce (to be replaced with swiotlb) - arm_iommu_attach_device (arm_iommu_attach_device is itself called from IOMMU and bus master device drivers) To allow the arch_setup_dma_ops() call to be moved from device add time to device probe time we must ensure that dma_ops already setup by any of the above callers will not be overridden. Aftering replacing dmabounce with swiotlb, converting IOMMU drivers to of_xlate and taking care of highbank and mvebu, the workaround should be removed. Signed-off-by: Laurent Pinchart --- arch/arm/mm/dma-mapping.c | 9 + 1 file changed, 9 insertions(+) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index ff7ed56..4686d66 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -2259,6 +2259,15 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, struct dma_map_ops *dma_ops; dev->archdata.dma_coherent = coherent; + + /* +* Don't override the dma_ops if they have already been set. Ideally +* this should be the only location where dma_ops are set, remove this +* check when all other callers of set_dma_ops will have disappeared. +*/ + if (dev->archdata.dma_ops) + return; + if (arm_setup_iommu_dma_ops(dev, dma_base, size, iommu)) dma_ops = arm_get_iommu_dma_map_ops(coherent); else Please add also set_dma_ops(dev, NULL) to arm_teardown_iommu_dma_ops() function to this patch, because otherwise dma_ops won't be configured properly if deferred probe of the given device happens: iommu dma ops will be set for the first probe try, then iommu structures will be teared down after EPROBEDEFER error, and then before next probe ties dma_ops will be already set, but no iommu structures are available. For a convenience, this a fixup patch: https://git.linaro.org/people/marek.szyprowski/linux-srpol.git/commitdiff/55adefd43cee9d4beb15cb1bbd805c5059b56b4f Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 7/8] iommu: of: Handle IOMMU lookup failure with deferred probing or error
Hi Sricharan, On 2016-08-09 00:49, Sricharan R wrote: > From: Laurent Pinchart > > Failures to look up an IOMMU when parsing the DT iommus property need to > be handled separately from the .of_xlate() failures to support deferred > probing. > > The lack of a registered IOMMU can be caused by the lack of a driver for > the IOMMU, the IOMMU device probe not having been performed yet, having > been deferred, or having failed. > > The first case occurs when the device tree describes the bus master and > IOMMU topology correctly but no device driver exists for the IOMMU yet > or the device driver has not been compiled in. Return NULL, the caller > will configure the device without an IOMMU. > > The second and third cases are handled by deferring the probe of the bus > master device which will eventually get reprobed after the IOMMU. > > The last case is currently handled by deferring the probe of the bus > master device as well. A mechanism to either configure the bus master > device without an IOMMU or to fail the bus master device probe depending > on whether the IOMMU is optional or mandatory would be a good > enhancement. > > Signed-off-by: Laurent Pinchart It is a common practice to briefly describe here what has been changed since the original patch if you have modified it (see commit 855ed04a3758b205e84b269f92d26ab36ed8e2f7 for the example). > Signed-off-by: Sricharan R > --- > drivers/iommu/of_iommu.c | 21 + > drivers/of/device.c | 2 ++ > 2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > index 4c4219d..3994cf5 100644 > --- a/drivers/iommu/of_iommu.c > +++ b/drivers/iommu/of_iommu.c > @@ -149,7 +149,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, > { > struct of_phandle_args iommu_spec; > struct device_node *np = NULL; > -struct iommu_ops *ops = NULL; > +const struct iommu_ops *ops = NULL; > int idx = 0; > > if (dev_is_pci(dev)) { > @@ -189,8 +189,21 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, > np = iommu_spec.np; > ops = of_iommu_get_ops(np); > > -if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec)) > +if (!ops) { > +const struct of_device_id *oid; > + > +oid = of_match_node(&__iommu_of_table, np); > +ops = oid ? ERR_PTR(-EPROBE_DEFER) : NULL; > goto err_put_node; > +} > + > +if (!ops->of_xlate || ops->of_xlate(dev, &iommu_spec)) { > +ops = NULL; > +goto err_put_node; > +} > + > +if (ops->add_device) > +ops = ops->add_device(dev) ? ops : NULL; ops->add_device() returns ZERO on success or error code on failure, so the above line should be changed to: ops = (ops->add_device(dev) == 0) ? ops : NULL; > of_node_put(np); > idx++; > @@ -200,7 +213,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, > > err_put_node: > of_node_put(np); > -return NULL; > +return ops; > } > > void __init of_iommu_init(void) > @@ -211,7 +224,7 @@ void __init of_iommu_init(void) > for_each_matching_node_and_match(np, matches, &match) { > const of_iommu_init_fn init_fn = match->data; > > -if (init_fn(np)) > +if (init_fn && init_fn(np)) > pr_err("Failed to initialise IOMMU %s\n", > of_node_full_name(np)); > } > diff --git a/drivers/of/device.c b/drivers/of/device.c > index e1fad50..92e02dc 100644 > --- a/drivers/of/device.c > +++ b/drivers/of/device.c > @@ -149,6 +149,8 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np) > coherent ? " " : " not "); > > iommu = of_iommu_configure(dev, np); > +if (IS_ERR(iommu)) > +return PTR_ERR(iommu); > dev_dbg(dev, "device is%sbehind an iommu\n", > iommu ? " " : " not "); > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 1/2] iommu/exynos: Remove excessive, useless debug
Remove excessive, useless debug about skipping TLB invalidation, which is a normal situation when more aggressive power management is enabled. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 75fbe5d30cb3..b0fa4d432e71 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -579,9 +579,6 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, sysmmu_unblock(data); } clk_disable(data->clk_master); - } else { - dev_dbg(data->master, - "disabled. Skipping TLB invalidation @ %#x\n", iova); } spin_unlock_irqrestore(&data->lock, flags); } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies)
Hello, This patch series finally implements proper runtime PM support in Exynos IOMMU driver. This has been achieved by using recently introduce device links, which lets SYSMMU controller's runtime PM to follow master's device runtime PM state (the device which actually performs DMA transaction). The main idea behind this solution is an observation that any DMA activity from master device can be done only when master device is active, thus when master device is suspended SYSMMU controller device can also be suspended. This patchset solves the situation that power domains are always enabled, because all SYSMMU controllers (which belongs to those domains) are permanently active (because existing driver was simplified and kept SYSMMU device active all the time after initialization). Patch requires second version of Rafeal's "Functional dependencies between devices" patchset, which is available here: https://lkml.org/lkml/2016/9/8/798 If one wants to test this patchset, I've provided a branch with all needed patches (some fixes for Exynos4 FIMC-IS driver are needed): https://git.linaro.org/people/marek.szyprowski/linux-srpol.git v4.8-iommu-pm-v3 Patches are based on vanilla v4.8-rc6 kernel with Rafael's patches applied. Best regards Marek Szyprowski Samsung R&D Institute Poland Changelog: v3: - rebased on top of latest device dependencies/links patchset - added proper locking between runtime pm, iommu_attach/detach and sysmmu enable/disable(added per iommu owner device's rpm lock) v2: - replaced PM notifiers with generic device dependencies/links developped by Rafael J. Wysocki v1: http://www.spinics.net/lists/arm-kernel/msg509600.html - initial version Patch summary: Marek Szyprowski (2): iommu/exynos: Remove excessive, useless debug iommu/exynos: Add proper runtime pm support drivers/iommu/exynos-iommu.c | 228 ++- 1 file changed, 94 insertions(+), 134 deletions(-) -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 2/2] iommu/exynos: Add proper runtime pm support
This patch uses recently introduced device links to track the runtime pm state of the master's device. This way each SYSMMU controller is runtime activated when its master's device is active and can save/restore its state instead of being enabled all the time. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not used. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 225 ++- 1 file changed, 94 insertions(+), 131 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index b0fa4d432e71..34717a0b1902 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -206,6 +206,7 @@ static const struct sysmmu_fault_info sysmmu_v5_faults[] = { struct exynos_iommu_owner { struct list_head controllers; /* list of sysmmu_drvdata.owner_node */ struct iommu_domain *domain;/* domain this device is attached */ + struct mutex rpm_lock; /* for runtime pm of all sysmmus */ }; /* @@ -237,8 +238,8 @@ struct sysmmu_drvdata { struct clk *aclk; /* SYSMMU's aclk clock */ struct clk *pclk; /* SYSMMU's pclk clock */ struct clk *clk_master; /* master's device clock */ - int activations;/* number of calls to sysmmu_enable */ spinlock_t lock;/* lock for modyfying state */ + int active; /* current status */ struct exynos_iommu_domain *domain; /* domain we belong to */ struct list_head domain_node; /* node for domain clients list */ struct list_head owner_node;/* node for owner controllers list */ @@ -251,25 +252,6 @@ static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom) return container_of(dom, struct exynos_iommu_domain, domain); } -static bool set_sysmmu_active(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU was not active previously - and it needs to be initialized */ - return ++data->activations == 1; -} - -static bool set_sysmmu_inactive(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU is needed to be disabled */ - BUG_ON(data->activations < 1); - return --data->activations == 0; -} - -static bool is_sysmmu_active(struct sysmmu_drvdata *data) -{ - return data->activations > 0; -} - static void sysmmu_unblock(struct sysmmu_drvdata *data) { writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); @@ -389,7 +371,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) unsigned short reg_status, reg_clear; int ret = -ENOSYS; - WARN_ON(!is_sysmmu_active(data)); + WARN_ON(!data->active); if (MMU_MAJ_VER(data->version) < 5) { reg_status = REG_INT_STATUS; @@ -435,40 +417,19 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) return IRQ_HANDLED; } -static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data) +static void __sysmmu_disable(struct sysmmu_drvdata *data) { + unsigned long flags; + clk_enable(data->clk_master); + spin_lock_irqsave(&data->lock, flags); writel(CTRL_DISABLE, data->sfrbase + REG_MMU_CTRL); writel(0, data->sfrbase + REG_MMU_CFG); - - __sysmmu_disable_clocks(data); -} - -static bool __sysmmu_disable(struct sysmmu_drvdata *data) -{ - bool disabled; - unsigned long flags; - - spin_lock_irqsave(&data->lock, flags); - - disabled = set_sysmmu_inactive(data); - - if (disabled) { - data->pgtable = 0; - data->domain = NULL; - - __sysmmu_disable_nocount(data); - - dev_dbg(data->sysmmu, "Disabled\n"); - } else { - dev_dbg(data->sysmmu, "%d times left to disable\n", - data->activations); - } - + data->active = false; spin_unlock_irqrestore(&data->lock, flags); - return disabled; + __sysmmu_disable_clocks(data); } static void __sysmmu_init_config(struct sysmmu_drvdata *data) @@ -485,17 +446,19 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data) writel(cfg, data->sfrbase + REG_MMU_CFG); } -static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) +static void __sysmmu_enable(struct sysmmu_drvdata *data) { + unsigned long flags; + __sysmmu_enable_clocks(data); + spin_lock_irqsave(&data->lock, flags); writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL); - __sysmmu_init_config(data); - __sysmmu_set_ptbase(data, data->pgtable); - writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); + data->ac
Re: [PATCH v3 2/2] iommu/exynos: Add proper runtime pm support
Hi Ulf, On 2016-09-13 16:20, Ulf Hansson wrote: On 13 September 2016 at 14:49, Marek Szyprowski wrote: This patch uses recently introduced device links to track the runtime pm state of the master's device. This way each SYSMMU controller is runtime activated when its master's device is active and can save/restore its state instead of being enabled all the time. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not used. Apologize for not reviewing earlier and if you find my questions/suggestions being silly. You may ignore them, if you don't think they deserves a proper answer. :-) No problem. There are no silly questions, but there might be some silly answers ;) I am not so familiar with the IOMMU subsystem, but I am wondering whether the issue you are solving, is similar to what can be observed for DMA and serial drivers. And of course also for other IOMMU drivers. In general the DMA/serial drivers requires to use the pm_runtime_irq_safe() option, to be able to easily deploy runtime PM support (of course there are some other workarounds as well). There are some similarities between IOMMU and DMA engine devices (serial drivers are imho completely different case). Both hw blocks do their work on behalf of some other hardware block, which I will call master device. DMA engine performs some DMA transaction on master's device request, while IOMMU usually sits between system memory and master's device memory interface, remapping addresses of each DMA transaction according to its configuration and provided mapping tables (master device has some kind of internal DMA controller and performs DMA transactions on their own). IOMMU is usually used for a) mapping physically discontinuous memory into contiguous DMA addresses and b) isolating devices, so they can access only memory, which is dedicated or allocated for them. DMA engine devices provide explicit API for their master's device drivers, while IOMMU drivers are usually hidden behind DMA-mapping API (for most use cases, although it would be possible for master's device driver to call IOMMU API directly and some GPU/DRM drivers do that). However from runtime pm perspective the DMA engine and IOMMU devices are a bit different. DMA engine drivers have well defined start and end of operation (queuing dma request and irq from hw about having it finished). During that time the device has to be runtime active all the time. The problem with using current implementation of runtime pm is the fact that both start and end of operation can be triggered from atomic context, what is not really suitable for runtime pm. So the problem is mainly about API incompatibility and lack of something like dma_engine_prepare()/unprepare() (as an analogy to clocks api). In case of IOMMU the main problem is determining weather IOMMU controller has to be activated. There is no calls in IOMMU and DMA-mapping API, which would bracket all DMA transactions performed by the master device. Someone proposed to keep IOMMU runtime active when there exist at least one mapping created by the IOMMU/DMA-mapping layers. This however does not cover all the cases. In case of our IOMMU, when it is disabled or runtime suspended, it enters "pass-thought" mode, so master device can still perform DMA operations with identity mappings (so DMA address equals to physical memory address). Till now Exynos IOMMU called pm_runtime_get() on attaching to the iommu domain (what happens during initialization of dma-mapping structures for given master device) and kept it active all the time. This patch series tries to address Exynos IOMMU runtime pm issue by forcing IOMMU controller to follow runtime pm status of its master device. This way we ensure that anytime when master's device is runtime activated, the iommu will be also active and master device won't be able to bypass during its DMA transactions mappings created by the IOMMU layer. Quite long answer, but I hope I managed to give you a bit more background on this topic. As we know, using the pm_runtime_irq_safe() option comes with some limitations, such as the runtime PM callbacks is not allowed to sleep. For a PM domain (genpd) that is attached to the device, this also means it must not be powered off. Right, if possible I would like to avoid using pm_runtime_irq_safe() option, because it is really impractical. To solve this problem, I was thinking we could convert to use the asynchronous pm_runtime_get() API, when trying to runtime resume the device from atomic contexts. I'm not sure if this will work for DMA engine devices. If I understand correctly some client's of DMA engine device might rely on the DMA engine being configured and operational after queuing a request and they might lock up if the DMA engine device activation if postponed because of async runtime pm activation. Of course when it turns o
Re: [PATCH v3 2/2] iommu/exynos: Add proper runtime pm support
s usually uses some additional signal lines for DMA, so the HW block might check if all needed signals from DMA engine HW are ready. If not it will fail to start avoid lockup of starting without DMA engine HW being ready. However I don't have much experience with DMA engine and drivers. I only helped in adding DMA engine support to Samsung UART driver and in the hardware manual there is information about additional lines between DMA controller and UART module, which are used in the DMA mode. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 22/22] iommu/dma: Avoid PCI host bridge windows
Hi Robin, On 2016-09-12 18:14, Robin Murphy wrote: With our DMA ops enabled for PCI devices, we should avoid allocating IOVAs which a host bridge might misinterpret as peer-to-peer DMA and lead to faults, corruption or other badness. To be safe, punch out holes for all of the relevant host bridge's windows when initialising a DMA domain for a PCI device. CC: Marek Szyprowski CC: Inki Dae Reported-by: Lorenzo Pieralisi Signed-off-by: Robin Murphy I don't know much about PCI and their IOMMU integration, but can't we use the direct mapping region feature of iommu core for it? There are already iommu_get_dm_regions(), iommu_put_dm_regions() and iommu_request_dm_for_dev() functions for handling them... --- - Squash in the previous drm/exynos fixup - If need be, this one can probably wait --- arch/arm64/mm/dma-mapping.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_iommu.h | 2 +- drivers/iommu/dma-iommu.c | 25 - include/linux/dma-iommu.h | 3 ++- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index c4284c432ae8..610d8e53011e 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -827,7 +827,7 @@ static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops, * then the IOMMU core will have already configured a group for this * device, and allocated the default domain for that group. */ - if (!domain || iommu_dma_init_domain(domain, dma_base, size)) { + if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) { pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n", dev_name(dev)); return false; diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.h b/drivers/gpu/drm/exynos/exynos_drm_iommu.h index c8de4913fdbe..87f6b5672e11 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_iommu.h +++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.h @@ -66,7 +66,7 @@ static inline int __exynos_iommu_create_mapping(struct exynos_drm_private *priv, if (ret) goto free_domain; - ret = iommu_dma_init_domain(domain, start, size); + ret = iommu_dma_init_domain(domain, start, size, NULL); if (ret) goto put_cookie; diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 4329d18080cf..c5ab8667e6f2 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -103,18 +104,38 @@ void iommu_put_dma_cookie(struct iommu_domain *domain) } EXPORT_SYMBOL(iommu_put_dma_cookie); +static void iova_reserve_pci_windows(struct pci_dev *dev, + struct iova_domain *iovad) +{ + struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus); + struct resource_entry *window; + unsigned long lo, hi; + + resource_list_for_each_entry(window, &bridge->windows) { + if (resource_type(window->res) != IORESOURCE_MEM && + resource_type(window->res) != IORESOURCE_IO) + continue; + + lo = iova_pfn(iovad, window->res->start - window->offset); + hi = iova_pfn(iovad, window->res->end - window->offset); + reserve_iova(iovad, lo, hi); + } +} + /** * iommu_dma_init_domain - Initialise a DMA mapping domain * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie() * @base: IOVA at which the mappable address space starts * @size: Size of IOVA space + * @dev: Device the domain is being initialised for * * @base and @size should be exact multiples of IOMMU page granularity to * avoid rounding surprises. If necessary, we reserve the page at address 0 * to ensure it is an invalid IOVA. It is safe to reinitialise a domain, but * any change which could make prior IOVAs invalid will fail. */ -int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 size) +int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, + u64 size, struct device *dev) { struct iova_domain *iovad = cookie_iovad(domain); unsigned long order, base_pfn, end_pfn; @@ -152,6 +173,8 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 size iovad->dma_32bit_pfn = end_pfn; } else { init_iova_domain(iovad, 1UL << order, base_pfn, end_pfn); + if (dev && dev_is_pci(dev)) + iova_reserve_pci_windows(to_pci_dev(dev), iovad); } return 0; } diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h index 5ee806e41b5c..32c589062bd9 100644 --- a/include/linux/dm
Re: [PATCH v7 22/22] iommu/dma: Avoid PCI host bridge windows
Hi Robin, On 2016-09-14 13:10, Robin Murphy wrote: On 14/09/16 11:55, Marek Szyprowski wrote: On 2016-09-12 18:14, Robin Murphy wrote: With our DMA ops enabled for PCI devices, we should avoid allocating IOVAs which a host bridge might misinterpret as peer-to-peer DMA and lead to faults, corruption or other badness. To be safe, punch out holes for all of the relevant host bridge's windows when initialising a DMA domain for a PCI device. CC: Marek Szyprowski CC: Inki Dae Reported-by: Lorenzo Pieralisi Signed-off-by: Robin Murphy I don't know much about PCI and their IOMMU integration, but can't we use the direct mapping region feature of iommu core for it? There are already iommu_get_dm_regions(), iommu_put_dm_regions() and iommu_request_dm_for_dev() functions for handling them... It's rather the opposite problem - in the direct-mapping case, we're making sure the iommu_domain has translations installed for the given IOVAs (which are also the corresponding physical address) before it goes live, whereas what we need to do here is make sure the these addresses never get used as IOVAs at all, because any attempt to do so them will likely go wrong. Thus we carve them out of the iova_domain such that they will never get near an actual IOMMU API call. This is a slightly generalised equivalent of e.g. amd_iommu.c's init_reserved_iova_ranges(). Hmmm. Each dm_region have protection parameter. Can't we reuse them to create prohibited/reserved regions by setting it to 0 (no read / no write) and mapping to physical 0 address? That's just a quick idea, because dm_regions and the proposed code for pci looks a bit similar for me... [...] Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/exynos: Improve page fault debug message
Add master device name to default IOMMU fault message to make easier to find which device triggered the fault. While at it, move printing some information (like page table base and first level entry addresses) to dev_dbg(), because those are typically not very useful for typical device driver user/developer not equipped with hardware debugging tools. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 30808e9..47f0d67 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -368,13 +368,14 @@ static void show_fault_information(struct sysmmu_drvdata *data, { sysmmu_pte_t *ent; - dev_err(data->sysmmu, "%s FAULT occurred at %#x (page table base: %pa)\n", - finfo->name, fault_addr, &data->pgtable); + dev_err(data->sysmmu, "%s: %s FAULT occurred at %#x\n", + dev_name(data->master), finfo->name, fault_addr); + dev_dbg(data->sysmmu, "Page table base: %pa\n", &data->pgtable); ent = section_entry(phys_to_virt(data->pgtable), fault_addr); - dev_err(data->sysmmu, "\tLv1 entry: %#x\n", *ent); + dev_dbg(data->sysmmu, "\tLv1 entry: %#x\n", *ent); if (lv1ent_page(ent)) { ent = page_entry(ent, fault_addr); - dev_err(data->sysmmu, "\t Lv2 entry: %#x\n", *ent); + dev_dbg(data->sysmmu, "\t Lv2 entry: %#x\n", *ent); } } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 22/22] iommu/dma: Avoid PCI host bridge windows
Hi Robin, On 2016-09-14 15:25, Robin Murphy wrote: On 14/09/16 13:35, Marek Szyprowski wrote: On 2016-09-14 13:10, Robin Murphy wrote: On 14/09/16 11:55, Marek Szyprowski wrote: On 2016-09-12 18:14, Robin Murphy wrote: With our DMA ops enabled for PCI devices, we should avoid allocating IOVAs which a host bridge might misinterpret as peer-to-peer DMA and lead to faults, corruption or other badness. To be safe, punch out holes for all of the relevant host bridge's windows when initialising a DMA domain for a PCI device. CC: Marek Szyprowski CC: Inki Dae Reported-by: Lorenzo Pieralisi Signed-off-by: Robin Murphy I don't know much about PCI and their IOMMU integration, but can't we use the direct mapping region feature of iommu core for it? There are already iommu_get_dm_regions(), iommu_put_dm_regions() and iommu_request_dm_for_dev() functions for handling them... It's rather the opposite problem - in the direct-mapping case, we're making sure the iommu_domain has translations installed for the given IOVAs (which are also the corresponding physical address) before it goes live, whereas what we need to do here is make sure the these addresses never get used as IOVAs at all, because any attempt to do so them will likely go wrong. Thus we carve them out of the iova_domain such that they will never get near an actual IOMMU API call. This is a slightly generalised equivalent of e.g. amd_iommu.c's init_reserved_iova_ranges(). Hmmm. Each dm_region have protection parameter. Can't we reuse them to create prohibited/reserved regions by setting it to 0 (no read / no write) and mapping to physical 0 address? That's just a quick idea, because dm_regions and the proposed code for pci looks a bit similar for me... It might look similar, but at different levels (iommu_domain vs. iova_domain) and with the opposite intent. The dm_region prot flag is just the standard flag as passed to iommu_map() - trying to map a region with no access in an empty pagetable isn't going to achieve anything anyway (it's effectively unmapping addresses that are already unmapped). But for this case, even if you _did_ map something in the pagetable (i.e. the iommu_domain), it wouldn't make any difference, because the thing we're mitigating against is handing out addresses which are going to cause a device's accesses to blow up inside the PCI root complex without ever even reaching the IOMMU. In short, dm_regions are about "these addresses are already being used for DMA, so make sure the IOMMU API doesn't block them", whereas reserved ranges are about "these addresses are unusable for DMA, so make sure the DMA API can't allocate them". Okay, thanks for the explanation. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies)
Hi All, On 2016-09-19 23:45, Tobias Jakobi wrote: I did some tests with the new version today. Sadly the reboot/shutdown issues are still present. Thanks for the report. I've managed to reproduce this issue and it is again caused by modifying device on devices_kset list before it will be finally added by device_add(). I thought that the new patchset allows creating links to a device, which has not been yet added to system device list. Rafael: What is your opinion? Should it be allowed to create a link to device, which has not yet been added to system device list by device_add()? My code used to do that, because IOMMUs are configured from of_platform_device_create_pdata() of_dma_configure() of_iommu_configure(), which happens before device_add(). To solve the reported corruption of devices_kset list, following change is needed: diff --git a/drivers/base/core.c b/drivers/base/core.c index aa8196508db9..4542ba9f60d4 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1039,11 +1039,18 @@ static void devices_kset_move_after(struct device *deva, struct device *devb) */ void devices_kset_move_last(struct device *dev) { + struct device *d; + if (!devices_kset) return; pr_debug("devices_kset: Moving %s to end of list\n", dev_name(dev)); spin_lock(&devices_kset->list_lock); - list_move_tail(&dev->kobj.entry, &devices_kset->list); + list_for_each_entry(d, &devices_kset->list, kobj.entry) { + if (d == dev) { + list_move_tail(&dev->kobj.entry, &devices_kset->list); + break; + } + } spin_unlock(&devices_kset->list_lock); } If you think that links can be created only to a device, which has been fully added to the system, I will register a bus notifier and create a link on consumers device probe then. [...] Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies)
Hi Rafael, On 2016-09-24 03:25, Rafael J. Wysocki wrote: On Friday, September 23, 2016 03:50:02 PM Lukas Wunner wrote: On Fri, Sep 23, 2016 at 02:49:20PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 20, 2016 10:51:13 AM Marek Szyprowski wrote: On 2016-09-19 23:45, Tobias Jakobi wrote: I did some tests with the new version today. Sadly the reboot/shutdown issues are still present. Thanks for the report. I've managed to reproduce this issue and it is again caused by modifying device on devices_kset list before it will be finally added by device_add(). I thought that the new patchset allows creating links to a device, which has not been yet added to system device list. Hm, Marek, why isn't it possible to set up the links from the consumer's ->probe hook in this case? Because consumers are unaware of the IOMMU presence, so they are also unaware of the links that have to be created. Should it be allowed to create a link to device, which has not yet been added to system device list by device_add()? While it would be easy to require both the consumer and producer devices to be registered for creating a link between them, that would just make it harder to use links in the first place. So ideally, it should be possible to create links between devices before registering them, but since I didn't take that into account in the current patch series, some quite substantial changes are needed to cover that. Additional link states come to mind, but then the "stateless" links are affected by this problem too. device_link_add() could be changed to call device_reorder_to_tail() only if device_is_registered(consumer) returns true. That's an inline function defined in which returns dev->kobj.state_in_sysfs, a flag which is set in kobject_add(). I know what that function is, but using it alone is not sufficient, because dev->kobj.state_in_sysfs is set before the device is added to dpm_list. I found that checking for dev->p was enough to check if device has been added to system or not, but this seems to be some kind of ugly workaround: diff --git a/drivers/base/core.c b/drivers/base/core.c index 4542ba9f60d4..780495918b53 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -180,9 +180,11 @@ struct device_link *device_link_add(struct device *consumer, * It is necessary to hold dpm_list locked throughout all that or else * we may end up suspending with a wrong ordering of it. */ - device_pm_lock(); - device_reorder_to_tail(consumer, NULL); - device_pm_unlock(); + if (consumer->p) { + device_pm_lock(); + device_reorder_to_tail(consumer, NULL); + device_pm_unlock(); + } list_add_tail_rcu(&link->s_node, &supplier->links_to_consumers); list_add_tail_rcu(&link->c_node, &consumer->links_to_suppliers); Then device_add() would have to check if any links are already set up and reorder the consumer behind the suppliers. Doesn't seem to be *that* complex, but probably I'm missing something, this is just off the cuff... There are some cases to consider and some races to avoid AFAICS. It all gets a lot simpler if device_link_add() is allowed to return NULL when the supplier device passed to it has not been registered yet. That looks like a reasonable thing to do to me, but I wonder if someone has a use case in which it would be a substantial limitation. Hmmm, you are talking here about the supplier, but my case is that supplier is already registered and probed, but the consumer is about to be created. If you think that supporting such case makes no sense, then I will use the workaround with bus notifier I mentioned earlier. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 2/2] iommu/exynos: Add proper runtime pm support
This patch uses recently introduced device links to track the runtime pm state of the master's device. This way each SYSMMU controller is runtime activated when its master's device is active and can save/restore its state instead of being enabled all the time. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not used. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 225 ++- 1 file changed, 94 insertions(+), 131 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index dfb44034b4ee..c8926e030713 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -206,6 +206,7 @@ static const struct sysmmu_fault_info sysmmu_v5_faults[] = { struct exynos_iommu_owner { struct list_head controllers; /* list of sysmmu_drvdata.owner_node */ struct iommu_domain *domain;/* domain this device is attached */ + struct mutex rpm_lock; /* for runtime pm of all sysmmus */ }; /* @@ -237,8 +238,8 @@ struct sysmmu_drvdata { struct clk *aclk; /* SYSMMU's aclk clock */ struct clk *pclk; /* SYSMMU's pclk clock */ struct clk *clk_master; /* master's device clock */ - int activations;/* number of calls to sysmmu_enable */ spinlock_t lock;/* lock for modyfying state */ + int active; /* current status */ struct exynos_iommu_domain *domain; /* domain we belong to */ struct list_head domain_node; /* node for domain clients list */ struct list_head owner_node;/* node for owner controllers list */ @@ -251,25 +252,6 @@ static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom) return container_of(dom, struct exynos_iommu_domain, domain); } -static bool set_sysmmu_active(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU was not active previously - and it needs to be initialized */ - return ++data->activations == 1; -} - -static bool set_sysmmu_inactive(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU is needed to be disabled */ - BUG_ON(data->activations < 1); - return --data->activations == 0; -} - -static bool is_sysmmu_active(struct sysmmu_drvdata *data) -{ - return data->activations > 0; -} - static void sysmmu_unblock(struct sysmmu_drvdata *data) { writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); @@ -388,7 +370,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) unsigned short reg_status, reg_clear; int ret = -ENOSYS; - WARN_ON(!is_sysmmu_active(data)); + WARN_ON(!data->active); if (MMU_MAJ_VER(data->version) < 5) { reg_status = REG_INT_STATUS; @@ -434,40 +416,19 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) return IRQ_HANDLED; } -static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data) +static void __sysmmu_disable(struct sysmmu_drvdata *data) { + unsigned long flags; + clk_enable(data->clk_master); + spin_lock_irqsave(&data->lock, flags); writel(CTRL_DISABLE, data->sfrbase + REG_MMU_CTRL); writel(0, data->sfrbase + REG_MMU_CFG); - - __sysmmu_disable_clocks(data); -} - -static bool __sysmmu_disable(struct sysmmu_drvdata *data) -{ - bool disabled; - unsigned long flags; - - spin_lock_irqsave(&data->lock, flags); - - disabled = set_sysmmu_inactive(data); - - if (disabled) { - data->pgtable = 0; - data->domain = NULL; - - __sysmmu_disable_nocount(data); - - dev_dbg(data->sysmmu, "Disabled\n"); - } else { - dev_dbg(data->sysmmu, "%d times left to disable\n", - data->activations); - } - + data->active = false; spin_unlock_irqrestore(&data->lock, flags); - return disabled; + __sysmmu_disable_clocks(data); } static void __sysmmu_init_config(struct sysmmu_drvdata *data) @@ -484,17 +445,19 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data) writel(cfg, data->sfrbase + REG_MMU_CFG); } -static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) +static void __sysmmu_enable(struct sysmmu_drvdata *data) { + unsigned long flags; + __sysmmu_enable_clocks(data); + spin_lock_irqsave(&data->lock, flags); writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL); - __sysmmu_init_config(data); - __sysmmu_set_ptbase(data, data->pgtable); - writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); + data->ac
[PATCH v4 1/2] iommu/exynos: Remove excessive, useless debug
Remove excessive, useless debug about skipping TLB invalidation, which is a normal situation when more aggressive power management is enabled. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 33dcc29ec200..dfb44034b4ee 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -578,9 +578,6 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, sysmmu_unblock(data); } clk_disable(data->clk_master); - } else { - dev_dbg(data->master, - "disabled. Skipping TLB invalidation @ %#x\n", iova); } spin_unlock_irqrestore(&data->lock, flags); } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 0/2] Exynos IOMMU: proper runtime PM support (use device dependencies)
Hello, This patch series finally implements proper runtime PM support in Exynos IOMMU driver. This has been achieved by using recently introduce device links, which lets SYSMMU controller's runtime PM to follow master's device runtime PM state (the device which actually performs DMA transaction). The main idea behind this solution is an observation that any DMA activity from master device can be done only when master device is active, thus when master device is suspended SYSMMU controller device can also be suspended. This patchset solves the situation that power domains are always enabled, because all SYSMMU controllers (which belongs to those domains) are permanently active (because existing driver was simplified and kept SYSMMU device active all the time after initialization). Patch requires fourth version of Rafeal's "Functional dependencies between devices" patchset, which is available here: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1241473.html If one wants to test this patchset, I've provided a branch with all needed patches (some fixes for Exynos4 FIMC-IS driver are needed): https://git.linaro.org/people/marek.szyprowski/linux-srpol.git v4.8-iommu-pm-v4 Patches are based on vanilla v4.8-rc8 kernel with Rafael's device dependencies v4 patchset applied. Best regards Marek Szyprowski Samsung R&D Institute Poland Changelog: v4: - rebased on top of v4 of device dependencies/links patchset, what resolved system hang on reboot v3: http://www.spinics.net/lists/linux-samsung-soc/msg55256.html - rebased on top of latest device dependencies/links patchset - added proper locking between runtime pm, iommu_attach/detach and sysmmu enable/disable(added per iommu owner device's rpm lock) v2: http://www.spinics.net/lists/arm-kernel/msg512082.html - replaced PM notifiers with generic device dependencies/links developped by Rafael J. Wysocki v1: http://www.spinics.net/lists/arm-kernel/msg509600.html - initial version Patch summary: Marek Szyprowski (2): iommu/exynos: Remove excessive, useless debug iommu/exynos: Add proper runtime pm support drivers/iommu/exynos-iommu.c | 228 ++- 1 file changed, 94 insertions(+), 134 deletions(-) -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V3 0/8] IOMMU probe deferral support
Hi Sricharan, On 2016-10-04 19:03, Sricharan R wrote: Initial post from Laurent Pinchart[1]. This is series calls the dma ops configuration for the devices at a generic place so that it works for all busses. The dma_configure_ops for a device is now called during the device_attach callback just before the probe of the bus/driver is called. Similarly dma_deconfigure is called during device/driver_detach path. pci_bus_add_devices(platform/amba)(_device_create/driver_register) | | pci_bus_add_device (device_add/driver_register) | | device_attach device_initial_probe | | __device_attach_driver__device_attach_driver | driver_probe_device | really_probe | dma_configure Similarly on the device/driver_unregister path __device_release_driver is called which inturn calls dma_deconfigure. If the ACPI bus code follows the same, we can add acpi_dma_configure at the same place as of_dma_configure. This series is based on the recently merged Generic DT bindings for PCI IOMMUs and ARM SMMU from Robin Murphy robin.mur...@arm.com [2] This time tested this with platform and pci device for probe deferral and reprobe on arm64 based platform. There is an issue on the cleanup path for arm64 though, where there is WARN_ON if the dma_ops is reset while device is attached to an domain in arch_teardown_dma_ops. But with iommu_groups created from the iommu driver, the device is always attached to a domain/default_domain. So so the WARN has to be removed/handled probably. Thanks for continuing work on this feature! Your can add my: Tested-by: Marek Szyprowski It works fine with Exynos SYSMMU driver, although a patch is needed to fix infinite loop due to list corruption (same element is added twice if master device fails with deferred probe): From: Marek Szyprowski Date: Mon, 10 Oct 2016 14:22:42 +0200 Subject: [PATCH] iommu/exynos: ensure that sysmmu is added only once to its master Since adding IOMMU deferred probing support, of_xlate() callback might be called more than once for given master device (for example it happens when masters device driver fails with EPROBE_DEFER), so ensure that SYSMMU controller is added to its master device (owner) only once. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 30808e91b775..1525a86eb829 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -1253,7 +1253,7 @@ static int exynos_iommu_of_xlate(struct device *dev, { struct exynos_iommu_owner *owner = dev->archdata.iommu; struct platform_device *sysmmu = of_find_device_by_node(spec->np); -struct sysmmu_drvdata *data; +struct sysmmu_drvdata *data, *entry; if (!sysmmu) return -ENODEV; @@ -1271,6 +1271,10 @@ static int exynos_iommu_of_xlate(struct device *dev, dev->archdata.iommu = owner; } +list_for_each_entry(entry, &owner->controllers, owner_node) +if (entry == data) +return 0; + list_add_tail(&data->owner_node, &owner->controllers); return 0; } -- 1.9.1 Previous post of this series [3]. [V3] * Removed the patch to split dma_masks/dma_ops configuration separately based on review comments that both masks and ops are required only during the device probe time. * Reworked the series based on Generic DT bindings series [2]. * Added call to iommu's remove_device in the cleanup path for arm and arm64. * Removed the notifier trick in arm64 to handle early device registration. * Added reset of dma_ops in cleanup path for arm based on comments. * Fixed the pci_iommu_configure path and tested with PCI device as well. * Fixed a bug to return the correct iommu_ops from patch 7 [4] in last post. * Fixed few other cosmetic comments. [V2] * Updated the Initial post to call dma_configure/deconfigure from generic code * Added iommu add_device callback from of_iommu_configure path [V1] * Initial post [1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html [2] http://www.spinics.net/lists/devicetree/msg142943.html [3] https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg13941.html [4] https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg13940.html Laurent Pinchart (4): arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops() of: dma: Move range size workaround to of_dma_get_range() of: dma: Make of_dma_deconfigure() public iommu: of: Handle IOMMU lookup failure with deferred probing or error Sricharan R (4): drivers: platform: Configure dma operations at probe time arm: dma-mapping: Rese
Re: [PATCH v4 2/2] iommu/exynos: Add proper runtime pm support
Hi Luis On 2016-10-06 19:37, Luis R. Rodriguez wrote: On Thu, Sep 29, 2016 at 10:12:31AM +0200, Marek Szyprowski wrote: This patch uses recently introduced device links to track the runtime pm state of the master's device. This way each SYSMMU controller is runtime activated when its master's device is active instead of? instead of keeping SYSMMU controller runtime active all the time. BTW what is the master device of a SYSMMU? I have no clue about these IOMMU devices here. Here is a more detailed description of IOMMU hardware I wrote a few days ago for Ulf: http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1231006.html In short: there is a SYSMMU controller and its master device - a device, which performs DMA operations. That SYSMMU sits in between system memory and the master device, so it performs mapping of DMA addresses to physical memory addresses on each DMA operation. and can save/restore its state instead of being enabled all the time. I take it this means currently even if the master device is disabled (whatever that is) all SYSMMU controllers are kept enabled, is that right? The issue here is this wastes power? Or what is the issue? Yes, the issue here is the fact that SYSMMU is kept active all the time, what in turn prevent the power domain for turning off even if master device doesn't do anything and is already suspended. This directly (some clocks enabled) and in-directly (leakage current) causes power looses. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not used. So when the master device is idle we want to also remove power from the controllers ? How much power does this save on a typical device in the market BTW ? The main purpose of this patchset is to let power domains to be turned off, because with the current code all domains are all the time turned on, because SYSMMU controllers prevent them from turning them off. If you want I can measure the power consumption of the idle board with all domains enabled and disabled if you want to see the numbers. On the other board disabling most power domains in idle state (the clocks were already disabled) gave me about 20mA savings (at 3.7V), what is a significant value for the battery powered device. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 225 ++- 1 file changed, 94 insertions(+), 131 deletions(-) I'm reviewing the device link patches now but since this is a demo of use of that I'll note the changes here are pretty large and it makes it terribly difficult for review. Is there any way this patch can be split up in to logical atomic pieces that only do one task upon change ? I will try to split it a bit, but I cannot promise that much can be done to improve readability for someone not very familiar with the driver internals. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 5/7] iommu/exynos: Rework and fix internal locking
This patch reworks locking in the exynos_iommu_attach/detach_device functions to ensure that all entries of the sysmmu_drvdata and exynos_iommu_owner structure are updated under the respective spinlocks, while runtime pm functions are called without any spinlocks held. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 28e570b53672..a959443e6f33 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -731,10 +731,12 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain) spin_lock_irqsave(&domain->lock, flags); list_for_each_entry_safe(data, next, &domain->clients, domain_node) { + spin_lock(&data->lock); __sysmmu_disable(data); data->pgtable = 0; data->domain = NULL; list_del_init(&data->domain_node); + spin_unlock(&data->lock); } spin_unlock_irqrestore(&domain->lock, flags); @@ -772,17 +774,22 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, if (!has_sysmmu(dev) || owner->domain != iommu_domain) return; + list_for_each_entry(data, &owner->controllers, owner_node) { + __sysmmu_disable(data); + pm_runtime_put(data->sysmmu); + } + spin_lock_irqsave(&domain->lock, flags); list_for_each_entry_safe(data, next, &domain->clients, domain_node) { - __sysmmu_disable(data); + spin_lock(&data->lock); data->pgtable = 0; data->domain = NULL; list_del_init(&data->domain_node); - pm_runtime_put(data->sysmmu); + spin_unlock(&data->lock); } + owner->domain = NULL; spin_unlock_irqrestore(&domain->lock, flags); - owner->domain = NULL; dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", __func__, &pagetable); @@ -803,18 +810,22 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, if (owner->domain) exynos_iommu_detach_device(owner->domain, dev); + spin_lock_irqsave(&domain->lock, flags); list_for_each_entry(data, &owner->controllers, owner_node) { + spin_lock(&data->lock); data->pgtable = pagetable; data->domain = domain; + list_add_tail(&data->domain_node, &domain->clients); + spin_unlock(&data->lock); + } + owner->domain = iommu_domain; + spin_unlock_irqrestore(&domain->lock, flags); + + list_for_each_entry(data, &owner->controllers, owner_node) { pm_runtime_get_sync(data->sysmmu); __sysmmu_enable(data); - - spin_lock_irqsave(&domain->lock, flags); - list_add_tail(&data->domain_node, &domain->clients); - spin_unlock_irqrestore(&domain->lock, flags); } - owner->domain = iommu_domain; dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__, &pagetable); -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 2/7] iommu/exynos: Remove dead code
__sysmmu_enable/disable functions were designed to do ref-count based operations, but current code always calls them only once, so the code for checking the conditions and invalid conditions can be simply removed without any influence to the driver operation. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 65 1 file changed, 17 insertions(+), 48 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 8ba0d6049a63..4056228b8e5f 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -460,9 +460,6 @@ static bool __sysmmu_disable(struct sysmmu_drvdata *data) __sysmmu_disable_nocount(data); dev_dbg(data->sysmmu, "Disabled\n"); - } else { - dev_dbg(data->sysmmu, "%d times left to disable\n", - data->activations); } spin_unlock_irqrestore(&data->lock, flags); @@ -508,29 +505,18 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable, struct exynos_iommu_domain *domain) { - int ret = 0; unsigned long flags; spin_lock_irqsave(&data->lock, flags); if (set_sysmmu_active(data)) { data->pgtable = pgtable; data->domain = domain; - __sysmmu_enable_nocount(data); - dev_dbg(data->sysmmu, "Enabled\n"); - } else { - ret = (pgtable == data->pgtable) ? 1 : -EBUSY; - - dev_dbg(data->sysmmu, "already enabled\n"); } - - if (WARN_ON(ret < 0)) - set_sysmmu_inactive(data); /* decrement count */ - spin_unlock_irqrestore(&data->lock, flags); - return ret; + return 0; } static void sysmmu_tlb_invalidate_flpdcache(struct sysmmu_drvdata *data, @@ -793,8 +779,8 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain) spin_lock_irqsave(&domain->lock, flags); list_for_each_entry_safe(data, next, &domain->clients, domain_node) { - if (__sysmmu_disable(data)) - data->master = NULL; + __sysmmu_disable(data); + data->master = NULL; list_del_init(&data->domain_node); } @@ -829,31 +815,23 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, phys_addr_t pagetable = virt_to_phys(domain->pgtable); struct sysmmu_drvdata *data, *next; unsigned long flags; - bool found = false; if (!has_sysmmu(dev) || owner->domain != iommu_domain) return; spin_lock_irqsave(&domain->lock, flags); list_for_each_entry_safe(data, next, &domain->clients, domain_node) { - if (data->master == dev) { - if (__sysmmu_disable(data)) { - data->master = NULL; - list_del_init(&data->domain_node); - } - pm_runtime_put(data->sysmmu); - found = true; - } + __sysmmu_disable(data); + data->master = NULL; + list_del_init(&data->domain_node); + pm_runtime_put(data->sysmmu); } spin_unlock_irqrestore(&domain->lock, flags); owner->domain = NULL; - if (found) - dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", - __func__, &pagetable); - else - dev_err(dev, "%s: No IOMMU is attached\n", __func__); + dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", __func__, + &pagetable); } static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, @@ -864,7 +842,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, struct sysmmu_drvdata *data; phys_addr_t pagetable = virt_to_phys(domain->pgtable); unsigned long flags; - int ret = -ENODEV; if (!has_sysmmu(dev)) return -ENODEV; @@ -874,27 +851,19 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, list_for_each_entry(data, &owner->controllers, owner_node) { pm_runtime_get_sync(data->sysmmu); - ret = __sysmmu_enable(data, pagetable, domain); - if (ret >= 0) { - data->master = dev; + __sysmmu_enable(data, pagetable, domain); + data->master = dev; - spi
[PATCH v5 1/7] iommu/exynos: Remove excessive, useless debug
Remove excessive, useless debug about skipping TLB invalidation, which is a normal situation when more aggressive power management is enabled. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 30808e91b775..8ba0d6049a63 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -578,9 +578,6 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, sysmmu_unblock(data); } clk_disable(data->clk_master); - } else { - dev_dbg(data->master, - "disabled. Skipping TLB invalidation @ %#x\n", iova); } spin_unlock_irqrestore(&data->lock, flags); } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 4/7] iommu/exynos: Set master device once on boot
To avoid possible races, set master device pointer in each SYSMMU controller once on boot. Suspend/resume callbacks now properly relies on the configured iommu domain to enable or disable SYSMMU controller. While changing the code, also update the sleep debug messages and make them conditional. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index f45b274513cc..28e570b53672 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -600,10 +600,12 @@ static int exynos_sysmmu_suspend(struct device *dev) struct sysmmu_drvdata *data = dev_get_drvdata(dev); struct device *master = data->master; - dev_dbg(dev, "suspend\n"); if (master) { - __sysmmu_disable(data); pm_runtime_put(dev); + if (data->domain) { + dev_dbg(data->sysmmu, "saving state\n"); + __sysmmu_disable(data); + } } return 0; } @@ -613,10 +615,12 @@ static int exynos_sysmmu_resume(struct device *dev) struct sysmmu_drvdata *data = dev_get_drvdata(dev); struct device *master = data->master; - dev_dbg(dev, "resume\n"); if (master) { pm_runtime_get_sync(dev); - __sysmmu_enable(data); + if (data->domain) { + dev_dbg(data->sysmmu, "restoring state\n"); + __sysmmu_enable(data); + } } return 0; } @@ -730,7 +734,6 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain) __sysmmu_disable(data); data->pgtable = 0; data->domain = NULL; - data->master = NULL; list_del_init(&data->domain_node); } @@ -772,7 +775,6 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, spin_lock_irqsave(&domain->lock, flags); list_for_each_entry_safe(data, next, &domain->clients, domain_node) { __sysmmu_disable(data); - data->master = NULL; data->pgtable = 0; data->domain = NULL; list_del_init(&data->domain_node); @@ -806,7 +808,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, data->domain = domain; pm_runtime_get_sync(data->sysmmu); __sysmmu_enable(data); - data->master = dev; spin_lock_irqsave(&domain->lock, flags); list_add_tail(&data->domain_node, &domain->clients); @@ -1192,6 +1193,7 @@ static int exynos_iommu_of_xlate(struct device *dev, } list_add_tail(&data->owner_node, &owner->controllers); + data->master = dev; return 0; } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 3/7] iommu/exynos: Simplify internal enable/disable functions
Remove remaining leftovers of the ref-count related code in the __sysmmu_enable/disable functions inline __sysmmu_enable/disable_nocount to them. Suspend/resume callbacks now checks if master device is set for given SYSMMU controller instead of relying on the activation count. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 104 --- 1 file changed, 29 insertions(+), 75 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 4056228b8e5f..f45b274513cc 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -237,8 +237,8 @@ struct sysmmu_drvdata { struct clk *aclk; /* SYSMMU's aclk clock */ struct clk *pclk; /* SYSMMU's pclk clock */ struct clk *clk_master; /* master's device clock */ - int activations;/* number of calls to sysmmu_enable */ spinlock_t lock;/* lock for modyfying state */ + int active; /* current status */ struct exynos_iommu_domain *domain; /* domain we belong to */ struct list_head domain_node; /* node for domain clients list */ struct list_head owner_node;/* node for owner controllers list */ @@ -251,25 +251,6 @@ static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom) return container_of(dom, struct exynos_iommu_domain, domain); } -static bool set_sysmmu_active(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU was not active previously - and it needs to be initialized */ - return ++data->activations == 1; -} - -static bool set_sysmmu_inactive(struct sysmmu_drvdata *data) -{ - /* return true if the System MMU is needed to be disabled */ - BUG_ON(data->activations < 1); - return --data->activations == 0; -} - -static bool is_sysmmu_active(struct sysmmu_drvdata *data) -{ - return data->activations > 0; -} - static void sysmmu_unblock(struct sysmmu_drvdata *data) { writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); @@ -388,7 +369,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) unsigned short reg_status, reg_clear; int ret = -ENOSYS; - WARN_ON(!is_sysmmu_active(data)); + WARN_ON(!data->active); if (MMU_MAJ_VER(data->version) < 5) { reg_status = REG_INT_STATUS; @@ -434,37 +415,19 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) return IRQ_HANDLED; } -static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data) +static void __sysmmu_disable(struct sysmmu_drvdata *data) { + unsigned long flags; + clk_enable(data->clk_master); + spin_lock_irqsave(&data->lock, flags); writel(CTRL_DISABLE, data->sfrbase + REG_MMU_CTRL); writel(0, data->sfrbase + REG_MMU_CFG); - - __sysmmu_disable_clocks(data); -} - -static bool __sysmmu_disable(struct sysmmu_drvdata *data) -{ - bool disabled; - unsigned long flags; - - spin_lock_irqsave(&data->lock, flags); - - disabled = set_sysmmu_inactive(data); - - if (disabled) { - data->pgtable = 0; - data->domain = NULL; - - __sysmmu_disable_nocount(data); - - dev_dbg(data->sysmmu, "Disabled\n"); - } - + data->active = false; spin_unlock_irqrestore(&data->lock, flags); - return disabled; + __sysmmu_disable_clocks(data); } static void __sysmmu_init_config(struct sysmmu_drvdata *data) @@ -481,17 +444,19 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data) writel(cfg, data->sfrbase + REG_MMU_CFG); } -static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) +static void __sysmmu_enable(struct sysmmu_drvdata *data) { + unsigned long flags; + __sysmmu_enable_clocks(data); + spin_lock_irqsave(&data->lock, flags); writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL); - __sysmmu_init_config(data); - __sysmmu_set_ptbase(data, data->pgtable); - writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL); + data->active = true; + spin_unlock_irqrestore(&data->lock, flags); /* * SYSMMU driver keeps master's clock enabled only for the short @@ -502,37 +467,18 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data) clk_disable(data->clk_master); } -static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable, - struct exynos_iommu_domain *domain) -{ - unsigned long flags; - - spin_lock_irqsave(&data->lock, flags); - if (set_sysmmu_active(data)) { - data->pgtable =
[PATCH v5 7/7] iommu/exynos: Use device dependency links to control runtime pm
This patch uses recently introduced device dependency links to track the runtime pm state of the master's device. This way each SYSMMU controller is set to runtime active only when its master's device is active and can restore or save its state instead of being activated all the time when attached to the given master device. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not being used. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 5e6d7bbf9b70..59b4f2ce4f5f 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -781,10 +781,6 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, if (!has_sysmmu(dev) || owner->domain != iommu_domain) return; - list_for_each_entry(data, &owner->controllers, owner_node) { - pm_runtime_put_sync(data->sysmmu); - } - mutex_lock(&owner->rpm_lock); list_for_each_entry(data, &owner->controllers, owner_node) { @@ -848,10 +844,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, mutex_unlock(&owner->rpm_lock); - list_for_each_entry(data, &owner->controllers, owner_node) { - pm_runtime_get_sync(data->sysmmu); - } - dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__, &pagetable); @@ -1232,6 +1224,14 @@ static int exynos_iommu_of_xlate(struct device *dev, list_add_tail(&data->owner_node, &owner->controllers); data->master = dev; + + /* +* SYSMMU will be runtime activated via device link (dependency) to its +* master device, so there are no direct calls to pm_runtime_get/put +* in this driver. +*/ + device_link_add(dev, data->sysmmu, DL_FLAG_PM_RUNTIME); + return 0; } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 0/7] Exynos IOMMU: proper runtime PM support (use device dependencies)
Hello, This is another update of the patchset for adding proper runtime PM support to Exynos IOMMU driver. This has been achieved by using recently introduced device links, which lets SYSMMU controller's runtime PM to follow master's device runtime PM state (the device which actually performs DMA transaction). The main idea behind this solution is an observation that any DMA activity from master device can be done only when master device is active, thus when master device is suspended SYSMMU controller device can also be suspended. This patchset solves the problem of all power domains being always enabled. It happened, because all SYSMMU controllers (which belongs to the same domains) are permanently kept active, because existing driver was simplified and kept SYSMMU device active all the time after initialization and attaching to the master device. Patch requires fifth version of Rafeal's "Functional dependencies between devices" patchset, which is available here: http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1246897.html or as git branch: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git device-links-test If one wants to test this patchset, I've provided a branch with all needed patches (a small fix for Exynos4 FIMC-IS DTS is still needed): https://git.linaro.org/people/marek.szyprowski/linux-srpol.git v4.9-iommu-pm-v5 Patches are based on vanilla v4.9-rc1 kernel with Rafael's device dependencies v5 patchset applied. Best regards Marek Szyprowski Samsung R&D Institute Poland Changelog: v5: - split main patch into several small changes for easier review (requested by Luis Rodriquez) - fixed usage of runtime_pm_active, now it is guarded by pm_runtime_get_noresume() and pm_runtime_put() pair v4: http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1241601.html - rebased on top of v4 of device dependencies/links patchset, what resolved system hang on reboot v3: http://www.spinics.net/lists/linux-samsung-soc/msg55256.html - rebased on top of latest device dependencies/links patchset - added proper locking between runtime pm, iommu_attach/detach and sysmmu enable/disable(added per iommu owner device's rpm lock) v2: http://www.spinics.net/lists/arm-kernel/msg512082.html - replaced PM notifiers with generic device dependencies/links developed by Rafael J. Wysocki v1: http://www.spinics.net/lists/arm-kernel/msg509600.html - initial version Patch summary: Marek Szyprowski (7): iommu/exynos: Remove excessive, useless debug iommu/exynos: Remove dead code iommu/exynos: Simplify internal enable/disable functions iommu/exynos: Set master device once on boot iommu/exynos: Rework and fix internal locking iommu/exynos: Add runtime pm support iommu/exynos: Use device dependency links to control runtime pm drivers/iommu/exynos-iommu.c | 230 ++- 1 file changed, 95 insertions(+), 135 deletions(-) -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 6/7] iommu/exynos: Add runtime pm support
This patch adds runtime pm implementation, which is based on previous suspend/resume code. SYSMMU controller is now being enabled/disabled mainly from the runtime pm callbacks. System sleep callbacks relies on generic pm_runtime_force_suspend/pm_runtime_force_resume helpers. To ensure internal state consistency, additional lock for runtime pm transitions was introduced. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 45 +++- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index a959443e6f33..5e6d7bbf9b70 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -206,6 +206,7 @@ struct sysmmu_fault_info { struct exynos_iommu_owner { struct list_head controllers; /* list of sysmmu_drvdata.owner_node */ struct iommu_domain *domain;/* domain this device is attached */ + struct mutex rpm_lock; /* for runtime pm of all sysmmus */ }; /* @@ -594,40 +595,46 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM_SLEEP -static int exynos_sysmmu_suspend(struct device *dev) +static int __maybe_unused exynos_sysmmu_suspend(struct device *dev) { struct sysmmu_drvdata *data = dev_get_drvdata(dev); struct device *master = data->master; if (master) { - pm_runtime_put(dev); + struct exynos_iommu_owner *owner = master->archdata.iommu; + + mutex_lock(&owner->rpm_lock); if (data->domain) { dev_dbg(data->sysmmu, "saving state\n"); __sysmmu_disable(data); } + mutex_unlock(&owner->rpm_lock); } return 0; } -static int exynos_sysmmu_resume(struct device *dev) +static int __maybe_unused exynos_sysmmu_resume(struct device *dev) { struct sysmmu_drvdata *data = dev_get_drvdata(dev); struct device *master = data->master; if (master) { - pm_runtime_get_sync(dev); + struct exynos_iommu_owner *owner = master->archdata.iommu; + + mutex_lock(&owner->rpm_lock); if (data->domain) { dev_dbg(data->sysmmu, "restoring state\n"); __sysmmu_enable(data); } + mutex_unlock(&owner->rpm_lock); } return 0; } -#endif static const struct dev_pm_ops sysmmu_pm_ops = { - SET_LATE_SYSTEM_SLEEP_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume) + SET_RUNTIME_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume, NULL) + SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, +pm_runtime_force_resume) }; static const struct of_device_id sysmmu_of_match[] __initconst = { @@ -775,7 +782,15 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, return; list_for_each_entry(data, &owner->controllers, owner_node) { - __sysmmu_disable(data); + pm_runtime_put_sync(data->sysmmu); + } + + mutex_lock(&owner->rpm_lock); + + list_for_each_entry(data, &owner->controllers, owner_node) { + pm_runtime_get_noresume(data->sysmmu); + if (pm_runtime_active(data->sysmmu)) + __sysmmu_disable(data); pm_runtime_put(data->sysmmu); } @@ -790,6 +805,7 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, owner->domain = NULL; spin_unlock_irqrestore(&domain->lock, flags); + mutex_unlock(&owner->rpm_lock); dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", __func__, &pagetable); @@ -810,6 +826,8 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, if (owner->domain) exynos_iommu_detach_device(owner->domain, dev); + mutex_lock(&owner->rpm_lock); + spin_lock_irqsave(&domain->lock, flags); list_for_each_entry(data, &owner->controllers, owner_node) { spin_lock(&data->lock); @@ -822,8 +840,16 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, spin_unlock_irqrestore(&domain->lock, flags); list_for_each_entry(data, &owner->controllers, owner_node) { + pm_runtime_get_noresume(data->sysmmu); + if (pm_runtime_active(data->sysmmu)) + __sysmmu_enable(data); + pm_runtime_put(data->sysmmu); + } + + mutex_unlock(&owner->rpm_lock); + + list_for_each_entry(data, &owner->controllers,
Re: [PATCH v5 6/7] iommu/exynos: Add runtime pm support
Hi Sricharan On 2016-10-22 07:50, Sricharan wrote: This patch adds runtime pm implementation, which is based on previous suspend/resume code. SYSMMU controller is now being enabled/disabled mainly > from the runtime pm callbacks. System sleep callbacks relies on generic pm_runtime_force_suspend/pm_runtime_force_resume helpers. To ensure internal state consistency, additional lock for runtime pm transitions was introduced. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 45 +++- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index a959443e6f33..5e6d7bbf9b70 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -206,6 +206,7 @@ struct sysmmu_fault_info { struct exynos_iommu_owner { struct list_head controllers; /* list of sysmmu_drvdata.owner_node */ struct iommu_domain *domain;/* domain this device is attached */ + struct mutex rpm_lock; /* for runtime pm of all sysmmus */ }; /* @@ -594,40 +595,46 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM_SLEEP -static int exynos_sysmmu_suspend(struct device *dev) +static int __maybe_unused exynos_sysmmu_suspend(struct device *dev) { struct sysmmu_drvdata *data = dev_get_drvdata(dev); struct device *master = data->master; if (master) { - pm_runtime_put(dev); + struct exynos_iommu_owner *owner = master->archdata.iommu; + + mutex_lock(&owner->rpm_lock); More of a device link question, To understand, i see that with device link + runtime, the supplier callbacks are not called for irqsafe clients, even if supplier is irqsafe. Why so ? Frankly I didn't care about irqsafe runtime pm, because there is no such need for Exynos platform and its drivers. Exynos power domain driver also doesn't support irqsafe mode. if (data->domain) { dev_dbg(data->sysmmu, "saving state\n"); __sysmmu_disable(data); } + mutex_unlock(&owner->rpm_lock); } return 0; } -static int exynos_sysmmu_resume(struct device *dev) +static int __maybe_unused exynos_sysmmu_resume(struct device *dev) { struct sysmmu_drvdata *data = dev_get_drvdata(dev); struct device *master = data->master; if (master) { - pm_runtime_get_sync(dev); + struct exynos_iommu_owner *owner = master->archdata.iommu; + + mutex_lock(&owner->rpm_lock); if (data->domain) { dev_dbg(data->sysmmu, "restoring state\n"); __sysmmu_enable(data); } + mutex_unlock(&owner->rpm_lock); } return 0; } -#endif static const struct dev_pm_ops sysmmu_pm_ops = { - SET_LATE_SYSTEM_SLEEP_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume) + SET_RUNTIME_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume, NULL) + SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, +pm_runtime_force_resume) }; Is this needed to be LATE_SYSTEM_SLEEP_PM_OPS with device links to take care of the order ? Hmmm. LASE_SYSTEM_SLEEP_PM_OPS is a left over from the previous versions of the driver, which doesn't use device links. You are right, that "normal" SYSTEM_SLEEP_PM_OPS should be enough assuming that device links will take care of the proper call sequence between consumer and supplier device. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 7/7] iommu/exynos: Use device dependency links to control runtime pm
Hi Sricharan, On 2016-10-23 11:49, Sricharan wrote: Hi Marek, This patch uses recently introduced device dependency links to track the runtime pm state of the master's device. This way each SYSMMU controller is set to runtime active only when its master's device is active and can restore or save its state instead of being activated all the time when attached to the given master device. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not being used. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 5e6d7bbf9b70..59b4f2ce4f5f 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -781,10 +781,6 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, if (!has_sysmmu(dev) || owner->domain != iommu_domain) return; - list_for_each_entry(data, &owner->controllers, owner_node) { - pm_runtime_put_sync(data->sysmmu); - } - mutex_lock(&owner->rpm_lock); list_for_each_entry(data, &owner->controllers, owner_node) { @@ -848,10 +844,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, mutex_unlock(&owner->rpm_lock); - list_for_each_entry(data, &owner->controllers, owner_node) { - pm_runtime_get_sync(data->sysmmu); - } - dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__, &pagetable); @@ -1232,6 +1224,14 @@ static int exynos_iommu_of_xlate(struct device *dev, list_add_tail(&data->owner_node, &owner->controllers); data->master = dev; + + /* +* SYSMMU will be runtime activated via device link (dependency) to its +* master device, so there are no direct calls to pm_runtime_get/put +* in this driver. +*/ + device_link_add(dev, data->sysmmu, DL_FLAG_PM_RUNTIME); + So in the case of master with multiple sids, this would be called multiple times for the same master ? I don't know what is "multiple sids" case, but if given SYSMMU master device (supplier) has multiple SYSMMU controllers (consumers), then links will be created for each SYSMMU controller. Please note that this code is based on vanilla v4.9-rc1, which calls of_xlate() callback only once for every iommu for given master device. Your IOMMU deferred probe patches change this, but I already posted a fix for Exynos IOMMU driver to handle such case. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V3 0/8] IOMMU probe deferral support
Hi Sricharan, On 2016-10-12 08:24, Sricharan wrote: On 2016-10-04 19:03, Sricharan R wrote: Initial post from Laurent Pinchart[1]. This is series calls the dma ops configuration for the devices at a generic place so that it works for all busses. The dma_configure_ops for a device is now called during the device_attach callback just before the probe of the bus/driver is called. Similarly dma_deconfigure is called during device/driver_detach path. pci_bus_add_devices(platform/amba)(_device_create/driver_register) | | pci_bus_add_device (device_add/driver_register) | | device_attach device_initial_probe | | __device_attach_driver__device_attach_driver | driver_probe_device | really_probe | dma_configure Similarly on the device/driver_unregister path __device_release_driver is called which inturn calls dma_deconfigure. If the ACPI bus code follows the same, we can add acpi_dma_configure at the same place as of_dma_configure. This series is based on the recently merged Generic DT bindings for PCI IOMMUs and ARM SMMU from Robin Murphy robin.mur...@arm.com [2] This time tested this with platform and pci device for probe deferral and reprobe on arm64 based platform. There is an issue on the cleanup path for arm64 though, where there is WARN_ON if the dma_ops is reset while device is attached to an domain in arch_teardown_dma_ops. But with iommu_groups created from the iommu driver, the device is always attached to a domain/default_domain. So so the WARN has to be removed/handled probably. Thanks for continuing work on this feature! Your can add my: Tested-by: Marek Szyprowski Thanks for testing this. So the for the below fix, the remove_device callback gets called on the dma_ops cleanup path, so would it be easy to remove the data for the device there ? I assumed that IOMMU driver cannot be removed reliably, so all structures that it creates are permanent. I didn't use device_add()/device_remove() callbacks, because in current implementation device_add() is called too late (after dma-mapping glue triggers device_attach_iommu()). Maybe once your patchset is merged, I will move creation and management of the all IOMMU related structures to device_add/remove callbacks. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 7/7] iommu/exynos: Use device dependency links to control runtime pm
Hi Sricharan, On 2016-10-24 14:29, Sricharan wrote: This patch uses recently introduced device dependency links to track the runtime pm state of the master's device. This way each SYSMMU controller is set to runtime active only when its master's device is active and can restore or save its state instead of being activated all the time when attached to the given master device. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not being used. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 5e6d7bbf9b70..59b4f2ce4f5f 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -781,10 +781,6 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain, if (!has_sysmmu(dev) || owner->domain != iommu_domain) return; - list_for_each_entry(data, &owner->controllers, owner_node) { - pm_runtime_put_sync(data->sysmmu); - } - mutex_lock(&owner->rpm_lock); list_for_each_entry(data, &owner->controllers, owner_node) { @@ -848,10 +844,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain, mutex_unlock(&owner->rpm_lock); - list_for_each_entry(data, &owner->controllers, owner_node) { - pm_runtime_get_sync(data->sysmmu); - } - dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__, &pagetable); @@ -1232,6 +1224,14 @@ static int exynos_iommu_of_xlate(struct device *dev, list_add_tail(&data->owner_node, &owner->controllers); data->master = dev; + + /* +* SYSMMU will be runtime activated via device link (dependency) to its +* master device, so there are no direct calls to pm_runtime_get/put +* in this driver. +*/ + device_link_add(dev, data->sysmmu, DL_FLAG_PM_RUNTIME); + So in the case of master with multiple sids, this would be called multiple times for the same master ? I don't know what is "multiple sids" case, but if given SYSMMU master device (supplier) has multiple SYSMMU controllers (consumers), then links will be created for each SYSMMU controller. Please note that this code is based on vanilla v4.9-rc1, which calls of_xlate() callback only once for every iommu for given master device. Your IOMMU deferred probe patches change this, but I already posted a fix for Exynos IOMMU driver to handle such case. By multiple sids, i meant iommus = <&phandle sid1 sid2 .. sidn> case, so xlate would be called multiples for the same master without deferred probing also. But the fix that you showed on the other thread would work here as well or maybe if you dont have masters with multiple sids you wont have any issues as well. Exynos SYSMMU driver always use "#iommu-cells = <0>", so it doesn't support multiple sids. However there is a case with 2 SYSMMU controllers attached to the same master device: "iommus = <&sysmmu_mfc_l>, <&sysmmu_mfc_r>;". Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/4] iommu/arm-smmu: Add the device_link between masters and smmu
Hi Sricharan, On 2016-10-21 19:14, Sricharan R wrote: The device link between master and its smmu is added so that the smmu gets runtime enabled/disabled when the master needs it. This is done from add_device callback which gets called once when the master is added to the smmu group. Signed-off-by: Sricharan R --- drivers/iommu/arm-smmu.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 578cdc2..71ce4b6 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1470,6 +1470,15 @@ static int arm_smmu_add_device(struct device *dev) goto out_free; pm_runtime_put_sync(smmu->dev); + /* +* Establish the link between smmu and master, so that the +* smmu gets runtime enabled/disabled as per the master's +* needs. +*/ + + device_link_add(dev, smmu->dev, DEVICE_LINK_AVAILABLE, + DEVICE_LINK_PM_RUNTIME); Please update to the latest version of Rafael's patches. In V5 the initial link state is not needed anymore and there was an important fix for creating links during master's driver probing, what happens after applying your IOMMU deferred probe patchset. + return 0; out_free: Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/exynos: Add support for page access protection bits
This patch add support for page access protection bits. Till now this feature was disabled and Exynos SYSMMU always mapped pages as read/write. Now page access bits are set according to the protection bits provided in iommu_map(), so Exynos SYSMMU is able to detect incorrect access to mapped pages. Exynos SYSMMU earlier than v5 doesn't support write-only mappings, so pages with such protection bits are mapped as read/write. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 63 +++- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 30808e9..e249e2f 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -70,6 +70,36 @@ #define SYSMMU_PG_ENT_SHIFT 0 #define SYSMMU_V5_PG_ENT_SHIFT 4 +static const sysmmu_pte_t *LV1_PROT; +static const sysmmu_pte_t SYSMMU_LV1_PROT[] = { + ((0 << 15) | (0 << 10)), /* no access */ + ((1 << 15) | (1 << 10)), /* IOMMU_READ only */ + ((0 << 15) | (1 << 10)), /* IOMMU_WRITE not supported, use read/write */ + ((0 << 15) | (1 << 10)), /* IOMMU_READ | IOMMU_WRITE */ +}; +static const sysmmu_pte_t SYSMMU_V5_LV1_PROT[] = { + (0 << 4), /* no access */ + (1 << 4), /* IOMMU_READ only */ + (2 << 4), /* IOMMU_WRITE only */ + (3 << 4), /* IOMMU_READ | IOMMU_WRITE */ +}; + +static const sysmmu_pte_t *LV2_PROT; +static const sysmmu_pte_t SYSMMU_LV2_PROT[] = { + ((0 << 9) | (0 << 4)), /* no access */ + ((1 << 9) | (1 << 4)), /* IOMMU_READ only */ + ((0 << 9) | (1 << 4)), /* IOMMU_WRITE not supported, use read/write */ + ((0 << 9) | (1 << 4)), /* IOMMU_READ | IOMMU_WRITE */ +}; +static const sysmmu_pte_t SYSMMU_V5_LV2_PROT[] = { + (0 << 2), /* no access */ + (1 << 2), /* IOMMU_READ only */ + (2 << 2), /* IOMMU_WRITE only */ + (3 << 2), /* IOMMU_READ | IOMMU_WRITE */ +}; + +#define SYSMMU_SUPPORTED_PROT_BITS (IOMMU_READ | IOMMU_WRITE) + #define sect_to_phys(ent) (((phys_addr_t) ent) << PG_ENT_SHIFT) #define section_phys(sent) (sect_to_phys(*(sent)) & SECT_MASK) #define section_offs(iova) (iova & (SECT_SIZE - 1)) @@ -97,16 +127,17 @@ static u32 lv2ent_offset(sysmmu_iova_t iova) #define SPAGES_PER_LPAGE (LPAGE_SIZE / SPAGE_SIZE) #define lv2table_base(sent) (sect_to_phys(*(sent) & 0xFFC0)) -#define mk_lv1ent_sect(pa) ((pa >> PG_ENT_SHIFT) | 2) +#define mk_lv1ent_sect(pa, prot) ((pa >> PG_ENT_SHIFT) | LV1_PROT[prot] | 2) #define mk_lv1ent_page(pa) ((pa >> PG_ENT_SHIFT) | 1) -#define mk_lv2ent_lpage(pa) ((pa >> PG_ENT_SHIFT) | 1) -#define mk_lv2ent_spage(pa) ((pa >> PG_ENT_SHIFT) | 2) +#define mk_lv2ent_lpage(pa, prot) ((pa >> PG_ENT_SHIFT) | LV2_PROT[prot] | 1) +#define mk_lv2ent_spage(pa, prot) ((pa >> PG_ENT_SHIFT) | LV2_PROT[prot] | 2) #define CTRL_ENABLE0x5 #define CTRL_BLOCK 0x7 #define CTRL_DISABLE 0x0 #define CFG_LRU0x1 +#define CFG_EAP(1 << 2) #define CFG_QOS(n) ((n & 0xF) << 7) #define CFG_ACGEN (1 << 24) /* System MMU 3.3 only */ #define CFG_SYSSEL (1 << 22) /* System MMU 3.2 only */ @@ -481,6 +512,8 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data) else cfg = CFG_QOS(15) | CFG_FLPDCACHE | CFG_ACGEN; + cfg |= CFG_EAP; /* enable access protection bits check */ + writel(cfg, data->sfrbase + REG_MMU_CFG); } @@ -652,10 +685,15 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) __sysmmu_get_version(data); if (PG_ENT_SHIFT < 0) { - if (MMU_MAJ_VER(data->version) < 5) + if (MMU_MAJ_VER(data->version) < 5) { PG_ENT_SHIFT = SYSMMU_PG_ENT_SHIFT; - else + LV1_PROT = SYSMMU_LV1_PROT; + LV2_PROT = SYSMMU_LV2_PROT; + } else { PG_ENT_SHIFT = SYSMMU_V5_PG_ENT_SHIFT; + LV1_PROT = SYSMMU_V5_LV1_PROT; + LV2_PROT = SYSMMU_V5_LV2_PROT; + } } pm_runtime_enable(dev); @@ -954,7 +992,7 @@ static sysmmu_pte_t *alloc_lv2entry(struct exynos_iommu_domain *domain, static int lv1set_section(struct exynos_iommu_domain *domain, sysmmu_pte_t *sent, sysmmu_iova_t iova, - phys_addr_t paddr, short *pgcnt) + phys_addr_t paddr, int prot, short *pgcnt) { if (lv1ent_section(sent)) { WARN(1, "Trying mapping on 1MiB@%#08x that is mapped", @@ -973,7 +1011,7 @@ static int lv1set_section(struct exynos_iommu_domain *domain,
[PATCH RESEND] iommu/exynos: Improve page fault debug message
Add master device name to default IOMMU fault message to make easier to find which device triggered the fault. While at it, move printing some information (like page table base and first level entry addresses) to dev_dbg(), because those are typically not very useful for typical device driver user/developer not equipped with hardware debugging tools. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 30808e9..47f0d67 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -368,13 +368,14 @@ static void show_fault_information(struct sysmmu_drvdata *data, { sysmmu_pte_t *ent; - dev_err(data->sysmmu, "%s FAULT occurred at %#x (page table base: %pa)\n", - finfo->name, fault_addr, &data->pgtable); + dev_err(data->sysmmu, "%s: %s FAULT occurred at %#x\n", + dev_name(data->master), finfo->name, fault_addr); + dev_dbg(data->sysmmu, "Page table base: %pa\n", &data->pgtable); ent = section_entry(phys_to_virt(data->pgtable), fault_addr); - dev_err(data->sysmmu, "\tLv1 entry: %#x\n", *ent); + dev_dbg(data->sysmmu, "\tLv1 entry: %#x\n", *ent); if (lv1ent_page(ent)) { ent = page_entry(ent, fault_addr); - dev_err(data->sysmmu, "\t Lv2 entry: %#x\n", *ent); + dev_dbg(data->sysmmu, "\t Lv2 entry: %#x\n", *ent); } } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/dma-iommu: properly respect configured address space size
When one called iommu_dma_init_domain() with size smaller than device's DMA mask, the alloc_iova() will not respect it and always assume that all IOVA addresses will be allocated from the the (base ... dev->dma_mask) range. This patch fixes this issue by taking the configured address space size parameter into account (if it is smaller than the device's dma_mask). Signed-off-by: Marek Szyprowski --- drivers/iommu/dma-iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index c5ab8667e6f2..8b4b72654359 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -212,11 +212,13 @@ static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size, if (domain->geometry.force_aperture) dma_limit = min(dma_limit, domain->geometry.aperture_end); + + dma_limit = min(dma_limit >> shift, (dma_addr_t)iovad->dma_32bit_pfn); /* * Enforce size-alignment to be safe - there could perhaps be an * attribute to control this per-device, or at least per-domain... */ - return alloc_iova(iovad, length, dma_limit >> shift, true); + return alloc_iova(iovad, length, dma_limit, true); } /* The IOVA allocator knows what we mapped, so just unmap whatever that was */ -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 7/7] iommu/exynos: Use device dependency links to control runtime pm
Hi Luis, On 2016-11-07 22:47, Luis R. Rodriguez wrote: On Thu, Oct 20, 2016 at 09:22:53AM +0200, Marek Szyprowski wrote: This patch uses recently introduced device dependency links to track the runtime pm state of the master's device. This way each SYSMMU controller is set to runtime active only when its master's device is active and can restore or save its state instead of being activated all the time when attached to the given master device. This way SYSMMU controllers no longer prevents respective power domains to be turned off when master's device is not being used. Its unclear why you need this based on this commit log -- is this needed only on platforms that lack ACPI and use device tree ? Nope, it has nothing to device tree nor ACPI. The dependency is a direct result of the way the devices operate. If so why? If this issue is present also on systems that only use ACPI is this possibly due to an ACPI firmware bug or the lack of some semantics in ACPI to express ordering in a better way? If the issue is device tree related only is this due to the lack of semantics in device tree to express some more complex dependency ? The main feature of device links that is used in this patch is enabling runtime pm dependency between Exynos SYSMMU controller (called it client device) and the device, for which it implements DMA address translation (called master device). The assumptions are following: 1. master device driver is completely unaware of the Exynos SYSMMU presence, IOMMU is transparently hooked up and managed by DMA-mapping framework 2. SYSMMU belongs to the same power domain as it's master device 3. SYSMMU is optional, master device can fully operate without it, with simple DMA address translation (DMA address == physical address) 4. Master device implements runtime pm, what in turn causes respective power domain to be turned on/off 5. DMA-mapping and IOMMU frameworks provides no calls to notify SYSMMU when its master device is performing DMA operations, so SYSMMU has to be runtime active 6. Currently SYSMMU always sets its runtime pm status to active after attaching to its master device to ensure proper hardware state. This prevents power domain to be turned off, even when master device sets its runtime pm status to suspended. 7. Exynos SYSMMU has to be runtime active at the same time when its master device is runtime active to it to perform DMA operations and allow the power domain to be turned off, when master device is runtime suspended. 8. The terms of device links, Exynos SYSMMU is a 'consumer' and master device is a 'supplier'. Has there been any review of the existing similar solutions out there such as the DRM / audio component framework? Would that help ? Nope, none of that solution deals with runtime pm. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 1/7] iommu/exynos: Remove excessive, useless debug
Remove excessive, useless debug about skipping TLB invalidation, which is a normal situation when more aggressive power management is enabled. Signed-off-by: Marek Szyprowski --- drivers/iommu/exynos-iommu.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 30808e9..8ba0d60 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -578,9 +578,6 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data, sysmmu_unblock(data); } clk_disable(data->clk_master); - } else { - dev_dbg(data->master, - "disabled. Skipping TLB invalidation @ %#x\n", iova); } spin_unlock_irqrestore(&data->lock, flags); } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu