Implement dma_sync_single_for_{cpu,device} translating the Linux
DMA_ flags to BUS_DMASYNC_ combinations.
Sponsored by: The FreeBSD Foundation
MFC after: 7 days
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
This needs careful review on BUS_DMASYNC_ flags as I constantly got confused trying to map them.
| sys/compat/linuxkpi/common/include/linux/dma-mapping.h | ||
|---|---|---|
| 210 | I think this is wrong. You need to do this in two sync operations. See sys/dev/usb/usb_busdma.c . | |
Comment Actions
Split PRE and POST syncs into two operations. They cannot go together.
Pointed out by @hsealsky.
We don't know which one we need when called. A bit silly?
Comment Actions
We don't know which one we need when called. A bit silly?
USB controllers don't always separate read and written data. Sometimes it is mixed. Might be the same for your Linux hardware.
For bidirectional, you first need to flush to RAM then invalidate I guess.
Comment Actions
Could you point to some places in driver code where these functions are used? I just want to verify the BUSDMA flags vs direction.
Comment Actions
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/tree/drivers/net/wireless/realtek/rtw88/pci.c#n249
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/tree/drivers/net/wireless/realtek/rtw88/pci.c#n1092
are two which are in official code but I also added debugging ones to iwlwifi given they change things after the fact.
Comment Actions
Hi,
I think you cannot pass multiple flags to bus_dmamap_sync() like you do.
Could you align bus_dmamap_sync() in the LinuxKPI with what you find here:
grep -C 10 bus_dmamap_sync sys/dev/mlx5/mlx5_en/*.c
There are roughly four cases, except for the bidirectional one.
Two for pre-RX and post-RX and two for pre-TX and post-TX. It should be obvious according to the code.
Comment Actions
Can you explain for what reason? Specifically to this Linux compatibility problem or in general?
In general we do "BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE" or "BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE" in gazillions of places in the drivers.
Comment Actions
That is only for the bidirectional case, from my understanding. Some drivers use bi-directional DMA segments. Others not.
Not sure who is the best busdma expert to ask.
I would expect that the non-bidirectional case maps to a single busdma operation, and not multiple.
Comment Actions
I think this is how you need to do it:
In dma_sync_single_for_device: DMA_BIDIRECTIONAL maps to BUS_DMASYNC_PREWRITE DMA_FROM_DEVICE maps to BUS_DMASYNC_PREREAD DMA_TO_DEVICE maps to BUS_DMASYNC_PREWRITE In dma_sync_single_for_cpu: DMA_BIDIRECTIONAL maps to BUS_DMASYNC_POSTREAD then BUS_DMASYNC_PREREAD DMA_FROM_DEVICE maps to BUS_DMASYNC_POSTREAD DMA_TO_DEVICE maps to BUS_DMASYNC_POSTWRITE
Please also note that dma_map_single() and dma_unmap_single() should be patched to sync_single_for_device() and sync_single_for_cpu() respectivly.
Comment Actions
And also linux_dma_map/unmap_sg_attrs() needs to be patched similarly to call memory sync operations based on "dir".
Comment Actions
Update based on various feedbac/request from @hselasky.
I've only done the s/g bit opportunistically.
Comment Actions
The S/G bits not at all.
I have been running with the sync bits for a while and it seemed to make a difference at least the Realtek drivers were behaving a lot better (more reliably) when I came back and tried them.
I am still not sure this is entirely correct and I am not sure how to reliably exercise it to say it is but I went over the man page and code a while back for one of the versions and it seemed so.
Do we need to ask someone else to have a look?