On Mon, Oct 28, 2019 at 08:48:41PM +0100, Mark Kettenis wrote: > > Date: Mon, 28 Oct 2019 19:28:29 +0100 > > From: Stefan Sperling <[email protected]> > > > > Newer iwm(4) firmware versions will require paging to host DRAM. > > This diff implements support for this. It was written by Imre Vadasz in > > 2017. > > > > I would like to get review by people with experience in the kernel memory > > subsystem and DMA, to check that this diff isn't doing something stupid. > > I have trouble understanding what the bus_dmamap_sync() calls in > iwm_alloc_fw_paging_mem() are supposed to achieve.
The driver has to initialize these pages with data provided in the firmware image. This is done with memcpy(). I guess these bus_dma_sync calls are supposed to ensure that the correct data appears on the device side when the pages are read from there. Note that the syncs occur both before memcpy() in iwm_fill_paging_mem() and after memcpy() in iwm_send_paging_cmd(). x86-specific behaviour aside, is there any reason why those syncs should not be required? I am a bit worried that iwm_fw_paging() is only called in error paths. Allocated pages should probably be torn down either when the device goes down, or when it is detached. We don't currently implement detach in this driver, so the pages are never freed if no errors occur (the same applies to Rx/Tx rings, so perhaps keeping the allocation persistent is OK). Do you think we should alloc/free these pages whenever we do up/down?
