swiotlb-xen question

swiotlb-xen question

[Oleksandr Dmytryshyn]

Hi to all.

Currently I'm using hypervisor on ARM Cortex A15 processor (DRA7xx
Jacinto 6 processor). I'm using mainline xen 4.4 with some patches on
top plus dom0 and domU linux kernel 3.8.

I've written a hack to make a camera working on my board in dom0
(drivers for camera and display system are in dom0). An user-space
application in dom0 is used to test camera (it reads camera captured
data and sends it to the framebuffer). In the kernel code I've implemented
.mmap callback function in the swiotlb-xen.c file (like xen_swiotlb_mmap())
and copied to this new callback all content from the original function
'arm_dma_mmap()' from the kernel. This function creates userspace
mapping for the DMA-coherent memory. With this hack camera is working
(I can see captured video on display).

For now I have little understanding how swiotlb-xen is working. Currently in the
mainline linux kernel this callback is not implemented.

Here is my question.
Does anyone know are there any plans to implement
this callback for swiotlb-xen (in linux kernel) in the right way?

Oleksandr Dmytryshyn | Product Engineering and Development
GlobalLogic
P x3657  M +38.067.382.2525
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt

Re: swiotlb-xen question

[Ian Campbell]

On Wed, 2014-03-26 at 12:35 +0200, Oleksandr Dmytryshyn wrote:
> Hi to all.
> 
> Currently I'm using hypervisor on ARM Cortex A15 processor (DRA7xx
> Jacinto 6 processor). I'm using mainline xen 4.4 with some patches on
> top plus dom0 and domU linux kernel 3.8.
> 
> I've written a hack to make a camera working on my board in dom0
> (drivers for camera and display system are in dom0). An user-space
> application in dom0 is used to test camera (it reads camera captured
> data and sends it to the framebuffer). In the kernel code I've implemented
> .mmap callback function in the swiotlb-xen.c file (like xen_swiotlb_mmap())
> and copied to this new callback all content from the original function
> 'arm_dma_mmap()' from the kernel. This function creates userspace
> mapping for the DMA-coherent memory. With this hack camera is working
> (I can see captured video on display).
> 
> For now I have little understanding how swiotlb-xen is working. Currently in the
> mainline linux kernel this callback is not implemented.
> 
> Here is my question.
> Does anyone know are there any plans to implement
> this callback for swiotlb-xen (in linux kernel) in the right way?

I've not heard of anyone doing so, please feel free to take this on.

Ian.

Re: swiotlb-xen question

[Konrad Rzeszutek Wilk]

On Wed, Mar 26, 2014 at 12:35:12PM +0200, Oleksandr Dmytryshyn wrote:
> Hi to all.
> 
> Currently I'm using hypervisor on ARM Cortex A15 processor (DRA7xx
> Jacinto 6 processor). I'm using mainline xen 4.4 with some patches on
> top plus dom0 and domU linux kernel 3.8.
> 
> I've written a hack to make a camera working on my board in dom0
> (drivers for camera and display system are in dom0). An user-space
> application in dom0 is used to test camera (it reads camera captured
> data and sends it to the framebuffer). In the kernel code I've implemented
> .mmap callback function in the swiotlb-xen.c file (like xen_swiotlb_mmap())
> and copied to this new callback all content from the original function
> 'arm_dma_mmap()' from the kernel. This function creates userspace
> mapping for the DMA-coherent memory. With this hack camera is working
> (I can see captured video on display).

Why can't you use the v4l API? Won't that work?
> 
> For now I have little understanding how swiotlb-xen is working. Currently in the
> mainline linux kernel this callback is not implemented.
> 
> Here is my question.
> Does anyone know are there any plans to implement
> this callback for swiotlb-xen (in linux kernel) in the right way?
> 
> Oleksandr Dmytryshyn | Product Engineering and Development
> GlobalLogic
> P x3657  M +38.067.382.2525
> www.globallogic.com
> 
> http://www.globallogic.com/email_disclaimer.txt
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: swiotlb-xen question

[Oleksandr Dmytryshyn]

On Wed, Mar 26, 2014 at 4:46 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Wed, Mar 26, 2014 at 12:35:12PM +0200, Oleksandr Dmytryshyn wrote:
>> Hi to all.
>>
>> Currently I'm using hypervisor on ARM Cortex A15 processor (DRA7xx
>> Jacinto 6 processor). I'm using mainline xen 4.4 with some patches on
>> top plus dom0 and domU linux kernel 3.8.
>>
>> I've written a hack to make a camera working on my board in dom0
>> (drivers for camera and display system are in dom0). An user-space
>> application in dom0 is used to test camera (it reads camera captured
>> data and sends it to the framebuffer). In the kernel code I've implemented
>> .mmap callback function in the swiotlb-xen.c file (like xen_swiotlb_mmap())
>> and copied to this new callback all content from the original function
>> 'arm_dma_mmap()' from the kernel. This function creates userspace
>> mapping for the DMA-coherent memory. With this hack camera is working
>> (I can see captured video on display).
>
> Why can't you use the v4l API? Won't that work?
>
Currently we are using v4l api. swiotlb-xen doesnt have mmap callback
implemented,
so kernel use standard function dma_common_mmap() to map memory. But this
function doesn't work correctly. We've also tried to use arm_dma_mmap() function
instead of the dma_common_mmap() (HACK in the file
include/asm-generic/dma-mapping-common.h only for dom0) and all is working
in this case.
>>
>> For now I have little understanding how swiotlb-xen is working. Currently in the
>> mainline linux kernel this callback is not implemented.
>>
>> Here is my question.
>> Does anyone know are there any plans to implement
>> this callback for swiotlb-xen (in linux kernel) in the right way?
>>
>> Oleksandr Dmytryshyn | Product Engineering and Development
>> GlobalLogic
>> P x3657  M +38.067.382.2525
>> www.globallogic.com
>>
>> http://www.globallogic.com/email_disclaimer.txt
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

Re: swiotlb-xen question

[Stefano Stabellini]

On Wed, 26 Mar 2014, Oleksandr Dmytryshyn wrote:
> On Wed, Mar 26, 2014 at 4:46 PM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> > On Wed, Mar 26, 2014 at 12:35:12PM +0200, Oleksandr Dmytryshyn wrote:
> >> Hi to all.
> >>
> >> Currently I'm using hypervisor on ARM Cortex A15 processor (DRA7xx
> >> Jacinto 6 processor). I'm using mainline xen 4.4 with some patches on
> >> top plus dom0 and domU linux kernel 3.8.
> >>
> >> I've written a hack to make a camera working on my board in dom0
> >> (drivers for camera and display system are in dom0). An user-space
> >> application in dom0 is used to test camera (it reads camera captured
> >> data and sends it to the framebuffer). In the kernel code I've implemented
> >> .mmap callback function in the swiotlb-xen.c file (like xen_swiotlb_mmap())
> >> and copied to this new callback all content from the original function
> >> 'arm_dma_mmap()' from the kernel. This function creates userspace
> >> mapping for the DMA-coherent memory. With this hack camera is working
> >> (I can see captured video on display).
> >
> > Why can't you use the v4l API? Won't that work?
> >
> Currently we are using v4l api. swiotlb-xen doesnt have mmap callback
> implemented,
> so kernel use standard function dma_common_mmap() to map memory. But this
> function doesn't work correctly. We've also tried to use arm_dma_mmap() function
> instead of the dma_common_mmap() (HACK in the file
> include/asm-generic/dma-mapping-common.h only for dom0) and all is working
> in this case.

