Re: [PATCH v6 11/19] swiotlb-xen: use xen_alloc/free_coherent_pages

2013-09-30 Thread Konrad Rzeszutek Wilk 
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 

Reviewed-by: Konrad Rzeszutek Wilk 
> ---
>  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;
>   intsuccess;
> + 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(_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 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   * 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,

Re: [PATCH v6 11/19] swiotlb-xen: use xen_alloc/free_coherent_pages

2013-09-30 Thread Konrad Rzeszutek Wilk 
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.stabell...@eu.citrix.com

Reviewed-by: Konrad Rzeszutek Wilk konrad.w...@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;
   intsuccess;
 + 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,

[PATCH v6 11/19] swiotlb-xen: use xen_alloc/free_coherent_pages

2013-09-27 Thread Stefano Stabellini 
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 
---
 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;
intsuccess;
+   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(_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 
 #include 
 #include 
+#include 
 
 /*
  * 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, _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

[PATCH v6 11/19] swiotlb-xen: use xen_alloc/free_coherent_pages

2013-09-27 Thread Stefano Stabellini 
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.stabell...@eu.citrix.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;
intsuccess;
+   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