linux-kernel - Re: [PATCH 3/4] riscv: Implement Zicbom-based cache management operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220620061607.GB10485@lst.de>
Date:   Mon, 20 Jun 2022 08:16:07 +0200
From:   Christoph Hellwig <hch@....de>
To:     Heiko Stuebner <heiko@...ech.de>
Cc:     palmer@...belt.com, paul.walmsley@...ive.com,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
        wefu@...hat.com, guoren@...nel.org, cmuellner@...ux.com,
        philipp.tomsich@...ll.eu, hch@....de, samuel@...lland.org,
        atishp@...shpatra.org, anup@...infault.org, mick@....forth.gr,
        robh+dt@...nel.org, krzk+dt@...nel.org, devicetree@...r.kernel.org,
        drew@...gleboard.org, rdunlap@...radead.org,
        Atish Patra <atish.patra@....com>
Subject: Re: [PATCH 3/4] riscv: Implement Zicbom-based cache management
 operations

On Sun, Jun 19, 2022 at 10:32:11PM +0200, Heiko Stuebner wrote:
> +#ifdef CONFIG_RISCV_DMA_NONCOHERENT
> +#define ARCH_DMA_MINALIGN L1_CACHE_BYTES
> +#endif

This needs to be greater or equal to riscv_cbom_block_size, but the
core code requires a compile time constant here.  So we'll need a big
fat comment here, and panic if riscv_cbom_block_size is >
L1_CACHE_BYTES/ARCH_DMA_MINALIGN in the code that queries
riscv_cbom_block_size.

Note that the arm64 folks are looking into making this variable or
killing it off in this current form, so things might be getting better
soon.

> +void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
> +			      enum dma_data_direction dir)
> +{
> +	void *vaddr = phys_to_virt(paddr);
> +
> +	switch (dir) {
> +	case DMA_TO_DEVICE:
> +		ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size);
> +		break;
> +	case DMA_FROM_DEVICE:
> +		ALT_CMO_OP(inval, vaddr, size, riscv_cbom_block_size);
> +		break;

For this also see:

https://lore.kernel.org/all/20220606152150.GA31568@willie-the-truck/

and

https://lore.kernel.org/linux-arm-kernel/20220610151228.4562-1-will@kernel.org/T/

> +void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> +		const struct iommu_ops *iommu, bool coherent)
> +{
> +	dev->dma_coherent = coherent;
> +}

This probably wants a sanity check warn if coherent if false without
any support for cache flushing as that will cause data corruption.