Re: [PATCH v2 -resend#1 1/1] V4L: videobuf, don't use dma addr as physical
Em 28-02-2011 15:20, Mauro Carvalho Chehab escreveu:
Em 28-02-2011 12:47, Jiri Slaby escreveu:
On 02/28/2011 03:53 PM, Konrad Rzeszutek Wilk wrote:
On Mon, Feb 28, 2011 at 10:37:02AM +0100, Jiri Slaby wrote:
mem-dma_handle is a dma address obtained by dma_alloc_coherent which
needn't be a physical address in presence of IOMMU. So ensure we are
Can you add a comment why you are fixing it? Is there a bug report for this?
Under what conditions did you expose this fault?
No, by a just peer review when I was looking for something completely
different.
You also might want to mention that needn't be a physical address as
a hardware IOMMU can (and most likely) will return a bus address where
physical != bus address.
Mauro, do you want me to resend this with such an udpate in the changelog?
Having it properly documented is always a good idea, especially since a
similar
fix might be needed on other drivers that also need contiguous memory. While
it
currently is used only on devices embedded on hardware with no iommu, there
are
some x86 hardware that doesn't allow DMA scatter/gather.
Btw, it may be worth to take a look at vb2 dma contig module, as it might have
similar issues.
Otherwise you can stick 'Reviewed-by: Konrad Rzeszutek Wilk
konrad.w...@oracle.com'
on it.
Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
As I got no return, and the patch looked sane, I've reviewed the comment myself,
in order to make it more explicit, as suggested by Konrad, and added his
reviewed-by: tag:
Author: Jiri Slaby jsl...@suse.cz
Date: Mon Feb 28 06:37:02 2011 -0300
[media] V4L: videobuf, don't use dma addr as physical
mem-dma_handle is a dma address obtained by dma_alloc_coherent which
needn't be a physical address in presence of IOMMU, as
a hardware IOMMU can (and most likely) will return a bus address where
physical != bus address.
So ensure we are remapping (remap_pfn_range) the right page in
__videobuf_mmap_mapper by using virt_to_phys(mem-vaddr) and not
mem-dma_handle.
While at it, use PFN_DOWN instead of explicit shift.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Reviewed-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c
index c969111..19d3e4a 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -300,7 +300,7 @@ static int __videobuf_mmap_mapper(struct videobuf_queue *q,
vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
retval = remap_pfn_range(vma, vma-vm_start,
-mem-dma_handle PAGE_SHIFT,
+ PFN_DOWN(virt_to_phys(mem-vaddr))
size, vma-vm_page_prot);
if (retval) {
dev_err(q-dev, mmap: remap failed with error %d. , retval);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -resend#1 1/1] V4L: videobuf, don't use dma addr as physical
On 03/21/2011 11:43 PM, Mauro Carvalho Chehab wrote:
As I got no return, and the patch looked sane, I've reviewed the comment
myself,
Aha, I forgot to send it. Sorry.
It looks OK.
Author: Jiri Slaby jsl...@suse.cz
Date: Mon Feb 28 06:37:02 2011 -0300
[media] V4L: videobuf, don't use dma addr as physical
mem-dma_handle is a dma address obtained by dma_alloc_coherent which
needn't be a physical address in presence of IOMMU, as
a hardware IOMMU can (and most likely) will return a bus address where
physical != bus address.
So ensure we are remapping (remap_pfn_range) the right page in
__videobuf_mmap_mapper by using virt_to_phys(mem-vaddr) and not
mem-dma_handle.
While at it, use PFN_DOWN instead of explicit shift.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Reviewed-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c
index c969111..19d3e4a 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -300,7 +300,7 @@ static int __videobuf_mmap_mapper(struct videobuf_queue
*q,
vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
retval = remap_pfn_range(vma, vma-vm_start,
-mem-dma_handle PAGE_SHIFT,
+ PFN_DOWN(virt_to_phys(mem-vaddr))
size, vma-vm_page_prot);
if (retval) {
dev_err(q-dev, mmap: remap failed with error %d. , retval);
thanks,
--
js
suse labs
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -resend#1 1/1] V4L: videobuf, don't use dma addr as physical
Hi Jiri,
On Monday 28 February 2011 10:37:02 Jiri Slaby wrote:
mem-dma_handle is a dma address obtained by dma_alloc_coherent which
needn't be a physical address in presence of IOMMU. So ensure we are
remapping (remap_pfn_range) the right page in __videobuf_mmap_mapper
by using virt_to_phys(mem-vaddr) and not mem-dma_handle.
Quoting arch/arm/include/asm/memory.h,
/*
* These are *only* valid on the kernel direct mapped RAM memory.
* Note: Drivers should NOT use these. They are the wrong
* translation for translating DMA addresses. Use the driver
* DMA support - see dma-mapping.h.
*/
static inline unsigned long virt_to_phys(const volatile void *x)
{
return __virt_to_phys((unsigned long)(x));
}
Why would you use physically contiguous memory if you have an IOMMU anyway ?
While at it, use PFN_DOWN instead of explicit shift.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
drivers/media/video/videobuf-dma-contig.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c index c969111..19d3e4a 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -300,7 +300,7 @@ static int __videobuf_mmap_mapper(struct videobuf_queue
*q,
vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
retval = remap_pfn_range(vma, vma-vm_start,
- mem-dma_handle PAGE_SHIFT,
+ PFN_DOWN(virt_to_phys(mem-vaddr))
size, vma-vm_page_prot);
if (retval) {
dev_err(q-dev, mmap: remap failed with error %d. , retval);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -resend#1 1/1] V4L: videobuf, don't use dma addr as physical
On 02/28/2011 11:53 AM, Laurent Pinchart wrote:
On Monday 28 February 2011 10:37:02 Jiri Slaby wrote:
mem-dma_handle is a dma address obtained by dma_alloc_coherent which
needn't be a physical address in presence of IOMMU. So ensure we are
remapping (remap_pfn_range) the right page in __videobuf_mmap_mapper
by using virt_to_phys(mem-vaddr) and not mem-dma_handle.
Quoting arch/arm/include/asm/memory.h,
/*
* These are *only* valid on the kernel direct mapped RAM memory.
Which the DMA allocation shall be.
* Note: Drivers should NOT use these.
This is weird.
They are the wrong
* translation for translating DMA addresses. Use the driver
* DMA support - see dma-mapping.h.
Yes, ACK, and vice versa. DMA addresses cannot be used as physical ones.
*/
static inline unsigned long virt_to_phys(const volatile void *x)
{
return __virt_to_phys((unsigned long)(x));
}
Why would you use physically contiguous memory if you have an IOMMU anyway ?
Sorry, what? When IOMMU is used, dma_alloc_* functions may return tags
as a DMA address, not a physical address. So using these DMA addresses
directly (e.g. in remap_pfn_range) is a bug.
Maybe there is a better way to convert a kernel address of the DMA
mapping into a physical frame, but I'm not aware of any...
While at it, use PFN_DOWN instead of explicit shift.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
drivers/media/video/videobuf-dma-contig.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c index c969111..19d3e4a 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -300,7 +300,7 @@ static int __videobuf_mmap_mapper(struct videobuf_queue
*q,
vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
retval = remap_pfn_range(vma, vma-vm_start,
- mem-dma_handle PAGE_SHIFT,
+ PFN_DOWN(virt_to_phys(mem-vaddr))
size, vma-vm_page_prot);
if (retval) {
dev_err(q-dev, mmap: remap failed with error %d. , retval);
regards,
--
js
suse labs
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -resend#1 1/1] V4L: videobuf, don't use dma addr as physical
Hi Jiri,
On Monday 28 February 2011 16:07:43 Jiri Slaby wrote:
On 02/28/2011 11:53 AM, Laurent Pinchart wrote:
On Monday 28 February 2011 10:37:02 Jiri Slaby wrote:
mem-dma_handle is a dma address obtained by dma_alloc_coherent which
needn't be a physical address in presence of IOMMU. So ensure we are
remapping (remap_pfn_range) the right page in __videobuf_mmap_mapper
by using virt_to_phys(mem-vaddr) and not mem-dma_handle.
Quoting arch/arm/include/asm/memory.h,
/*
* These are *only* valid on the kernel direct mapped RAM memory.
Which the DMA allocation shall be.
* Note: Drivers should NOT use these.
This is weird.
They are the wrong
* translation for translating DMA addresses. Use the driver
* DMA support - see dma-mapping.h.
Yes, ACK, and vice versa. DMA addresses cannot be used as physical ones.
*/
static inline unsigned long virt_to_phys(const volatile void *x)
{
return __virt_to_phys((unsigned long)(x));
}
Why would you use physically contiguous memory if you have an IOMMU
anyway ?
Sorry, what? When IOMMU is used, dma_alloc_* functions may return tags
as a DMA address, not a physical address. So using these DMA addresses
directly (e.g. in remap_pfn_range) is a bug.
What I mean is that videobuf-dma-contig is meant to be used by drivers that
require physically contiguous memory. If the system has an IOMMU, why would
drivers need that ?
Maybe there is a better way to convert a kernel address of the DMA
mapping into a physical frame, but I'm not aware of any...
While at it, use PFN_DOWN instead of explicit shift.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
drivers/media/video/videobuf-dma-contig.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c index c969111..19d3e4a
100644 --- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -300,7 +300,7 @@ static int __videobuf_mmap_mapper(struct
videobuf_queue *q,
vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
retval = remap_pfn_range(vma, vma-vm_start,
- mem-dma_handle PAGE_SHIFT,
+ PFN_DOWN(virt_to_phys(mem-vaddr))
size, vma-vm_page_prot);
if (retval) {
dev_err(q-dev, mmap: remap failed with error %d. , retval);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -resend#1 1/1] V4L: videobuf, don't use dma addr as physical
On Mon, Feb 28, 2011 at 10:37:02AM +0100, Jiri Slaby wrote:
mem-dma_handle is a dma address obtained by dma_alloc_coherent which
needn't be a physical address in presence of IOMMU. So ensure we are
Can you add a comment why you are fixing it? Is there a bug report for this?
Under what conditions did you expose this fault?
You also might want to mention that needn't be a physical address as
a hardware IOMMU can (and most likely) will return a bus address where
physical != bus address.
Otherwise you can stick 'Reviewed-by: Konrad Rzeszutek Wilk
konrad.w...@oracle.com'
on it.
remapping (remap_pfn_range) the right page in __videobuf_mmap_mapper
by using virt_to_phys(mem-vaddr) and not mem-dma_handle.
While at it, use PFN_DOWN instead of explicit shift.
Signed-off-by: Jiri Slaby jsl...@suse.cz
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
drivers/media/video/videobuf-dma-contig.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/videobuf-dma-contig.c
b/drivers/media/video/videobuf-dma-contig.c
index c969111..19d3e4a 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -300,7 +300,7 @@ static int __videobuf_mmap_mapper(struct videobuf_queue
*q,
vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
retval = remap_pfn_range(vma, vma-vm_start,
- mem-dma_handle PAGE_SHIFT,
+ PFN_DOWN(virt_to_phys(mem-vaddr))
size, vma-vm_page_prot);
if (retval) {
dev_err(q-dev, mmap: remap failed with error %d. , retval);
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -resend#1 1/1] V4L: videobuf, don't use dma addr as physical
On 02/28/2011 04:14 PM, Laurent Pinchart wrote:
Hi Jiri,
On Monday 28 February 2011 16:07:43 Jiri Slaby wrote:
On 02/28/2011 11:53 AM, Laurent Pinchart wrote:
On Monday 28 February 2011 10:37:02 Jiri Slaby wrote:
mem-dma_handle is a dma address obtained by dma_alloc_coherent which
needn't be a physical address in presence of IOMMU. So ensure we are
remapping (remap_pfn_range) the right page in __videobuf_mmap_mapper
by using virt_to_phys(mem-vaddr) and not mem-dma_handle.
Quoting arch/arm/include/asm/memory.h,
/*
* These are *only* valid on the kernel direct mapped RAM memory.
Which the DMA allocation shall be.
* Note: Drivers should NOT use these.
This is weird.
They are the wrong
* translation for translating DMA addresses. Use the driver
* DMA support - see dma-mapping.h.
Yes, ACK, and vice versa. DMA addresses cannot be used as physical ones.
*/
static inline unsigned long virt_to_phys(const volatile void *x)
{
return __virt_to_phys((unsigned long)(x));
}
Why would you use physically contiguous memory if you have an IOMMU
anyway ?
Sorry, what? When IOMMU is used, dma_alloc_* functions may return tags
as a DMA address, not a physical address. So using these DMA addresses
directly (e.g. in remap_pfn_range) is a bug.
What I mean is that videobuf-dma-contig is meant to be used by drivers that
require physically contiguous memory. If the system has an IOMMU, why would
drivers need that ?
Aha. They actually need not but they would need do the mapping
themselves which they currently do not.
IOW the vbuf-dma-contig allocator is used unconditionally in the few
drivers I checked.
BUT Even if they need only one page and use vbuf-dma-contig, which I
don't see a reason not to, it will cause problems too.
regards,
--
js
suse labs
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -resend#1 1/1] V4L: videobuf, don't use dma addr as physical
On 02/28/2011 03:53 PM, Konrad Rzeszutek Wilk wrote: On Mon, Feb 28, 2011 at 10:37:02AM +0100, Jiri Slaby wrote: mem-dma_handle is a dma address obtained by dma_alloc_coherent which needn't be a physical address in presence of IOMMU. So ensure we are Can you add a comment why you are fixing it? Is there a bug report for this? Under what conditions did you expose this fault? No, by a just peer review when I was looking for something completely different. You also might want to mention that needn't be a physical address as a hardware IOMMU can (and most likely) will return a bus address where physical != bus address. Mauro, do you want me to resend this with such an udpate in the changelog? Otherwise you can stick 'Reviewed-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com' on it. thanks, -- js suse labs -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 -resend#1 1/1] V4L: videobuf, don't use dma addr as physical
Em 28-02-2011 12:47, Jiri Slaby escreveu: On 02/28/2011 03:53 PM, Konrad Rzeszutek Wilk wrote: On Mon, Feb 28, 2011 at 10:37:02AM +0100, Jiri Slaby wrote: mem-dma_handle is a dma address obtained by dma_alloc_coherent which needn't be a physical address in presence of IOMMU. So ensure we are Can you add a comment why you are fixing it? Is there a bug report for this? Under what conditions did you expose this fault? No, by a just peer review when I was looking for something completely different. You also might want to mention that needn't be a physical address as a hardware IOMMU can (and most likely) will return a bus address where physical != bus address. Mauro, do you want me to resend this with such an udpate in the changelog? Having it properly documented is always a good idea, especially since a similar fix might be needed on other drivers that also need contiguous memory. While it currently is used only on devices embedded on hardware with no iommu, there are some x86 hardware that doesn't allow DMA scatter/gather. Btw, it may be worth to take a look at vb2 dma contig module, as it might have similar issues. Otherwise you can stick 'Reviewed-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com' on it. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
