linux-kernel - Re: [PATCH 1/2] rust: Add initial interconnect framework abstractions

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: <aaf6dff3-30fc-44a9-b88f-da7347e0b7d4@oss.qualcomm.com>
Date: Wed, 23 Jul 2025 13:53:56 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: Konrad Dybcio <konradybcio@...nel.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 <lossin@...nel.org>,
        Andreas Hindborg <a.hindborg@...nel.org>,
        Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
        Danilo Krummrich <dakr@...nel.org>, Georgi Djakov <djakov@...nel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
        Bjorn Andersson <bjorn.andersson@....qualcomm.com>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
        linux-pm@...r.kernel.org
Subject: Re: [PATCH 1/2] rust: Add initial interconnect framework abstractions

On 7/23/25 1:41 PM, Miguel Ojeda wrote:
> On Wed, Jul 23, 2025 at 1:32 PM Konrad Dybcio
> <konrad.dybcio@....qualcomm.com> wrote:
>>
>> The C framework makes no effort to check for that, so panicking is at
>> least something.. That said, what would you suggest to do here?
> 
> If you want to mimic the C side, then you will need to use one of the
> non-panicking operations, such as e.g. `wrapping_mul()`.
> 
> Otherwise, you could make it a fallible method, i.e. return `Result`.
> 
> Otherwise, I think the panic should be documented in the docs of the
> methods (because callers then really need to be careful).
> 
> Which option to takes depends a bit on the use case and what C
> maintainers consider best for a particular operation.
> 
> For instance, sometimes people have used `build_assert!` because they
> expect that the value is always known at compile-time (after
> optimizations).

build_assert sounds nice, but the value is often retrieved dynamically
(e.g. from the device tree itself). Result seems like the best option.
I'll also add a verse in the docs about the return variants.

> 
>> I was debating that. icc_path represents the interconnect consumer part
>> (i.e. used in device drivers that just need to toggle a bus endpoint),
>> whereas the corresponding provider part (which manages said bus) is not
>> yet abstracted.
>>
>> It would make logical sense to split these two.. with the latter going
>> to icc_provider.rs, perhaps?
> 
> Ah, so I just meant that you could have the `icc_path` as a `mod
> icc_path;`, and move it to its own file, rather than inline. Other
> reorganizations makes sense, but I was only suggesting that :)

So, IccBwUnit stays in icc.rs and icc_path -> icc_path.rs?

> 
>> No, it compiles fine here.. Strangely, I didn't get any warnings or
>> errors with this patch. Maybe because the struct is pub and within the
>> same file?
> 
> It likely happens if `CONFIG_INTERCONNECT` is not set, because then
> the entire module above is gone.

OK that makes sense!

> 
>> I almost wanna say `make rustfmt` produced slightly different results
>> (one or two lines of difference) than make rust-analyzer + vscode
>> extension.. hmm.. Perhaps PEBKAC..
> 
> In mainline we currently enforce that code is formatted with `make ...
> rustfmt` (there is `make ... rustfmtcheck` to check, too), so if some
> extension gives you different formatting, please double-check before
> submitting the code.

I assumed that since the extension name is the same as the make target,
they were meant to be ;)

Konrad