login
Header Space

 
 

RE: [PATCH] Microblaze: implement dma-coherent API and refactorcache flush code.

Previous thread: BUG: unable to handle kernel paging request by thomas on Monday, May 5, 2008 - 6:33 pm. (2 messages)

Next thread: [patch 1/8] PNP: set IRQ index in sysfs "set irq" interface by Bjorn Helgaas on Monday, May 5, 2008 - 6:36 pm. (1 message)
To: <arnd@...>, <linux-arch@...>, John Linn <linnj@...>, <john.williams@...>, <matthew@...>, <will.newton@...>, <drepper@...>, <microblaze-uclinux@...>, <grant.likely@...>, Michal Simek <monstr@...>, <linux-kernel@...>
Date: Monday, May 5, 2008 - 6:37 pm

Primarily, this patch implements the dma-coherent API.  In addition,
it cleans up some of the code that deals with caches, in order to
match the usage in dma-coherent.

In particular, the dcache in the microblaze is write through, so the
existing code is more easily thought of as 'invalidation' than
'flushing'.  In addition, some of the flush_* methods were old, and
some shouldn't need to be implemented (since currently no mmu is
supported).

I'd appreciate if someone would ACK my interpretation of
Documentation/cachetlb.txt.  In particular:

flush_cache_mm(mm) (NOOP because nommu)
flush_cache_range(mm, start, end) (Does this need to be implemented
since nommu?)
flush_cache_page(vma, vmaddr) (NOOP because nommu)
flush_dcache_page(page) (NOOP because write through cache.)
flush_dcache_range(start, end) (NOOP because write through cache.)
flush_dcache_mmap_lock(mapping) (NOOP because nommu)
flush_dcache_mmap_unlock(mapping) (NOOP because nommu)

flush_icache_page(vma,pg) (Does this need to be implemented? Doc is
unclear, but I assume it is used as flush_icache_range)
flush_icache_range(start, end) (Must be implemented because icache
doesn't snoop dcache on code loads)

Signed-off-by: Stephen Neuendorffer &lt;stephen.neuendorffer@xilinx.com&gt;
---
 arch/microblaze/kernel/cpu/cache.c  |   37 +---------
 arch/microblaze/kernel/setup.c      |    4 +-
 arch/microblaze/mm/dma-coherent.c   |  122
+++++++++++++++++++++++++++++++++++
 include/asm-microblaze/cacheflush.h |   71 +++++++++------------
 4 files changed, 158 insertions(+), 76 deletions(-)
 create mode 100644 arch/microblaze/mm/dma-coherent.c

diff --git a/arch/microblaze/kernel/cpu/cache.c
b/arch/microblaze/kernel/cpu/cache.c
index d6a1eab..1247b9e 100644
--- a/arch/microblaze/kernel/cpu/cache.c
+++ b/arch/microblaze/kernel/cpu/cache.c
@@ -129,7 +129,7 @@ void _invalidate_dcache(unsigned int addr)
 				: "r" (addr));
 }
 
-void __flush_icache_all(void)
+void __invalidate_icache_all(void)
 {
 	unsigned int i;
 	u...
To: Stephen Neuendorffer <stephen.neuendorffer@...>
Cc: <arnd@...>, <linux-arch@...>, John Linn <linnj@...>, <john.williams@...>, <matthew@...>, <will.newton@...>, <drepper@...>, <microblaze-uclinux@...>, <grant.likely@...>, <linux-kernel@...>
Date: Tuesday, May 6, 2008 - 5:28 am

removed.

M
--
To: Stephen Neuendorffer <stephen.neuendorffer@...>
Cc: <arnd@...>, <linux-arch@...>, John Linn <linnj@...>, <matthew@...>, <will.newton@...>, <drepper@...>, <microblaze-uclinux@...>, <grant.likely@...>, Michal Simek <monstr@...>, <linux-kernel@...>
Date: Monday, May 5, 2008 - 6:57 pm

Hi Steve,


I agree - this renaming is a long time coming.

[snip]

Does the DMA API insist upon the dma_cache_sync call to guarantee
sensible results?  If so, your implementation looks fine.  If not, then
the results will clearly be bogus as there's nothing magical about the
memory being allocated in dma_alloc.

To that end, can it just call kmalloc(), and similarly kfree() for
dma_free?

I'd still like to see the uncached shadow stuff make a return one day,
it is an effective way of solving the problem, we'll just need to find a
cleaner way to implement it.  A linear walk through the cache to
invalidate is so slow, it destroys any benefit gained from DMA in the
first place.

Cheers,

John


--
To: John Williams <john.williams@...>
Cc: <arnd@...>, <linux-arch@...>, John Linn <linnj@...>, <matthew@...>, <will.newton@...>, <drepper@...>, <microblaze-uclinux@...>, <grant.likely@...>, Michal Simek <monstr@...>, <linux-kernel@...>
Date: Monday, May 5, 2008 - 7:12 pm

Yes, in fact this is one of the keys to getting the lltemac driver to

My understanding is that on other architectures (x86, for instance)
'dma' memory ensures other things, like it's accessible in PCI memory
space.  On microblaze, there's nothing really special about dma memory,

I think it's definitely a simple way of solving the problem (in fact,
the version of the code that's currently at git.xilinx.com includes it).
Would it be better dealt with at the page level, rather than at the
address level, then one of the architecture-reserved flags could be used
for it?
If it really does have value, then I want to make sure it goes in, along
with bits in EDK that make it straightforward to use.  For me, the
biggest barrier to using it is understanding exactly what assumptions it
is making on the hardware, and making those assumptions bulletproof.

Steve


--
To: Stephen Neuendorffer <stephen.neuendorffer@...>
Cc: <arnd@...>, <linux-arch@...>, John Linn <linnj@...>, <matthew@...>, <will.newton@...>, <drepper@...>, <microblaze-uclinux@...>, <grant.likely@...>, Michal Simek <monstr@...>, <linux-kernel@...>
Date: Monday, May 5, 2008 - 8:14 pm

I agree.  Michal did some work on that when he did the initial DTS stuff
with us over the (southern) summer this year.  We looked at ways to
validate the HW design and Kconfig params to make sure that
uncached_shadow worked as expected.

If forget now the outcome, there's plenty of ways to get it wrong.

It would be easy at runtime to do some validation tests - write to a
pointer, cache flush (noop..), read back from the twiddled pointer, make
sure it matches.  Repeat a few times until you're happy it wasn't a
fluke, that sort of thing.

-- 
John Williams, PhD, B.Eng, B.IT
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com  p: +61-7-30090663  f: +61-7-30090663


--
To: John Williams <john.williams@...>
Cc: <arnd@...>, <linux-arch@...>, John Linn <linnj@...>, <matthew@...>, <will.newton@...>, <drepper@...>, <microblaze-uclinux@...>, <grant.likely@...>, Michal Simek <monstr@...>, <linux-kernel@...>
Date: Monday, May 5, 2008 - 8:31 pm

work?

I scanned through Linux Device Drivers, and it appears that calling
get_free_pages is:
1) more efficient than kmalloc for large allocations
2) allocates physically contiguous memory, which kmalloc doesn't
necessarily do if there's an mmu.

Steve

--
To: Stephen Neuendorffer <stephen.neuendorffer@...>
Cc: John Williams <john.williams@...>, <arnd@...>, <linux-arch@...>, John Linn <linnj@...>, <matthew@...>, <will.newton@...>, <drepper@...>, <microblaze-uclinux@...>, <grant.likely@...>, Michal Simek <monstr@...>, <linux-kernel@...>
Date: Tuesday, May 6, 2008 - 3:23 am

kmalloc() allocates physically contiguous memory, vmalloc() doesn't.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
--
To: Geert Uytterhoeven <geert@...>
Cc: Stephen Neuendorffer <stephen.neuendorffer@...>, John Williams <john.williams@...>, <arnd@...>, <linux-arch@...>, John Linn <linnj@...>, <matthew@...>, <will.newton@...>, <drepper@...>, <microblaze-uclinux@...>, <grant.likely@...>, <linux-kernel@...>
Date: Tuesday, May 6, 2008 - 5:33 am

Hi John, Steve and others,

I'll only rename flush to invalidate. Thant's all for now.


--
Previous thread: BUG: unable to handle kernel paging request by thomas on Monday, May 5, 2008 - 6:33 pm. (2 messages)

Next thread: [patch 1/8] PNP: set IRQ index in sysfs "set irq" interface by Bjorn Helgaas on Monday, May 5, 2008 - 6:36 pm. (1 message)
speck-geostationary