LinuxKPI: implement dma_sync_single_for_{cpu,device}

[bz]

Summary

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

[bz]

This needs careful review on BUS_DMASYNC_ flags as I constantly got confused trying to map them.

[• hselasky]

sys/compat/linuxkpi/common/include/linux/dma-mapping.h

216

I think this is wrong. You need to do this in two sync operations.

See sys/dev/usb/usb_busdma.c .

[bz]

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?

[• hselasky]

sys/compat/linuxkpi/common/include/linux/dma-mapping.h

218

Missing "break" statements here.

250

ditto

[• hselasky]

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.

[bz]

Take a break take a ...

[• hselasky]

Could you point to some places in driver code where these functions are used? I just want to verify the BUSDMA flags vs direction.

[bz]

In D32255#728733, @hselasky wrote:

Could you point to some places in driver code where these functions are used? I just want to verify the BUSDMA flags vs direction.

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.

[• hselasky]

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.

[bz]

In D32255#728739, @hselasky wrote:

Hi,

I think you cannot pass multiple flags to bus_dmamap_sync() like you do.

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.

[• hselasky]

In D32255#728749, @bz wrote:

In D32255#728739, @hselasky wrote:

Hi,

I think you cannot pass multiple flags to bus_dmamap_sync() like you do.

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.

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.

[• hselasky]

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.

[• hselasky]

And also linux_dma_map/unmap_sg_attrs() needs to be patched similarly to call memory sync operations based on "dir".

[• hselasky]

bz: Ping

[bz]

In D32255#730145, @hselasky wrote:

bz: Ping

I'll try to get back to you about this tomorrow.

[bz]

Update based on various feedbac/request from @hselasky.

I've only done the s/g bit opportunistically.

[• hselasky]

Were you able to test this?

[bz]

In D32255#776531, @hselasky wrote:

Were you able to test this?

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?

[• hselasky]

I think this is good.