Add warm_boot_entry for suspend support by m4tx · Pull Request #40 · google/aarch64-rt

m4tx · 2026-01-14T15:17:13Z

No description provided.

m4tx · 2026-01-14T15:18:33Z

Disclaimer: note that while the code is very similar to secondary_entry, I wasn't able to properly test this. See google/ritm#8 (comment) for more context - any help or tips on how to test this will be greatly appreciated!

src/entry.rs

qwandor · 2026-01-15T16:48:01Z

src/entry.rs

+/// `stack_ptr` must be a valid stack pointer.
+/// `entry_point` must be a valid function pointer taking one argument.
+#[unsafe(naked)]
+pub unsafe extern "C" fn warm_boot_entry<T>(context: *mut SuspendContext<T>) -> ! {


How is this intended to be used? Does it need to be separate from secondary_entry? Would it make sense to provide a wrapper around psci::cpu_suspend the same way as we have a wrapper around psci::cpu_on?

The idea is that this is deliberately low-level so that the user can implement suspending however they like. In case of ritm, where this is going to be used, ritm is just a thin hypervisor which doesn't need to restore any internal state after resuming.

Sure, I think it would make sense to provide a wrapper around psci::cpu_suspend which could emulate returning from a function even after powerdown. It could store the SuspendContext on the stack, along with the state of the registers, and just restore everything after resuming and call ret. This wrapper would still use warm_boot_entry internally. However, because such wrapper would be useless for ritm, I don't want to contribute functionality I won't actively use, and therefore, most importantly, test.

I've updated the docs of the function with this info.

qwandor

Whoops, didn't mean to approve before, sorry.

m4tx · 2026-02-12T13:29:54Z

Disclaimer: note that while the code is very similar to secondary_entry, I wasn't able to properly test this. See google/ritm#8 (comment) for more context - any help or tips on how to test this will be greatly appreciated!

Update: I was able to test this in google/ritm#8 by using Rusted Firmware-A inside QEMU, and it seems to be working just fine.

qwandor · 2026-02-18T00:53:29Z

src/entry.rs

+#[repr(C)]
+pub struct SuspendContext<T> {
+    pub stack_ptr: u64,
+    pub entry: extern "C" fn(&Self) -> !,


Would it be simpler for this function to take &T rather than &Self?

I was initially thinking to do this, but then I realized if someone wants to allocate SuspendContext on the heap, there will be no easy way to deallocate it after unsuspending. Hence I went with passing Self here.

qwandor · 2026-02-18T00:55:27Z

src/entry.rs

+///
+/// The caller guarantees that the `SuspendContext` instance passed to the function
+/// will be valid and safe to read when the CPU resumes (especially important when
+/// the data is wrapped in a mutex, for instance).


For how long does it need to remain valid? I guess as long as entry runs, which is potentially forever? This seems like a difficult requirement for the caller to meet. I wonder if it would make more sense for entry to take a pointer rather than a reference, then it would be up to the caller of warm_boot_entry to choose an appropriate lifetime to match what their entry function expects.

Ah, that's a very good point and indeed a very good reason to pass a pointer instead of a reference here.

qwandor · 2026-02-18T01:01:10Z

src/entry.rs

+        "ldr x0, [x19, #{stack_ptr_offset}]",
+        "ldr x1, [x19, #{entry_offset}]",
+        // Set the exception vector.
+        "bl {set_exception_vector}",


The stack pointer needs to be set before calling set_exception_vector, as it is a Rust function. It may also clobber x0 and x1, so you need to get the entry function pointer after this.

qwandor · 2026-02-18T01:06:21Z

src/entry.rs

+///
+/// The function expects to be passed a pointer to a `SuspendContext` instance
+/// that will be valid after returning from suspend. It should therefore be
+/// a static, or allocated on the heap, to avoid being deallocated before resuming.


It would be nice if we could put the SuspendContext on the stack of the core being resumed. What about if we made the data field of SuspendContext a pointer rather than a value? Then the SuspendContext itself could go on the stack, as it isn't needed once its contents is loaded into registers, and only the value behind the data pointer would need to be allocated statically.

Alternatively, looking at how this is used in RITM, you could remove the data field from SuspendContext entirely, and the entry function could just find its own data based on the MPIDR. (In the case of RITM, SUSPEND_CONTEXTS could store pairs of SuspendContext and SuspendCoreData.)

Either way, I think it should work to always put the entry pointer and data pointer (if we keep it) on the stack of the core that is being resumed. That only requires that there is enough spare stack space to fit, and if the stack is so small or full that there isn't, then resuming isn't going to go very well anyway. The stack pointer itself doesn't need to go on the stack, as it can be passed directly as the context of the PSCI CPU_SUSPEND call. This would work with a wrapper function in this crate for CPU_SUSPEND to put them on the stack.

I've pushed a couple of commits implementing my suggestions here, see what you think.

I've changed data to a u64 rather than an arbitrary type. This means that SuspendContext can always go on the stack of the core being resumed, so there's no need to worry about deallocating it. I realised that we do still need to keep the stack pointer there too, as depending on how you want to handle resuming it may be the bottom of the stack or somewhere near the top, and won't necessarily be a constant offset from the SuspendContext. This also maintains the ability to put the SuspendContext somewhere else if you really want to.

If you want to pass more data than a u64 then you can cast a pointer to it, in which case you can handle the deallocation yourself (if you need to). But I don't think you need this for RITM anyway.

I've also added a suspend_core function which constructs a SuspendContext on the stack and passes it to the PSCI cpu_suspend. I think this should work for RITM.

Thanks a lot for helping me with this! Yes, I think the current version is probably best of both worlds - still low level enough to be a good fit for RITM and easy to make it memory safe.

This means that the SuspendContext can be stored on the stack of the resuming core. Larger values can be passed via a pointer to some other location.

m4tx · 2026-02-20T11:41:35Z

src/lib.rs

+/// want to handle resuming this could either be the bottom of the stack (if you want to treat
+/// resuming like `CPU_ON`) or the top (if `entry` will restore register state and return from the
+/// point where the suspend happened).
+pub unsafe fn suspend_core<C: smccc::Call, T: Sized>(


We don't use T anywhere, I think it can be dropped.

m4tx · 2026-02-20T11:44:56Z

src/lib.rs

+/// point where the suspend happened).
+pub unsafe fn suspend_core<C: smccc::Call, T: Sized>(
+    power_state: u32,
+    stack_ptr: *mut u64,


nitpick: it feels like this should be *mut u8 (to match the internals of StackPage, and basically treat at as an untyped pointer), or perhaps some other type.

Add warm_boot_entry for suspend support

b5783ed

qwandor approved these changes Jan 15, 2026

View reviewed changes

qwandor requested changes Jan 15, 2026

View reviewed changes

fix issues

4283ea3

m4tx requested a review from qwandor February 12, 2026 13:30

qwandor reviewed Feb 18, 2026

View reviewed changes

qwandor added 2 commits February 19, 2026 02:44

Make SuspendContext.data a u64 rather than an arbitrary T.

a21cf53

This means that the SuspendContext can be stored on the stack of the resuming core. Larger values can be passed via a pointer to some other location.

Add wrapper function construct a SuspendContext and call CPU_SUSPEND.

4b70c95

m4tx commented Feb 20, 2026

View reviewed changes

Comments

Conversation

m4tx commented Jan 14, 2026

Uh oh!

m4tx commented Jan 14, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m4tx Feb 12, 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!

qwandor left a comment

Choose a reason for hiding this comment

Uh oh!

m4tx commented Feb 12, 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!

m4tx Feb 18, 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

m4tx Feb 12, 2026 •

edited

Loading

m4tx Feb 18, 2026 •

edited

Loading