Skip to content

Conversation

@sparchatus
Copy link
Contributor

Add support for ACPI fixed power buttons. This is based on #350

I wanted to add the functionality to change the power button handler because it would be useful for triggering experiments for me. This opened up a race condition between the interrupt and the function changing the handler. The interrupt may not wait for a spinlock so I added a try_lock function but I am not very convinced by this approach.

@sparchatus sparchatus requested a review from a team as a code owner November 11, 2025 15:08
@sparchatus sparchatus force-pushed the feat_power_button branch 2 times, most recently from 155fb4d to 114c5ad Compare November 14, 2025 09:24
Driver for ACPI fixed power buttons. It allows overwriting the default handler so the button can be used for custom purposes. One such purpose might be to trigger an experiment to run.

Signed-off-by: Sandro Rüegge <[email protected]>
@sparchatus
Copy link
Contributor Author

I have removed the spinlock changes. I have added another commit that adds a struct cpu reference to the percpu variable such that we can assert that the handler is indeed running on the BSP. Not sure if that is needed and it might introduce other issues if there is task migration later on. If sb thinks it is a bad idea, I can just drop that commit again.

Adding a self reference to the percpu allows
checking what CPU some code is currently
running on. We use this in pb_set_handler
to ensure it runs on the bsp.

static void default_handler(void *);

spinlock_t pb_handler_lock = SPINLOCK_INIT;
Copy link
Contributor

Choose a reason for hiding this comment

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

pb_handler_lock isn't used any more, can remove its definition as well.

Copy link
Contributor

@minipli-oss minipli-oss left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM!

unsigned long usermode_private;
volatile unsigned long apic_ticks;
bool apic_timer_enabled;
struct cpu *cpu;
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess, using the proper typedef'd type cpu_t would cause a cyclic header include dependency?

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