> Date: Tue, 29 Oct 2019 11:31:31 +0100 > From: Stefan Sperling <[email protected]> > > 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?
Unless the deviceactually writes to those pages, the syncs before the memcpy() should not be necessary. You do need to sync after the memcpy() although on x86 that is mostly for the compiler's benefit, to make sure it doesn't reorder things in a way that pokes the hardware before the memcpy() is done. > 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? That might not be the best approach as you run the risk that allocations will fail if you up the interface again later on. This is more likely to be an issue if you need phys contig memory, as such allocations may fail even when there is plenty of free memory due to fragmentations.
