| Message ID | 67223d4b1ff57f3f46e8c3102e663a063a50a7f7.1449246988.git.robin.murphy@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 04, 2015 at 05:52:59PM +0000, Robin Murphy wrote: > IOMMU hardware with range-based TLB maintenance commands can work > happily with the iova and size arguments passed via the tlb_add_flush > callback, but for IOMMUs which require separate commands per entry in > the range, it is not straightforward to infer the necessary granularity > when it comes to issuing the actual commands. > > Add an additional argument indicating the granularity for the benefit > of drivers needing to know, and update the ARM LPAE code appropriately > (for non-leaf invalidations we currently just assume the worst-case > page granularity rather than walking the table to check). > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > --- > drivers/iommu/arm-smmu-v3.c | 2 +- > drivers/iommu/arm-smmu.c | 2 +- > drivers/iommu/io-pgtable-arm.c | 27 +++++++++++++++------------ > drivers/iommu/io-pgtable.h | 4 ++-- > drivers/iommu/ipmmu-vmsa.c | 4 ++-- > 5 files changed, 21 insertions(+), 18 deletions(-) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index 4e5118a..c302b65 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -1335,7 +1335,7 @@ static void arm_smmu_tlb_inv_context(void *cookie) > } > > static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size, > - bool leaf, void *cookie) > + size_t granule, bool leaf, void *cookie) > { > struct arm_smmu_domain *smmu_domain = cookie; > struct arm_smmu_device *smmu = smmu_domain->smmu; > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 47dc7a7..601e3dd 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -582,7 +582,7 @@ static void arm_smmu_tlb_inv_context(void *cookie) > } > > static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size, > - bool leaf, void *cookie) > + size_t granule, bool leaf, void *cookie) > { > struct arm_smmu_domain *smmu_domain = cookie; > struct arm_smmu_cfg *cfg = &smmu_domain->cfg; > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c > index 366a354..9088d27 100644 > --- a/drivers/iommu/io-pgtable-arm.c > +++ b/drivers/iommu/io-pgtable-arm.c > @@ -58,8 +58,10 @@ > ((((d)->levels - ((l) - ARM_LPAE_START_LVL(d) + 1)) \ > * (d)->bits_per_level) + (d)->pg_shift) > > +#define ARM_LPAE_GRANULE(d) (1UL << (d)->pg_shift) > + > #define ARM_LPAE_PAGES_PER_PGD(d) \ > - DIV_ROUND_UP((d)->pgd_size, 1UL << (d)->pg_shift) > + DIV_ROUND_UP((d)->pgd_size, ARM_LPAE_GRANULE(d)) > > /* > * Calculate the index at level l used to map virtual address a using the > @@ -169,7 +171,7 @@ > /* IOPTE accessors */ > #define iopte_deref(pte,d) \ > (__va((pte) & ((1ULL << ARM_LPAE_MAX_ADDR_BITS) - 1) \ > - & ~((1ULL << (d)->pg_shift) - 1))) > + & ~(ARM_LPAE_GRANULE(d) - 1))) Do we run the risk of truncating the VA on 32-bit ARM here? Will
On 07/12/15 11:08, Will Deacon wrote: [...] >> +#define ARM_LPAE_GRANULE(d) (1UL << (d)->pg_shift) >> + >> #define ARM_LPAE_PAGES_PER_PGD(d) \ >> - DIV_ROUND_UP((d)->pgd_size, 1UL << (d)->pg_shift) >> + DIV_ROUND_UP((d)->pgd_size, ARM_LPAE_GRANULE(d)) >> >> /* >> * Calculate the index at level l used to map virtual address a using the >> @@ -169,7 +171,7 @@ >> /* IOPTE accessors */ >> #define iopte_deref(pte,d) \ >> (__va((pte) & ((1ULL << ARM_LPAE_MAX_ADDR_BITS) - 1) \ >> - & ~((1ULL << (d)->pg_shift) - 1))) >> + & ~(ARM_LPAE_GRANULE(d) - 1))) > > Do we run the risk of truncating the VA on 32-bit ARM here? Indeed, in all honesty I'd missed that, but since __va is going to truncate it to a 32-bit void * anyway, doing it earlier in the expression actually seems to result in better code - with iopte_deref wrapped like so to make it easier to pick out: arm_lpae_iopte *__iopte_deref(arm_lpae_iopte pte, struct arm_lpae_io_pgtable *d) { return iopte_deref(pte, d); } the result goes from this: 1568: e92d4070 push {r4, r5, r6, lr} 156c: e3a0e001 mov lr, #1 1570: e592c060 ldr ip, [r2, #96] ; 0x60 1574: e3e04000 mvn r4, #0 1578: e30f5fff movw r5, #65535 ; 0xffff 157c: e26c6020 rsb r6, ip, #32 1580: e1a02c1e lsl r2, lr, ip 1584: e2722000 rsbs r2, r2, #0 1588: e0000002 and r0, r0, r2 158c: e0000004 and r0, r0, r4 1590: e2400481 sub r0, r0, #-2130706432 ; 0x81000000 1594: e8bd8070 pop {r4, r5, r6, pc} to this: 151c: e5922060 ldr r2, [r2, #96] ; 0x60 1520: e3e03000 mvn r3, #0 1524: e1a03213 lsl r3, r3, r2 1528: e0000003 and r0, r0, r3 152c: e2400481 sub r0, r0, #-2130706432 ; 0x81000000 1530: e12fff1e bx lr which, given how mind-bogglingly silly some of the former code looks, seems like an inadvertent win. Robin. > > Will >
On Mon, Dec 07, 2015 at 12:09:56PM +0000, Robin Murphy wrote: > On 07/12/15 11:08, Will Deacon wrote: > [...] > >>+#define ARM_LPAE_GRANULE(d) (1UL << (d)->pg_shift) > >>+ > >> #define ARM_LPAE_PAGES_PER_PGD(d) \ > >>- DIV_ROUND_UP((d)->pgd_size, 1UL << (d)->pg_shift) > >>+ DIV_ROUND_UP((d)->pgd_size, ARM_LPAE_GRANULE(d)) > >> > >> /* > >> * Calculate the index at level l used to map virtual address a using the > >>@@ -169,7 +171,7 @@ > >> /* IOPTE accessors */ > >> #define iopte_deref(pte,d) \ > >> (__va((pte) & ((1ULL << ARM_LPAE_MAX_ADDR_BITS) - 1) \ > >>- & ~((1ULL << (d)->pg_shift) - 1))) > >>+ & ~(ARM_LPAE_GRANULE(d) - 1))) > > > >Do we run the risk of truncating the VA on 32-bit ARM here? > > Indeed, in all honesty I'd missed that, but since __va is going to truncate > it to a 32-bit void * anyway, doing it earlier in the expression actually > seems to result in better code - with iopte_deref wrapped like so to make it > easier to pick out: Gah, I meant PA not VA, sorry. The PA is 40-bit, but with ARM_LPAE_GRANULE using unsigned long, we drop the top 8 bits of the output address field in the pte. Will
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 4e5118a..c302b65 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1335,7 +1335,7 @@ static void arm_smmu_tlb_inv_context(void *cookie) } static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size, - bool leaf, void *cookie) + size_t granule, bool leaf, void *cookie) { struct arm_smmu_domain *smmu_domain = cookie; struct arm_smmu_device *smmu = smmu_domain->smmu; diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 47dc7a7..601e3dd 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -582,7 +582,7 @@ static void arm_smmu_tlb_inv_context(void *cookie) } static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size, - bool leaf, void *cookie) + size_t granule, bool leaf, void *cookie) { struct arm_smmu_domain *smmu_domain = cookie; struct arm_smmu_cfg *cfg = &smmu_domain->cfg; diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c index 366a354..9088d27 100644 --- a/drivers/iommu/io-pgtable-arm.c +++ b/drivers/iommu/io-pgtable-arm.c @@ -58,8 +58,10 @@ ((((d)->levels - ((l) - ARM_LPAE_START_LVL(d) + 1)) \ * (d)->bits_per_level) + (d)->pg_shift) +#define ARM_LPAE_GRANULE(d) (1UL << (d)->pg_shift) + #define ARM_LPAE_PAGES_PER_PGD(d) \ - DIV_ROUND_UP((d)->pgd_size, 1UL << (d)->pg_shift) + DIV_ROUND_UP((d)->pgd_size, ARM_LPAE_GRANULE(d)) /* * Calculate the index at level l used to map virtual address a using the @@ -169,7 +171,7 @@ /* IOPTE accessors */ #define iopte_deref(pte,d) \ (__va((pte) & ((1ULL << ARM_LPAE_MAX_ADDR_BITS) - 1) \ - & ~((1ULL << (d)->pg_shift) - 1))) + & ~(ARM_LPAE_GRANULE(d) - 1))) #define iopte_type(pte,l) \ (((pte) >> ARM_LPAE_PTE_TYPE_SHIFT) & ARM_LPAE_PTE_TYPE_MASK) @@ -326,7 +328,7 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova, /* Grab a pointer to the next level */ pte = *ptep; if (!pte) { - cptep = __arm_lpae_alloc_pages(1UL << data->pg_shift, + cptep = __arm_lpae_alloc_pages(ARM_LPAE_GRANULE(data), GFP_ATOMIC, cfg); if (!cptep) return -ENOMEM; @@ -412,7 +414,7 @@ static void __arm_lpae_free_pgtable(struct arm_lpae_io_pgtable *data, int lvl, if (lvl == ARM_LPAE_START_LVL(data)) table_size = data->pgd_size; else - table_size = 1UL << data->pg_shift; + table_size = ARM_LPAE_GRANULE(data); start = ptep; end = (void *)ptep + table_size; @@ -473,7 +475,7 @@ static int arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data, __arm_lpae_set_pte(ptep, table, cfg); iova &= ~(blk_size - 1); - cfg->tlb->tlb_add_flush(iova, blk_size, true, data->iop.cookie); + cfg->tlb->tlb_add_flush(iova, blk_size, blk_size, true, data->iop.cookie); return size; } @@ -501,12 +503,13 @@ static int __arm_lpae_unmap(struct arm_lpae_io_pgtable *data, if (!iopte_leaf(pte, lvl)) { /* Also flush any partial walks */ - tlb->tlb_add_flush(iova, size, false, cookie); + tlb->tlb_add_flush(iova, size, ARM_LPAE_GRANULE(data), + false, cookie); tlb->tlb_sync(cookie); ptep = iopte_deref(pte, data); __arm_lpae_free_pgtable(data, lvl + 1, ptep); } else { - tlb->tlb_add_flush(iova, size, true, cookie); + tlb->tlb_add_flush(iova, size, size, true, cookie); } return size; @@ -572,7 +575,7 @@ static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops, return 0; found_translation: - iova &= ((1 << data->pg_shift) - 1); + iova &= (ARM_LPAE_GRANULE(data) - 1); return ((phys_addr_t)iopte_to_pfn(pte,data) << data->pg_shift) | iova; } @@ -670,7 +673,7 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg, void *cookie) (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) | (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT); - switch (1 << data->pg_shift) { + switch (ARM_LPAE_GRANULE(data)) { case SZ_4K: reg |= ARM_LPAE_TCR_TG0_4K; break; @@ -771,7 +774,7 @@ arm_64_lpae_alloc_pgtable_s2(struct io_pgtable_cfg *cfg, void *cookie) sl = ARM_LPAE_START_LVL(data); - switch (1 << data->pg_shift) { + switch (ARM_LPAE_GRANULE(data)) { case SZ_4K: reg |= ARM_LPAE_TCR_TG0_4K; sl++; /* SL0 format is different for 4K granule size */ @@ -891,8 +894,8 @@ static void dummy_tlb_flush_all(void *cookie) WARN_ON(cookie != cfg_cookie); } -static void dummy_tlb_add_flush(unsigned long iova, size_t size, bool leaf, - void *cookie) +static void dummy_tlb_add_flush(unsigned long iova, size_t size, + size_t granule, bool leaf, void *cookie) { WARN_ON(cookie != cfg_cookie); WARN_ON(!(size & cfg_cookie->pgsize_bitmap)); diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h index ac9e234..2e18469 100644 --- a/drivers/iommu/io-pgtable.h +++ b/drivers/iommu/io-pgtable.h @@ -26,8 +26,8 @@ enum io_pgtable_fmt { */ struct iommu_gather_ops { void (*tlb_flush_all)(void *cookie); - void (*tlb_add_flush)(unsigned long iova, size_t size, bool leaf, - void *cookie); + void (*tlb_add_flush)(unsigned long iova, size_t size, size_t granule, + bool leaf, void *cookie); void (*tlb_sync)(void *cookie); }; diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c index 8cf605f..5b1166d 100644 --- a/drivers/iommu/ipmmu-vmsa.c +++ b/drivers/iommu/ipmmu-vmsa.c @@ -277,8 +277,8 @@ static void ipmmu_tlb_flush_all(void *cookie) ipmmu_tlb_invalidate(domain); } -static void ipmmu_tlb_add_flush(unsigned long iova, size_t size, bool leaf, - void *cookie) +static void ipmmu_tlb_add_flush(unsigned long iova, size_t size, + size_t granule, bool leaf, void *cookie) { /* The hardware doesn't support selective TLB flush. */ }
IOMMU hardware with range-based TLB maintenance commands can work happily with the iova and size arguments passed via the tlb_add_flush callback, but for IOMMUs which require separate commands per entry in the range, it is not straightforward to infer the necessary granularity when it comes to issuing the actual commands. Add an additional argument indicating the granularity for the benefit of drivers needing to know, and update the ARM LPAE code appropriately (for non-leaf invalidations we currently just assume the worst-case page granularity rather than walking the table to check). Signed-off-by: Robin Murphy <robin.murphy@arm.com> --- drivers/iommu/arm-smmu-v3.c | 2 +- drivers/iommu/arm-smmu.c | 2 +- drivers/iommu/io-pgtable-arm.c | 27 +++++++++++++++------------ drivers/iommu/io-pgtable.h | 4 ++-- drivers/iommu/ipmmu-vmsa.c | 4 ++-- 5 files changed, 21 insertions(+), 18 deletions(-)