Loose ends: private-tier consumer, OwnNode for minor nodes, properties and LDI, nightly doctests in CI #3

Merged

forge merged 7 commits from loose-ends-20260928 into main 2026-09-29 19:11:03 +00:00

Conversation 0 Commits 7 Files changed 11 +436 -97

forge commented 2026-09-28 18:35:58 +00:00

Contributor

Copy link

Loose ends after merging #1 (drm-r0) and #2 (dma-own-node).

1. Private tier: first consumer (docs/DESIGN.md section 4)

This applies the pending design-doc edit and brings it up to date. The Rust DRM core lives at https://code.aopc.cloud/illumos/illumos-drm. R0 and R1 needed only the public tier, plus the clone-open and devmap(9E) glue from #1. The later milestones need illumos-kernel-private-sys:

• R2 needs falloc/setf for PRIME, syncobj export and sync_file descriptors.
• R4 needs the virtio framework.
• The console hand-over needs gfx_private.
• Write-combining pools need hat_*, but only a later TTM-like layer uses them.

2. OwnNode follow-ups from #2

Each decision was checked against the gate source of illumos cda5b9a49f (os/sunddi.c, os/driver_lyr.c, os/devcfg.c, os/sunpm.c, vm/seg_dev.c). The new DESIGN.md section 5.12 table records all of them.

Interface	Decision	Why	Guard
ddi_create_minor_node	Moved: OwnNode::create_minor_node replaces DevInfo::create_minor_node	The minor node's dev_t takes the major number of the node's driver, so opens go to that driver. detach_node (devcfg.c) removes a node's minor nodes when that driver detaches, after which our MinorNode drop would touch a node that may be gone. An unbound node trips ASSERT(major != DDI_MAJOR_T_NONE) on DEBUG kernels.	DdiError
ddi_prop_update_*, ddi_prop_remove, ddi_prop_remove_all, ddi_prop_undefine	Moved: Props<'a, A = ReadOnly>. DevInfo::props() gives a lookup-only view; OwnNode::props() gives Props<'_, ReadWrite> with the update and removal methods	These functions write the node's driver property list (ddi_prop_add, ddi_prop_remove_common). That list belongs to the node's driver: detach_node deletes it, and the framework reads settings from it (pm-components in sunpm.c). remove_all on a foreign node would delete another driver's properties.	PropError::NotOwnNode (new variant)
ldi_ident_from_dip	Moved: LdiIdent::from_dip(&OwnNode, ..)	ident_alloc (driver_lyr.c) stores the dip without a hold, and ldi_usage_walker_helper later calls e_ddi_hold_devi on it, so an identifier for a foreign node can outlive that node.	ENXIO
devmap_devmem_setup / _remap	Not wrapped. DevmapHandle now documents that the dip must be the own node. A future safe wrapper takes &OwnNode	They call ddi_map(dip) through the parent nexus, which is the same root-node hazard as RegMap.	For the wrapper
devmap_umem_setup / _remap	Not wrapped; nothing to enforce	seg_dev.c never reads the dip.	None
Property lookups, ddi_driver_name, ddi_get_instance, ddi_report_dev, dev_err	Stay on DevInfo	They are queries and logging and create nothing.	None

I considered API churn. node.props() still resolves for an OwnNode, because the inherent method is found before the deref, and lookups compile unchanged. What changes:

• Calls that update properties through a DevInfo must use the OwnNode.
• Props::remove now returns Result<bool, PropError> and remove_all returns Result<(), PropError>, so the guard can report a refusal.
• LdiIdent::from_dip and create_minor_node need the OwnNode. A driver that uses them after attach keeps the node in its instance, as rselftest now does.

No legitimate use becomes impossible. Leaf drivers only act on their own node, and nexus drivers update child properties with the unwrapped ndi_prop_*.

Type-rule tests are compile_fail doctests with compiling twins:

• DevInfo::create_minor_node: E0599
• DevInfo::props().set_int: E0599
• LdiIdent::from_dip(&DevInfo): E0308

Guard tests are rselftest checks against ddi_root_node: "minor node on root node refused by guard", "prop update on root node refused by guard", "ldi ident for root node refused by guard", plus "prop remove missing". None of these can panic even without the guard (see the doc comment of test_own_node_guards).

3. CI

• (a) On main at da82578, run 17: check-hosted passed, including the host-test step on the Linux runner (6 lib tests, 5 doctests, 7 ignored). Nothing needed fixing.
• (b) New step in check-hosted: install nightly-2026-04-16 with the illumos target and run cargo +nightly-2026-04-16 test -p ddi --no-default-features --doc --target x86_64-unknown-illumos. Stable rustdoc ignores the error code after compile_fail; the nightly checks it. As a negative control I changed the E0308 in the from_dip doctest to E0599: the nightly run failed with Some expected error codes were not found, and stable still passed.

Verification

Local checks on the Mac, targeting x86_64-unknown-illumos:

• check, clippy -D warnings, fmt --check and doc -D warnings pass.
• Host lib tests: 6 passed.
• Doctests: 11 passed and 7 ignored, on stable and on the nightly.
• cargo xtask lint-man --coverage: 1020 references, 0 errors, 1183/1183 9F names bound.
• kernel-check was not run, because no binding changed.

In-kernel test: rselftest ran in a fresh golden guest on Hephaistos (illumos cda5b9a49f):

• 49 of 49 checks passed (42 at attach, 7 LDI), with 0 unresolved symbols.
• No guard minor node was left under /devices.
• rem_drv was clean and there was no panic.

Loose ends after merging #1 (`drm-r0`) and #2 (`dma-own-node`). ## 1. Private tier: first consumer (`docs/DESIGN.md` section 4) This applies the pending design-doc edit and brings it up to date. The Rust DRM core lives at https://code.aopc.cloud/illumos/illumos-drm. R0 and R1 needed only the public tier, plus the clone-open and `devmap(9E)` glue from #1. The later milestones need `illumos-kernel-private-sys`: - R2 needs `falloc`/`setf` for PRIME, syncobj export and `sync_file` descriptors. - R4 needs the virtio framework. - The console hand-over needs `gfx_private`. - Write-combining pools need `hat_*`, but only a later TTM-like layer uses them. ## 2. OwnNode follow-ups from #2 Each decision was checked against the gate source of illumos `cda5b9a49f` (`os/sunddi.c`, `os/driver_lyr.c`, `os/devcfg.c`, `os/sunpm.c`, `vm/seg_dev.c`). The new DESIGN.md section 5.12 table records all of them. | Interface | Decision | Why | Guard | | --- | --- | --- | --- | | `ddi_create_minor_node` | Moved: `OwnNode::create_minor_node` replaces `DevInfo::create_minor_node` | The minor node's `dev_t` takes the major number of the node's driver, so opens go to that driver. `detach_node` (`devcfg.c`) removes a node's minor nodes when that driver detaches, after which our `MinorNode` drop would touch a node that may be gone. An unbound node trips `ASSERT(major != DDI_MAJOR_T_NONE)` on DEBUG kernels. | `DdiError` | | `ddi_prop_update_*`, `ddi_prop_remove`, `ddi_prop_remove_all`, `ddi_prop_undefine` | Moved: `Props<'a, A = ReadOnly>`. `DevInfo::props()` gives a lookup-only view; `OwnNode::props()` gives `Props<'_, ReadWrite>` with the update and removal methods | These functions write the node's driver property list (`ddi_prop_add`, `ddi_prop_remove_common`). That list belongs to the node's driver: `detach_node` deletes it, and the framework reads settings from it (`pm-components` in `sunpm.c`). `remove_all` on a foreign node would delete another driver's properties. | `PropError::NotOwnNode` (new variant) | | `ldi_ident_from_dip` | Moved: `LdiIdent::from_dip(&OwnNode, ..)` | `ident_alloc` (`driver_lyr.c`) stores the dip without a hold, and `ldi_usage_walker_helper` later calls `e_ddi_hold_devi` on it, so an identifier for a foreign node can outlive that node. | `ENXIO` | | `devmap_devmem_setup` / `_remap` | Not wrapped. `DevmapHandle` now documents that the dip must be the own node. A future safe wrapper takes `&OwnNode` | They call `ddi_map(dip)` through the parent nexus, which is the same root-node hazard as `RegMap`. | For the wrapper | | `devmap_umem_setup` / `_remap` | Not wrapped; nothing to enforce | `seg_dev.c` never reads the dip. | None | | Property lookups, `ddi_driver_name`, `ddi_get_instance`, `ddi_report_dev`, `dev_err` | Stay on `DevInfo` | They are queries and logging and create nothing. | None | I considered API churn. `node.props()` still resolves for an `OwnNode`, because the inherent method is found before the deref, and lookups compile unchanged. What changes: - Calls that update properties through a `DevInfo` must use the `OwnNode`. - `Props::remove` now returns `Result<bool, PropError>` and `remove_all` returns `Result<(), PropError>`, so the guard can report a refusal. - `LdiIdent::from_dip` and `create_minor_node` need the `OwnNode`. A driver that uses them after attach keeps the node in its instance, as `rselftest` now does. No legitimate use becomes impossible. Leaf drivers only act on their own node, and nexus drivers update child properties with the unwrapped `ndi_prop_*`. Type-rule tests are `compile_fail` doctests with compiling twins: - `DevInfo::create_minor_node`: E0599 - `DevInfo::props().set_int`: E0599 - `LdiIdent::from_dip(&DevInfo)`: E0308 Guard tests are `rselftest` checks against `ddi_root_node`: "minor node on root node refused by guard", "prop update on root node refused by guard", "ldi ident for root node refused by guard", plus "prop remove missing". None of these can panic even without the guard (see the doc comment of `test_own_node_guards`). ## 3. CI - (a) On `main` at `da82578`, run 17: `check-hosted` passed, including the host-test step on the Linux runner (6 lib tests, 5 doctests, 7 ignored). Nothing needed fixing. - (b) New step in `check-hosted`: install `nightly-2026-04-16` with the illumos target and run `cargo +nightly-2026-04-16 test -p ddi --no-default-features --doc --target x86_64-unknown-illumos`. Stable rustdoc ignores the error code after `compile_fail`; the nightly checks it. As a negative control I changed the E0308 in the `from_dip` doctest to E0599: the nightly run failed with `Some expected error codes were not found`, and stable still passed. ## Verification Local checks on the Mac, targeting `x86_64-unknown-illumos`: - `check`, `clippy -D warnings`, `fmt --check` and `doc -D warnings` pass. - Host lib tests: 6 passed. - Doctests: 11 passed and 7 ignored, on stable and on the nightly. - `cargo xtask lint-man --coverage`: 1020 references, 0 errors, 1183/1183 9F names bound. - `kernel-check` was not run, because no binding changed. In-kernel test: `rselftest` ran in a fresh golden guest on Hephaistos (illumos `cda5b9a49f`): - 49 of 49 checks passed (42 at attach, 7 LDI), with 0 unresolved symbols. - No guard minor node was left under `/devices`. - `rem_drv` was clean and there was no panic.

