illumos/illumos-ddi
Watch
2
0
Fork
You've already forked illumos-ddi
 0
Code Issues Pull requests Projects Releases 1 Packages Wiki Activity Actions

Framework modules: CharTable glue, Driver::load, unsafe cb_ops; private current_context/servicing_interrupt/module_loaded #8

Merged
toasterson merged 13 commits from r5a-char-table into main 2026-10-05 16:35:04 +00:00
Conversation 0 Commits 13 Files changed 12 +1125 -141
toasterson commented 2026-10-03 16:46:04 +00:00
Owner
Copy link

Framework-module support for illumos-drm R5a (misc/drm_core), stacked on #7 (g1-gate-pin).

illumos-drm's DRM core became one kernel module, misc/drm_core, that serves the character devices its driver modules attach and exports a C facade. This branch adds what such a framework module needs from illumos-ddi, under the panic-to-type rule (docs/DESIGN.md §5.12 rows for every new rule).

Public layer

  • module: CharTable (unsafe trait) and table_glue — entry points for devices a framework module serves, found by device number instead of the module's own soft state; the module! { driver: X, cb_ops: unsafe EXPR } form (the unsafe is the caller's promise that the pointer is a valid, 'static cb_ops whose entry points find its devices; compile_fail doctest for a bare pointer).
  • Driver::load(&Sleepable) -> Result<(), Errno>: a check in _init(9E) before anything is set up (illumos-drm's drivers check the facade version there).
  • TaskQueue::without_node (a module without a device node).
  • IntrPriority::new refuses 0 and every priority at or above ddi_intr_get_hilevel_pri() (host tests of the bound, an rselftest check); IntrError::HighLevel/VirtioError::HighLevel carry the raw u32, so no high-level IntrPriority can be taken out of an error.

Private tier (unstable-gate-cda5b9a49f)

  • private::fd::FileRef::char_dev.
  • private::current_context is an unsafe fn whose contract names the contexts servicing_interrupt cannot tell from a thread (callouts, quiesce(9E), chpoll(9E), raised PIL, spin locks held); compile_fail doctest.
  • private::servicing_interrupt(): the context question without minting a token.
  • private::module_loaded(modname, &Sleepable): whether a module is loaded, without loading it (the DDI has none: ddi_modopen(9F) loads). Walks modules under mod_lock by module name (the framework's own identity; a module's file name is whatever path its latest load used), takes no hold (a hold waits on a busy module, so two modules guarding against each other could deadlock), calls nothing with the lock held. New illumos-kernel-private-sys::modctl: modctl_t (size 144, 25 offsets), mod_lock, modules, cited at cda5b9a49f. Host tests (name matching, flags), compile_fail doctests (no token, an Atomic token), rselftest checks (genunix and unix loaded, a made-up name not).

Evidence

  • Local: fmt; clippy in all CI configurations and rselftest private-gate; ddi lib tests 12 / 18 (virtio feature); doctests 34 passed, 7 ignored on stable and on the pinned nightly (error codes checked); cargo doc -D warnings; lint-man clean for the new citations.
  • Guest (illumos-drm's hephaistos guest, golden-20260930, illumos 7e802e88a3, 3 Oct 2026): illumos-drm's cargo xtask kernel-check verified the private tier with misc/drm_core loaded — illumos-kernel-private-sys: sizes 3/3, offsets 46/46, symbols 15/15 (the modctl layout, mod_lock, modules); misc/drm_core's _init refused with module_loaded(c"drm") true while gfx-drm's misc/drm was loaded, and loaded once it was gone; the char-table glue, Driver::load and cb_ops: unsafe ran in illumos-drm's full R5a suite (167 checks).
  • Not run: the in-kernel rselftest checks added here, and the CI kmod-illumos job (owner's CI).

Review

Two review rounds against the anima-review bar (with the illumos-drm R5a branch). Round 1 findings fixed here: B3a (current_context unsafe), B3b (IntrPriority::new bound), B3c (cb_ops: unsafe).

Framework-module support for illumos-drm R5a (`misc/drm_core`), stacked on #7 (`g1-gate-pin`). illumos-drm's DRM core became one kernel module, `misc/drm_core`, that serves the character devices its driver modules attach and exports a C facade. This branch adds what such a framework module needs from illumos-ddi, under the panic-to-type rule (docs/DESIGN.md §5.12 rows for every new rule). ## Public layer - `module`: `CharTable` (unsafe trait) and `table_glue` — entry points for devices a framework module serves, found by device number instead of the module's own soft state; the `module! { driver: X, cb_ops: unsafe EXPR }` form (the `unsafe` is the caller's promise that the pointer is a valid, 'static `cb_ops` whose entry points find its devices; `compile_fail` doctest for a bare pointer). - `Driver::load(&Sleepable) -> Result<(), Errno>`: a check in `_init(9E)` before anything is set up (illumos-drm's drivers check the facade version there). - `TaskQueue::without_node` (a module without a device node). - `IntrPriority::new` refuses 0 and every priority at or above `ddi_intr_get_hilevel_pri()` (host tests of the bound, an rselftest check); `IntrError::HighLevel`/`VirtioError::HighLevel` carry the raw `u32`, so no high-level `IntrPriority` can be taken out of an error. ## Private tier (`unstable-gate-cda5b9a49f`) - `private::fd::FileRef::char_dev`. - `private::current_context` is an `unsafe fn` whose contract names the contexts `servicing_interrupt` cannot tell from a thread (callouts, `quiesce(9E)`, `chpoll(9E)`, raised PIL, spin locks held); `compile_fail` doctest. - `private::servicing_interrupt()`: the context question without minting a token. - `private::module_loaded(modname, &Sleepable)`: whether a module is loaded, without loading it (the DDI has none: `ddi_modopen(9F)` loads). Walks `modules` under `mod_lock` by module name (the framework's own identity; a module's file name is whatever path its latest load used), takes no hold (a hold waits on a busy module, so two modules guarding against each other could deadlock), calls nothing with the lock held. New `illumos-kernel-private-sys::modctl`: `modctl_t` (size 144, 25 offsets), `mod_lock`, `modules`, cited at `cda5b9a49f`. Host tests (name matching, flags), `compile_fail` doctests (no token, an `Atomic` token), rselftest checks (genunix and unix loaded, a made-up name not). ## Evidence - Local: fmt; clippy in all CI configurations and rselftest `private-gate`; `ddi` lib tests 12 / 18 (virtio feature); doctests 34 passed, 7 ignored on stable and on the pinned nightly (error codes checked); `cargo doc -D warnings`; `lint-man` clean for the new citations. - Guest (illumos-drm's hephaistos guest, golden-20260930, illumos `7e802e88a3`, 3 Oct 2026): illumos-drm's `cargo xtask kernel-check` verified the private tier with `misc/drm_core` loaded — `illumos-kernel-private-sys: sizes 3/3, offsets 46/46, symbols 15/15` (the `modctl` layout, `mod_lock`, `modules`); `misc/drm_core`'s `_init` refused with `module_loaded(c"drm")` true while gfx-drm's `misc/drm` was loaded, and loaded once it was gone; the char-table glue, `Driver::load` and `cb_ops: unsafe` ran in illumos-drm's full R5a suite (167 checks). - Not run: the in-kernel rselftest checks added here, and the CI `kmod-illumos` job (owner's CI). ## Review Two review rounds against the anima-review bar (with the illumos-drm R5a branch). Round 1 findings fixed here: B3a (`current_context` unsafe), B3b (`IntrPriority::new` bound), B3c (`cb_ops: unsafe`). 
A framework module such as illumos-drm's misc/drm keeps the state of
devices that driver modules attach. The device numbers carry the driver's
major and instance, so the framework cannot find a device in its own soft
state, and it cannot write its own entry points either: IoctlArg with its
device number, DevmapHandle and PollRegistration are made only by the glue.

The char glue becomes generic over a lookup (crate-private chario). glue::*
keep their signatures and behaviour over ModuleState; table_glue::* do the
same over a CharTable, an unsafe trait with the soft state's contract. The
module! macro gains a cb_ops: form for drivers whose cb_ops point at such a
framework.
misc/drm serves the dma-bufs and sync files of every DRM driver, so it must
recognise a descriptor of another driver's device, which own_minor cannot
(it compares with the caller's own major).
ddi_taskq_create(9F) takes a NULL dip for kernel modules without a
dev_info; misc/drm destroys the fences of its drivers' C structures on such
a queue.
A C driver hands its ddi_intr_get_pri(9F) result to misc/drm as a number,
and misc/drm initialises its fence locks with it.
misc/drm's facade functions dma_fence_signal, dma_fence_put and
drm_gem_object_put may be called from a C driver's interrupt handler, as
on Linux, and carry no context token.
A driver that depends on a framework module checks the framework's
interface version before anything attaches: illumos-drm's drivers
compare misc/drm's facade version and structure sizes with their own
and refuse to load on a mismatch. The default succeeds.
servicing_interrupt only tells interrupt threads apart, so the safe
current_context handed a Sleepable to timeout(9F) callouts, quiesce(9E),
chpoll(9E) and threads at raised priority or holding a spin mutex, all
contexts in which the rest of the crate gives no Sleepable. The function
is now unsafe; its Safety section names the contexts the caller must
exclude. compile_fail doctest (E0133) with a compiling twin, and a row in
DESIGN.md section 5.12.
IntrPriority::new accepted 1 to 15, although every other source of an
IntrPriority refuses priorities at or above ddi_intr_get_hilevel_pri(9F),
and Mutex::with_priority and RwLock::with_priority rely on that. new now
returns None for 0 and for high-level priorities, asking the kernel for
the threshold (no longer a const fn). IntrError::HighLevel and
VirtioError::HighLevel carry the raw u32 instead of an IntrPriority, so
the error values no longer hand out a high-level priority either.

Host unit tests of the bound and of new against a stubbed
ddi_intr_get_hilevel_pri, an rselftest check at attach, the lock
documentation corrected, and a row in DESIGN.md section 5.12.
module! { driver: X, cb_ops: <expr> } accepted any *mut cb_ops in safe
code, a null pointer included, which the kernel dereferences at the first
open(2). The form is now cb_ops: unsafe <expr>; the keyword is the
invoker's assertion that the pointer is a valid 'static cb_ops of
table_glue entry points (or functions with their rules) whose module stays
loaded, documented on the macro, CharTable and table_glue. The internal
@common arm takes the same unsafe, so it is no way around the rule. The
ops: forms are unchanged.

compile_fail doctest (no rule matches without unsafe) with a compiling
twin, the section 5.2 paragraph updated, and a row in DESIGN.md section
5.12.
For a framework module that only needs to know whether it runs on an
interrupt thread (to take a thread-level lock or not), without the
Sleepable that current_context hands out under its contract.
illumos-drm's misc/drm_core must refuse to load while gfx-drm's legacy
misc/drm is loaded, and the DDI has no way to ask that without loading
the module (ddi_modopen(9F) loads). The gate's private
mod_find_by_filename (uts/common/os/modctl.c:3200) does: it looks a
module record up by mod_filename under mod_lock and returns it without a
hold, which is safe because modctl records are never freed.

illumos-kernel-private-sys gains a modctl module: modctl_t
(sys/modctl.h:499, size 144 and all 25 field offsets asserted for
xtask kernel-check) and mod_find_by_filename, behind
unstable-gate-cda5b9a49f. ddi::private::module_loaded(filename, &Sleepable)
returns whether the record exists with mod_loaded or mod_installed set,
read with volatile loads, documented as a snapshot.

The Sleepable token is the type rule: mod_find_by_filename takes
mod_lock, an adaptive mutex, and asserts the caller does not hold it,
which only the module framework could. compile_fail doctests (no token,
E0061; an Atomic, E0308) with a compiling twin, a host unit test of the
flag decision, two rselftest private-gate checks (genunix is loaded, an
unknown name is not), and a row in DESIGN.md section 5.12. The layout and
symbol are not yet checked against a running kernel.
private: module_loaded looks a module up by name under mod_lock
All checks were successful
ci / check-hosted (pull_request) Successful in 17m58s
Details
ci / kmod-illumos (pull_request) Successful in 29m36s
Details
10bdfba866 
mod_filename is whatever path the latest hold of a module used, so a
lookup by file name (mod_find_by_filename) misses a module loaded with
an explicit path, such as modload /kernel/misc/amd64/drm, which is the
case misc/drm_core must catch. The framework's identity of a module is
mod_modname (mod_hold_by_name_common, one record per name).

module_loaded now takes the module name (c"drm", c"genunix") and walks
the list as mod_name_to_modid does (os/modctl.c:4235): mod_lock held
only for the walk, a NULL-safe comparison of mod_modname, the
mod_loaded and mod_installed flags read with volatile loads, nothing
called while the lock is held and no hold taken (a hold would wait for
a busy module, and two modules guarding against each other while
loading could deadlock).

illumos-kernel-private-sys binds mod_lock and modules
(sys/modctl.h:552, :563) as extern statics with their contracts and
drops mod_find_by_filename, which nothing uses any more. Host tests of
the name comparison, the doctests and the section 5.12 row follow the
new design; rselftest checks that genunix and unix (the list head) are
loaded and no_such_module_xyz is not.
toasterson changed target branch from g1-gate-pin to main 2026-10-05 16:35:00 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
illumos/illumos-ddi!8
No description provided.