This is not a Xen specific issue.
In fact unless some of the dma pages involved could be foreign guest
pages (for example because you have a PV framebuffer frontend in a guest
sharing pages with a framebuffer backend in Dom0, and these pages are
used directly in your camera driver), swiotlb-xen shouldn't even be
involved.

The problem is that on ARM, when you call virt_to_phys on a virtual
address returned by dma_alloc_from_coherent, it doesn't return the right
physical address.
For example:

virt_address = dma_alloc_from_coherent(dev, size, dma_addr, &ret);
virt_to_phys(virt_address) != dma_addr

The issue is not that bus addresses are different from physical
addresses. The problem is that the virtual address is an ioremap address
therefore virt_to_phys and virt_to_page don't work as expected.

To fix your problem you can simply:

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 0ce39a3..052a5ed 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -248,7 +248,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
 #ifdef CONFIG_MMU
 	unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
 	unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
-	unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
+	unsigned long pfn = dma_addr >> PAGE_SHIFT;
 	unsigned long off = vma->vm_pgoff;
 
 	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);

however that would cause problems on architectures for which bus
addresses are different from physical addresses.
Alternatively you could:

+	unsigned long pfn = bus_to_phys(dma_addr) >> PAGE_SHIFT;

however bus_to_phys is not defined on all architectures.
I am not sure what is the best way to fix this in common code such us
drivers/base/dma-mapping.c.

Re: swiotlb-xen question

[Konrad Rzeszutek Wilk]

On Wed, Mar 26, 2014 at 06:49:57PM +0000, Stefano Stabellini wrote:
> On Wed, 26 Mar 2014, Oleksandr Dmytryshyn wrote:
> > On Wed, Mar 26, 2014 at 4:46 PM, Konrad Rzeszutek Wilk
> > <konrad.wilk@oracle.com> wrote:
> > > On Wed, Mar 26, 2014 at 12:35:12PM +0200, Oleksandr Dmytryshyn wrote:
> > >> Hi to all.
> > >>
> > >> Currently I'm using hypervisor on ARM Cortex A15 processor (DRA7xx
> > >> Jacinto 6 processor). I'm using mainline xen 4.4 with some patches on
> > >> top plus dom0 and domU linux kernel 3.8.
> > >>
> > >> I've written a hack to make a camera working on my board in dom0
> > >> (drivers for camera and display system are in dom0). An user-space
> > >> application in dom0 is used to test camera (it reads camera captured
> > >> data and sends it to the framebuffer). In the kernel code I've implemented
> > >> .mmap callback function in the swiotlb-xen.c file (like xen_swiotlb_mmap())
> > >> and copied to this new callback all content from the original function
> > >> 'arm_dma_mmap()' from the kernel. This function creates userspace
> > >> mapping for the DMA-coherent memory. With this hack camera is working
> > >> (I can see captured video on display).
> > >
> > > Why can't you use the v4l API? Won't that work?
> > >
> > Currently we are using v4l api. swiotlb-xen doesnt have mmap callback
> > implemented,
> > so kernel use standard function dma_common_mmap() to map memory. But this
> > function doesn't work correctly. We've also tried to use arm_dma_mmap() function
> > instead of the dma_common_mmap() (HACK in the file
> > include/asm-generic/dma-mapping-common.h only for dom0) and all is working
> > in this case.
> 
> This is not a Xen specific issue.
> In fact unless some of the dma pages involved could be foreign guest
> pages (for example because you have a PV framebuffer frontend in a guest
> sharing pages with a framebuffer backend in Dom0, and these pages are
> used directly in your camera driver), swiotlb-xen shouldn't even be
> involved.
> 
> The problem is that on ARM, when you call virt_to_phys on a virtual
> address returned by dma_alloc_from_coherent, it doesn't return the right
> physical address.
> For example:
> 
> virt_address = dma_alloc_from_coherent(dev, size, dma_addr, &ret);
> virt_to_phys(virt_address) != dma_addr
> 
> The issue is not that bus addresses are different from physical
> addresses. The problem is that the virtual address is an ioremap address
> therefore virt_to_phys and virt_to_page don't work as expected.
> 
> To fix your problem you can simply:
> 
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index 0ce39a3..052a5ed 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -248,7 +248,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
>  #ifdef CONFIG_MMU
>  	unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>  	unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> -	unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
> +	unsigned long pfn = dma_addr >> PAGE_SHIFT;
>  	unsigned long off = vma->vm_pgoff;
>  
>  	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> 
> however that would cause problems on architectures for which bus
> addresses are different from physical addresses.
> Alternatively you could:
> 
> +	unsigned long pfn = bus_to_phys(dma_addr) >> PAGE_SHIFT;
> 
> however bus_to_phys is not defined on all architectures.
> I am not sure what is the best way to fix this in common code such us
> drivers/base/dma-mapping.c.

Ugh. Usually drivers have an 'dma_addr_t[]' array to go along with the
virtual addresses (or the array of the 'struct page'.). That way they
have both the CPU and the bus addresses for the same region.

Could that be used here?

Re: swiotlb-xen question

[Oleksandr Dmytryshyn]

Hi, Stefano.

Thank You for advice.

I'll try this few days later.

Oleksandr Dmytryshyn | Product Engineering and Development
GlobalLogic
M +38.067.382.2525
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt


On Wed, Mar 26, 2014 at 8:49 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Wed, 26 Mar 2014, Oleksandr Dmytryshyn wrote:
>> On Wed, Mar 26, 2014 at 4:46 PM, Konrad Rzeszutek Wilk
>> <konrad.wilk@oracle.com> wrote:
>> > On Wed, Mar 26, 2014 at 12:35:12PM +0200, Oleksandr Dmytryshyn wrote:
>> >> Hi to all.
>> >>
>> >> Currently I'm using hypervisor on ARM Cortex A15 processor (DRA7xx
>> >> Jacinto 6 processor). I'm using mainline xen 4.4 with some patches on
>> >> top plus dom0 and domU linux kernel 3.8.
>> >>
>> >> I've written a hack to make a camera working on my board in dom0
>> >> (drivers for camera and display system are in dom0). An user-space
>> >> application in dom0 is used to test camera (it reads camera captured
>> >> data and sends it to the framebuffer). In the kernel code I've implemented
>> >> .mmap callback function in the swiotlb-xen.c file (like xen_swiotlb_mmap())
>> >> and copied to this new callback all content from the original function
>> >> 'arm_dma_mmap()' from the kernel. This function creates userspace
>> >> mapping for the DMA-coherent memory. With this hack camera is working
>> >> (I can see captured video on display).
>> >
>> > Why can't you use the v4l API? Won't that work?
>> >
>> Currently we are using v4l api. swiotlb-xen doesnt have mmap callback
>> implemented,
>> so kernel use standard function dma_common_mmap() to map memory. But this
>> function doesn't work correctly. We've also tried to use arm_dma_mmap() function
>> instead of the dma_common_mmap() (HACK in the file
>> include/asm-generic/dma-mapping-common.h only for dom0) and all is working
>> in this case.
>
> This is not a Xen specific issue.
> In fact unless some of the dma pages involved could be foreign guest
> pages (for example because you have a PV framebuffer frontend in a guest
> sharing pages with a framebuffer backend in Dom0, and these pages are
> used directly in your camera driver), swiotlb-xen shouldn't even be
> involved.
>
> The problem is that on ARM, when you call virt_to_phys on a virtual
> address returned by dma_alloc_from_coherent, it doesn't return the right
> physical address.
> For example:
>
> virt_address = dma_alloc_from_coherent(dev, size, dma_addr, &ret);
> virt_to_phys(virt_address) != dma_addr
>
> The issue is not that bus addresses are different from physical
> addresses. The problem is that the virtual address is an ioremap address
> therefore virt_to_phys and virt_to_page don't work as expected.
>
> To fix your problem you can simply:
>
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index 0ce39a3..052a5ed 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -248,7 +248,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
>  #ifdef CONFIG_MMU
>         unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>         unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> -       unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
> +       unsigned long pfn = dma_addr >> PAGE_SHIFT;
>         unsigned long off = vma->vm_pgoff;
>
>         vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>
> however that would cause problems on architectures for which bus
> addresses are different from physical addresses.
> Alternatively you could:
>
> +       unsigned long pfn = bus_to_phys(dma_addr) >> PAGE_SHIFT;
>
> however bus_to_phys is not defined on all architectures.
> I am not sure what is the best way to fix this in common code such us
> drivers/base/dma-mapping.c.

