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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <DBDBU8IFSBBR.1KWPOHV33NCPW@nvidia.com>
Date: Wed, 16 Jul 2025 17:12:53 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Danilo Krummrich" <dakr@...nel.org>
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:04 PM JST, Danilo Krummrich wrote:
> 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.

I agree that a dedicated type limiting the possible values to inputs
that make sense would be nice.