Skip to content

Support soft shutdown via ACPI PWRBTN_STS - #1217

Open
lifning wants to merge 3 commits into
masterfrom
lif/acpi-shutdown-ft-luiz
Open

lifning wants to merge 3 commits into
masterfrom
lif/acpi-shutdown-ft-luiz

Conversation

@lifning

@lifning lifning commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Changes the instance state put API to support the propolis-server behaviors specified by RFD 622.

(migration-from-base phd-test failure: propolis-client is the new v7, so phd-runner can't talk to the v6 propolis-server artifact)

@lifning
lifning force-pushed the lif/acpi-shutdown-ft-luiz branch from f3aa4de to 2453efc Compare September 19, 2026 08:14
Implements behavior specified in RFD 622, based on earlier work on
platform support for ACPI fixed power button by @lgfa29.

This adds an optional `acpi_timeout_secs` parameter
to instance state PUT, which if specified will cause
propolis-server to send the guest an [ACPI fixed power
button](https://uefi.org/specs/ACPI/6.6/04_ACPI_Hardware_Specification.html#fixed-power-button)
signal. After sending this to the guest, propolis-server waits for the
guest to shut itself down for the specified number of seconds before
enqueueing the explicit hard-stop or hard-reset the request would have
otherwise been without the timeout parameter specified.

If an instance state PUT request to stop or reset is received while
shutting down, it is honored immediately, possibly interrupting the
guest's safer shutdown process (unmounting disks, et al).

If an instance state PUT with a timeout is received while still waiting
for a previously-requested ACPI shutdown, the newer request is ignored.

If a guest which is expected to be shutting down sends propolis *any*
CPU-halting event, be it the expected halt, a chipset-driven reset, or
a triple-fault, the action which was specified in the pending shutdown
request is taken, overriding what the guest's action would normally
imply absent the pending shutdown with with the authority of the user's
intent.\*

(\* In essence, we expect that if the control plane tells us the
instance should be turned off gently, it could well be because the
sled is about to be upgraded, and the customer is likely to want their
VM guests' filesystems to be left in as coherent a state as possible.
An unclean unmount from a triple-fault followed shortly by the system
rebooting, starting a filesystem repair, and then being interrupted
during *that* by a hard-cut of its power is unlikely to be a positive
customer experience)

Co-authored-by: Luiz Aoqui <lgfa29@gmail.com>
@lifning
lifning force-pushed the lif/acpi-shutdown-ft-luiz branch from 2453efc to 0b3e6b4 Compare September 19, 2026 08:16
@lifning
lifning marked this pull request as ready for review September 30, 2026 21:30

@iximeow iximeow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a smattering of thoughts, this is easily half me trying to put the state bits of propolis-server back into my head (sorry!)

thanks for the work on this, and alpine-sans-apicd ignoring the button press makes for a very useful test case..

Comment thread lib/propolis/src/hw/chipset/i440fx.rs Outdated
Comment on lines +581 to +584
self.put_instance_state(
InstanceStateRequested::Stop,
Some(timeout_secs),
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fwiw a thought on my mind is if we might want to spell the ACPI operation as an InstanceStateRequested::AcpiStop (and below, InstanceStateRequested::AcpiReboot) where the optional timeout says "if it didn't X in Yms, then X it ourselves".. I suppose in reality the difference isn't all that important. but it comes to mind in part because "what's InstanceStateRequested::Run with a "30s timeout" mean?"

other than the absurd Run state request, I mostly feel like "get the OS to seen SCI pulse with no later obligation for the VMM" would be interesting for testing. but in our bold systemd future we've given up on userland getting to notice power buttons itself..

Comment thread bin/propolis-server/src/lib/vm/state_driver.rs Outdated
// TODO? might watch for OSPM clearing the status bit to
// determine if guest acknowledges the button press

// timeout to hard-stop/reset if guest takes too long

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

stray comment maybe? or do you mean re. all the remaining lines in this arm?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

in some respect it's probably slightly conceptually misplaced for this comment as-written to be in StateDriver, ultimately being in reference to how "OSPM responds to the event by clearing the PWRBTN_STS bit"[ACPI 4.8.2.2.1.1]. my thought is that if the OS doesn't do this, we can assume the kernel (if any) is completely wedged and not bother with the longer timeout, and this was the place i figured we would put such a tighter timeout task to check if the bit was cleared.

Comment on lines +988 to +992
// first abandon any timeout task that may send a redundant request
if let Some((_, timeout_task)) = self.soft_off.take() {
timeout_task.abort();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

something I had to think about a bit: a guest-initiated reboot could race with the timeout and mean that there's already a pending ExternalRequest::reboot() that we'll process pretty much as soon as we're out of this function. that's ok, because in the reset case it's no different from someone poking the reset button a bunch. but the comment probably could note that we may have already lost the race with a redundant op enqueued? (it really seems like we don't want to go remove it from the queue, since we'd need to distinguish it from a bona fide API request and that would need more different plumbing..)

in the halt case you're.. turning off the thing that's off, so abstractly that's OK. but in reality as soon as we've processed the first stop we're out of event_loop anyway, so the second would never be seen.

// state.
ExternalRequest::State(StateChangeRequest::Start) => {
if self.awaiting_stop {
if self.awaiting_stop || self.awaiting_shutdown {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm having a hard time puzzling through the bools we're accumulating here.. we could be awaiting_shutdown because of a soft-off+reboot, which means denying a start because of HaltPending is both true and .. not? concretely, I'm wondering things like, "what is awaiting_shutdown if we had an ACPI restart but then preempted that with a force stop?" - I think we'd end up in awaiting_shutdown = true after stopping the VM?

so overall I'm both wondering if this should still operate in terms of "awaiting stop" and "awaiting reboot", since we're kind of processing ACPIShutdown as a stand-in for the eventual fate, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"what is awaiting_shutdown if we had an ACPI restart but then preempted that with a force stop?" - I think we'd end up in awaiting_shutdown = true after stopping the VM?

if i'm understanding your scenario correctly, i believe we'd end up with awaiting_shutdown = false per this part of try_queue

ExternalRequest::State(StateChangeRequest::Stop) => {
assert!(!self.awaiting_stop);
self.awaiting_shutdown = false;
self.awaiting_stop = true;
}

meanwhile i think if we treated an ACPIShutdown as an awaiting_stop/awaiting_reboot according to its fate, we may end up ignoring such a force-stop request 'for idempotency'

// If the queue is in a terminal state, deny the request straightaway
// (unless it's a stop request, which can be ignored for idempotency).
if let Some(reason) = self.state.deny_reason() {
if request.is_stop() {
return Ok(false);

which we could of course change, but at some point we do need to distinguish between the two if we're to allow force-stops/reboots through while we're awaiting an unforceful shutdown (and the way it's currently wired, to allow the timeout's request to be processed as well)

with all that said i do agree that the number of anticipatory booleans does begin to make me nervous!

Comment on lines +534 to +535
} else if self.awaiting_shutdown {
return Ok(false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

my read is that at this point we'd ignore a soft-power-off if the VM had seen a previous soft-reboot. that seems fine (especially in consideration that we don't want to ever-extend the deadline), but I'm thinking through this from the console PoV: say I've got an instance with a 10-minute default timer on shutdown/reboot events. if I've tried to reboot my instance and it seems stuck, then I wait 10 seconds and get bored and irritated and try to stop it.

then if that's gotten to Propolis, we'll ignore the shutdown request and say "ok" to the API. if the timer elapses and you did not force-stop it (maybe you went to get water instead), you come back to find that the shutdown was forgotten and your instance might be back at a login prompt. that seems unfortunate!

if this is handled in Nexus in ways I haven't seen yet, maybe I'm overthinking it a bit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the opinion the RFD seems to have is that once you've made a request, no changing your mind on timeout, and right now i suppose i'm extending that to 'no changing your mind on fate either' - but i certainly don't like the operator experience in the aforementioned hypothetical, so perhaps we should consider letting its fate be changed.. if you've sent a soft-reboot and a timeout began, the OS was already given a signal, so changing your mind on what you want it to do (make it a soft-off) after either the guest sends the power event after completing its shutdown or the existing timeout runs out certainly doesn't strike me as an unreasonable thing to allow

Comment thread lib/propolis/src/hw/chipset/i440fx.rs
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.

2 participants