Kernel: Adjustments to `unsafe` code by jamesmunns · Pull Request #2600 · oxidecomputer/hubris

jamesmunns · 2026-07-17T12:23:04Z

The following is a number of tweaks to the kernel code that in my opinion, make some of the unsafe code a bit more direct to follow. In particular, these changes:

Consolidates switch_to as a single method on Task, rather than one version in syscalls.rs, and a handful of semantically-equal copy/pastes
Removes as many single/double blind as casts as is reasonable, e.g. x as *const _ as *mut _, replacing them with discrete steps that note the implicit casts (references to pointer), and explicit casts, using ptr::cast()
Reduces unsafe usage a bit in startup when initializing each task slot
Makes some methods take &mut Task instead of &Task, as these are used for setting CURRENT_TASK_PTR, which semantically allows us to write-through to that address (often in assembly) when returning to the kernel, and that feels a little sketch to me doing from a pointer with shared provenance
Adjusts the codegen for Task slots from MaybeUninit<[Task; N]> to [MaybeUninit<Task>; N], as this makes startup a little less awkward

In my opinion, none of these are strictly necessary for soundness, so if we don't want to touch it, I could definitely understand that!

I think 1, 2, and 3 are pretty clear readability/clarity wins.

I think 4 is maybe a little nitpicky, and is a bit more semantically correct, but also unlikely to cause miscompilations in practice.

I think 5 is maybe a mixed bag (we can still make the changes for 3 without 5), and I need to check it isn't going to make humility upset as the types have changed (even though the before/after are guaranteed to have the same layout and semantics).

Happy to revert any chunks, or rework these into separate commits if that makes reviewing easier.

This is extracted from #2592, and most of these were noticed while I was hacking around on that.

jamesmunns · 2026-07-17T13:22:35Z

Annoyingly, the RAM usage for g070 didn't increase because we are using more, it's just a quirk that the linker isn't packing statics as intelligently as it possibly could.

labbott · 2026-07-17T13:50:34Z

+                [core::mem::MaybeUninit<crate::task::Task>; HUBRIS_TASK_COUNT] =
+                [const { core::mem::MaybeUninit::uninit() }; HUBRIS_TASK_COUNT];


You certainly know more about this area than I do but is this a functional change? I don't think so but this is also accessed via humility so is there any chance this could be disruptive there?

To the best of my knowledge an "executable" perspective, this is not a functional change: MaybeUninit is repr(transparent), therefore:

MaybeUninit<T> is the same size + align as T

MaybeUninit<[T; N]> is the same size + align as [T; N]

If this wasn't the case, this AsRef impl which is stable and safe, wouldn't be sound.

It might cause problems for humility, depending on how we access this symbol. The name and size are the same, but it might be a noticeable type change. I'll test this before going to merge it.

I agree that this is probably not a functional change from Hubris' perspective and shouldn't actually impact how this is initialized. But, I think we should definitely check if it changes the DWARF in a way that makes Humility sad. If it does, it should be okay to change Humility to handle either type, just a bit annoying.

Hilariously, I think we're good on this. Running humility tasks and humility dump, both over SWD and net-hiffy, works to totally fine, and I think it's because for legacy compat reasons, we return a base pointer and task count here:

https://github.com/oxidecomputer/humility/blob/68e7d145f633706fcf51a28071c637b48c70ca96/humility-core/src/hubris.rs#L1929-L1957

We then read back Task sized objects from the read blobs, which means it doesn't care about this shape change!

hawkw · 2026-07-17T16:51:19Z

    // this frame.
-    task.save_mut().psp = frame as *const _ as u32;
+    let frame: *const ExtendedExceptionFrame = frame;
+    task.save_mut().psp = frame as u32;


i wonder if it would be more pedantically correct to use expose_provenance here? i do not think it actually matters, since from strict provenance's perspective, we are never actually turning the u32 back into a pointer in the kernel (while this does happen, it happens in the task when we return from the syscall and not in a way that's visible to Rust)... but it seems like this could technically be treated as a provenance exposing cast?

I'm somewhat of the opinion that "that's another program's data", and it isn't necessarily relevant to us, the kernel.

The hardware itself swaps from psp back to msp before re-entering the kernel in an interrupt, so I don't think it's necessary. That explanation is all a little handwavey though.

for the record, i also don't think it's necessary, i was thinking more from a position of "is this something we should generally be in the habit of doing?"

The expose_provenance docs specifically state: "This is equivalent to self as usize ... Furthermore, this (like the as cast) has the [...] effect of marking the provenance of 'exposed'" so there doesn't seem to be absolute any difference between these to.

What would be interesting if you could use addr(): probably not if it's the hardware using this data.

…hanges

aapoalas

Please excuse my unrequested comments; the topic called out to me.

aapoalas · 2026-07-17T14:03:30Z

 [[config.i2c.devices]]
 controller = 1
 mux = 1
-segment = 1 


thought: Meh, I wish there was toml formatting. (Mostly because my editor formats it automatically, and has caused annoyance in PRs :) )

aapoalas · 2026-07-17T18:49:01Z

    // this frame.
-    task.save_mut().psp = frame as *const _ as u32;
+    let frame: *const ExtendedExceptionFrame = frame;
+    task.save_mut().psp = frame as u32;


The expose_provenance docs specifically state: "This is equivalent to self as usize ... Furthermore, this (like the as cast) has the [...] effect of marking the provenance of 'exposed'" so there doesn't seem to be absolute any difference between these to.

What would be interesting if you could use addr(): probably not if it's the hardware using this data.

also add a contrasting comment vs static-cell Similar to #2600.

Nitpicky kernel fixes to reduce raw pointer as-casting and copy-pasting

b4a4165

jamesmunns requested review from cbiffle, hawkw and labbott July 17, 2026 12:23

jamesmunns added 2 commits July 17, 2026 15:08

Fix armv6m

c89042f

Bump g070 ram (under protest)

849ac98

jamesmunns added 3 commits July 17, 2026 15:27

Bump a bunch more armv6s

a436bb6

More ram for the linker

dcffc5b

More ram

3d87adb

labbott reviewed Jul 17, 2026

View reviewed changes

hawkw reviewed Jul 17, 2026

View reviewed changes

Address some EVEN MORE pedantic nits noticed while explaining other c…

d8180e8

…hanges

aapoalas reviewed Jul 17, 2026

View reviewed changes

hawkw reviewed Jul 17, 2026

View reviewed changes

Comment thread sys/kern/src/task.rs Outdated

jamesmunns added 2 commits July 20, 2026 09:20

addr_of(_mut) to &raw (const|mut)

0bbf4ce

Revert one &raw mut usage

950f1ab

This was referenced Jul 20, 2026

WIP: Kernel accounting of task CPU usage #2592

Draft

Clean up some mutable-static unsafe #2605

Merged

jamesmunns added a commit that referenced this pull request Aug 6, 2026

Clean up some mutable-static unsafe (#2605)

4924a12

also add a contrasting comment vs static-cell Similar to #2600.

		[core::mem::MaybeUninit<crate::task::Task>; HUBRIS_TASK_COUNT] =
		[const { core::mem::MaybeUninit::uninit() }; HUBRIS_TASK_COUNT];

Uh oh!

Conversation

jamesmunns commented Jul 17, 2026

Uh oh!

jamesmunns commented Jul 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aapoalas left a comment

Choose a reason for hiding this comment

Uh oh!

aapoalas Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aapoalas Jul 17, 2026 •

edited

Loading