Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933249AbbENDiF (ORCPT ); Wed, 13 May 2015 23:38:05 -0400 Received: from e23smtp03.au.ibm.com ([202.81.31.145]:56815 "EHLO e23smtp03.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753170AbbENDiB (ORCPT ); Wed, 13 May 2015 23:38:01 -0400 Date: Thu, 14 May 2015 13:36:55 +1000 From: Gavin Shan To: Alexey Kardashevskiy Cc: linuxppc-dev@lists.ozlabs.org, David Gibson , Benjamin Herrenschmidt , Paul Mackerras , Alex Williamson , Gavin Shan , Wei Yang , linux-kernel@vger.kernel.org Subject: Re: [PATCH kernel v10 19/34] powerpc/iommu: Fix IOMMU ownership control functions Message-ID: <20150514033655.GA13032@gwshan> Reply-To: Gavin Shan References: <1431358763-24371-1-git-send-email-aik@ozlabs.ru> <1431358763-24371-20-git-send-email-aik@ozlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1431358763-24371-20-git-send-email-aik@ozlabs.ru> User-Agent: Mutt/1.5.23 (2014-03-12) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15051403-0009-0000-0000-000001631049 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3564 Lines: 113 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 >Reviewed-by: David Gibson Reviewed-by: Gavin Shan 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/