iwn: DMA fixes, and race condition fix

[gonzo]

Summary

This review contains several fixes:

• Fix some dma sync operations to use matching tags
• Perform BUS_DMASYNC_PREWRITE op before accessing ICT table

Test Plan

I ran following sequence for couple of hours:

• load if_iwn
• associate, obtain address and run iperf for 30 seconds
• stop wlan0
• unload if_iwn

I didn't see any "iwn0: device timeout" or BAD_COMMAND firmware errors

[adrian]

weird, how'd we not notice that command race before? How are they trampling on each other? is it some shared buffer, or is it just allocating the same command entry in the TX ring?

[avos]

sys/dev/iwn/if_iwn.c

5120 ↗

(On Diff #22941)

AFAIR, this is wrong; it should be just locked everywhere.

[gonzo]

sys/dev/iwn/if_iwn.c

5120 ↗

(On Diff #22941)

Yes, I thought so and tried to fix accordingly but it may affect performance in non-subtle way and I have only one iwn(4) card to test all code paths. On the other hand - having dedicated mutex for command queue is very non-intrusive fix that makes sure iwn_cmd accesses command slots sequentially

[gonzo]

In D8804#182208, @adrian wrote:

weird, how'd we not notice that command race before? How are they trampling on each other? is it some shared buffer, or is it just allocating the same command entry in the TX ring?

There are several reports in bugzilla that may be relevant to this issue. If two threads call iwn_cmd in parallel they may end up copying cmd data to the same cmd slot, because slot index is increased at the end of the function.

[avos]

sys/dev/iwn/if_iwn.c

5120 ↗

(On Diff #22941)

And, in theory, can also allow to cleanup locking logic (like in rS280071 )

P.S. Probably, writes to IWN_HBUS_TARG_WRPTR register from other threads should be protected too?

[gonzo]

Leave only DMA fixes

[gonzo]

I think locking stuff requires more thought/investigation, so I removed race fix from this review