linux-kernel - Re: [PATCH 2/5] rust: dma: add DMA addressing capabilities

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <DBDBNRC9VEU5.2MXQF7KLR2HA7@kernel.org>
Date: Wed, 16 Jul 2025 10:04:26 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Alexandre Courbot" <acourbot@...dia.com>
Cc: <abdiel.janulgue@...il.com>, <daniel.almeida@...labora.com>,
 <robin.murphy@....com>, <a.hindborg@...nel.org>, <ojeda@...nel.org>,
 <alex.gaynor@...il.com>, <boqun.feng@...il.com>, <gary@...yguo.net>,
 <bjorn3_gh@...tonmail.com>, <lossin@...nel.org>, <aliceryhl@...gle.com>,
 <tmgross@...ch.edu>, <bhelgaas@...gle.com>, <kwilczynski@...nel.org>,
 <gregkh@...uxfoundation.org>, <rafael@...nel.org>,
 <rust-for-linux@...r.kernel.org>, <linux-pci@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/5] rust: dma: add DMA addressing capabilities

On Wed Jul 16, 2025 at 5:18 AM CEST, Alexandre Courbot wrote:
> On Fri Jul 11, 2025 at 4:45 AM JST, Danilo Krummrich wrote:
>> @@ -18,7 +18,83 @@
>>  /// The [`dma::Device`](Device) trait should be implemented by bus specific device representations,
>>  /// where the underlying bus is DMA capable, such as [`pci::Device`](::kernel::pci::Device) or
>>  /// [`platform::Device`](::kernel::platform::Device).
>> -pub trait Device: AsRef<device::Device<Core>> {}
>> +pub trait Device: AsRef<device::Device<Core>> {
>> +    /// Set up the device's DMA streaming addressing capabilities.
>> +    ///
>> +    /// This method is usually called once from `probe()` as soon as the device capabilities are
>> +    /// known.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// This method must not be called concurrently with any DMA allocation or mapping primitives,
>> +    /// such as [`CoherentAllocation::alloc_attrs`].
>
> I'm a bit confused by the use of "concurrently" in this sentence. Do you
> mean that it must be called *before* any DMA allocation of mapping
> primitives? In this case, wouldn't it be clearer to make the order
> explicit?

Setting the mask before any DMA allocations might only be relevant from a
semantical point of view, but not safety wise.

We need to prevent concurrent accesses to dev->dma_mask and
dev->coherent_dma_mask.

>> +    unsafe fn dma_set_mask(&self, mask: u64) -> Result {
>
> Do we want to allow any u64 as a valid mask? If not, shall we restrict
> the accepted values by taking either the parameter to give to
> `dma_bit_mask`, or a bit range (similarly to Daniel's bitmask series
> [1], which it might make sense to leverage)?
>
> [1]
> https://lore.kernel.org/rust-for-linux/20250714-topics-tyr-genmask2-v9-1-9e6422cbadb6@collabora.com/

I think it would make sense to make dma_bit_mask() return a new type, e.g.
DmaMask and take this instead.

Taking the parameter dma_bit_mask() takes directly in dma_set_mask() etc. makes
sense, but changes the semantics of the mask parameter *subtly* compared to the
C versions, which I want to avoid.

Using the infrastructure in [1] doesn't seem to provide much value, since we
don't want a range [M..N], but [0..N], so we should rather only ask for N.