Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751988AbbHEJum (ORCPT ); Wed, 5 Aug 2015 05:50:42 -0400 Received: from smtp.citrix.com ([66.165.176.89]:31624 "EHLO SMTP.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751231AbbHEJuk (ORCPT ); Wed, 5 Aug 2015 05:50:40 -0400 X-IronPort-AV: E=Sophos;i="5.15,615,1432598400"; d="scan'208";a="288221998" Date: Wed, 5 Aug 2015 10:49:15 +0100 From: Stefano Stabellini X-X-Sender: sstabellini@kaball.uk.xensource.com To: Julien Grall CC: , , , , "Russell King" , Konrad Rzeszutek Wilk , Boris Ostrovsky , "David Vrabel" , Thomas Gleixner , "Ingo Molnar" , "H. Peter Anvin" , , Subject: Re: [PATCH v2 2/8] xen: Make clear that swiotlb and biomerge are dealing with DMA address In-Reply-To: <1438711972-18752-3-git-send-email-julien.grall@citrix.com> Message-ID: References: <1438711972-18752-1-git-send-email-julien.grall@citrix.com> <1438711972-18752-3-git-send-email-julien.grall@citrix.com> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-DLP: MIA2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8601 Lines: 237 On Tue, 4 Aug 2015, Julien Grall wrote: > The swiotlb is required when programming a DMA address on ARM when a > device is not protected by an IOMMU. > > In this case, the DMA address should always be equal to the machine address. > For DOM0 memory, Xen ensure it by have an identity mapping between the > guest address and host address. However, when mapping a foreign grant > reference, the 1:1 model doesn't work. > > For ARM guest, most of the callers of pfn_to_mfn expects to get a GFN > (Guest Frame Number), i.e a PFN (Page Frame Number) from the Linux point > of view given that all ARM guest are auto-translated. > > Even though the name pfn_to_mfn is misleading, we need to ensure that > those caller get a GFN and not by mistake a MFN. In pratical, I haven't > seen error related to this but we should fix it for the sake of > correctness. > > In order to fix the implementation of pfn_to_mfn on ARM in a follow-up > patch, we have to introduce new helpers to return the DMA from a PFN and > the invert. > > On x86, the new helpers will be an alias of pfn_to_mfn and mfn_to_pfn. > > The helpers will be used in swiotlb and xen_biovec_phys_mergeable. > > This is necessary in the latter because we have to ensure that the > biovec code will not try to merge a biovec using foreign page and > another using Linux memory. > > Lastly, the helper mfn_to_local_pfn has been renamed to dnf_to_local_pfn ^ please update > given that the only usage was in swiotlb. > > Signed-off-by: Julien Grall > Cc: Stefano Stabellini > Cc: Russell King > Cc: Konrad Rzeszutek Wilk > Cc: Boris Ostrovsky > Cc: David Vrabel > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: x86@kernel.org > Cc: linux-arm-kernel@lists.infradead.org > > --- > Changes in v2: > - Use bfn (Bus Frame Number) rather than dfn to match the > proposed terminology for pv-iommu hypercall. > --- > arch/arm/include/asm/xen/page.h | 23 +++++++++++++++++++++-- > arch/arm/xen/mm.c | 4 ++-- > arch/x86/include/asm/xen/page.h | 8 ++++++-- > drivers/xen/biomerge.c | 6 +++--- > drivers/xen/swiotlb-xen.c | 16 ++++++++-------- > 5 files changed, 40 insertions(+), 17 deletions(-) > > diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h > index 98b1084..bc5e77c 100644 > --- a/arch/arm/include/asm/xen/page.h > +++ b/arch/arm/include/asm/xen/page.h > @@ -52,7 +52,26 @@ static inline unsigned long mfn_to_pfn(unsigned long mfn) > return mfn; > } > > -#define mfn_to_local_pfn(mfn) mfn_to_pfn(mfn) > +/* Pseudo-physical <-> BUS conversion */ > +static inline unsigned long pfn_to_bfn(unsigned long pfn) > +{ > + unsigned long mfn; > + > + if (phys_to_mach.rb_node != NULL) { > + mfn = __pfn_to_mfn(pfn); > + if (mfn != INVALID_P2M_ENTRY) > + return mfn; > + } > + > + return pfn; > +} > + > +static inline unsigned long bfn_to_pfn(unsigned long bfn) > +{ > + return bfn; > +} > + > +#define bfn_to_local_pfn(bfn) bfn_to_pfn(bfn) > > /* VIRT <-> MACHINE conversion */ > #define virt_to_mfn(v) (pfn_to_mfn(virt_to_pfn(v))) > @@ -96,7 +115,7 @@ static inline bool set_phys_to_machine(unsigned long pfn, unsigned long mfn) > > bool xen_arch_need_swiotlb(struct device *dev, > unsigned long pfn, > - unsigned long mfn); > + unsigned long dfn); > unsigned long xen_get_swiotlb_free_pages(unsigned int order); You missed a bunch of dfn->bfn renamings. Aside from those and the commit message error: Reviewed-by: Stefano Stabellini > #endif /* _ASM_ARM_XEN_PAGE_H */ > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c > index 03e75fe..12bde72 100644 > --- a/arch/arm/xen/mm.c > +++ b/arch/arm/xen/mm.c > @@ -139,9 +139,9 @@ void __xen_dma_sync_single_for_device(struct device *hwdev, > > bool xen_arch_need_swiotlb(struct device *dev, > unsigned long pfn, > - unsigned long mfn) > + unsigned long dfn) > { > - return (!hypercall_cflush && (pfn != mfn) && !is_device_dma_coherent(dev)); > + return (!hypercall_cflush && (pfn != dfn) && !is_device_dma_coherent(dev)); > } > > int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order, > diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h > index c44a5d5..8ba04b8 100644 > --- a/arch/x86/include/asm/xen/page.h > +++ b/arch/x86/include/asm/xen/page.h > @@ -178,6 +178,10 @@ static inline xpaddr_t machine_to_phys(xmaddr_t machine) > return XPADDR(PFN_PHYS(mfn_to_pfn(PFN_DOWN(machine.maddr))) | offset); > } > > +/* Pseudo-physical <-> Bus conversion */ > +#define pfn_to_bfn(pfn) pfn_to_mfn(pfn) > +#define bfn_to_pfn(bfn) mfn_to_pfn(bfn) > + > /* > * We detect special mappings in one of two ways: > * 1. If the MFN is an I/O page then Xen will set the m2p entry > @@ -198,7 +202,7 @@ static inline xpaddr_t machine_to_phys(xmaddr_t machine) > * require. In all the cases we care about, the FOREIGN_FRAME bit is > * masked (e.g., pfn_to_mfn()) so behaviour there is correct. > */ > -static inline unsigned long mfn_to_local_pfn(unsigned long mfn) > +static inline unsigned long bfn_to_local_pfn(unsigned long mfn) > { > unsigned long pfn; > > @@ -264,7 +268,7 @@ void make_lowmem_page_readwrite(void *vaddr); > > static inline bool xen_arch_need_swiotlb(struct device *dev, > unsigned long pfn, > - unsigned long mfn) > + unsigned long bfn) > { > return false; > } > diff --git a/drivers/xen/biomerge.c b/drivers/xen/biomerge.c > index 0edb91c..8ae2fc90 100644 > --- a/drivers/xen/biomerge.c > +++ b/drivers/xen/biomerge.c > @@ -6,10 +6,10 @@ > bool xen_biovec_phys_mergeable(const struct bio_vec *vec1, > const struct bio_vec *vec2) > { > - unsigned long mfn1 = pfn_to_mfn(page_to_pfn(vec1->bv_page)); > - unsigned long mfn2 = pfn_to_mfn(page_to_pfn(vec2->bv_page)); > + unsigned long bfn1 = pfn_to_bfn(page_to_pfn(vec1->bv_page)); > + unsigned long bfn2 = pfn_to_bfn(page_to_pfn(vec2->bv_page)); > > return __BIOVEC_PHYS_MERGEABLE(vec1, vec2) && > - ((mfn1 == mfn2) || ((mfn1+1) == mfn2)); > + ((bfn1 == bfn2) || ((bfn1+1) == bfn2)); > } > EXPORT_SYMBOL(xen_biovec_phys_mergeable); > diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c > index 4c54932..d757a3e 100644 > --- a/drivers/xen/swiotlb-xen.c > +++ b/drivers/xen/swiotlb-xen.c > @@ -82,8 +82,8 @@ static u64 start_dma_addr; > */ > static inline dma_addr_t xen_phys_to_bus(phys_addr_t paddr) > { > - unsigned long mfn = pfn_to_mfn(PFN_DOWN(paddr)); > - dma_addr_t dma = (dma_addr_t)mfn << PAGE_SHIFT; > + unsigned long bfn = pfn_to_bfn(PFN_DOWN(paddr)); > + dma_addr_t dma = (dma_addr_t)bfn << PAGE_SHIFT; > > dma |= paddr & ~PAGE_MASK; > > @@ -92,7 +92,7 @@ static inline dma_addr_t xen_phys_to_bus(phys_addr_t paddr) > > static inline phys_addr_t xen_bus_to_phys(dma_addr_t baddr) > { > - unsigned long pfn = mfn_to_pfn(PFN_DOWN(baddr)); > + unsigned long pfn = bfn_to_pfn(PFN_DOWN(baddr)); > dma_addr_t dma = (dma_addr_t)pfn << PAGE_SHIFT; > phys_addr_t paddr = dma; > > @@ -110,15 +110,15 @@ static int check_pages_physically_contiguous(unsigned long pfn, > unsigned int offset, > size_t length) > { > - unsigned long next_mfn; > + unsigned long next_bfn; > int i; > int nr_pages; > > - next_mfn = pfn_to_mfn(pfn); > + next_bfn = pfn_to_bfn(pfn); > nr_pages = (offset + length + PAGE_SIZE-1) >> PAGE_SHIFT; > > for (i = 1; i < nr_pages; i++) { > - if (pfn_to_mfn(++pfn) != ++next_mfn) > + if (pfn_to_bfn(++pfn) != ++next_bfn) > return 0; > } > return 1; > @@ -138,8 +138,8 @@ static inline int range_straddles_page_boundary(phys_addr_t p, size_t size) > > static int is_xen_swiotlb_buffer(dma_addr_t dma_addr) > { > - unsigned long mfn = PFN_DOWN(dma_addr); > - unsigned long pfn = mfn_to_local_pfn(mfn); > + unsigned long bfn = PFN_DOWN(dma_addr); > + unsigned long pfn = bfn_to_local_pfn(bfn); > phys_addr_t paddr; > > /* If the address is outside our domain, it CAN > -- > 2.1.4 > -- 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/