forge added 4 commits 2026-09-28 18:35:58 +00:00

Record the Rust DRM core as the first consumer of the private tier ... 75e6978cf7

The DRM core lives in illumos-drm. R0 and R1 needed only the public
tier; PRIME, sync_file, virtio-gpu, the console hand-over and
write-combining page pools need illumos-kernel-private-sys, which
therefore moves ahead of the other planned crates.

Require OwnNode for minor nodes, property updates and LDI identifiers ... 79e6277cea

The kernel ties each of these to the node and its driver: detach_node
(os/devcfg.c) removes a node's minor nodes and driver properties when
that node's driver detaches, a minor node takes the major number of the
node's driver, and ldi_ident_from_dip keeps the node without a hold for
the layered usage walker (os/driver_lyr.c).

- OwnNode::create_minor_node replaces DevInfo::create_minor_node.
- Props takes the access as a type parameter: DevInfo::props gives a
  ReadOnly view for lookups, OwnNode::props a ReadWrite view that also
  updates and removes. remove and remove_all now return a Result.
- LdiIdent::from_dip takes &OwnNode.
- Each applies the own-node guard: DdiError, PropError::NotOwnNode and
  ENXIO before the kernel is called.
- DevmapHandle documents that the devmap setup functions need the own
  node (devmem_setup maps through the parent nexus; umem_setup ignores
  the node); they stay unwrapped.

compile_fail doctests cover each type rule, rselftest checks each guard
against the root node, and DESIGN.md section 5.12 records the decision
for every interface that takes a node.

Run the doctests on the pinned nightly in CI ... 9ebd16add0

Stable rustdoc does not check the error code of a compile_fail doctest,
so a type rule could fail to compile for the wrong reason unnoticed. The
check-hosted job now runs the doctests again with nightly-2026-04-16,
which checks the codes.

Record the rselftest run of the own-node guard checks ... eb0150cadc

All 49 checks (42 at attach, 7 LDI) passed on a fresh Hephaistos guest
with illumos cda5b9a49f, including the guard checks for minor nodes,
property updates and LDI identifiers.

forge referenced this pull request from illumos/illumos-drm 2026-09-28 18:37:29 +00:00

Loose ends: repin illumos-ddi, ENOMEM for failed DMA binds, page-size constant in rdrmnull #4

forge added 3 commits 2026-09-29 19:03:58 +00:00

State that property writes to child or parent nodes are out of scope ... 5ac4601c86

Gate drivers write properties of other nodes: nexus drivers on children
(lofi, fcoe, npe_misc, mpt_sas) and a few leaf drivers on their parent
(sil3xxx, dnet). The own-node view cannot express that; say so in
DESIGN.md sections 5.8 and 5.12 and in the prop module, and name the
raw ddi::sys functions as the way until a child-node capability exists.
Also reflow an over-long doc line on OwnNode.

Re-export the property access markers ReadOnly and ReadWrite at the crate root 2e7a55727e

Make DmaError::check public for drivers that call raw DMA functions ... 449bfe73e9

It decodes all ten DDI_DMA_* failure codes the way the safe layer does,
so a driver that binds through the raw bindings (illumos-drm binds
ddi_umem memory with ddi_dma_addr_bind_handle) decodes them
consistently. A host test covers every code.

forge merged commit 5d21301137 into main 2026-09-29 19:11:03 +00:00

forge referenced this pull request from a commit 2026-09-29 19:11:05 +00:00

Merge pull request #3 from loose-ends-20260928: OwnNode follow-ups, nightly doctests, public DMA decoder

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

Subscribe

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!3

No description provided.