[PATCH v2 0/2] XEN SWIOTLB for ARM dma operations extension

[PATCH v2 0/2] XEN SWIOTLB for ARM dma operations extension

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

Some drivers need additional dma ops to be provided by xen swiotlb. Because
common operations implementation came from x86 world and does not suite ARM
needs we have to provide needed XEN SWIOTLB for ARM callbacks.

dma_mmap patch is a port of an antique and lost patch discussed here:
http://marc.info/?l=xen-devel&m=139695901430667&w=4

Changes in V2:
 - patches are rebased and checked for compilation for x86, arm, arm64 with
   the current LK master HEAD 83346fbc07d267de777e2597552f785174ad0373

Andrii Anisov (1):
  swiotlb-xen: implement xen_swiotlb_get_sgtable callback

Stefano Stabellini (1):
  swiotlb-xen: implement xen_swiotlb_dma_mmap callback

 arch/arm/xen/mm.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 1/2] swiotlb-xen: implement xen_swiotlb_dma_mmap callback

[Andrii Anisov]

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

This function creates userspace mapping for the DMA-coherent memory.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 arch/arm/xen/mm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index bd62d94..ff812a2 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -163,6 +163,19 @@ bool xen_arch_need_swiotlb(struct device *dev,
 		!is_device_dma_coherent(dev));
 }
 
+/*
+ * Create userspace mapping for the DMA-coherent memory.
+ */
+static int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+			 void *cpu_addr, dma_addr_t dma_addr, size_t size,
+			 unsigned long attrs)
+{
+	if (__generic_dma_ops(dev)->mmap)
+		return __generic_dma_ops(dev)->mmap(dev, vma, cpu_addr, dma_addr, size, attrs);
+
+	return dma_common_mmap(dev, vma, cpu_addr, dma_addr, size);
+}
+
 int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
 				 unsigned int address_bits,
 				 dma_addr_t *dma_handle)
@@ -198,6 +211,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
 	.unmap_page = xen_swiotlb_unmap_page,
 	.dma_supported = xen_swiotlb_dma_supported,
 	.set_dma_mask = xen_swiotlb_set_dma_mask,
+	.mmap = xen_swiotlb_dma_mmap,
 };
 
 int __init xen_mm_init(void)
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
 arch/arm/xen/mm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index ff812a2..dc83a35 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -176,6 +176,16 @@ static int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 	return dma_common_mmap(dev, vma, cpu_addr, dma_addr, size);
 }
 
+static int xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
+				 void *cpu_addr, dma_addr_t handle, size_t size,
+				 unsigned long attrs)
+{
+	if (__generic_dma_ops(dev)->get_sgtable)
+		return __generic_dma_ops(dev)->get_sgtable(dev, sgt, cpu_addr, handle,
+								 size, attrs);
+	return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size);
+}
+
 int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
 				 unsigned int address_bits,
 				 dma_addr_t *dma_handle)
@@ -212,6 +222,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
 	.dma_supported = xen_swiotlb_dma_supported,
 	.set_dma_mask = xen_swiotlb_set_dma_mask,
 	.mmap = xen_swiotlb_dma_mmap,
+	.get_sgtable = xen_swiotlb_get_sgtable,
 };
 
 int __init xen_mm_init(void)
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback

[Stefano Stabellini]

On Mon, 16 Jan 2017, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@epam.com>
> 
> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>

Thanks for the patch!


>  arch/arm/xen/mm.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index ff812a2..dc83a35 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -176,6 +176,16 @@ static int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
>  	return dma_common_mmap(dev, vma, cpu_addr, dma_addr, size);
>  }

As for the other patch, I would move xen_swiotlb_get_sgtable to
drivers/xen/swiotlb-xen.c, if Konrad agrees.


> +static int xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
> +				 void *cpu_addr, dma_addr_t handle, size_t size,
> +				 unsigned long attrs)
> +{
> +	if (__generic_dma_ops(dev)->get_sgtable)
> +		return __generic_dma_ops(dev)->get_sgtable(dev, sgt, cpu_addr, handle,
> +								 size, attrs);
> +	return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size);
> +}
> +

__generic_dma_ops(dev)->get_sgtable on ARM is implemented by
arm_dma_get_sgtable, which doesn't work on foreign pages (pages for
which bfn != pfn).

