Re: [PATCH] powerpc: Fix swiotlb ops for ppc64
On Dec 12, 2011, at 10:27 PM, Benjamin Herrenschmidt wrote: On Mon, 2011-12-12 at 21:55 -0600, Becky Bruce wrote: 1) dma_direct_alloc_coherent strips GFP_HIGHMEM out of the flags field when calling the actual allocator and the iotlb version does not. I don't know how much this matters - I did a quick grep and I don't see any users that specify that, but somebody went through the trouble of putting it in there in the first place and without knowing why I wasn't willing to get rid of it. Now, since my patch it looks like someone added a VM_BUG_ON into __get_free_pages() if GFP_HIGHMEM so this would get caught. However, I don't know if we really want to throw a bug there. 2) The iotlb code doesn't deal with the !coherent parts like 8xx. We can work around that by setting up the dma_ops differently for that case instead. Does the rest of it handle them ? I mean swiotlb_map_sg_attrs etc... The non-coherent specialness is in the dma sync stuff and no, I don't think the iotlb stuff deals with that properly. Do you not have a problem with 1)? If not then I think we can look at switching over; 2) was more of a secondary thing. -B ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix swiotlb ops for ppc64
On Tue, 2011-12-13 at 20:53 -0600, Becky Bruce wrote: The non-coherent specialness is in the dma sync stuff and no, I don't think the iotlb stuff deals with that properly. Do you not have a problem with 1)? If not then I think we can look at switching over; 2) was more of a secondary thing. So let's switch over, avoid an #ifdef which is always a good thing... Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix swiotlb ops for ppc64
On Dec 7, 2011, at 11:46 PM, Kumar Gala wrote:
On Dec 7, 2011, at 9:23 PM, Benjamin Herrenschmidt wrote:
On Wed, 2011-12-07 at 11:19 -0600, Kumar Gala wrote:
struct dma_map_ops swiotlb_dma_ops = {
+#ifdef CONFIG_PPC64
+ .alloc_coherent = swiotlb_alloc_coherent,
+ .free_coherent = swiotlb_free_coherent,
+#else
.alloc_coherent = dma_direct_alloc_coherent,
.free_coherent = dma_direct_free_coherent,
+#endif
.map_sg = swiotlb_map_sg_attrs,
.unmap_sg = swiotlb_unmap_sg_attrs,
.dma_supported = swiotlb_dma_supported,
Do we really need the ifdef ? What happens if we use
swiotlb_alloc_coherent() on ppc32 ? Won't it allocate lowmem, realize
that it doesn't need bouncing and be happy ?
Cheers,
Ben.
Becky any comment?
I know its been a while, but wondering if you had any reason to not do what
Ben's suggesting ?
Well, as you say, it's been a while, and but I think:
1) dma_direct_alloc_coherent strips GFP_HIGHMEM out of the flags field when
calling the actual allocator and the iotlb version does not. I don't know how
much this matters - I did a quick grep and I don't see any users that specify
that, but somebody went through the trouble of putting it in there in the first
place and without knowing why I wasn't willing to get rid of it. Now, since my
patch it looks like someone added a VM_BUG_ON into __get_free_pages() if
GFP_HIGHMEM so this would get caught. However, I don't know if we really want
to throw a bug there.
2) The iotlb code doesn't deal with the !coherent parts like 8xx. We can work
around that by setting up the dma_ops differently for that case instead.
-Becky
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix swiotlb ops for ppc64
On Mon, 2011-12-12 at 21:55 -0600, Becky Bruce wrote: 1) dma_direct_alloc_coherent strips GFP_HIGHMEM out of the flags field when calling the actual allocator and the iotlb version does not. I don't know how much this matters - I did a quick grep and I don't see any users that specify that, but somebody went through the trouble of putting it in there in the first place and without knowing why I wasn't willing to get rid of it. Now, since my patch it looks like someone added a VM_BUG_ON into __get_free_pages() if GFP_HIGHMEM so this would get caught. However, I don't know if we really want to throw a bug there. 2) The iotlb code doesn't deal with the !coherent parts like 8xx. We can work around that by setting up the dma_ops differently for that case instead. Does the rest of it handle them ? I mean swiotlb_map_sg_attrs etc... If not then it's broken anyway so may as well not care for now. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc: Fix swiotlb ops for ppc64
We assumed before that alloc_coherent free_coherent ops would always
be direct because of 32-bit systems and how we utilize highmem lowmem.
However, on 64-bit systems we typically treat all memory as lowmem so
the same assumptions are not valid. We need to utilze the swiotlb
versions of alloc_coherent free_coherent on 64-bit systems.
Signed-off-by: Kumar Gala ga...@kernel.crashing.org
---
arch/powerpc/kernel/dma-swiotlb.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/dma-swiotlb.c
b/arch/powerpc/kernel/dma-swiotlb.c
index 1ebc918..5000fd4 100644
--- a/arch/powerpc/kernel/dma-swiotlb.c
+++ b/arch/powerpc/kernel/dma-swiotlb.c
@@ -40,15 +40,20 @@ static u64 swiotlb_powerpc_get_required(struct device *dev)
}
/*
- * At the moment, all platforms that use this code only require
- * swiotlb to be used if we're operating on HIGHMEM. Since
+ * We assume that 32-bit systems will utilize HIGHMEM and that we're
+ * able to DMA directly to anything in the LOWMEM region. Since
* we don't ever call anything other than map_sg, unmap_sg,
* map_page, and unmap_page on highmem, use normal dma_ops
* for everything else.
*/
struct dma_map_ops swiotlb_dma_ops = {
+#ifdef CONFIG_PPC64
+ .alloc_coherent = swiotlb_alloc_coherent,
+ .free_coherent = swiotlb_free_coherent,
+#else
.alloc_coherent = dma_direct_alloc_coherent,
.free_coherent = dma_direct_free_coherent,
+#endif
.map_sg = swiotlb_map_sg_attrs,
.unmap_sg = swiotlb_unmap_sg_attrs,
.dma_supported = swiotlb_dma_supported,
--
1.7.3.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix swiotlb ops for ppc64
On Wed, 2011-12-07 at 11:19 -0600, Kumar Gala wrote:
struct dma_map_ops swiotlb_dma_ops = {
+#ifdef CONFIG_PPC64
+ .alloc_coherent = swiotlb_alloc_coherent,
+ .free_coherent = swiotlb_free_coherent,
+#else
.alloc_coherent = dma_direct_alloc_coherent,
.free_coherent = dma_direct_free_coherent,
+#endif
.map_sg = swiotlb_map_sg_attrs,
.unmap_sg = swiotlb_unmap_sg_attrs,
.dma_supported = swiotlb_dma_supported,
Do we really need the ifdef ? What happens if we use
swiotlb_alloc_coherent() on ppc32 ? Won't it allocate lowmem, realize
that it doesn't need bouncing and be happy ?
Cheers,
Ben.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix swiotlb ops for ppc64
On Dec 7, 2011, at 9:23 PM, Benjamin Herrenschmidt wrote:
On Wed, 2011-12-07 at 11:19 -0600, Kumar Gala wrote:
struct dma_map_ops swiotlb_dma_ops = {
+#ifdef CONFIG_PPC64
+.alloc_coherent = swiotlb_alloc_coherent,
+.free_coherent = swiotlb_free_coherent,
+#else
.alloc_coherent = dma_direct_alloc_coherent,
.free_coherent = dma_direct_free_coherent,
+#endif
.map_sg = swiotlb_map_sg_attrs,
.unmap_sg = swiotlb_unmap_sg_attrs,
.dma_supported = swiotlb_dma_supported,
Do we really need the ifdef ? What happens if we use
swiotlb_alloc_coherent() on ppc32 ? Won't it allocate lowmem, realize
that it doesn't need bouncing and be happy ?
Cheers,
Ben.
Becky any comment?
I know its been a while, but wondering if you had any reason to not do what
Ben's suggesting ?
- k
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
