On Mon, Jan 09, 2023 at 01:59:12PM +0100, Arnd Bergmann wrote: > On Mon, Jan 9, 2023, at 13:03, Lad, Prabhakar wrote: > > On Sun, Jan 8, 2023 at 12:08 AM Arnd Bergmann wrote: > >> >> > +struct riscv_cache_ops { > >> >> > + void (*clean_range)(unsigned long addr, unsigned long size); > >> >> > + void (*inv_range)(unsigned long addr, unsigned long size); > >> >> > + void (*flush_range)(unsigned long addr, unsigned long size); > >> >> > + void (*riscv_dma_noncoherent_cmo_ops)(void *vaddr, size_t size, > >> >> > + enum dma_data_direction dir, > >> >> > + enum dma_noncoherent_ops ops); > >> >> > +}; > >> >> > >> >> I don't quite see how the fourth operation is used here. > >> >> Are there cache controllers that need something beyond > >> >> clean/inv/flush? > >> >> > >> > This is for platforms that dont follow standard cache operations (like > >> > done in patch 5/6) and there drivers decide on the operations > >> > depending on the ops and dir. > >> > >> My feeling is that the set of operations that get called should > >> not depend on the cache controller but at best the CPU. I tried to > >> enumerate how zicbom and ax45 differ here, and how that compares > >> to other architectures: > >> > >> zicbom ax45,mips,arc arm arm64 > >> fromdevice clean/flush inval/inval inval/inval clean/inval > >> todevice clean/- clean/- clean/- clean/- > >> bidi flush/flush flush/inval clean/inval clean/inval I did a bit of digging on lore for context on why the ops are what they are.. In v3 of the Zicbom enablement patchset, things looked like: fromdevice inval/inval todevice clean/- bidi flush/inval v3: https://lore.kernel.org/linux-riscv/20220610004308.1903626-3-heiko@sntech.de/ Samuel had some comments about the invals: https://lore.kernel.org/linux-riscv/342e3c12-ebb0-badf-7d4c-c444a2b842b2@sholland.org/ In v4 it was changed to: fromdevice inval/flush todevice clean/- bidi flush/flush v4: https://lore.kernel.org/linux-riscv/20220619203212.3604485-4-heiko@sntech.de/ Christoph replied to that one, linking the thread belonging to the commit you pointed out earlier: https://lore.kernel.org/linux-riscv/20220620061607.GB10485@lst.de/ v5 produced what you have in your table above: https://lore.kernel.org/linux-riscv/20220629215944.397952-4-heiko@sntech.de/ > >> > >> So everyone does the same operation for DMA_TO_DEVICE, but > >> they differ in the DMA_FROM_DEVICE handling, for reasons I > >> don't quite see: > >> > >> Your ax45 code does the same as arc and mips. arm and > >> arm64 skip invalidating the cache before bidi mappings, > >> but arm has a FIXME comment about that. arm64 does a > >> 'clean' instead of 'inval' when mapping a fromdevice > >> page, which seems valid but slower than necessary. > >> > >> Could the zicbom operations be changed to do the same > >> things as the ax45/mips/arc ones, or are there specific > >> details in the zicbom spec that require this? > >> > > I'll let the RISC-V experts respond here. > > Adding Christoph Hellwig and Will Deacon to Cc as well. > > I had another look at the arm64 side, which (like the zicbom > variant) uses 'clean' on dma_sync_single_for_device(DMA_FROM_DEVICE), > as that has changed not that long ago, see > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c50f11c6196f45c92ca48b16a5071615d4ae0572 > > I'm still not sure what the correct set of operations has > to be, but nothing in that patch description sounds ISA > or even microarchitecture specific. Hope the lore archaeology helps jog people's memories... Conor