Re: [RFC PATCH v6 1/6] riscv: mm: dma-noncoherent: Switch using function pointers for cache management

From: Conor Dooley <conor@kernel.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Prabhakar <prabhakar.csengg@gmail.com>,
	"Conor.Dooley" <conor.dooley@microchip.com>,
	"Geert Uytterhoeven" <geert+renesas@glider.be>,
	"Heiko Stübner" <heiko@sntech.de>, guoren <guoren@kernel.org>,
	"Andrew Jones" <ajones@ventanamicro.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"open list:RISC-V ARCHITECTURE" <linux-riscv@lists.infradead.org>,
	"open list" <linux-kernel@vger.kernel.org>,
	devicetree@vger.kernel.org,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"Lad, Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	"Philipp Tomsich" <philipp.tomsich@vrull.eu>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Atish Patra" <atishp@rivosinc.com>,
	"Anup Patel" <apatel@ventanamicro.com>,
	"Tsukasa OI" <research_trasio@irq.a4lg.com>,
	"Jisheng Zhang" <jszhang@kernel.org>,
	"Mayuresh Chitale" <mchitale@ventanamicro.com>
Subject: Re: [RFC PATCH v6 1/6] riscv: mm: dma-noncoherent: Switch using function pointers for cache management
Date: Sat, 7 Jan 2023 22:21:52 +0000	[thread overview]
Message-ID: <Y7nwgEs3n8kzPq5X@spud> (raw)
In-Reply-To: <fb3b34ae-e35e-4dc2-a8f4-19984a2f58a8@app.fastmail.com>

[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]

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@bp.renesas.com>
> >> > +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@ventanamicro.com/

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]