If get_sgtable is guaranteed to be always called passing references to
pages previously allocated with dma_alloc_coherent, then we don't have
any issues, because those can't be foreign pages. I suggest we add an
in-code comment to explain why this is safe, as for the previous patch.
I think this is the case, but I am not 100% sure.

On the other hand, if this function can be called passing as parameters
cpu_addr and handle that could potentially refer to a foreign page, then
we have a problem. On ARM, virt_to_phys doesn't work on some pages, in
fact that is the reason why ARM has its own separate get_sgtable
implementation (arm_dma_get_sgtable). But with Xen foreign pages,
dma_to_pfn doesn't work either, because we have no way of finding out
the pfn address corresponding to the mfn of the foreign page. Both
arm_dma_get_sgtable and dma_common_get_sgtable wouldn't work. I have no
solution to this problem, but maybe we could add a check like the
following (also to the previous patch?). I haven't tested it, but I
think it should work as long as page_is_ram is returns the correct value
for the handle parameter.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index dc83a35..cd0441c 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -18,6 +18,7 @@
 #include <xen/page.h>
 #include <xen/swiotlb-xen.h>
 
+#include <asm/dma-mapping.h>
 #include <asm/cacheflush.h>
 #include <asm/xen/hypercall.h>
 #include <asm/xen/interface.h>
@@ -180,9 +181,18 @@ static int xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
 				 void *cpu_addr, dma_addr_t handle, size_t size,
 				 unsigned long attrs)
 {
-	if (__generic_dma_ops(dev)->get_sgtable)
+
+	if (__generic_dma_ops(dev)->get_sgtable) {
+		/* We can't handle foreign pages here. */
+#ifdef CONFIG_ARM
+		unsigned long bfn = dma_to_pfn(dev, handle);
+#else
+		unsigned long bfn = handle >> PAGE_SHIFT;
+#endif
+		BUG_ON (!page_is_ram(bfn));
 		return __generic_dma_ops(dev)->get_sgtable(dev, sgt, cpu_addr, handle,
 								 size, attrs);
+	}
 	return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size);
 }
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback

[Konrad Rzeszutek Wilk]

On Mon, Jan 16, 2017 at 05:09:24PM -0800, Stefano Stabellini wrote:
> On Mon, 16 Jan 2017, Andrii Anisov wrote:
> > From: Andrii Anisov <andrii_anisov@epam.com>
> > 
> > Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
> 
> Thanks for the patch!
> 
> 
> >  arch/arm/xen/mm.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> > index ff812a2..dc83a35 100644
> > --- a/arch/arm/xen/mm.c
> > +++ b/arch/arm/xen/mm.c
> > @@ -176,6 +176,16 @@ static int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> >  	return dma_common_mmap(dev, vma, cpu_addr, dma_addr, size);
> >  }
> 
> As for the other patch, I would move xen_swiotlb_get_sgtable to
> drivers/xen/swiotlb-xen.c, if Konrad agrees.

That is fine.
> 
> 
> > +static int xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
> > +				 void *cpu_addr, dma_addr_t handle, size_t size,
> > +				 unsigned long attrs)
> > +{
> > +	if (__generic_dma_ops(dev)->get_sgtable)
> > +		return __generic_dma_ops(dev)->get_sgtable(dev, sgt, cpu_addr, handle,
> > +								 size, attrs);
> > +	return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size);
> > +}
> > +
> 
> __generic_dma_ops(dev)->get_sgtable on ARM is implemented by
> arm_dma_get_sgtable, which doesn't work on foreign pages (pages for
> which bfn != pfn).
> 
> If get_sgtable is guaranteed to be always called passing references to
> pages previously allocated with dma_alloc_coherent, then we don't have
> any issues, because those can't be foreign pages. I suggest we add an
> in-code comment to explain why this is safe, as for the previous patch.
> I think this is the case, but I am not 100% sure.
> 
> On the other hand, if this function can be called passing as parameters
> cpu_addr and handle that could potentially refer to a foreign page, then
> we have a problem. On ARM, virt_to_phys doesn't work on some pages, in
> fact that is the reason why ARM has its own separate get_sgtable
> implementation (arm_dma_get_sgtable). But with Xen foreign pages,
> dma_to_pfn doesn't work either, because we have no way of finding out
> the pfn address corresponding to the mfn of the foreign page. Both
> arm_dma_get_sgtable and dma_common_get_sgtable wouldn't work. I have no
> solution to this problem, but maybe we could add a check like the
> following (also to the previous patch?). I haven't tested it, but I
> think it should work as long as page_is_ram is returns the correct value
> for the handle parameter.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index dc83a35..cd0441c 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -18,6 +18,7 @@
>  #include <xen/page.h>
>  #include <xen/swiotlb-xen.h>
>  
> +#include <asm/dma-mapping.h>
>  #include <asm/cacheflush.h>
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/interface.h>
> @@ -180,9 +181,18 @@ static int xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
>  				 void *cpu_addr, dma_addr_t handle, size_t size,
>  				 unsigned long attrs)
>  {
> -	if (__generic_dma_ops(dev)->get_sgtable)
> +
> +	if (__generic_dma_ops(dev)->get_sgtable) {
> +		/* We can't handle foreign pages here. */
> +#ifdef CONFIG_ARM
> +		unsigned long bfn = dma_to_pfn(dev, handle);
> +#else
> +		unsigned long bfn = handle >> PAGE_SHIFT;
> +#endif
> +		BUG_ON (!page_is_ram(bfn));
>  		return __generic_dma_ops(dev)->get_sgtable(dev, sgt, cpu_addr, handle,
>  								 size, attrs);
> +	}
>  	return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size);
>  }
>  
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback

