linux-kernel - Re: [PATCH v6 2/2] 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 for Android: free password hash cracker in your pocket

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

Message-ID: <6e0209be-c58c-4f42-ae0a-eaa016cd05a5@arm.com>
Date: Thu, 5 Dec 2024 16:47:51 +0000
From: Robin Murphy <robin.murphy@....com>
To: Daniel Almeida <daniel.almeida@...labora.com>,
 Abdiel Janulgue <abdiel.janulgue@...il.com>
Cc: 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>, Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>,
 Valentin Obst <kernel@...entinobst.de>,
 "open list:RUST" <rust-for-linux@...r.kernel.org>,
 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 v6 2/2] rust: add dma coherent allocator abstraction.

On 05/12/2024 4:41 pm, Daniel Almeida wrote:
> Hi Abdiel,
> 
>> On 5 Dec 2024, at 11:25, Abdiel Janulgue <abdiel.janulgue@...il.com> wrote:
>>
>> On 05/12/2024 15:07, Robin Murphy wrote:
>>>> +    pub fn alloc_coherent(
>>>> +        dev: &Device,
>>>> +        count: usize,
>>>> +        flags: kernel::alloc::Flags,
>>> Might it be worth adding at least a placeholder type and argument for "attrs" as well, so there's less churn involved in coming back and adding it later? Or is the intention to have a separate dma_alloc_attrs() constructor for such users?
>>
>> It certainly is possible to include attrs in the constructor using Option:
>> ```
>>     pub fn alloc_coherent(
>>         dev: &Device,
>>         count: usize,
>>         flags: kernel::alloc::Flags,
>>         attrs: Option<u64>,
>>     ) -> Result<CoherentAllocation<T>> {
>>
>> <cut>
>>         let ret = unsafe {
>>             bindings::dma_alloc_attrs(
>>                 dev.as_raw(),
>>                 size,
>>                 &mut dma_handle, flags.as_raw(),
>>                 attrs.unwrap_or(0),

Note that you'd also want to store the attrs in the CoherentAllocation 
itself, since they must be passed to the free and (eventually) mmap 
calls too.

>>             )
>>         };
>> <cut>
>>     }
>>
>> ```
>> So the signature of the constructor could be:
>>
>> let c = CoherentAllocation::alloc_coherent(dev, 4, GFP_KERNEL, None)?;
>>
>> Andreas, Miguel, Daniel, any thoughts?
>>
>> Regards,
>> Abdiel
>>
> 
> Instead of a raw u64, you should take inspiration from the work in the Alloc crate and elsewhere to define
> a proper `flags` type, which you can name `attr` in this case.

Indeed, some type-checking would be rather neat, especially given that 
there are actually different sets of DMA attributes which apply to the 
coherent vs. streaming APIs (and no, that isn't very well documented...)

Thanks,
Robin.