Date: Mon, 29 Jun 2009 11:11:20 +0200 From: Piotr =?utf-8?q?Zi=C4=99cik?= <kosmo@semihalf.com> To: Hans Petter Selasky <hselasky@c2i.net> Cc: freebsd-arm@freebsd.org, freebsd-usb@freebsd.org, thompsa@freebsd.org Subject: Re: CPU Cache and busdma usage in USB Message-ID: <200906291111.20725.kosmo@semihalf.com> In-Reply-To: <200906281154.43392.hselasky@c2i.net> References: <200906231035.43096.kosmo@semihalf.com> <200906231912.20741.hselasky@c2i.net> <200906281154.43392.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Sunday 28 June 2009 11:54:40 Hans Petter Selasky napisa=C5=82(a):
> Hi Piotr and Rafal,
>
> Your patch is not fully correct. It will break support for x86 and more
> when bounce pages are uses.
>
> Let's get the definitions right:
>
> man busdma
> (...)
>
> My view:
>
> XXX_PREXXX functions should be used prior to read/write device access.
>
> In other words, PRE has to be a flush operation.
>
> XXX_POSTXXX functions should be used after read/write device access.
>
> In other words, POST has to be an invalidate operation.
>
> Reading:
>
> src/sys/arm/arm/busdma_machdep.c
>
> I find bus_dmamap_sync_buf() to be coherent with this view.
If everything is OK, then why USB does not work on ARM and MIPS ?
Let's look into busdma for these platforms:
usb_pc_cpu_invalidate(): [BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE]
i386: NOP
arm: Invalidate + hacks for VIPT cache.
mips: NOP
usb_pc_cpu_flush(): [BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE]
i386: NOP
arm: Writeback invalidate
mips: Writeback invalidate.
I do not see direct coherency between flags passed to bus_dma and cache=20
operations, which could be source of problem.
Let's also look at usb_pc_cpu_invalidate() usage in=20
sys/dev/usb/controller/ehci.c:
<cite>
while (1) {
usb_pc_cpu_invalidate(td->page_cache);
status =3D hc32toh(sc, td->qtd_status);
(...)
if (status & EHCI_QTD_HALTED) {
break;
}
(...)
td =3D td->obj_next;
</cite>
In my oppinion usb_pc_cpu_invalidate() used here suggests that it is doing=
=20
cache invalidation not "Perform any synchronization required after =
=20
an update of host memory by the device and prior to CPU access to host=20
memory".
As this function is implemented as bus_dmamap_sync() all busdma rules shoul=
d=20
be applied:
<cite>
If read and write operations are not preceded and followed by the
appropriate synchronization operations, behavior is undefined.
</cite>
In code shown above (and many more places in USB stack) there is no followi=
ng
synchronization operation, which also could be source of problem.
My major question here is why bus_dma is used for flushing and invalidation
CPU caches instead of cpu_*() functions wich was written for this purpuose.
> Can you check if the COHERENT bit is set for your allocation?
>
> if (map->flags & DMAMAP_COHERENT)
> return;
>
No. This bit is not set.
> Summed up:
>
> The existing code is doing correct. What is known is a problem with the
> memory mapping, so that the same memory page can get mapped with different
> attributes, which makes the problem appear.
I don't think so:
$ man busdma
<cite>
BUS_DMA_COHERENT
Attempt to map this memory such that cache sync operations are as cheap as
possible. This flag is typically set on memory that will be accessed by b=
oth
a CPU and a DMA engine, frequently. Use of this flag does not remove the=
=20
requirement of using bus_dmamap_sync, but it may reduce the cost of =09
performing these operations.=20
</cite>
This means that BUS_DMA_COHERENT does not guarantee no-cache mapping - this=
is
only a hint for busdma subsystem that region will be synced frequently. Ple=
ase=20
look at discussion at=20
http://kerneltrap.org/mailarchive/freebsd-current/2009/5/27/5817243
=2D-=20
Best regards.
Piotr Ziecik
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200906291111.20725.kosmo>