Page MenuHomeFreeBSD

intrng: expose lower-level device interface for INTRNG
Needs ReviewPublic

Authored by ehem_freebsd_m5p.com on Mar 29 2023, 7:33 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 7, 5:26 AM
Unknown Object (File)
Fri, Oct 4, 5:57 AM
Unknown Object (File)
Tue, Oct 1, 5:17 PM
Unknown Object (File)
Wed, Sep 25, 8:37 AM
Unknown Object (File)
Sun, Sep 22, 7:53 AM
Unknown Object (File)
Sat, Sep 21, 7:38 PM
Unknown Object (File)
Thu, Sep 19, 10:07 PM
Unknown Object (File)
Aug 19 2024, 11:02 PM
Subscribers

Details

Reviewers
markj
imp
mmel
mw
Group Reviewers
Core Team
Summary

Specialized peripheral PIC drivers may need to handle most interrupt
setup steps themselves without touching newbus. Two cases are
intr_add_handler() and intr_describe() which are internally used by
intr_setup_irq()/intr_describe_irq(). Exposing these allow for
alternative use case.

In fact the BUS interface for INTRNG is arguably a layering violation.
Those 6 functions could just as well be in nexus.c or a kernel library.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 58512
Build 55400: arc lint + arc unit

Event Timeline

Any suggestions for reviewers?

sys/kern/subr_intr.c
719–721

Hopefully the reasoning for argument order change is obvious, this is to move closer to matching x86's intr_add_handler() function (doesn't yet match precisely).

mmel requested changes to this revision.May 9 2023, 4:56 AM
mmel added a subscriber: mmel.

I strongly disagree, it's a clear step backwards. Intrng was designed with strong layer separation and you are trying to break it in a similar way to x86.
The consumer interface for interrupts is in the kern_intr.c file and no one else is allowed to use it. The provider interface is intr_isrc_*. All other functions should (and must) remain private or unpublished outside of intrng.

This revision now requires changes to proceed.May 9 2023, 4:56 AM

I am in fact taking inspiration from x86 since the general idea was to fully split the layers apart (move the outermost functions to perhaps sys/kern/subr_nexus.c). Then it might also match x86's needs and push architectures closer together. In particular, D35559 plus D39178 greatly help with merging.

Unfortunately this really isn't a single-person task and needs more guidance to achieve (or perhaps the sluggish handling of patches for FreeBSD has worn me down).

Looks like sys/arm/mv/gpio.c has a situation similar to what I'm looking at. The approach of doing local calls to intr_event_create() has been tested and functions, but this seemed unlikely to be accepted by a reviewer. Yet this makes me the second person to run into troubles with INTRNG due to assumptions in its interface.

Update to current tree status (reflecting GitHub). Now retains INTRNG's argument order for intr_add_handler(). x86 was convinced to match INTRNG and thus the two prototypes are now quite similar.

The interesting part is the combination of D39333 with D35559 and D39178. The result would be most of this low-level interface would be highly similar to x86. If the "BUS" interface was split into a separate file, it might be possible to share this portion between INTRNG and x86 with some work (INTRNG presently has no support for processor domains).

Yet everything needs to start with admitting there is an interface here. Trying to glue this interface shut makes work towards merging the interrupt infrastructures that much harder.

I doubt it is possible to converge the interrupt infrastructures without exposing this interface. This interface is the common point duplicated on every architecture, so things go through here. The way to discourage access to this interface for your alternative interface to provide valuable services which simplify consumers. For my case (Xen) where the drivers already exist, using the higher-level interface would be much harder.

Since all attempts at convergence are basically certain to go through here, the plain interpretation is you (@mmel) are against converging. My guess is you want to rip out the x86 interrupt infrastructure and simply slot INTRNG in. Problem is that would be too drastic a change resulting in too many bugs, so it will never happen.