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

[Heiko Stübner]

Hi,

Am Mittwoch, 15. Juni 2022, 19:49:10 CEST schrieb Christoph Hellwig:
> On Wed, Jun 15, 2022 at 06:56:40PM +0200, Heiko Stübner wrote:
> > If I'm reading things correctly [0], the default for those functions
> > is for those to be empty - but defined in the coherent case.
> 
> That's not the point.
> 
> Zicbom is just an extension that allows the CPU to support managing
> cache state.  Non-coherent DMA is just one of the use cases there
> are others like persistent memory.  And when a CPU core supports
> Zicbom it might or might not have any non-coherent periphals.  Or
> even some coherent and some non-coherent ones, something that
> is pretty common in arm/arm64 CPUs, where PCIe is usually cache
> coherent, but some other cheap periphals might not be.
> 
> That is why Linux ports require the plaform (usually through
> DT or ACPI) to mark which devices are coherent and which ones
> are not.

I "get" it now I think. I was somewhat struggling what you were aiming
at, but that was something of not seeing "the forest for the trees" on
my part. And of course you were right in recognizing that issue :-) .

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.


As we already have detection-points for non-coherent systems (zicbom
detection, t-head errata detection) I guess just also switching some boolean
might solve that, so that arch_setup_dma_ops() will set the dma_coherent
attribute to true always except when some non-coherent system is detected.


void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
                const struct iommu_ops *iommu, bool coherent)
{
        /* only track coherent attributes, if cache-management is available */
        if (enable_noncoherency)
                dev->dma_coherent = coherent;
        else
                dev->dma_coherent = true;
}


Heiko