Re: swiotlb-xen question

[Oleksandr Dmytryshyn]

Hi to all. I've investigated my issue deeper.

Currently we use mmap in 2 cases: video-in (camera) and video-out (display).

In case video-out all is working because mmap callback in the video-out device
leads to the remap_pfn_range() function (this function doesn't use unimplemented
.mmap callback in the swiotlb-xen.c file).

In case video-out mmap callback in the video-in device leads to the .mmap
callback from the dma ops (unimplemented callback in the swiotlb-xen.c file)

I've investigated kernel code. We have patch from the Stefano Stabellini
(068a6e62 - arm/xen: get_dma_ops: return xen_dma_ops if we are running as
xen_initial_domain)

Before this patch the function *get_dma_ops() always returned
__generic_dma_ops()
which returned &arm_dma_ops (if archdata.dma_ops is absent). In this case
for arm architecture .mmap callback is arm_dma_mmap() function.

After this patch for domU all is the same as before this patch. But in case
dom0 kernel uses xen_dma_ops with unimplemented .mmap callback. If some
function tries to use .mmap callback in dom0 then kernel calls generic
function dma_common_mmap() which doesn't work properly.

May be some mechanism for swiotlb-xen should be implemented for unimplemented
callbacks: if callback is not implemented then right architectural callback
should be used (.mmap - arm_dma_mmap() for ARM)?

Oleksandr Dmytryshyn | Product Engineering and Development
GlobalLogic
M +38.067.382.2525
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt


On Thu, Mar 27, 2014 at 12:11 PM, Oleksandr Dmytryshyn
<oleksandr.dmytryshyn@globallogic.com> wrote:
> Hi, Stefano.
>
> Thank You for advice.
>
> I'll try this few days later.
>
> Oleksandr Dmytryshyn | Product Engineering and Development
> GlobalLogic
> M +38.067.382.2525
> www.globallogic.com
>
> http://www.globallogic.com/email_disclaimer.txt
>
>
> On Wed, Mar 26, 2014 at 8:49 PM, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
>> On Wed, 26 Mar 2014, Oleksandr Dmytryshyn wrote:
>>> On Wed, Mar 26, 2014 at 4:46 PM, Konrad Rzeszutek Wilk
>>> <konrad.wilk@oracle.com> wrote:
>>> > On Wed, Mar 26, 2014 at 12:35:12PM +0200, Oleksandr Dmytryshyn wrote:
>>> >> Hi to all.
>>> >>
>>> >> Currently I'm using hypervisor on ARM Cortex A15 processor (DRA7xx
>>> >> Jacinto 6 processor). I'm using mainline xen 4.4 with some patches on
>>> >> top plus dom0 and domU linux kernel 3.8.
>>> >>
>>> >> I've written a hack to make a camera working on my board in dom0
>>> >> (drivers for camera and display system are in dom0). An user-space
>>> >> application in dom0 is used to test camera (it reads camera captured
>>> >> data and sends it to the framebuffer). In the kernel code I've implemented
>>> >> .mmap callback function in the swiotlb-xen.c file (like xen_swiotlb_mmap())
>>> >> and copied to this new callback all content from the original function
>>> >> 'arm_dma_mmap()' from the kernel. This function creates userspace
>>> >> mapping for the DMA-coherent memory. With this hack camera is working
>>> >> (I can see captured video on display).
>>> >
>>> > Why can't you use the v4l API? Won't that work?
>>> >
>>> Currently we are using v4l api. swiotlb-xen doesnt have mmap callback
>>> implemented,
>>> so kernel use standard function dma_common_mmap() to map memory. But this
>>> function doesn't work correctly. We've also tried to use arm_dma_mmap() function
>>> instead of the dma_common_mmap() (HACK in the file
>>> include/asm-generic/dma-mapping-common.h only for dom0) and all is working
>>> in this case.
>>
>> This is not a Xen specific issue.
>> In fact unless some of the dma pages involved could be foreign guest
>> pages (for example because you have a PV framebuffer frontend in a guest
>> sharing pages with a framebuffer backend in Dom0, and these pages are
>> used directly in your camera driver), swiotlb-xen shouldn't even be
>> involved.
>>
>> The problem is that on ARM, when you call virt_to_phys on a virtual
>> address returned by dma_alloc_from_coherent, it doesn't return the right
>> physical address.
>> For example:
>>
>> virt_address = dma_alloc_from_coherent(dev, size, dma_addr, &ret);
>> virt_to_phys(virt_address) != dma_addr
>>
>> The issue is not that bus addresses are different from physical
>> addresses. The problem is that the virtual address is an ioremap address
>> therefore virt_to_phys and virt_to_page don't work as expected.
>>
>> To fix your problem you can simply:
>>
>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
>> index 0ce39a3..052a5ed 100644
>> --- a/drivers/base/dma-mapping.c
>> +++ b/drivers/base/dma-mapping.c
>> @@ -248,7 +248,7 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma,
>>  #ifdef CONFIG_MMU
>>         unsigned long user_count = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>>         unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> -       unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
>> +       unsigned long pfn = dma_addr >> PAGE_SHIFT;
>>         unsigned long off = vma->vm_pgoff;
>>
>>         vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>>
>> however that would cause problems on architectures for which bus
>> addresses are different from physical addresses.
>> Alternatively you could:
>>
>> +       unsigned long pfn = bus_to_phys(dma_addr) >> PAGE_SHIFT;
>>
>> however bus_to_phys is not defined on all architectures.
>> I am not sure what is the best way to fix this in common code such us
>> drivers/base/dma-mapping.c.

Re: swiotlb-xen question

[Stefano Stabellini]