[Andrii Anisov]

Stefano,

About this piece:

>
> -       if (__generic_dma_ops(dev)->get_sgtable)
> +
> +       if (__generic_dma_ops(dev)->get_sgtable) {
> +               /* We can't handle foreign pages here. */
> +#ifdef CONFIG_ARM
> +               unsigned long bfn = dma_to_pfn(dev, handle);
> +#else
> +               unsigned long bfn = handle >> PAGE_SHIFT;
> +#endif
> +               BUG_ON (!page_is_ram(bfn));
>                 return __generic_dma_ops(dev)->get_sgtable(dev, sgt, cpu_addr, handle,
>                                                                  size, attrs);
> +       }
>         return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size);
>  }


Would it be in drivers/xen/swiotlb-xen.c as you suggested, the whole
"if (__generic_dma_ops(dev)->get_sgtable) {}" should be under ifdef.

IMO it would be better to avoid ifdefs in drivers/xen/swiotlb-xen.c,
but I haven't find out how to do that.

Sincerely,
Andrii Anisov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback

[Stefano Stabellini]

On Wed, 18 Jan 2017, Andrii Anisov wrote:
> Stefano,
> 
> About this piece:
> 
> >
> > -       if (__generic_dma_ops(dev)->get_sgtable)
> > +
> > +       if (__generic_dma_ops(dev)->get_sgtable) {
> > +               /* We can't handle foreign pages here. */
> > +#ifdef CONFIG_ARM
> > +               unsigned long bfn = dma_to_pfn(dev, handle);
> > +#else
> > +               unsigned long bfn = handle >> PAGE_SHIFT;
> > +#endif
> > +               BUG_ON (!page_is_ram(bfn));
> >                 return __generic_dma_ops(dev)->get_sgtable(dev, sgt, cpu_addr, handle,
> >                                                                  size, attrs);
> > +       }
> >         return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size);
> >  }
> 
> 
> Would it be in drivers/xen/swiotlb-xen.c as you suggested, the whole
> "if (__generic_dma_ops(dev)->get_sgtable) {}" should be under ifdef.
> 
> IMO it would be better to avoid ifdefs in drivers/xen/swiotlb-xen.c,
> but I haven't find out how to do that.

Yes, I dislike ifdef like everybody else, but sometimes they are
unavoidable.

But please test this code because I only compile tested it :-)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 0/2] XEN SWIOTLB for ARM dma operations extension

[Andrii Anisov]

From: Andrii Anisov <andrii_anisov@epam.com>

Some drivers need additional dma ops to be provided by xen swiotlb. Because
common operations implementation came from x86 world and does not suite ARM
needs we have to provide needed XEN SWIOTLB for ARM callbacks.

dma_mmap patch is a port of an antique and lost patch discussed here:
http://marc.info/?l=xen-devel&m=139695901430667&w=4

Changes in V2:
 - patches are rebased and checked for compilation for x86, arm, arm64 with
   the current LK master HEAD 83346fbc07d267de777e2597552f785174ad0373

Andrii Anisov (1):
  swiotlb-xen: implement xen_swiotlb_get_sgtable callback

Stefano Stabellini (1):
  swiotlb-xen: implement xen_swiotlb_dma_mmap callback

 arch/arm/xen/mm.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel