linux-kernel - Re: [RFC PATCH v3 24/37] kvx: Add memory management

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: <dc2a28b6-566c-4c17-9834-874513f1d4f1@arm.com>
Date: Tue, 30 Jul 2024 14:48:21 +0100
From: Robin Murphy <robin.murphy@....com>
To: Christoph Hellwig <hch@...radead.org>, ysionneau@...rayinc.com
Cc: linux-kernel@...r.kernel.org, Will Deacon <will@...nel.org>,
 "Aneesh Kumar K.V" <aneesh.kumar@...nel.org>,
 Andrew Morton <akpm@...ux-foundation.org>, Nick Piggin <npiggin@...il.com>,
 Peter Zijlstra <peterz@...radead.org>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Jonathan Borne <jborne@...rayinc.com>, Julian Vetter
 <jvetter@...rayinc.com>, Clement Leger <clement@...ment-leger.fr>,
 Guillaume Thouvenin <thouveng@...il.com>,
 Jean-Christophe Pince <jcpince@...il.com>,
 Jules Maselbas <jmaselbas@...v.net>, Julien Hascoet
 <jhascoet@...rayinc.com>, Louis Morhet <lmorhet@...rayinc.com>,
 Marc Poulhiès <dkm@...aplop.net>,
 Marius Gligor <mgligor@...rayinc.com>,
 Vincent Chardon <vincent.chardon@...ys-design.com>,
 linux-arch@...r.kernel.org, linux-mm@...ck.org,
 linux-riscv@...ts.infradead.org
Subject: Re: [RFC PATCH v3 24/37] kvx: Add memory management

On 22/07/2024 3:58 pm, Christoph Hellwig wrote:
>> +#include "../../../drivers/iommu/dma-iommu.h"
> 
> This is not a public header as you can guess from the file path.
> 
>> +	switch (dir) {
>> +	case DMA_TO_DEVICE:
>> +		break;
>> +	case DMA_FROM_DEVICE:
>> +		break;
>> +
>> +	case DMA_BIDIRECTIONAL:
>> +		inval_dcache_range(paddr, size);
> 
> Doing this just for bidirectional is weird unless your architecture
> never does any speculative prefetching.  Other architectures
> include DMA_FROM_DEVICE here.
> 
>> +#ifdef CONFIG_IOMMU_DMA
>> +void arch_teardown_dma_ops(struct device *dev)
>> +{
>> +	dev->dma_ops = NULL;
>> +}
>> +#endif /* CONFIG_IOMMU_DMA*/
> 
> This should not be needed right now.

More than that, per 8b80549f1bc6, it's now actually a latent bug.

>  And will be completley
> useless once we do the direct calls to dma-iommu which we plan
> to do for Linux 6.12.
> 
>> +void arch_setup_dma_ops(struct device *dev, bool coherent)
>> +{
>> +	dev->dma_coherent = coherent;
>> +	if (device_iommu_mapped(dev))
>> +		iommu_setup_dma_ops(dev);
>> +}
> 
> And this seems odd, as iommu_setup_dma_ops is called from the iommu
> code and you shouldn't need it here.

Yeah, this smells like it was based on the old arm64 code, but then 
rebased without reference to the equivalent arm64 changes, hence the 
#include hack. Enabling iommu-dma for an architecture should now be a 
one-liner in drivers/iommu/Kconfig, however I don't see an IOMMU driver 
being added in this series so it's not clear it's actually necessary (yet).

Thanks,
Robin.