[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOZdJXU1ftLfem40v82NJp3S0WqZoMbqYrqQMw4vZEUbpa6Uag@mail.gmail.com>
Date: Sat, 31 May 2025 09:38:24 -0500
From: Timur Tabi <timur@...nel.org>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: Timur Tabi <timur@...nel.org>, Alexandre Courbot <acourbot@...dia.com>,
Danilo Krummrich <dakr@...nel.org>, John Hubbard <jhubbard@...dia.com>, 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>, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org, Timur Tabi <ttabi@...dia.com>
Subject: Re: [PATCH] rust: add basic ELF sections parser
On Sat, May 31, 2025 at 7:25 AM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> What exactly do you mean by this? That is what I have been asking, what
> is the specific reason why this can't be done in userspace? What
> hardware "thing" can't be read by userspace, and why not? Userspace has
> access to PCI devices directly, surely there is nothing "secret" here.
Why in the world would you want user space to read hardware registers,
when the driver is already doing it???????
And please note that the driver has to read and parse a lot of other
register in order to know which register contains the fuse settings,
and even whether that register exists, and at what address. The fuse
register is hardware-specific. It doesn't exist on Turing, it does
exist on Ampere and Ada (but just GA10x, not GA100), and it's not used
on Hopper and Blackwell. You want to duplicate all this code in
user-space (assuming that registers really are accessible in user
space), just avoid a 12-line function that already exists and works in
Nouveau?????????
Please make it make sense, Greg.
> > Please note that there other drivers in Linux that iterate over ELF
> > sections in order to parse their firmware images:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/imagination/pvr_fw_util.c#n29
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c#n925
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/remoteproc/qcom_q6v5_mss.c#n1374
>
> As pointed out before, those have "slipped in" and should not be used to
> justify continuing to do the same thing.
>
> Again, just do it in userspace, if it's "just" 12 lines in the kernel,
> then put those 12 lines in userspace and you are fine.
Those 12 lines are used to determine how to patch the image. We would
need to move all that patching code and all the GPU detection code, as
well as the list of all the relevant GPU registers into user-space.
> And the proposed patch was NOT 12 lines of rust, so please don't
> conflate the two things here. That's not what we are talking about.
We can easily strip it down to the bare minimum and keep it private to
Nova, if you want. After all, that's how Nouveau does this.
Powered by blists - more mailing lists