On Fri, Jan 09, 2009 at 10:27:21PM +0000, Russell King wrote:
> On Fri, Jan 09, 2009 at 11:19:36AM +0000, Ralf Baechle wrote:
> > On Thu, Jan 08, 2009 at 09:36:03AM +0100, Haavard Skinnemoen wrote:
> > > In the general case, however, I think MIPS has a bug: I've seen drivers
> > > DMA to/from tiny buffers stored inside another struct. This is legal
> > > because the driver can guarantee that the other fields in the struct
> > > aren't accessed in the mean time, but any fields sharing a cacheline
> > > with the buffer must be written back before the lines are invalidated.
> >
> > Depending on the implementation details, the use of such a struct might be
> > relying on implementation-specific behaviour. This is what
> > Documentation/DMA-API.txt has to say:
> >
> > [...]
> > int
> > dma_get_cache_alignment(void)
> >
> > Returns the processor cache alignment. This is the absolute minimum
> > alignment *and* width that you must observe when either mapping
> > memory or doing partial flushes.
> >
> > Notes: This API may return a number *larger* than the actual cache
> > line, but it will guarantee that one or more cache lines fit exactly
> > into the width returned by this call. It will also always be a power
> > of two for easy alignment.
> > [...]
> >
> > Since dma_get_cache_alignment() is a function not a constant its result
> > can't be used in the definition of a struct unless possibly excessive
> > padding is used.
> >
> > The debate has shown that we problably need BUG_ON() assertions in the
> > DMA API implementations to catch this sort of dangerous use.
>
> I really don't think that's a realistic option. You're asking for
> every call to the DMA API to ensure that the buffer and length are
> a multiple of the cache line size.
>
> So, what happens if, eg, SPI wants to send a 16 byte buffer, and your
> cache lines are 64 bytes? Does the SPI driver have to kmalloc a new
> chunk of memory 64 bytes long and copy the data into that before
> passing it into the DMA API?
>
> If you start enforcing that kind of thing, I think the cache coherent
> people will take violent exception and refuse to play such games - and
> quite rightly so.
I only want to force people to be aware of what they're doing. So far I've
seen cache lines of up to 256 bytes in size on non-coherent systems. Be
paranoid, very paranoid ...
Below patch should solve Dan William's concerns. It will peform a writeback
and invalidation operation on the first and last cacheline worth of data.
The instruction costs around a dozen cycles so I won't even try to optimize
possible double cache operations away; that'd probably be more expensive.
Ralf
Signed-off-by: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index 6e99665..56290a7 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -618,8 +618,11 @@ static void r4k_dma_cache_inv(unsigned long addr, unsigned long size)
if (cpu_has_inclusive_pcaches) {
if (size >= scache_size)
r4k_blast_scache();
- else
+ else {
+ cache_op(Hit_Writeback_Inv_SD, addr);
+ cache_op(Hit_Writeback_Inv_SD, addr + size - 1);
blast_inv_scache_range(addr, addr + size);
+ }
return;
}
@@ -627,6 +630,8 @@ static void r4k_dma_cache_inv(unsigned long addr, unsigned long size)
r4k_blast_dcache();
} else {
R4600_HIT_CACHEOP_WAR_IMPL;
+ cache_op(Hit_Writeback_Inv_D, addr);
+ cache_op(Hit_Writeback_Inv_D, addr + size - 1);
blast_inv_dcache_range(addr, addr + size);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
