Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1422880AbbD2Jvq (ORCPT ); Wed, 29 Apr 2015 05:51:46 -0400 Received: from mail-pd0-f180.google.com ([209.85.192.180]:35715 "EHLO mail-pd0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422657AbbD2Jv3 (ORCPT ); Wed, 29 Apr 2015 05:51:29 -0400 Message-ID: <5540A999.9060507@ozlabs.ru> Date: Wed, 29 Apr 2015 19:51:21 +1000 From: Alexey Kardashevskiy User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: David Gibson CC: linuxppc-dev@lists.ozlabs.org, Benjamin Herrenschmidt , Paul Mackerras , Alex Williamson , Gavin Shan , linux-kernel@vger.kernel.org Subject: Re: [PATCH kernel v9 18/32] powerpc/iommu/powernv: Release replaced TCE References: <1429964096-11524-1-git-send-email-aik@ozlabs.ru> <1429964096-11524-19-git-send-email-aik@ozlabs.ru> <20150429041808.GO32589@voom.redhat.com> In-Reply-To: <20150429041808.GO32589@voom.redhat.com> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3148 Lines: 82 On 04/29/2015 02:18 PM, David Gibson wrote: > On Sat, Apr 25, 2015 at 10:14:42PM +1000, Alexey Kardashevskiy wrote: >> At the moment writing new TCE value to the IOMMU table fails with EBUSY >> if there is a valid entry already. However PAPR specification allows >> the guest to write new TCE value without clearing it first. >> >> Another problem this patch is addressing is the use of pool locks for >> external IOMMU users such as VFIO. The pool locks are to protect >> DMA page allocator rather than entries and since the host kernel does >> not control what pages are in use, there is no point in pool locks and >> exchange()+put_page(oldtce) is sufficient to avoid possible races. >> >> This adds an exchange() callback to iommu_table_ops which does the same >> thing as set() plus it returns replaced TCE and DMA direction so >> the caller can release the pages afterwards. The exchange() receives >> a physical address unlike set() which receives linear mapping address; >> and returns a physical address as the clear() does. >> >> This implements exchange() for P5IOC2/IODA/IODA2. This adds a requirement >> for a platform to have exchange() implemented in order to support VFIO. >> >> This replaces iommu_tce_build() and iommu_clear_tce() with >> a single iommu_tce_xchg(). >> >> This makes sure that TCE permission bits are not set in TCE passed to >> IOMMU API as those are to be calculated by platform code from DMA direction. >> >> This moves SetPageDirty() to the IOMMU code to make it work for both >> VFIO ioctl interface in in-kernel TCE acceleration (when it becomes >> available later). >> >> Signed-off-by: Alexey Kardashevskiy >> [aw: for the vfio related changes] >> Acked-by: Alex Williamson > > This looks mostly good, but there are couple of details that need fixing. > [...] >> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c >> index ba75aa5..e8802ac 100644 >> --- a/arch/powerpc/platforms/powernv/pci.c >> +++ b/arch/powerpc/platforms/powernv/pci.c >> @@ -598,6 +598,23 @@ int pnv_tce_build(struct iommu_table *tbl, long index, long npages, >> return 0; >> } >> >> +#ifdef CONFIG_IOMMU_API >> +int pnv_tce_xchg(struct iommu_table *tbl, long index, >> + unsigned long *tce, enum dma_data_direction *direction) >> +{ >> + u64 proto_tce = iommu_direction_to_tce_perm(*direction); >> + unsigned long newtce = *tce | proto_tce; >> + unsigned long idx = index - tbl->it_offset; > > Should this have a BUG_ON or WARN_ON if the supplied tce has bits set > below the page mask? Why? The caller checks these bits, do we really need to duplicate it here? >> + *tce = xchg(pnv_tce(tbl, idx), cpu_to_be64(newtce)); >> + *tce = be64_to_cpu(*tce); >> + *direction = iommu_tce_direction(*tce); >> + *tce &= ~(TCE_PCI_READ | TCE_PCI_WRITE); >> + >> + return 0; >> +} >> +#endif -- Alexey -- 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/