On Sat, Jan 07, 2023 at 10:21:52PM +0000, Conor Dooley wrote:
> 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..
Should pop up here in a few minutes..
https://lore.kernel.org/linux-riscv/20230108163356.3063839-1-conor@xxxxxxxxxx/
Hopefully that both works & makes life easier. Certainly from a CI
coverage point of view, relaxing toolchain requirements makes *my* life
easier!
Attachment:
signature.asc
Description: PGP signature