On Tue, 1 Apr 2014, Oleksandr Dmytryshyn wrote:
> Hi to all. I've investigated my issue deeper.
> 
> Currently we use mmap in 2 cases: video-in (camera) and video-out (display).
> 
> In case video-out all is working because mmap callback in the video-out device
> leads to the remap_pfn_range() function (this function doesn't use unimplemented
> .mmap callback in the swiotlb-xen.c file).
> 
> In case video-out mmap callback in the video-in device leads to the .mmap
> callback from the dma ops (unimplemented callback in the swiotlb-xen.c file)
> 
> I've investigated kernel code. We have patch from the Stefano Stabellini
> (068a6e62 - arm/xen: get_dma_ops: return xen_dma_ops if we are running as
> xen_initial_domain)
> 
> Before this patch the function *get_dma_ops() always returned
> __generic_dma_ops()
> which returned &arm_dma_ops (if archdata.dma_ops is absent). In this case
> for arm architecture .mmap callback is arm_dma_mmap() function.
> 
> After this patch for domU all is the same as before this patch. But in case
> dom0 kernel uses xen_dma_ops with unimplemented .mmap callback. If some
> function tries to use .mmap callback in dom0 then kernel calls generic
> function dma_common_mmap() which doesn't work properly.
> 
> May be some mechanism for swiotlb-xen should be implemented for unimplemented
> callbacks: if callback is not implemented then right architectural callback
> should be used (.mmap - arm_dma_mmap() for ARM)?

I understand now. Thanks for investingating!
I think that we'll have to introduce our own version of .mmap,
because dma_to_pfn doesn't work correctly on Xen on ARM.
The implementation would be something like (untested):

int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
		 void *cpu_addr, dma_addr_t dma_addr, size_t size,
		 struct dma_attrs *attrs)
{
	int ret = -ENXIO;
#ifdef CONFIG_MMU
	unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
	unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
	unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dev, dma_addr));
	unsigned long off = vma->vm_pgoff;

	vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);

	if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
		return ret;

	if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
		ret = remap_pfn_range(vma, vma->vm_start,
				      pfn + off,
				      vma->vm_end - vma->vm_start,
				      vma->vm_page_prot);
	}
#endif	/* CONFIG_MMU */

	return ret;
}

then we can add the function to xen_swiotlb_dma_ops.
Would you be up for coming up with a proper patch?

Re: swiotlb-xen question

[Oleksandr Dmytryshyn]

Hi, Stefano.

Thank You for advice!
I see that You just copied the content from the original function arm_dma_mmap()
to the swiotlb-xen driver.
I've tested the same solution (except that I've copied __get_dma_pgprot()
function to the swiotlb-xen.c file too because this function is static in the
original file). This solution is working (I've tested it). But there is
an easier solution:

diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index b0e77de..f3e820f 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -48,6 +48,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
  .sync_sg_for_device = xen_swiotlb_sync_sg_for_device,
  .map_sg = xen_swiotlb_map_sg_attrs,
  .unmap_sg = xen_swiotlb_unmap_sg_attrs,
+ .mmap = arm_dma_mmap,
  .map_page = xen_swiotlb_map_page,
  .unmap_page = xen_swiotlb_unmap_page,
  .dma_supported = xen_swiotlb_dma_supported,

The original arm_dma_mmap() function is implemented in the file
arch/arm/mm/dma-mapping.c which always is compiled for arm arhitecture.
I've checked this solution too. It is working.

I have a question. Is my solution correct?

Oleksandr Dmytryshyn | Product Engineering and Development
GlobalLogic
M +38.067.382.2525
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt


On Thu, Apr 3, 2014 at 1:51 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Tue, 1 Apr 2014, Oleksandr Dmytryshyn wrote:
>> Hi to all. I've investigated my issue deeper.
>>
>> Currently we use mmap in 2 cases: video-in (camera) and video-out (display).
>>
>> In case video-out all is working because mmap callback in the video-out device
>> leads to the remap_pfn_range() function (this function doesn't use unimplemented
>> .mmap callback in the swiotlb-xen.c file).
>>
>> In case video-out mmap callback in the video-in device leads to the .mmap
>> callback from the dma ops (unimplemented callback in the swiotlb-xen.c file)
>>
>> I've investigated kernel code. We have patch from the Stefano Stabellini
>> (068a6e62 - arm/xen: get_dma_ops: return xen_dma_ops if we are running as
>> xen_initial_domain)
>>
>> Before this patch the function *get_dma_ops() always returned
>> __generic_dma_ops()
>> which returned &arm_dma_ops (if archdata.dma_ops is absent). In this case
>> for arm architecture .mmap callback is arm_dma_mmap() function.
>>
>> After this patch for domU all is the same as before this patch. But in case
>> dom0 kernel uses xen_dma_ops with unimplemented .mmap callback. If some
>> function tries to use .mmap callback in dom0 then kernel calls generic
>> function dma_common_mmap() which doesn't work properly.
>>
>> May be some mechanism for swiotlb-xen should be implemented for unimplemented
>> callbacks: if callback is not implemented then right architectural callback
>> should be used (.mmap - arm_dma_mmap() for ARM)?
>
> I understand now. Thanks for investingating!
> I think that we'll have to introduce our own version of .mmap,
> because dma_to_pfn doesn't work correctly on Xen on ARM.
> The implementation would be something like (untested):
>
> int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
>                  void *cpu_addr, dma_addr_t dma_addr, size_t size,
>                  struct dma_attrs *attrs)
> {
>         int ret = -ENXIO;
> #ifdef CONFIG_MMU
>         unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>         unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>         unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dev, dma_addr));
>         unsigned long off = vma->vm_pgoff;
>
>         vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
>
>         if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
>                 return ret;
>
>         if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
>                 ret = remap_pfn_range(vma, vma->vm_start,
>                                       pfn + off,
>                                       vma->vm_end - vma->vm_start,
>                                       vma->vm_page_prot);
>         }
> #endif  /* CONFIG_MMU */
>
>         return ret;
> }
>
> then we can add the function to xen_swiotlb_dma_ops.
> Would you be up for coming up with a proper patch?

Re: swiotlb-xen question

[Stefano Stabellini]

On Thu, 3 Apr 2014, Oleksandr Dmytryshyn wrote:
> Hi, Stefano.
> 
> Thank You for advice!
> I see that You just copied the content from the original function arm_dma_mmap()
> to the swiotlb-xen driver.

I did not :-)
I changed the function used to translate from dma addresses to pfn from
dma_to_pfn to xen_bus_to_phys.


