Re: [PATCH 2/3] riscv: Implement Zicbom-based cache management operations

From: "Heiko Stübner" <heiko@sntech.de>
To: Christoph Hellwig <hch@lst.de>
Cc: Christoph Hellwig <hch@lst.de>,
	palmer@dabbelt.com, paul.walmsley@sifive.com,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	wefu@redhat.com, guoren@kernel.org, cmuellner@linux.com,
	philipp.tomsich@vrull.eu, samuel@sholland.org,
	atishp@atishpatra.org, anup@brainfault.org, mick@ics.forth.gr,
	robh+dt@kernel.org, krzk+dt@kernel.org,
	devicetree@vger.kernel.org, drew@beagleboard.org,
	Atish Patra <atish.patra@wdc.com>
Subject: Re: [PATCH 2/3] riscv: Implement Zicbom-based cache management operations
Date: Thu, 16 Jun 2022 14:09:47 +0200	[thread overview]
Message-ID: <2041345.KlZ2vcFHjT@diego> (raw)
In-Reply-To: <20220616115342.GA11289@lst.de>

Am Donnerstag, 16. Juni 2022, 13:53:42 CEST schrieb Christoph Hellwig:
> On Thu, Jun 16, 2022 at 11:46:45AM +0200, Heiko Stübner wrote:
> > Without CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE and friends
> > dev_is_dma_coherent() will always return true otherwise the dma_coherent
> > attribute. Hence the "coherent" value for every system not managing things
> > will suddenly show as non-coherent where it showed as coherent before.
> 
> Yes.
> 
> > As we already have detection-points for non-coherent systems (zicbom
> > detection, t-head errata detection)
> 
> No, we don't.  There are plenty of reasons to support Zicbom without
> every having any non-coherent DMA periphals.  Or just some non-coherent
> ones.  So that alone is not a good indicator at all.
> 
> The proper interface for that in DT-based setups i of_dma_is_coherent(),
> which looks at the dma-coherent DT property.  And given that RISC-V
> started out as all coherent we need to follow the powerpc way
> (CONFIG_OF_DMA_DEFAULT_COHERENT) and default to coherent with an
> explcit propery for non-coherent devices, and not the arm/arm64 way
> where non-coherent is the default and coherent devices need the property.

I did look at the dma-coherent-property -> of_dma_is_coherent()
	 -> of_dma_configure_id() -> arch_setup_dma_ops() chain yesterday
which setups the value dev_is_dma_coherent() returns.

The Zicbom extension will only be in new platforms, so none of the currently
supported ones will have that. So my thinking was that we can default to
true in arch_setup_dma_ops() and only use the read coherency value when
actual cache-managment-operations are defined.

My guess was that new platforms implementing cache-management will want
to be non-coherent by default?

So if the kernel is running on a platform not implementing cache-management
dev_is_dma_coherent() would always return true as it does now, while on a
new platform implementing cache-management we'd default to non-coherent
and expect that new devicetree/firmware to specify the coherent devices.