Skip to content

Fix halting behavior preventing humility probe from working - #725

Merged
labbott merged 2 commits into
masterfrom
correct_halting
Aug 21, 2026
Merged

Fix halting behavior preventing humility probe from working#725
labbott merged 2 commits into
masterfrom
correct_halting

Conversation

@labbott

@labbott labbott commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

@adamlouis flagged this in a facade change that we were getting errors from humility probe "The core has to be halted for the operation, but was not." . This introduces two change: a way to get the previous CPU state from run and halt and a fix in validate

This makes it possible for code calling `halt`/`run` to
keep state consistent.

@evan-oxide evan-oxide left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, sorry about that

Changes in humility#711 can result in the CPU unexpectedly running
again after a `validate` call. Only make the CPU run if the CPU
was previously running.
Comment thread humility-core/src/core.rs
use thiserror::Error;

/// State of the CPU before the halt/run operation
pub enum PreviousCpuState {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we mark this #[must_use] to catch places where we don't check this but should?

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.

I tried this and I think it causes more problems than it's solving. Most other places in the code are very deliberate about wanting halt/run so the logic seems to result in let _ = core.run()?. Maybe this is a sign we need a fn run_while_halted(&self, fn ...) for parts of the code? That also seems like follow up work.

@labbott
labbott merged commit ad03c66 into master Aug 21, 2026
13 checks passed
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.

3 participants