> I've tested the same solution (except that I've copied __get_dma_pgprot()
> function to the swiotlb-xen.c file too because this function is static in the
> original file). This solution is working (I've tested it). But there is
> an easier solution:
> 
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index b0e77de..f3e820f 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -48,6 +48,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
>   .sync_sg_for_device = xen_swiotlb_sync_sg_for_device,
>   .map_sg = xen_swiotlb_map_sg_attrs,
>   .unmap_sg = xen_swiotlb_unmap_sg_attrs,
> + .mmap = arm_dma_mmap,
>   .map_page = xen_swiotlb_map_page,
>   .unmap_page = xen_swiotlb_unmap_page,
>   .dma_supported = xen_swiotlb_dma_supported,
> 
> The original arm_dma_mmap() function is implemented in the file
> arch/arm/mm/dma-mapping.c which always is compiled for arm arhitecture.
> I've checked this solution too. It is working.
> 
> I have a question. Is my solution correct?

No, because if any foreign pages are involved the physical address
corresponding to the dma address would not be the one returned by
dma_to_pfn. We need to call xen_bus_to_phys.


> Oleksandr Dmytryshyn | Product Engineering and Development
> GlobalLogic
> M +38.067.382.2525
> www.globallogic.com
> 
> http://www.globallogic.com/email_disclaimer.txt
> 
> 
> On Thu, Apr 3, 2014 at 1:51 PM, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
> > On Tue, 1 Apr 2014, Oleksandr Dmytryshyn wrote:
> >> Hi to all. I've investigated my issue deeper.
> >>
> >> Currently we use mmap in 2 cases: video-in (camera) and video-out (display).
> >>
> >> In case video-out all is working because mmap callback in the video-out device
> >> leads to the remap_pfn_range() function (this function doesn't use unimplemented
> >> .mmap callback in the swiotlb-xen.c file).
> >>
> >> In case video-out mmap callback in the video-in device leads to the .mmap
> >> callback from the dma ops (unimplemented callback in the swiotlb-xen.c file)
> >>
> >> I've investigated kernel code. We have patch from the Stefano Stabellini
> >> (068a6e62 - arm/xen: get_dma_ops: return xen_dma_ops if we are running as
> >> xen_initial_domain)
> >>
> >> Before this patch the function *get_dma_ops() always returned
> >> __generic_dma_ops()
> >> which returned &arm_dma_ops (if archdata.dma_ops is absent). In this case
> >> for arm architecture .mmap callback is arm_dma_mmap() function.
> >>
> >> After this patch for domU all is the same as before this patch. But in case
> >> dom0 kernel uses xen_dma_ops with unimplemented .mmap callback. If some
> >> function tries to use .mmap callback in dom0 then kernel calls generic
> >> function dma_common_mmap() which doesn't work properly.
> >>
> >> May be some mechanism for swiotlb-xen should be implemented for unimplemented
> >> callbacks: if callback is not implemented then right architectural callback
> >> should be used (.mmap - arm_dma_mmap() for ARM)?
> >
> > I understand now. Thanks for investingating!
> > I think that we'll have to introduce our own version of .mmap,
> > because dma_to_pfn doesn't work correctly on Xen on ARM.
> > The implementation would be something like (untested):
> >
> > int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> >                  void *cpu_addr, dma_addr_t dma_addr, size_t size,
> >                  struct dma_attrs *attrs)
> > {
> >         int ret = -ENXIO;
> > #ifdef CONFIG_MMU
> >         unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> >         unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> >         unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dev, dma_addr));
> >         unsigned long off = vma->vm_pgoff;
> >
> >         vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
> >
> >         if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
> >                 return ret;
> >
> >         if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
> >                 ret = remap_pfn_range(vma, vma->vm_start,
> >                                       pfn + off,
> >                                       vma->vm_end - vma->vm_start,
> >                                       vma->vm_page_prot);
> >         }
> > #endif  /* CONFIG_MMU */
> >
> >         return ret;
> > }
> >
> > then we can add the function to xen_swiotlb_dma_ops.
> > Would you be up for coming up with a proper patch?
> 

Re: swiotlb-xen question

[Oleksandr Dmytryshyn]

On Thu, Apr 3, 2014 at 3:52 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Thu, 3 Apr 2014, Oleksandr Dmytryshyn wrote:
>> Hi, Stefano.
>>
>> Thank You for advice!
>> I see that You just copied the content from the original function arm_dma_mmap()
>> to the swiotlb-xen driver.
>
> I did not :-)
> I changed the function used to translate from dma addresses to pfn from
> dma_to_pfn to xen_bus_to_phys.
>
>
>> I've tested the same solution (except that I've copied __get_dma_pgprot()
>> function to the swiotlb-xen.c file too because this function is static in the
>> original file). This solution is working (I've tested it). But there is
>> an easier solution:
>>
>> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
>> index b0e77de..f3e820f 100644
>> --- a/arch/arm/xen/mm.c
>> +++ b/arch/arm/xen/mm.c
>> @@ -48,6 +48,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
>>   .sync_sg_for_device = xen_swiotlb_sync_sg_for_device,
>>   .map_sg = xen_swiotlb_map_sg_attrs,
>>   .unmap_sg = xen_swiotlb_unmap_sg_attrs,
>> + .mmap = arm_dma_mmap,
>>   .map_page = xen_swiotlb_map_page,
>>   .unmap_page = xen_swiotlb_unmap_page,
>>   .dma_supported = xen_swiotlb_dma_supported,
>>
>> The original arm_dma_mmap() function is implemented in the file
>> arch/arm/mm/dma-mapping.c which always is compiled for arm arhitecture.
>> I've checked this solution too. It is working.
>>
>> I have a question. Is my solution correct?
>
> No, because if any foreign pages are involved the physical address
> corresponding to the dma address would not be the one returned by
> dma_to_pfn. We need to call xen_bus_to_phys.
>
>
>> Oleksandr Dmytryshyn | Product Engineering and Development
>> GlobalLogic
>> M +38.067.382.2525
>> www.globallogic.com
>>
>> http://www.globallogic.com/email_disclaimer.txt
>>
>>
>> On Thu, Apr 3, 2014 at 1:51 PM, Stefano Stabellini
>> <stefano.stabellini@eu.citrix.com> wrote:
>> > On Tue, 1 Apr 2014, Oleksandr Dmytryshyn wrote:
>> >> Hi to all. I've investigated my issue deeper.
>> >>
>> >> Currently we use mmap in 2 cases: video-in (camera) and video-out (display).
>> >>
>> >> In case video-out all is working because mmap callback in the video-out device
>> >> leads to the remap_pfn_range() function (this function doesn't use unimplemented
>> >> .mmap callback in the swiotlb-xen.c file).
>> >>
>> >> In case video-out mmap callback in the video-in device leads to the .mmap
>> >> callback from the dma ops (unimplemented callback in the swiotlb-xen.c file)
>> >>
>> >> I've investigated kernel code. We have patch from the Stefano Stabellini
>> >> (068a6e62 - arm/xen: get_dma_ops: return xen_dma_ops if we are running as
>> >> xen_initial_domain)
>> >>
>> >> Before this patch the function *get_dma_ops() always returned
>> >> __generic_dma_ops()
>> >> which returned &arm_dma_ops (if archdata.dma_ops is absent). In this case
>> >> for arm architecture .mmap callback is arm_dma_mmap() function.
>> >>
>> >> After this patch for domU all is the same as before this patch. But in case
>> >> dom0 kernel uses xen_dma_ops with unimplemented .mmap callback. If some
>> >> function tries to use .mmap callback in dom0 then kernel calls generic
>> >> function dma_common_mmap() which doesn't work properly.
>> >>
>> >> May be some mechanism for swiotlb-xen should be implemented for unimplemented
>> >> callbacks: if callback is not implemented then right architectural callback
>> >> should be used (.mmap - arm_dma_mmap() for ARM)?
>> >
>> > I understand now. Thanks for investingating!
>> > I think that we'll have to introduce our own version of .mmap,
>> > because dma_to_pfn doesn't work correctly on Xen on ARM.
>> > The implementation would be something like (untested):
>> >
>> > int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
>> >                  void *cpu_addr, dma_addr_t dma_addr, size_t size,
>> >                  struct dma_attrs *attrs)
>> > {
>> >         int ret = -ENXIO;
>> > #ifdef CONFIG_MMU
>> >         unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>> >         unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> >         unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dev, dma_addr));
>> >         unsigned long off = vma->vm_pgoff;
>> >
>> >         vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
>> >
>> >         if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
>> >                 return ret;
>> >
>> >         if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
>> >                 ret = remap_pfn_range(vma, vma->vm_start,
>> >                                       pfn + off,
>> >                                       vma->vm_end - vma->vm_start,
>> >                                       vma->vm_page_prot);
>> >         }
>> > #endif  /* CONFIG_MMU */
>> >
>> >         return ret;
>> > }
>> >
>> > then we can add the function to xen_swiotlb_dma_ops.
>> > Would you be up for coming up with a proper patch?
>>

