linux-kernel - Re: [PATCH v14 02/11] rust: add dma coherent allocator abstraction.

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: <20250321182539.GP126678@ziepe.ca>
Date: Fri, 21 Mar 2025 15:25:39 -0300
From: Jason Gunthorpe <jgg@...pe.ca>
To: Abdiel Janulgue <abdiel.janulgue@...il.com>
Cc: rust-for-linux@...r.kernel.org, daniel.almeida@...labora.com,
	dakr@...nel.org, robin.murphy@....com, aliceryhl@...gle.com,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Trevor Gross <tmgross@...ch.edu>,
	Valentin Obst <kernel@...entinobst.de>,
	open list <linux-kernel@...r.kernel.org>,
	Christoph Hellwig <hch@....de>,
	Marek Szyprowski <m.szyprowski@...sung.com>, airlied@...hat.com,
	"open list:DMA MAPPING HELPERS" <iommu@...ts.linux.dev>
Subject: Re: [PATCH v14 02/11] rust: add dma coherent allocator abstraction.

On Tue, Mar 11, 2025 at 07:47:58PM +0200, Abdiel Janulgue wrote:
> +pub struct CoherentAllocation<T: AsBytes + FromBytes> {
> +    dev: ARef<Device>,
> +    dma_handle: bindings::dma_addr_t,
> +    count: usize,
> +    cpu_addr: *mut T,
> +    dma_attrs: Attrs,
> +}

I'd like to point out how memory wasteful this is from what real
drivers are doing today when they use the coherent API. Let's compare
against SMMUv3's use for the CD table..

This would be the code in arm_smmu_alloc_cd_ptr()

It is making a 2 level radix tree.

The cpu_addr is stored in a linear array of pointers:

			struct arm_smmu_cdtab_l2 **l2ptrs;

The dma_addr is encoded into the HW data structure itself:

		arm_smmu_write_cd_l1_desc(&cd_table->l2.l1tab[idx],
						  l2ptr_dma);

The size of the allocation is fixed size:
			*l2ptr = dma_alloc_coherent(smmu->dev, sizeof(**l2ptr),
                                                                ^^^^^^^^^^^^
						    &l2ptr_dma, GFP_KERNEL);

It doesn't need a struct device pointer or reference because this uses
the usual kernel 'fence' reasoning for destruction.

It doesn't even use dma_attrs. (why is this in a long term struct?)

So, smmu manages to do this with a single array of 8 bytes/entry to shadow
the CPU pointer, and recovers the dma_addr from the HW data structure:

			dma_free_coherent(smmu->dev,
					  sizeof(*cd_table->l2.l2ptrs[i]),
					  cd_table->l2.l2ptrs[i],
					  arm_smmu_cd_l1_get_desc(&cd_table->l2.l1tab[i]));

Basically, it was designed to be very memory efficient.

If we imagine driving the same HW in rust the array storing the CPU
pointer would have to expand to 40 bytes/entry to hold every
CoherentAllocation. This means rust would need a new high order memory
allocation to hold the CoherentAllocation memory array!

Jason