Date: Tue, 30 Jun 2009 10:37:48 +0200 From: Piotr =?iso-8859-2?q?Zi=EAcik?= <kosmo@semihalf.com> To: Hans Petter Selasky <hselasky@c2i.net> Cc: Rafal Jaworowski <raj@semihalf.com>, freebsd-arm@freebsd.org, thompsa@freebsd.org, freebsd-usb@freebsd.org Subject: Re: CPU Cache and busdma usage in USB Message-ID: <200906301037.49367.kosmo@semihalf.com> In-Reply-To: <200906291516.58725.hselasky@c2i.net> References: <200906231035.43096.kosmo@semihalf.com> <200906291516.58725.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Monday 29 June 2009 15:16:56 Hans Petter Selasky napisa=B3(a):
>
> You want to change the flags passed to bus_dmamap_sync() so that the
> flush/invalidate mapping gets right. I understand why your patch makes it
> work. That's not the problem.
>
> In "src/sys/arm/arm/busdma_machdep.c" there is a function called
> "_bus_dmamap_sync_bp()". If you look at that function you see that it only
> triggers on the "BUS_DMASYNC_PREWRITE" and "BUS_DMASYNC_POSTREAD" flags.
> After your patching only the PREXXXX flags are used, so if bouce pages are
> used on ARM and x86 and amd64 +++, then only BUS_DMASYNC_PREWRITE will do
> anything. This indicates that your patch is not fully correct.
That is true. I have missed "bounce page" case. I can change flags passed t=
o=20
bus_dmamap_sync() to fix this on ARM, but this will break MIPS.
This clearly shows the problem - using side effect of busdma to manage CPU=
=20
cache. Current USB implementation relies of this side effect assuming that=
=20
bus_dmamap_sync() with given flags will do cpu cache flush/invalidation.
This is not true even on i386 !
This thread is not about my patch. It is only a fast and dirty hack
making USB working on platforms without hardware cache coherency
and showing the problem. I see two proper solutions:
1. Implement usb_pc_cpu_invalidate() / usb_pc_cpu_flush() with
cpu_*() functions realizing requested operation.
2. Using busdma in proper way:
[... prepare data to transfer ...]
bus_dmamap_sync(..., PREREAD | PREWRITE);
[... do transfer ...]
bus_dmamap_sync(..., POSTREAD | POSTWRITE);
[... check results ...]
as manpage says:
<cite>
If read and write operations are not preceded and followed by the
appropriate synchronization operations, behavior is undefined.
</cite>
Requirement cited above is currently not met in USB stack. Funtion
shown in http://fxr.watson.org/fxr/source/dev/usb/controller/ehci.c#L1294
is just an example of improper usage of bus_dmamap_sync(), which is hidden=
=20
under usb_pc_cpu_invalidate().
This thread started from my question about general usage of usb_pc_cpu_*()=
=20
functions. So I am asking once again - why these functions relies on side=20
effect of busdma instead of simply doing cache flush/invalidation through=20
cpu_*() functions ?
> Grepping through the source code for ARM, I found a line like this:
> (...)
> You see that it only performs purge and no prior flush. This is what needs
> fixing! If semihalf could go through the "arm/arm/cpufunc.c" file and fix
> those flush and invalidate functions then many people would become happy!
My developement platform is sheeva. CPU functions for this platform are=20
implemented correctly.
> Again, it is not a problem in USB firstly, it is a problem in "arm/xxx".
You are suggesting that the problem is in ARM busdma (and in MIPS busdma, a=
s=20
on this platform USB also does not work). Could you give me example of=20
platform _without_ hardware coherency where new USB stack simply works ?
=2D-=20
Best regards.
Piotr Ziecik
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200906301037.49367.kosmo>