Hi, Stefano.

Thank You for the response.
I'll test Your solution and write about results.

Oleksandr Dmytryshyn | Product Engineering and Development
GlobalLogic
M +38.067.382.2525
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt

Re: swiotlb-xen question

[Oleksandr Dmytryshyn]

On Thu, Apr 3, 2014 at 1:51 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Tue, 1 Apr 2014, Oleksandr Dmytryshyn wrote:
>> Hi to all. I've investigated my issue deeper.
>>
>> Currently we use mmap in 2 cases: video-in (camera) and video-out (display).
>>
>> In case video-out all is working because mmap callback in the video-out device
>> leads to the remap_pfn_range() function (this function doesn't use unimplemented
>> .mmap callback in the swiotlb-xen.c file).
>>
>> In case video-out mmap callback in the video-in device leads to the .mmap
>> callback from the dma ops (unimplemented callback in the swiotlb-xen.c file)
>>
>> I've investigated kernel code. We have patch from the Stefano Stabellini
>> (068a6e62 - arm/xen: get_dma_ops: return xen_dma_ops if we are running as
>> xen_initial_domain)
>>
>> Before this patch the function *get_dma_ops() always returned
>> __generic_dma_ops()
>> which returned &arm_dma_ops (if archdata.dma_ops is absent). In this case
>> for arm architecture .mmap callback is arm_dma_mmap() function.
>>
>> After this patch for domU all is the same as before this patch. But in case
>> dom0 kernel uses xen_dma_ops with unimplemented .mmap callback. If some
>> function tries to use .mmap callback in dom0 then kernel calls generic
>> function dma_common_mmap() which doesn't work properly.
>>
>> May be some mechanism for swiotlb-xen should be implemented for unimplemented
>> callbacks: if callback is not implemented then right architectural callback
>> should be used (.mmap - arm_dma_mmap() for ARM)?
>
> I understand now. Thanks for investingating!
> I think that we'll have to introduce our own version of .mmap,
> because dma_to_pfn doesn't work correctly on Xen on ARM.
> The implementation would be something like (untested):
>
> int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
>                  void *cpu_addr, dma_addr_t dma_addr, size_t size,
>                  struct dma_attrs *attrs)
> {
>         int ret = -ENXIO;
> #ifdef CONFIG_MMU
>         unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>         unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>         unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dev, dma_addr));
>         unsigned long off = vma->vm_pgoff;
>
>         vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
>
>         if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
>                 return ret;
>
>         if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
>                 ret = remap_pfn_range(vma, vma->vm_start,
>                                       pfn + off,
>                                       vma->vm_end - vma->vm_start,
>                                       vma->vm_page_prot);
>         }
> #endif  /* CONFIG_MMU */
>
>         return ret;
> }
>
> then we can add the function to xen_swiotlb_dma_ops.
> Would you be up for coming up with a proper patch?

Hi, Stefano.

Here is a slightly modified solution from You (I've only replaced a
__get_dma_pgprot() function with its original implementation):

diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index b0e77de..91408b1 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -48,6 +48,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
  .sync_sg_for_device = xen_swiotlb_sync_sg_for_device,
  .map_sg = xen_swiotlb_map_sg_attrs,
  .unmap_sg = xen_swiotlb_unmap_sg_attrs,
+ .mmap = xen_swiotlb_dma_mmap,
  .map_page = xen_swiotlb_map_page,
  .unmap_page = xen_swiotlb_unmap_page,
  .dma_supported = xen_swiotlb_dma_supported,
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 5403855..acf0c06 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -407,6 +407,42 @@ dma_addr_t xen_swiotlb_map_page(struct device
*dev, struct page *page,
 EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);

 /*
+ * Create userspace mapping for the DMA-coherent memory.
+ */
+int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+ void *cpu_addr, dma_addr_t dma_addr, size_t size,
+ struct dma_attrs *attrs)
+{
+ int ret = -ENXIO;
+#ifdef CONFIG_MMU
+ unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+ unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+ unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dma_addr));
+ unsigned long off = vma->vm_pgoff;
+ pgprot_t prot = vma->vm_page_prot;
+
+ prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ?
+    pgprot_writecombine(prot) :
+    pgprot_dmacoherent(prot);
+
+ vma->vm_page_prot = prot;
+
+ if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
+ return ret;
+
+ if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
+ ret = remap_pfn_range(vma, vma->vm_start,
+ pfn + off,
+ vma->vm_end - vma->vm_start,
+ vma->vm_page_prot);
+ }
+#endif  /* CONFIG_MMU */
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(xen_swiotlb_dma_mmap);
+
+/*
  * Unmap a single streaming mode DMA translation.  The dma_addr and size must
  * match what was provided for in a previous xen_swiotlb_map_page call.  All
  * other usages are undefined.
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index 7b64465..930fa94 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -15,6 +15,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size,
   void *vaddr, dma_addr_t dma_handle,
   struct dma_attrs *attrs);

+extern int
+xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+     void *cpu_addr, dma_addr_t dma_addr, size_t size,
+     struct dma_attrs *attrs);
+
 extern dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
        unsigned long offset, size_t size,
        enum dma_data_direction dir,


I've checked it on Xen 4.4 stable + some our architecture specific
patches on top
plus Dom0 and DomU Linux Kernel 3.8 on DRA7xxx based board. Camera application
is working (I can see a captured data from the camera on my board). Can we use
this implementation of the .mmap callback as the correct solution?

Oleksandr Dmytryshyn | Product Engineering and Development
GlobalLogic
M +38.067.382.2525
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt

Re: swiotlb-xen question

[Stefano Stabellini]

On Fri, 4 Apr 2014, Oleksandr Dmytryshyn wrote:
> On Thu, Apr 3, 2014 at 1:51 PM, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
> > On Tue, 1 Apr 2014, Oleksandr Dmytryshyn wrote:
> >> Hi to all. I've investigated my issue deeper.
> >>
> >> Currently we use mmap in 2 cases: video-in (camera) and video-out (display).
> >>
> >> In case video-out all is working because mmap callback in the video-out device
> >> leads to the remap_pfn_range() function (this function doesn't use unimplemented
> >> .mmap callback in the swiotlb-xen.c file).
> >>
> >> In case video-out mmap callback in the video-in device leads to the .mmap
> >> callback from the dma ops (unimplemented callback in the swiotlb-xen.c file)
> >>
> >> I've investigated kernel code. We have patch from the Stefano Stabellini
> >> (068a6e62 - arm/xen: get_dma_ops: return xen_dma_ops if we are running as
> >> xen_initial_domain)
> >>
> >> Before this patch the function *get_dma_ops() always returned
> >> __generic_dma_ops()
> >> which returned &arm_dma_ops (if archdata.dma_ops is absent). In this case
> >> for arm architecture .mmap callback is arm_dma_mmap() function.
> >>
> >> After this patch for domU all is the same as before this patch. But in case
> >> dom0 kernel uses xen_dma_ops with unimplemented .mmap callback. If some
> >> function tries to use .mmap callback in dom0 then kernel calls generic
> >> function dma_common_mmap() which doesn't work properly.
> >>
> >> May be some mechanism for swiotlb-xen should be implemented for unimplemented
> >> callbacks: if callback is not implemented then right architectural callback
> >> should be used (.mmap - arm_dma_mmap() for ARM)?
> >
> > I understand now. Thanks for investingating!
> > I think that we'll have to introduce our own version of .mmap,
> > because dma_to_pfn doesn't work correctly on Xen on ARM.
> > The implementation would be something like (untested):
> >
> > int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> >                  void *cpu_addr, dma_addr_t dma_addr, size_t size,
> >                  struct dma_attrs *attrs)
> > {
> >         int ret = -ENXIO;
> > #ifdef CONFIG_MMU
> >         unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> >         unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> >         unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dev, dma_addr));
> >         unsigned long off = vma->vm_pgoff;
> >
> >         vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
> >
> >         if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
> >                 return ret;
> >
> >         if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
> >                 ret = remap_pfn_range(vma, vma->vm_start,
> >                                       pfn + off,
> >                                       vma->vm_end - vma->vm_start,
> >                                       vma->vm_page_prot);
> >         }
> > #endif  /* CONFIG_MMU */
> >
> >         return ret;
> > }
> >
> > then we can add the function to xen_swiotlb_dma_ops.
> > Would you be up for coming up with a proper patch?
> 
> Hi, Stefano.
> 
> Here is a slightly modified solution from You (I've only replaced a
> __get_dma_pgprot() function with its original implementation):

Looks fine.
Could you resend to the list as a proper patch and with a signed-off-by
line? Otherwise I can do it.


> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index b0e77de..91408b1 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -48,6 +48,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
>   .sync_sg_for_device = xen_swiotlb_sync_sg_for_device,
>   .map_sg = xen_swiotlb_map_sg_attrs,
>   .unmap_sg = xen_swiotlb_unmap_sg_attrs,
> + .mmap = xen_swiotlb_dma_mmap,
>   .map_page = xen_swiotlb_map_page,
>   .unmap_page = xen_swiotlb_unmap_page,
>   .dma_supported = xen_swiotlb_dma_supported,
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 5403855..acf0c06 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -407,6 +407,42 @@ dma_addr_t xen_swiotlb_map_page(struct device
> *dev, struct page *page,
>  EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
> 
>  /*
> + * Create userspace mapping for the DMA-coherent memory.
> + */
> +int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> + void *cpu_addr, dma_addr_t dma_addr, size_t size,
> + struct dma_attrs *attrs)
> +{
> + int ret = -ENXIO;
> +#ifdef CONFIG_MMU
> + unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> + unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> + unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dma_addr));
> + unsigned long off = vma->vm_pgoff;
> + pgprot_t prot = vma->vm_page_prot;
> +
> + prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ?
> +    pgprot_writecombine(prot) :
> +    pgprot_dmacoherent(prot);
> +
> + vma->vm_page_prot = prot;
> +
> + if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
> + return ret;
> +
> + if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
> + ret = remap_pfn_range(vma, vma->vm_start,
> + pfn + off,
> + vma->vm_end - vma->vm_start,
> + vma->vm_page_prot);
> + }
> +#endif  /* CONFIG_MMU */
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(xen_swiotlb_dma_mmap);
> +
> +/*
>   * Unmap a single streaming mode DMA translation.  The dma_addr and size must
>   * match what was provided for in a previous xen_swiotlb_map_page call.  All
>   * other usages are undefined.
> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> index 7b64465..930fa94 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -15,6 +15,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size,
>    void *vaddr, dma_addr_t dma_handle,
>    struct dma_attrs *attrs);
> 
> +extern int
> +xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> +     void *cpu_addr, dma_addr_t dma_addr, size_t size,
> +     struct dma_attrs *attrs);
> +
>  extern dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
>         unsigned long offset, size_t size,
>         enum dma_data_direction dir,
> 
> 
> I've checked it on Xen 4.4 stable + some our architecture specific
> patches on top
> plus Dom0 and DomU Linux Kernel 3.8 on DRA7xxx based board. Camera application
> is working (I can see a captured data from the camera on my board). Can we use
> this implementation of the .mmap callback as the correct solution?
> 
> Oleksandr Dmytryshyn | Product Engineering and Development
> GlobalLogic
> M +38.067.382.2525
> www.globallogic.com
> 
> http://www.globallogic.com/email_disclaimer.txt
> 

Re: swiotlb-xen question

[Oleksandr Dmytryshyn]

>From f52bafc1ba51324ca5044df22778aacf815c59bf Mon Sep 17 00:00:00 2001
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Date: Fri, 4 Apr 2014 12:16:27 +0300
Subject: [PATCH] swiotlb-xen: implement xen_swiotlb_dma_mmap callback

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>

diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index b0e77de..91408b1 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -48,6 +48,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
  .sync_sg_for_device = xen_swiotlb_sync_sg_for_device,
  .map_sg = xen_swiotlb_map_sg_attrs,
  .unmap_sg = xen_swiotlb_unmap_sg_attrs,
+ .mmap = xen_swiotlb_dma_mmap,
  .map_page = xen_swiotlb_map_page,
  .unmap_page = xen_swiotlb_unmap_page,
  .dma_supported = xen_swiotlb_dma_supported,
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 5403855..acf0c06 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -407,6 +407,42 @@ dma_addr_t xen_swiotlb_map_page(struct device
*dev, struct page *page,
 EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);

 /*
+ * Create userspace mapping for the DMA-coherent memory.
+ */
+int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+ void *cpu_addr, dma_addr_t dma_addr, size_t size,
+ struct dma_attrs *attrs)
+{
+ int ret = -ENXIO;
+#ifdef CONFIG_MMU
+ unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+ unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+ unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dma_addr));
+ unsigned long off = vma->vm_pgoff;
+ pgprot_t prot = vma->vm_page_prot;
+
+ prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ?
+    pgprot_writecombine(prot) :
+    pgprot_dmacoherent(prot);
+
+ vma->vm_page_prot = prot;
+
+ if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
+ return ret;
+
+ if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
+ ret = remap_pfn_range(vma, vma->vm_start,
+ pfn + off,
+ vma->vm_end - vma->vm_start,
+ vma->vm_page_prot);
+ }
+#endif  /* CONFIG_MMU */
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(xen_swiotlb_dma_mmap);
+
+/*
  * Unmap a single streaming mode DMA translation.  The dma_addr and size must
  * match what was provided for in a previous xen_swiotlb_map_page call.  All
  * other usages are undefined.
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index 7b64465..930fa94 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -15,6 +15,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size,
   void *vaddr, dma_addr_t dma_handle,
   struct dma_attrs *attrs);

+extern int
+xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
+     void *cpu_addr, dma_addr_t dma_addr, size_t size,
+     struct dma_attrs *attrs);
+
 extern dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
        unsigned long offset, size_t size,
        enum dma_data_direction dir,

Re: swiotlb-xen question

[Konrad Rzeszutek Wilk]

On Fri, Apr 04, 2014 at 02:30:22PM +0300, Oleksandr Dmytryshyn wrote:
> >From f52bafc1ba51324ca5044df22778aacf815c59bf Mon Sep 17 00:00:00 2001
> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Date: Fri, 4 Apr 2014 12:16:27 +0300
> Subject: [PATCH] swiotlb-xen: implement xen_swiotlb_dma_mmap callback
> 
> 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>
> 
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index b0e77de..91408b1 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -48,6 +48,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
>   .sync_sg_for_device = xen_swiotlb_sync_sg_for_device,
>   .map_sg = xen_swiotlb_map_sg_attrs,
>   .unmap_sg = xen_swiotlb_unmap_sg_attrs,
> + .mmap = xen_swiotlb_dma_mmap,
>   .map_page = xen_swiotlb_map_page,
>   .unmap_page = xen_swiotlb_unmap_page,
>   .dma_supported = xen_swiotlb_dma_supported,
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 5403855..acf0c06 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -407,6 +407,42 @@ dma_addr_t xen_swiotlb_map_page(struct device
> *dev, struct page *page,
>  EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
> 
>  /*
> + * Create userspace mapping for the DMA-coherent memory.
> + */
> +int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> + void *cpu_addr, dma_addr_t dma_addr, size_t size,
> + struct dma_attrs *attrs)

Wow. Your editor ate all the nice tabs and spaces. Could you
please also run

scripts/checkpatch.pl on the patch and fix the issues it would
complain about.

Thank you!
> +{
> + int ret = -ENXIO;
> +#ifdef CONFIG_MMU
> + unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> + unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> + unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dma_addr));
> + unsigned long off = vma->vm_pgoff;
> + pgprot_t prot = vma->vm_page_prot;
> +
> + prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ?
> +    pgprot_writecombine(prot) :
> +    pgprot_dmacoherent(prot);
> +
> + vma->vm_page_prot = prot;
> +
> + if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
> + return ret;
> +
> + if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
> + ret = remap_pfn_range(vma, vma->vm_start,
> + pfn + off,
> + vma->vm_end - vma->vm_start,
> + vma->vm_page_prot);
> + }
> +#endif  /* CONFIG_MMU */
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(xen_swiotlb_dma_mmap);
> +
> +/*
>   * Unmap a single streaming mode DMA translation.  The dma_addr and size must
>   * match what was provided for in a previous xen_swiotlb_map_page call.  All
>   * other usages are undefined.
> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> index 7b64465..930fa94 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -15,6 +15,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size,
>    void *vaddr, dma_addr_t dma_handle,
>    struct dma_attrs *attrs);
> 
> +extern int
> +xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> +     void *cpu_addr, dma_addr_t dma_addr, size_t size,
> +     struct dma_attrs *attrs);
> +
>  extern dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
>         unsigned long offset, size_t size,
>         enum dma_data_direction dir,

Re: swiotlb-xen question

[Oleksandr Dmytryshyn]

On Fri, Apr 4, 2014 at 7:26 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Fri, Apr 04, 2014 at 02:30:22PM +0300, Oleksandr Dmytryshyn wrote:
>> >From f52bafc1ba51324ca5044df22778aacf815c59bf Mon Sep 17 00:00:00 2001
>> From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Date: Fri, 4 Apr 2014 12:16:27 +0300
>> Subject: [PATCH] swiotlb-xen: implement xen_swiotlb_dma_mmap callback
>>
>> 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>
>>
>> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
>> index b0e77de..91408b1 100644
>> --- a/arch/arm/xen/mm.c
>> +++ b/arch/arm/xen/mm.c
>> @@ -48,6 +48,7 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
>>   .sync_sg_for_device = xen_swiotlb_sync_sg_for_device,
>>   .map_sg = xen_swiotlb_map_sg_attrs,
>>   .unmap_sg = xen_swiotlb_unmap_sg_attrs,
>> + .mmap = xen_swiotlb_dma_mmap,
>>   .map_page = xen_swiotlb_map_page,
>>   .unmap_page = xen_swiotlb_unmap_page,
>>   .dma_supported = xen_swiotlb_dma_supported,
>> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
>> index 5403855..acf0c06 100644
>> --- a/drivers/xen/swiotlb-xen.c
>> +++ b/drivers/xen/swiotlb-xen.c
>> @@ -407,6 +407,42 @@ dma_addr_t xen_swiotlb_map_page(struct device
>> *dev, struct page *page,
>>  EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
>>
>>  /*
>> + * Create userspace mapping for the DMA-coherent memory.
>> + */
>> +int xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
>> + void *cpu_addr, dma_addr_t dma_addr, size_t size,
>> + struct dma_attrs *attrs)
>
> Wow. Your editor ate all the nice tabs and spaces. Could you
> please also run
>
> scripts/checkpatch.pl on the patch and fix the issues it would
> complain about.
>
> Thank you!
>> +{
>> + int ret = -ENXIO;
>> +#ifdef CONFIG_MMU
>> + unsigned long nr_vma_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>> + unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> + unsigned long pfn = PFN_DOWN(xen_bus_to_phys(dma_addr));
>> + unsigned long off = vma->vm_pgoff;
>> + pgprot_t prot = vma->vm_page_prot;
>> +
>> + prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ?
>> +    pgprot_writecombine(prot) :
>> +    pgprot_dmacoherent(prot);
>> +
>> + vma->vm_page_prot = prot;
>> +
>> + if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
>> + return ret;
>> +
>> + if (off < nr_pages && nr_vma_pages <= (nr_pages - off)) {
>> + ret = remap_pfn_range(vma, vma->vm_start,
>> + pfn + off,
>> + vma->vm_end - vma->vm_start,
>> + vma->vm_page_prot);
>> + }
>> +#endif  /* CONFIG_MMU */
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(xen_swiotlb_dma_mmap);
>> +
>> +/*
>>   * Unmap a single streaming mode DMA translation.  The dma_addr and size must
>>   * match what was provided for in a previous xen_swiotlb_map_page call.  All
>>   * other usages are undefined.
>> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
>> index 7b64465..930fa94 100644
>> --- a/include/xen/swiotlb-xen.h
>> +++ b/include/xen/swiotlb-xen.h
>> @@ -15,6 +15,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size,
>>    void *vaddr, dma_addr_t dma_handle,
>>    struct dma_attrs *attrs);
>>
>> +extern int
>> +xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
>> +     void *cpu_addr, dma_addr_t dma_addr, size_t size,
>> +     struct dma_attrs *attrs);
>> +
>>  extern dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
>>         unsigned long offset, size_t size,
>>         enum dma_data_direction dir,

Hi to all.
I've sent a patch in another thread (used git send).

Oleksandr Dmytryshyn | Product Engineering and Development
GlobalLogic
M +38.067.382.2525
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt