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

Message-ID: <aFAfhsuvEQFOd4MJ@pollux>
Date: Mon, 16 Jun 2025 15:43:34 +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: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.

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

vs.

	// `req` is an `IrqRequest`.
	let req = pdev.irq_by_name()?;

	// `irq` is an `irq::Registration`
	let irq = irq::ThreadedRegistration::new(req)?;