Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

split INTRNG main file into two files #1285

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

split INTRNG main file into two files #1285

wants to merge 9 commits into from

Conversation

ehem
Copy link
Contributor

@ehem ehem commented Jun 11, 2024

Hopefully SSIA. INTRNG presents multiple interfaces and tries to hide some. There seems to be a border between the PIC side and the event side. Split these two apart.

The PIC side bears fair resemblance to what is in x86/nexus.c. I suspect it might be possible to have this take over some of what is done for x86. Theory is to shrink the architecture specific bits.

Copy link
Contributor Author

@ehem ehem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers, I notice intr_event_describe_handler() is one of the few intr_event functions not to the cookie for NULL. As such handling this outside.

@ehem
Copy link
Contributor Author

ehem commented Jun 11, 2024

I'll confess this is mostly meant to suggest there really is another interface being hidden here. Mainly intr_add_handler() and intr_describe() very definitely point to a border. Some drivers would benefit from access to this extra interface.

@ehem
Copy link
Contributor Author

ehem commented Jun 11, 2024

Gah! Keep trying to draw a borderline and some bits are quite unclear.

@ehem
Copy link
Contributor Author

ehem commented Jun 12, 2024

This is an alternative. I'm pretty sure I've got intr_describe_irq() split correctly. Problem is I'm rather less confident of how intr_isrc_assign_cpu() should be split. Then there are a bunch of pieces which could fall on either side.

@ehem
Copy link
Contributor Author

ehem commented Jun 12, 2024

I'm rather unsure where the border goes here. To tell the truth, right now I'm primarily interested in the two at the front. I'm trying to port a driver for an unusual PIC. Due to extra knowledge about its bus, the implementation skips some steps and therefore doesn't match to INTRNG's existing interface.

intr_describe_irq() was mixing two operations together.  There was the
mapping step and the underlying intr_event_describe_handler() call.
Split these two steps apart to match other portions of the interface.

Differential Revision: https://reviews.freebsd.org/D39333
Use of the macro CK_SLIST_EMPTY() on ->is_event->ie_handlers readily
substitues for the value ->is_handlers.  While only minor shrinkage of
struct intsrc, small reductions do accumulate.

Update the i8254 hack to work with this setup.
Making INTRNG's intr_add_handler() act similarly to x86's would be a
bit troublesome.  As such modify x86's functionality.  Currently INTRNG
uses a bit array to indicate processor restriction, so remove the
domain to allow matching.
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.

Differential Revision: https://reviews.freebsd.org/D39333
Handle the situation as a request to update the interrupt table name,
without updating the event name.
intr_teardown_irq() is PIC-level, while intrcnt_updatename() is
interrupt-level.  As such, intr_teardown_irq() should use the next layer
of function, instead of an interrupt-internal function.
intr_isrc_assign_cpu() failed to split the intr_event functionality
from the assignment functionality.  The actual assignment is a PIC-level
operation and as such should be separate.
Using isrc_table_lock for these functions violates layering.  Instead
use pic_list_lock, which isn't quite proper, but matches the layer these
functions are located.
This should serve to truly split things into layers.  Perhaps this could
then be used on x86 and serve to continue unifying interrupt frameworks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant