On Sat, Jan 07, 2023 at 10:52:55PM +0100, Arnd Bergmann wrote: > On Sat, Jan 7, 2023, at 00:29, Conor Dooley wrote: > > On Fri, Jan 06, 2023 at 11:31:33PM +0100, Arnd Bergmann wrote: > >> On Fri, Jan 6, 2023, at 19:55, Prabhakar wrote: > >> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > >> > +struct riscv_cache_ops zicbom_cmo_ops = { > >> > + .clean_range = &zicbom_cmo_clean_range, > >> > + .inv_range = &zicbom_cmo_inval_range, > >> > + .flush_range = &zicbom_cmo_flush_range, > >> > +}; > >> > +#else > >> > +struct riscv_cache_ops zicbom_cmo_ops = { > >> > + .clean_range = NULL, > >> > + .inv_range = NULL, > >> > + .flush_range = NULL, > >> > + .riscv_dma_noncoherent_cmo_ops = NULL, > >> > +}; > >> > +#endif > >> > +EXPORT_SYMBOL(zicbom_cmo_ops); > >> > >> Same here: If the ZICBOM ISA is disabled, nothing should > >> reference zicbom_cmo_ops. > > > >> Also, since ZICBOM is a standard > >> extension, I think it makes sense to always have it enabled, > >> at least whenever noncoherent DMA is supported, that way > >> it can be the default that gets used in the absence of any > >> nonstandard cache controller. > > > > While I think of it, this is not possible as Zicbom requires toolchain > > support whereas the alternative methods for non-coherent DMA do not. > > Ah, I see. Would it be possible to use the same .long trick > as in the other ones though? Something like > > #if CONFIG_AS_VERSION >= 23600 /* or whichever version */ > /* proper inline asm */ > #else > /* .long hack */ > #endif > > That way everyone can use it, and the hack would automatically > go away in a few years after linux requires a newer toolchain. > Alternatively, the entire noncoherent-dma support could be > made to depend on whichever toolchain introduced Zicbom. Ehh, I don't think that's a great idea. It'd require far too recent a toolchain IMO. Ideally, in my opinion, we'd just do something like what Drew has proposed for Zicboz, negating the need for a check at all: https://lore.kernel.org/linux-riscv/20221027130247.31634-4-ajones@xxxxxxxxxxxxxxxx/ Been waiting for that to be re-spun and Palmer to accept it before doing the same thing for Zicbom. At present we have this in the arch Kconfig: config TOOLCHAIN_HAS_ZICBOM bool default y depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicbom) depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicbom) depends on LLD_VERSION >= 150000 || LD_VERSION >= 23800 config RISCV_ISA_ZICBOM bool "Zicbom extension support for non-coherent DMA operation" depends on TOOLCHAIN_HAS_ZICBOM The linker version check is entirely due to the linker having issues if it sees zicbom in the ISA string in object files. I'd been intending to do that for Zicbom anyway, so I guess I'll just go do it & Prabhakar can attach it to his v7.. Thanks, Conor.
Attachment:
signature.asc
Description: PGP signature