Message-ID: <msbgnz45owgcnx2q5e6kamhsfmslyed6kqbrprrttkoeruradr@nd3rbstjxds5>
Date: Tue, 16 Dec 2025 16:23:53 -0800
From: Bjorn Andersson <andersson@...nel.org>
To: Matthew Maurer <mmaurer@...gle.com>
Cc: Konrad Dybcio <konradybcio@...nel.org>, 
	Satya Durga Srinivasu Prabhala <satyap@...cinc.com>, Miguel Ojeda <ojeda@...nel.org>, 
	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>, 
	Trilok Soni <tsoni@...cinc.com>, linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org, 
	rust-for-linux@...r.kernel.org
Subject: Re: [PATCH RFC] soc: qcom: socinfo: Re-implement in Rust

On Tue, Dec 16, 2025 at 03:32:53PM -0800, Matthew Maurer wrote:
> On Tue, Dec 16, 2025 at 3:05 PM Bjorn Andersson <andersson@...nel.org> wrote:
> >
> > On Tue, Dec 16, 2025 at 02:13:08PM -0800, Matthew Maurer wrote:
> > > On Tue, Dec 16, 2025 at 1:53 PM Bjorn Andersson <andersson@...nel.org> wrote:
> > > >
> > > > On Tue, Dec 16, 2025 at 12:53:28PM -0800, Matthew Maurer wrote:
> > > > > On Tue, Dec 16, 2025 at 12:49 PM Bjorn Andersson <andersson@...nel.org> wrote:
> > > > > >
> > > > > > On Sat, Dec 13, 2025 at 12:36:00AM +0000, Matthew Maurer wrote:
> > > > > > > Re-implements qcom-socinfo driver in Rust, using `Scoped`-based DebugFS
> > > > > > > bindings.
> > > > > >
> > > > > > Okay, but why?
> > > > > >
> > > > > > https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > > > > >
> > > > > > [..]
> > > > >
> > > > > I'll move more of the text from the cover letter into the commit
> > > > > message, but the short version is that it's intended to be a low-risk
> > > > > driver to pilot vendor drivers in Android.
> > > > >
> > > >
> > > > The answers I'm looking for isn't in your cover letter either. I want to
> > > > know what benefits Rust provides in this particular case.
> > > >
> > >
> > > Rust likely does not provide any significant benefits for the socinfo
> > > driver itself, aside from having caught the slight disconnect
> > > (mentioned on the other thread) where it should probably be an
> > > auxiliary device because it assumes that the `qcom-smem` device will
> > > remain active as long as it does.
> > >
> >
> > Yes, this isn't unique to Rust. The smem API would benefit from an
> > overhaul in general... At the time I wrote it, you couldn't really boot
> > the system without SMEM, so I added the suppress_bind_attrs, these days
> > you can exercise a fair amount of use cases without it.
> >
> > > The primary intention of converting this driver is to be a low risk
> > > trial for GKI's ABI stability for vendor modules and a first Rust
> > > vendor module (we have platform modules already for binder and ashmem)
> > > on Android. This was discussed informally with Trilok (a long time
> > > ago) to act as an example for other vendor drivers and encourage usage
> > > of Rust for new drivers or those where memory safety has been a
> > > problem in the past, but nothing was ever formally committed to.
> >
> > But if the test bed for GKI ABI stability is the goal, don't you need
> > something with a more interesting interface?
> 
> We've been watching things manually for the 6.12 release and manually
> backtested on the 6.6 release (by looking at our ABI dumps) and
> believe that it works. The only way we can really progress further is
> by deploying a vendor driver in the field.

But this is the upstream kernel, you're not contributing to 6.6, 6.12,
or even 6.18. If we merge this rewrite for v6.20 it will be deploying
into the field in 2027.

Perhaps I'm misunderstanding what you're saying here though?

> If we pick something more
> interesting, there was concern around it being caught up on concerns
> related to the specific driver. The hope was that this driver would
> simultaneously be real enough to count as a driver in the eyes of
> other vendors watching for someone to take the lead, and simple enough
> to be low risk enough to be possible.
> 

Are you talking about upstream drivers, or downstream vendor drivers?

> We do have more interesting drivers in process, e.g. the tyr graphics
> driver, but those are longer term efforts.
> 
> >
> > Also, how would this manifest itself in the upstream kernel? Do you have
> > a test bed where you build GKI + vendor "kernel" out of the upstream
> > kernel tree today?
> 
> In Android, kernel modules are split into two categories - "GKI
> modules", which get shipped with the primary kernel to every system,
> and vendor modules, which are shipped and loaded by particular
> devices. Neither current builtin modules [1] nor current GKI modules
> [2] contain the qcom socinfo, so it is a vendor module. This
> categorization is not an "in-tree" / "out of tree" categorization, but
> a "every Android device" / "some Android devices" one.
> 

