[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2025051647-urology-think-b8e0@gregkh>
Date: Fri, 16 May 2025 15:15:14 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: John Hubbard <jhubbard@...dia.com>
Cc: Timur Tabi <timur@...nel.org>, Alexandre Courbot <acourbot@...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>,
Danilo Krummrich <dakr@...nel.org>, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org
Subject: Re: [PATCH] rust: add basic ELF sections parser
On Thu, May 15, 2025 at 12:17:00PM -0700, John Hubbard wrote:
> On 5/15/25 7:30 AM, Timur Tabi wrote:
> > On Thu, May 15, 2025 at 6:43 AM Greg KH <gregkh@...uxfoundation.org> wrote:
> >>> Or if ELF is the problem, I don't mind introducing a WAD loader. ;)
> >>
> >> The "problem" I'm not understanding is why does the kernel have to do
> >> any of this parsing at all?
> >
> > Nova will need to parse ELF headers in order to properly load and boot
> > Nvidia firmware images. Nouveau does this already:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c#n2931
>
> Hi Greg!
>
> Nouveau influenced us heavily here, because having firmware that we
> can post once, and use everywhere (Nouveau and Nova), is very attractive.
>
> Alex and Timur discuss other details that explain why the standard
> user-space approach is less simple and clean than it might appear at
> first glance, but I wanted to emphasize that the firmware re-use point
> a little bit, too.
>
> Oh, and also: the ELF images are going to remain extremely simple,
> because there is nothing now (nor can I see anything in the future)
> that would drive anyone to do complicated things. For example, if
> there is some exotic new thing in the future, it could be put into
> its own firmware image if necessary--because we understand that
> this parser here is intended to be a simple subset of ELF, and
> left alone really.
Ok, then why not just bury this down in the driver that is going to
actually use it? This patch series was adding it to ALL kernels, if you
need/want it or not, and as such would be seen as a generic way to
handle all ELF images. But as that's not the case here, just copy what
you did in the existing C driver and make it private to your code, so
that no one else has to worry about accidentally thinking it would also
work for their code :)
And I still think that having the kernel do this is a mistake, firmware
should always just be a "pass through" otherwise you open yourself up to
all sorts of complexity and vulnerabilities in the kernel, both of which
is generally not a good idea.
thanks,
greg k-h
Powered by blists - more mailing lists