Message ID | 1431358763-24371-20-git-send-email-aik@ozlabs.ru (mailing list archive) |
---|---|
State | Accepted |
Commit | b82c75bfbeb5b4c7c23ee09dd6f295c7f06aeb25 |
Delegated to: | Benjamin Herrenschmidt |
Headers | show |
On Tue, May 12, 2015 at 01:39:08AM +1000, Alexey Kardashevskiy wrote: >This adds missing locks in iommu_take_ownership()/ >iommu_release_ownership(). > >This marks all pages busy in iommu_table::it_map in order to catch >errors if there is an attempt to use this table while ownership over it >is taken. > >This only clears TCE content if there is no page marked busy in it_map. >Clearing must be done outside of the table locks as iommu_clear_tce() >called from iommu_clear_tces_and_put_pages() does this. > >In order to use bitmap_empty(), the existing code clears bit#0 which >is set even in an empty table if it is bus-mapped at 0 as >iommu_init_table() reserves page#0 to prevent buggy drivers >from crashing when allocated page is bus-mapped at zero >(which is correct). This restores the bit in the case of failure >to bring the it_map to the state it was in when we called >iommu_take_ownership(). > >Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Gavin Shan <gwshan@linux.vnet.ibm.com> Thanks, Gavin >--- >Changes: >v9: >* iommu_table_take_ownership() did not return @ret (and ignored EBUSY), >now it does return correct error. >* updated commit log about setting bit#0 in the case of failure > >v5: >* do not store bit#0 value, it has to be set for zero-based table >anyway >* removed test_and_clear_bit >--- > arch/powerpc/kernel/iommu.c | 30 +++++++++++++++++++++++++----- > 1 file changed, 25 insertions(+), 5 deletions(-) > >diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c >index 7e54714..6275164 100644 >--- a/arch/powerpc/kernel/iommu.c >+++ b/arch/powerpc/kernel/iommu.c >@@ -1032,31 +1032,51 @@ EXPORT_SYMBOL_GPL(iommu_tce_build); > > int iommu_take_ownership(struct iommu_table *tbl) > { >- unsigned long sz = (tbl->it_size + 7) >> 3; >+ unsigned long flags, i, sz = (tbl->it_size + 7) >> 3; >+ int ret = 0; >+ >+ spin_lock_irqsave(&tbl->large_pool.lock, flags); >+ for (i = 0; i < tbl->nr_pools; i++) >+ spin_lock(&tbl->pools[i].lock); > > if (tbl->it_offset == 0) > clear_bit(0, tbl->it_map); > > if (!bitmap_empty(tbl->it_map, tbl->it_size)) { > pr_err("iommu_tce: it_map is not empty"); >- return -EBUSY; >+ ret = -EBUSY; >+ /* Restore bit#0 set by iommu_init_table() */ >+ if (tbl->it_offset == 0) >+ set_bit(0, tbl->it_map); >+ } else { >+ memset(tbl->it_map, 0xff, sz); > } > >- memset(tbl->it_map, 0xff, sz); >+ for (i = 0; i < tbl->nr_pools; i++) >+ spin_unlock(&tbl->pools[i].lock); >+ spin_unlock_irqrestore(&tbl->large_pool.lock, flags); > >- return 0; >+ return ret; > } > EXPORT_SYMBOL_GPL(iommu_take_ownership); > > void iommu_release_ownership(struct iommu_table *tbl) > { >- unsigned long sz = (tbl->it_size + 7) >> 3; >+ unsigned long flags, i, sz = (tbl->it_size + 7) >> 3; >+ >+ spin_lock_irqsave(&tbl->large_pool.lock, flags); >+ for (i = 0; i < tbl->nr_pools; i++) >+ spin_lock(&tbl->pools[i].lock); > > memset(tbl->it_map, 0, sz); > > /* Restore bit#0 set by iommu_init_table() */ > if (tbl->it_offset == 0) > set_bit(0, tbl->it_map); >+ >+ for (i = 0; i < tbl->nr_pools; i++) >+ spin_unlock(&tbl->pools[i].lock); >+ spin_unlock_irqrestore(&tbl->large_pool.lock, flags); > } > EXPORT_SYMBOL_GPL(iommu_release_ownership); > >-- >2.4.0.rc3.8.gfb3e7d5 >
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c index 7e54714..6275164 100644 --- a/arch/powerpc/kernel/iommu.c +++ b/arch/powerpc/kernel/iommu.c @@ -1032,31 +1032,51 @@ EXPORT_SYMBOL_GPL(iommu_tce_build); int iommu_take_ownership(struct iommu_table *tbl) { - unsigned long sz = (tbl->it_size + 7) >> 3; + unsigned long flags, i, sz = (tbl->it_size + 7) >> 3; + int ret = 0; + + spin_lock_irqsave(&tbl->large_pool.lock, flags); + for (i = 0; i < tbl->nr_pools; i++) + spin_lock(&tbl->pools[i].lock); if (tbl->it_offset == 0) clear_bit(0, tbl->it_map); if (!bitmap_empty(tbl->it_map, tbl->it_size)) { pr_err("iommu_tce: it_map is not empty"); - return -EBUSY; + ret = -EBUSY; + /* Restore bit#0 set by iommu_init_table() */ + if (tbl->it_offset == 0) + set_bit(0, tbl->it_map); + } else { + memset(tbl->it_map, 0xff, sz); } - memset(tbl->it_map, 0xff, sz); + for (i = 0; i < tbl->nr_pools; i++) + spin_unlock(&tbl->pools[i].lock); + spin_unlock_irqrestore(&tbl->large_pool.lock, flags); - return 0; + return ret; } EXPORT_SYMBOL_GPL(iommu_take_ownership); void iommu_release_ownership(struct iommu_table *tbl) { - unsigned long sz = (tbl->it_size + 7) >> 3; + unsigned long flags, i, sz = (tbl->it_size + 7) >> 3; + + spin_lock_irqsave(&tbl->large_pool.lock, flags); + for (i = 0; i < tbl->nr_pools; i++) + spin_lock(&tbl->pools[i].lock); memset(tbl->it_map, 0, sz); /* Restore bit#0 set by iommu_init_table() */ if (tbl->it_offset == 0) set_bit(0, tbl->it_map); + + for (i = 0; i < tbl->nr_pools; i++) + spin_unlock(&tbl->pools[i].lock); + spin_unlock_irqrestore(&tbl->large_pool.lock, flags); } EXPORT_SYMBOL_GPL(iommu_release_ownership);