linux-kernel - Re: [PATCH v4 4/6] rust: irq: add support for threaded IRQs and handlers

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] [day] [month] [year] [list]

Message-ID: <aFBZOWt_W2Gku5ZW@pollux>
Date: Mon, 16 Jun 2025 19:49:45 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Daniel Almeida <daniel.almeida@...labora.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>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Trevor Gross <tmgross@...ch.edu>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Benno Lossin <lossin@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>,
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	linux-pci@...r.kernel.org
Subject: Re: [PATCH v4 4/6] rust: irq: add support for threaded IRQs and
 handlers

On Mon, Jun 16, 2025 at 03:43:40PM +0200, Danilo Krummrich wrote:
> On Mon, Jun 16, 2025 at 03:33:06PM +0200, Alice Ryhl wrote:
> > On Mon, Jun 9, 2025 at 8:13 PM Danilo Krummrich <dakr@...nel.org> wrote:
> > > I think with specialization it'd be trivial to generalize, but this isn't
> > > stable yet. The enum approach is probably unnecessarily complicated, so I agree
> > > to leave it as it is.
> > >
> > > Maybe a comment that this can be generalized once we get specialization would be
> > > good?
> > 
> > Specialization is really far out. I don't think we should try to take
> > it into account when designing things today. I think that the
> > duplication in this case is perfectly acceptable and trying to
> > deduplicate makes things too hard to read.
> 
> As mentioned above, I agree with the latter. But I think leaving a note that
> this could be deduplicated rather easily with specialization probably doesn't
> hurt?
> 
> > > I'm thinking of something like
> > >
> > >         /// # Invariant
> > >         ///
> > >         /// `ěrq` is the number of an interrupt source of `dev`.
> > >         struct IrqRequest<'a> {
> > >            dev: &'a Device<Bound>,
> > >            irq: u32,
> > >         }
> > >
> > > and from the caller you could create an instance like this:
> > >
> > >         // INVARIANT: [...]
> > >         let req = IrqRequest { dev, irq };
> > >
> > > I'm not sure whether this needs an unsafe constructor though.
> > 
> > The API you shared would definitely work. It pairs the irq number with
> > the device it matches. Yes, I would probably give it an unsafe
> > constructor, but I imagine that most methods that return an irq number
> > could be changed to just return this type so that drivers do not need
> > to use said unsafe.
> 
> Driver don't need to use unsafe already. It's only the IRQ accessors in this
> patch series (in platform.rs and pci.rs) that are affected.
> 
> Let's also keep those accessors, from a driver perspective it's much nicer to
> have an API like this, i.e.

Just to clarify, I meant to additionally keep the accessors, since

> 
> 	// `irq` is an `irq::Registration`
> 	let irq = pdev.threaded_irq_by_name()?

this should be the most common case.

> 
> vs.
> 
> 	// `req` is an `IrqRequest`.
> 	let req = pdev.irq_by_name()?;
> 
> 	// `irq` is an `irq::Registration`
> 	let irq = irq::ThreadedRegistration::new(req)?;

But this can be useful as well, e.g. if a driver can handle devices from
multiple busses, the driver could obtain the IrqRequest and pass it down to bus
independent layers of the driver which then create the final irq::Registration.