On 1/10/26 11:53, Peter Maydell wrote:
On Wed, 30 Sept 2026 at 18:50, Philippe Mathieu-Daudé
<[email protected]> wrote:
PSCI can update the power state of a vCPU other than the caller.
arm_cpu_has_work() reads env->halt_reason to decide whether that
vCPU has work to do, and is called from the vCPU thread without the
BQL held. A remote power_state/halt_reason update is therefore not
observed until the target vCPU is rescheduled, so the vCPU stays
halted and a PSCI CPU_ON of a secondary vCPU can hang.
Kick the vCPU after changing its state so it re-evaluates promptly.
Reported-by: Paolo Bonzini <[email protected]>
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
---
target/arm/arm-powerctl.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index ef14a1f8a94..1ba5277b023 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -48,9 +48,14 @@ struct CpuOnInfo {
void arm_set_cpu_power_state(ARMCPU *cpu, ARMPSCIState state)
{
CPUARMState *env = &cpu->env;
+ CPUState *cs = CPU(cpu);
qatomic_set(&cpu->power_state, state);
qatomic_set(&env->halt_reason, state == PSCI_OFF ? HALT_PSCI :
NOT_HALTED);
+
+ if (!qemu_cpu_is_self(cs)) {
+ qemu_cpu_kick(cs);
+ }
}
Looking at the callsites of this function:
* three places in arm-powerctl.c call it from an async_run_on_cpu
callback function which runs it in the context of the vCPU it
is changing the power state for
* target/arm/cpu.c calls it from within reset for the vCPU being
reset, so we definitely don't want to kick the vCPU
* target/arm/kvm.c calls it as part of the "sync the vCPU state
to/from KVM" function, which should only be running from
that vCPU's thread
* target/arm/machine.c calls it as part of an inbound migration
to set the vCPU's power-state to whatever it was on the source
So I think that in the cases where we're actually running the VM
the if() check will fail and we do nothing anyway, and in the other
cases (reset, migration) we don't want to be kicking the vCPU.
Indeed... Paolo identified a possible race problem when discussing
the generic sync changes around has_work() so I ran to address it but
failed to correctly audit the call sites for the ARM case, and this
indeed does not occurs. Hopefully the new comment added in commit
fbf08c0fbd5 will help reminding we need to be careful and not try
to protect unreachable cases.
Thanks for the audit,
Phil.
