linux-kernel - Re: [PATCH v12 2/3] 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] [day] [month] [year] [list]

Message-ID: <Z8GJduPzVBZ7ugWv@pollux>
Date: Fri, 28 Feb 2025 11:01:26 +0100
From: Danilo Krummrich <dakr@...nel.org>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: Abdiel Janulgue <abdiel.janulgue@...il.com>,
	Andreas Hindborg <a.hindborg@...nel.org>, aliceryhl@...gle.com,
	robin.murphy@....com, daniel.almeida@...labora.com,
	rust-for-linux@...r.kernel.org, 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>,
	Trevor Gross <tmgross@...ch.edu>,
	Valentin Obst <kernel@...entinobst.de>,
	linux-kernel@...r.kernel.org, Christoph Hellwig <hch@....de>,
	Marek Szyprowski <m.szyprowski@...sung.com>, airlied@...hat.com,
	iommu@...ts.linux.dev
Subject: Re: [PATCH v12 2/3] rust: add dma coherent allocator abstraction.

On Fri, Feb 28, 2025 at 05:35:26PM +0900, Alexandre Courbot wrote:
> On Tue Feb 25, 2025 at 1:27 AM JST, Abdiel Janulgue wrote:
> >
> > On 24/02/2025 16:40, Andreas Hindborg wrote:
> >> "Abdiel Janulgue" <abdiel.janulgue@...il.com> writes:
> >> 
> >> [...]
> >> 
> >>> +/// Inform the kernel about the device's DMA addressing capabilities. This will set the mask for
> >>> +/// both streaming and coherent APIs together.
> >>> +pub fn dma_set_mask_and_coherent(dev: &Device, mask: u64) -> i32 {
> >>> +    // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
> >>> +    unsafe { bindings::dma_set_mask_and_coherent(dev.as_raw(), mask) }
> >>> +}
> >>> +
> >>> +/// Same as `dma_set_mask_and_coherent`, but set the mask only for streaming mappings.
> >>> +pub fn dma_set_mask(dev: &Device, mask: u64) -> i32 {
> >>> +    // SAFETY: device pointer is guaranteed as valid by invariant on `Device`.
> >>> +    unsafe { bindings::dma_set_mask(dev.as_raw(), mask) }
> >>> +}
> >> 
> >> Sorry if it was asked before, I am late to the party. But would it make
> >> sense to put these to functions on `Device` and make them take `&self`.
> >
> > Thanks for checking this. The API is about the dma addressing 
> > capabalities of the device, my thoughts would be to group them with the 
> > rest of the dma API? But either way, I don't have a strong preference. 
> > I'll let others comment.
> 
> FWIW I was about to make the same comment as Andreas. The mask is set on
> a Device, it should thus be part of its implementation.

Yes, this should be Device methods. Please also add them in a separate commit.

> You can still
> keep them with the rest of the DMA API in this file by just adding an
> `impl Device` block here - since Device resides in the same crate, it is
> allowed.

Eventually, the build system will support to move rust code to the corresponding
subsystem entires in separate crates.