Forgive me, but I'm reading this as "Android downstream" + "Qualcomm
downstream", my question was how this relates to the upstream kernel.

> [1] https://ci.android.com/builds/submitted/14027866/kernel_aarch64/latest/modules.builtin
> [2] https://android.googlesource.com/kernel/common/+/refs/heads/android16-6.12/modules.bzl
> >
> > > Trying to apply Rust to more significant existing modules (where it
> > > would provide a greater benefit) raises concern about whether they
> > > will operate identically. Socinfo in particular can be more thoroughly
> > > tested for equivalency, as aside from the runtime DSP information
> > > modification that was mentioned in this discussion, the output of the
> > > socinfo driver is static and can just be checked. This means that a
> > > socinfo driver swap will essentially have the language change be the
> > > *only* significant thing about the change, keeping the risk to a
> > > minimum.
> > >
> > > Part of the reason for the RFC prefix on this patch is to solicit your
> > > feedback on whether you are open to this. If you have somewhere else
> > > you'd prefer we try this, I'd be open to that as well.
> > >
> >
> > Rust is a new dependency and it's a foreign language to the vast
> > majority of the contributors in the upstream Qualcomm community.
> >
> > So, IMHO sufficient value needs to come with such rewrite, and this
> > value needs to be properly documented in the git history. Rewriting
> > things for the sake of rewriting things, or the facilitate a downstream
> > experiment, is hard for me to force onto our community.
> 
> Since you brought it up, I took a brief look and found CVE-2024-58007
> [3] , which would have been prevented through use of Rust, as the
> access would have been bounds-checked. I understand that such
> vulnerabilities in socinfo.c are likely to be uncommon, but even for a
> driver this simple memory safety issues do happen, and have even
> happened recently.
> 

Yes, this is exactly the kind of argumentation I would like to see.

This is something we can weigh against the added cost of the Rust
implementation.

Regards,
Bjorn

> [3]: https://nvd.nist.gov/vuln/detail/CVE-2024-58007
> >
> > Regards,
> > Bjorn
> >
> > > > > >
> > > > > > > ---
> > > > > > > base-commit: 4b31c90f86768367c84cc6e99e1deee0ec55197b
> > > > > > > change-id: 20251029-qcom-socinfo-d8387c7fdb1c
> > > > > > > prerequisite-change-id: 20251029-soc-bindings-9b0731bcdbed:v1
> > > > > > > prerequisite-patch-id: e4da423ddabec93bd9a64004691266f9b740e0c5
> > > > > > > prerequisite-patch-id: 5097ff622f8cb1d38354674cf70c1c946ac87f6c
> > > > > > > prerequisite-change-id: 20251029-add-entropy-f57e12ebe110:v1
> > > > > > > prerequisite-patch-id: 657de204912d2c5efff9898d3f4c51e52684d8e6
> > > > > > > prerequisite-change-id: 20251212-transmute-8ab6076700a8:v1
> > > > > > > prerequisite-patch-id: 4f5f7cb002d02d232083ab5c3ce6b3cb90650bd6
> > > > > > > prerequisite-patch-id: 01a1f3d727e549c173dd142180741f5b6a3c866e
> > > > > > > prerequisite-patch-id: ff801a7ae439876b1554b4d1132d94c825bbe74f
> > > > > >
> > > > > > So, it doesn't work on a clean v6.19-rc1 tree?
> > > > >
> > > > > Correct. Those other patchsets are patchsets I'm landing which add
> > > > > bindings used in this one.
> > > > >
> > > >
> > > > Okay, please make that explicit when submitting patches, so that the
> > > > maintainers know that your patch(es) can't be merged.
> > >
> > > I thought that was what I had done by using the combination of "RFC"
> > > and putting the prerequisite change IDs from b4, but I will include a
> > > statement to that effect in my cover letter in the future.
> > >
> > > >
> > > > Regards,
> > > > Bjorn
> > > >
> > > > > >
> > > > > > Regards,
> > > > > > Bjorn
> > > > > >
> > > > > > >
> > > > > > > Best regards,
> > > > > > > --
> > > > > > > Matthew Maurer <mmaurer@...gle.com>
> > > > > > >

Powered by blists - more mailing lists