From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: xen-devel@lists.xensource.com, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Ian.Campbell@citrix.com
Subject: Re: [PATCH v6 11/19] swiotlb-xen: use xen_alloc/free_coherent_pages
Date: Mon, 30 Sep 2013 11:34:48 -0400 [thread overview]
Message-ID: <20130930153448.GR3106@phenom.dumpdata.com> (raw)
In-Reply-To: <1380298207-29151-11-git-send-email-stefano.stabellini@eu.citrix.com>
On Fri, Sep 27, 2013 at 05:09:59PM +0100, Stefano Stabellini wrote:
> Use xen_alloc_coherent_pages and xen_free_coherent_pages to allocate or
> free coherent pages.
>
> We need to be careful handling the pointer returned by
> xen_alloc_coherent_pages, because on ARM the pointer is not equal to
> phys_to_virt(*dma_handle). In fact virt_to_phys only works for kernel
> direct mapped RAM memory.
> In ARM case the pointer could be an ioremap address, therefore passing
> it to virt_to_phys would give you another physical address that doesn't
> correspond to it.
>
> Make xen_create_contiguous_region take a phys_addr_t as start parameter to
> avoid the virt_to_phys calls which would be incorrect.
>
> Changes in v6:
> - remove extra spaces.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> arch/arm/xen/mm.c | 7 +++----
> arch/x86/xen/mmu.c | 7 +++++--
> drivers/xen/swiotlb-xen.c | 31 +++++++++++++++++++++----------
> include/xen/xen-ops.h | 4 ++--
> 4 files changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index 4330b15..b305b94 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -55,11 +55,10 @@ static int xen_exchange_memory(xen_ulong_t extents_in,
> return success;
> }
>
> -int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
> +int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
> unsigned int address_bits,
> dma_addr_t *dma_handle)
> {
> - phys_addr_t pstart = __pa(vstart);
> xen_pfn_t in_frame, out_frame;
> int success;
>
> @@ -78,9 +77,9 @@ int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
> }
> EXPORT_SYMBOL_GPL(xen_create_contiguous_region);
>
> -void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order)
> +void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
> {
> - xen_pfn_t in_frame = __pa(vstart) >> PAGE_SHIFT;
> + xen_pfn_t in_frame = pstart >> PAGE_SHIFT;
> struct xen_unpin unpin = {
> .in = {
> .nr_extents = 1,
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 6c34d7c..8830883 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -2328,13 +2328,14 @@ static int xen_exchange_memory(unsigned long extents_in, unsigned int order_in,
> return success;
> }
>
> -int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
> +int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
> unsigned int address_bits,
> dma_addr_t *dma_handle)
> {
> unsigned long *in_frames = discontig_frames, out_frame;
> unsigned long flags;
> int success;
> + unsigned long vstart = (unsigned long)phys_to_virt(pstart);
>
> /*
> * Currently an auto-translated guest will not perform I/O, nor will
> @@ -2374,11 +2375,12 @@ int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
> }
> EXPORT_SYMBOL_GPL(xen_create_contiguous_region);
>
> -void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order)
> +void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
> {
> unsigned long *out_frames = discontig_frames, in_frame;
> unsigned long flags;
> int success;
> + unsigned long vstart;
>
> if (xen_feature(XENFEAT_auto_translated_physmap))
> return;
> @@ -2386,6 +2388,7 @@ void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order)
> if (unlikely(order > MAX_CONTIG_ORDER))
> return;
>
> + vstart = (unsigned long)phys_to_virt(pstart);
> memset((void *) vstart, 0, PAGE_SIZE << order);
>
> spin_lock_irqsave(&xen_reservation_lock, flags);
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index deb9131..96ad316 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -46,6 +46,7 @@
> #include <xen/hvc-console.h>
> #include <xen/features.h>
> #include <asm/dma-mapping.h>
> +#include <asm/xen/page-coherent.h>
>
> /*
> * Used to do a quick range check in swiotlb_tbl_unmap_single and
> @@ -244,6 +245,7 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
> {
> int i, j, rc;
> int dma_bits;
> + phys_addr_t p = virt_to_phys(buf);
>
> dma_bits = get_order(IO_TLB_SEGSIZE << IO_TLB_SHIFT) + PAGE_SHIFT;
>
> @@ -253,7 +255,7 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long nslabs)
>
> do {
> rc = xen_create_contiguous_region(
> - (unsigned long)buf + (i << IO_TLB_SHIFT),
> + p + (i << IO_TLB_SHIFT),
> get_order(slabs << IO_TLB_SHIFT),
> dma_bits, &xen_dma_seg[j].dma_addr);
> } while (rc && dma_bits++ < max_dma_bits);
> @@ -389,7 +391,6 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> void *ret;
> int order = get_order(size);
> u64 dma_mask = DMA_BIT_MASK(32);
> - unsigned long vstart;
> phys_addr_t phys;
> dma_addr_t dev_addr;
> struct xen_dma_info *dma_info = NULL;
> @@ -405,8 +406,12 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> if (dma_alloc_from_coherent(hwdev, size, dma_handle, &ret))
> return ret;
>
> - vstart = __get_free_pages(flags, order);
> - ret = (void *)vstart;
> + /* On ARM this function returns an ioremap'ped virtual address for
> + * which virt_to_phys doesn't return the corresponding physical
> + * address. In fact on ARM virt_to_phys only works for kernel direct
> + * mapped RAM memory. Also see comment below.
> + */
> + ret = xen_alloc_coherent_pages(hwdev, size, dma_handle, flags, attrs);
>
> if (!ret)
> return ret;
> @@ -414,16 +419,20 @@ xen_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> if (hwdev && hwdev->coherent_dma_mask)
> dma_mask = dma_alloc_coherent_mask(hwdev, flags);
>
> - phys = virt_to_phys(ret);
> + /* At this point dma_handle is the physical address, next we are
> + * going to set it to the machine address.
> + * Do not use virt_to_phys(ret) because on ARM it doesn't correspond
> + * to *dma_handle. */
> + phys = *dma_handle;
> dev_addr = xen_phys_to_bus(phys);
> if (!xen_feature(XENFEAT_auto_translated_physmap) &&
> ((dev_addr + size - 1 <= dma_mask)) &&
> !range_straddles_page_boundary(phys, size))
> *dma_handle = dev_addr;
> else {
> - if (xen_create_contiguous_region(vstart, order,
> + if (xen_create_contiguous_region(phys, order,
> fls64(dma_mask), dma_handle) != 0) {
> - free_pages(vstart, order);
> + xen_free_coherent_pages(hwdev, size, ret, (dma_addr_t)phys, attrs);
> return NULL;
> }
>
> @@ -463,18 +472,20 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
> if (hwdev && hwdev->coherent_dma_mask)
> dma_mask = hwdev->coherent_dma_mask;
>
> - phys = virt_to_phys(vaddr);
> + /* do not use virt_to_phys because on ARM it doesn't return you the
> + * physical address */
> + phys = xen_bus_to_phys(dev_addr);
>
> if (xen_feature(XENFEAT_auto_translated_physmap) ||
> (((dev_addr + size - 1 > dma_mask)) ||
> range_straddles_page_boundary(phys, size))) {
> - xen_destroy_contiguous_region((unsigned long)vaddr, order);
> + xen_destroy_contiguous_region(phys, order);
> dma_info = xen_get_dma_info_from_dma(dev_addr);
> rb_erase(&dma_info->rbnode, &bus_to_phys);
> kfree(dma_info);
> }
>
> - free_pages((unsigned long)vaddr, order);
> + xen_free_coherent_pages(hwdev, size, vaddr, (dma_addr_t)phys, attrs);
> }
> EXPORT_SYMBOL_GPL(xen_swiotlb_free_coherent);
>
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index 9ef704d..fb2ea8f 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -19,11 +19,11 @@ void xen_arch_resume(void);
> int xen_setup_shutdown_event(void);
>
> extern unsigned long *xen_contiguous_bitmap;
> -int xen_create_contiguous_region(unsigned long vstart, unsigned int order,
> +int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
> unsigned int address_bits,
> dma_addr_t *dma_handle);
>
> -void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order);
> +void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order);
>
> struct vm_area_struct;
> int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
> --
> 1.7.2.5
>
next prev parent reply other threads:[~2013-09-30 15:35 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-27 16:09 [PATCH v6 0/19] enable swiotlb-xen on arm and arm64 Stefano Stabellini
2013-09-27 16:09 ` [PATCH v6 01/19] arm: make SWIOTLB available Stefano Stabellini
2013-09-27 16:09 ` [PATCH v6 02/19] arm64: define DMA_ERROR_CODE Stefano Stabellini
2013-09-30 10:47 ` Catalin Marinas
2013-09-27 16:09 ` [PATCH v6 03/19] xen: introduce XENMEM_exchange_and_pin and XENMEM_unpin Stefano Stabellini
2013-09-30 14:54 ` Konrad Rzeszutek Wilk
2013-09-27 16:09 ` [PATCH v6 04/19] xen: make xen_create_contiguous_region return the dma address Stefano Stabellini
2013-09-27 16:09 ` [PATCH v6 05/19] swiotlb-xen: support autotranslate guests Stefano Stabellini
2013-09-27 16:09 ` [PATCH v6 06/19] xen/arm,arm64: enable SWIOTLB_XEN Stefano Stabellini
2013-09-30 15:14 ` Konrad Rzeszutek Wilk
2013-09-27 16:09 ` [PATCH v6 07/19] swiotlb-xen: introduce xen_swiotlb_set_dma_mask Stefano Stabellini
2013-09-27 16:09 ` [PATCH v6 08/19] arm/xen: get_dma_ops: return xen_dma_ops if we are running on Xen Stefano Stabellini
2013-09-30 15:17 ` Konrad Rzeszutek Wilk
2013-09-27 16:09 ` [PATCH v6 09/19] arm64/xen: " Stefano Stabellini
2013-09-30 10:48 ` Catalin Marinas
2013-09-27 16:09 ` [PATCH v6 10/19] xen: introduce xen_alloc/free_coherent_pages Stefano Stabellini
2013-09-30 15:31 ` Konrad Rzeszutek Wilk
2013-10-01 13:40 ` Catalin Marinas
2013-10-02 17:03 ` Stefano Stabellini
2013-10-02 17:07 ` Catalin Marinas
2013-10-02 17:14 ` Stefano Stabellini
2013-09-27 16:09 ` [PATCH v6 11/19] swiotlb-xen: use xen_alloc/free_coherent_pages Stefano Stabellini
2013-09-30 15:34 ` Konrad Rzeszutek Wilk [this message]
2013-09-27 16:10 ` [PATCH v6 12/19] swiotlb: don't assume that io_tlb_start-io_tlb_end is coherent Stefano Stabellini
2013-09-30 15:56 ` Konrad Rzeszutek Wilk
2013-10-02 17:31 ` Stefano Stabellini
2013-10-04 13:23 ` Konrad Rzeszutek Wilk
2013-09-27 16:10 ` [PATCH v6 13/19] ASoC: Samsung: Rename dma_ops by samsung_dma_ops Stefano Stabellini
2013-09-27 16:10 ` [PATCH v6 14/19] swiotlb: print a warning when the swiotlb is full Stefano Stabellini
2013-09-30 15:58 ` Konrad Rzeszutek Wilk
2013-10-09 17:31 ` Stefano Stabellini
2013-09-27 16:10 ` [PATCH v6 15/19] swiotlb-xen: call dma_capable only if dev->dma_mask is allocated Stefano Stabellini
2013-09-30 16:02 ` Konrad Rzeszutek Wilk
2013-10-02 17:13 ` Stefano Stabellini
2013-10-02 17:22 ` Konrad Rzeszutek Wilk
2013-10-03 18:35 ` Rob Herring
2013-09-27 16:10 ` [PATCH v6 16/19] arm,arm64: do not always merge biovec if we are running on Xen Stefano Stabellini
2013-09-30 16:06 ` Konrad Rzeszutek Wilk
2013-10-09 17:54 ` Stefano Stabellini
2013-09-27 16:10 ` [PATCH v6 17/19] xen: introduce XENMEM_pin Stefano Stabellini
2013-09-30 17:22 ` Konrad Rzeszutek Wilk
2013-09-27 16:10 ` [PATCH v6 18/19] swiotlb-xen: introduce a rbtree to track phys to bus mappings Stefano Stabellini
2013-09-30 17:27 ` Konrad Rzeszutek Wilk
2013-10-02 17:23 ` Stefano Stabellini
2013-10-09 17:25 ` Stefano Stabellini
2013-09-27 16:10 ` [PATCH v6 19/19] swiotlb-xen: instead of bouncing on the swiotlb, pin single pages Stefano Stabellini
2013-09-30 17:39 ` Konrad Rzeszutek Wilk
2013-10-09 17:27 ` Stefano Stabellini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130930153448.GR3106@phenom.dumpdata.com \
--to=konrad.wilk@oracle.com \
--cc=Ian.Campbell@citrix.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stefano.stabellini@eu.citrix.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox