target/ppc/cpu.h | 11 +++++++++++ hw/ppc/pnv_core.c | 6 +++++- hw/ppc/spapr_cpu_core.c | 6 ++++++ hw/ppc/spapr_rtas.c | 5 ++++- target/ppc/excp_helper.c | 4 ++++ 5 files changed, 30 insertions(+), 2 deletions(-)
This change takes the CPUPPCState 'quiesced' field added for powernv
hardware CPU core controls (used to stop and start cores), and extends
it to spapr to model the "RTAS stopped" state. This prevents the
schedulers attempting to run stopped CPUs unexpectedly, which can cause
hangs and possibly other unexpected behaviour.
The detail of the problematic situation is this:
A KVM spapr guest boots with all secondary CPUs defined to be in the
"RTAS stopped" state. In this state, the CPU is only responsive to the
start-cpu RTAS call. This behaviour is modeled in QEMU with the
start_powered_off feature, which sets ->halted on secondary CPUs at
boot. ->halted=true looks like an idle / sleep / power-save state which
typically is responsive to asynchronous interrupts, but spapr clears
wake-on-interrupt bits in the LPCR SPR. This more-or-less works.
Commit e8291ec16da8 ("target/ppc: fix timebase register reset state")
recently caused the decrementer to expire sooner at boot, causing a
decrementer exception on secondary CPUs in RTAS stopped state. This
was not a problem on TCG, but KVM limits how a guest can modify LPCR, in
particular it prevents the clearing of wake-on-interrupt bits, and so in
the course of CPU register synchronisation, the LPCR as set by spapr to
model the RTAS stopped state is overwritten with KVM's LPCR value, and
that then causes QEMU's interrupt code to notice the expired decrementer
exception, turn that into an interrupt, and set CPU_INTERRUPT_HARD.
That causes the CPU to be kicked, and the KVM vCPU thread to loop
calling kvm_cpu_exec(). kvm_cpu_exec() calls
kvm_arch_process_async_events(), which on ppc just returns ->halted.
This is still true, so it returns immediately with EXCP_HLT, and the
vCPU never goes to sleep because qemu_wait_io_event() sees
CPU_INTERRUPT_HARD is set. All this while the vCPU holds the bql. This
causes the boot CPU to eventually lock up when it needs the bql.
So make 'quiesced' represent the "RTAS stopped" state, and have it
explicitly not respond to exceptions (interrupt conditions) rather than
rely on machine register state to model that state. This matches the
powernv quiesced state very well because it essentially turns off the
CPU core via a side-band control unit.
There are still issues with QEMU and KVM idea of LPCR diverging and that
is quite ugly and fragile that should be fixed. spapr should synchronize
its LPCR properly with KVM, and not try to use values that KVM does not
support.
Reported-by: Misbah Anjum N <misanjum@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/cpu.h | 11 +++++++++++
hw/ppc/pnv_core.c | 6 +++++-
hw/ppc/spapr_cpu_core.c | 6 ++++++
hw/ppc/spapr_rtas.c | 5 ++++-
target/ppc/excp_helper.c | 4 ++++
5 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index efab54a0683..3ee83517dcd 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1356,6 +1356,17 @@ struct CPUArchState {
* special way (such as routing some resume causes to 0x100, i.e. sreset).
*/
bool resume_as_sreset;
+
+ /*
+ * On powernv, quiesced means the CPU has been stopped using PC direct
+ * control xscom registers.
+ *
+ * On spapr, quiesced means it is in the "RTAS stopped" state.
+ *
+ * The core halted/stopped variables aren't sufficient for this, because
+ * they can be changed with various side-band operations like qmp cont,
+ * powersave interrupts, etc.
+ */
bool quiesced;
#endif
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 99d9644ee38..a33977da188 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -248,21 +248,25 @@ static void pnv_core_power10_xscom_write(void *opaque, hwaddr addr,
if (val & PPC_BIT(7 + 8 * i)) { /* stop */
val &= ~PPC_BIT(7 + 8 * i);
- cpu_pause(cs);
env->quiesced = true;
+ ppc_maybe_interrupt(env);
+ cpu_pause(cs);
}
if (val & PPC_BIT(6 + 8 * i)) { /* start */
val &= ~PPC_BIT(6 + 8 * i);
env->quiesced = false;
+ ppc_maybe_interrupt(env);
cpu_resume(cs);
}
if (val & PPC_BIT(4 + 8 * i)) { /* sreset */
val &= ~PPC_BIT(4 + 8 * i);
env->quiesced = false;
+ ppc_maybe_interrupt(env);
pnv_cpu_do_nmi_resume(cs);
}
if (val & PPC_BIT(3 + 8 * i)) { /* clear maint */
env->quiesced = false;
+ ppc_maybe_interrupt(env);
/*
* Hardware has very particular cases for where clear maint
* must be used and where start must be used to resume a
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 0671d9e44b4..faf9170ba6b 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -37,6 +37,9 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
cpu_reset(cs);
+ env->quiesced = true; /* set "RTAS stopped" state. */
+ ppc_maybe_interrupt(env);
+
/*
* "PowerPC Processor binding to IEEE 1275" defines the initial MSR state
* as 32bit (MSR_SF=0) with MSR_ME=1 and MSR_FP=1 in "8.2.1. Initial
@@ -98,6 +101,9 @@ void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip,
CPU(cpu)->halted = 0;
/* Enable Power-saving mode Exit Cause exceptions */
ppc_store_lpcr(cpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
+
+ env->quiesced = false; /* clear "RTAS stopped" state. */
+ ppc_maybe_interrupt(env);
}
/*
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 503d441b48e..78309dbb09d 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -110,7 +110,8 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU *cpu_,
id = rtas_ld(args, 0);
cpu = spapr_find_cpu(id);
if (cpu != NULL) {
- if (CPU(cpu)->halted) {
+ CPUPPCState *env = &cpu->env;
+ if (env->quiesced) {
rtas_st(rets, 1, 0);
} else {
rtas_st(rets, 1, 2);
@@ -215,6 +216,8 @@ static void rtas_stop_self(PowerPCCPU *cpu, SpaprMachineState *spapr,
* For the same reason, set PSSCR_EC.
*/
env->spr[SPR_PSSCR] |= PSSCR_EC;
+ env->quiesced = true; /* set "RTAS stopped" state. */
+ ppc_maybe_interrupt(env);
cs->halted = 1;
ppc_store_lpcr(cpu, env->spr[SPR_LPCR] & ~pcc->lpcr_pm);
kvmppc_set_reg_ppc_online(cpu, 0);
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 44e19aacd8d..c941c89806e 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1951,6 +1951,10 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
target_ulong lpcr = env->spr[SPR_LPCR];
bool async_deliver;
+ if (unlikely(env->quiesced)) {
+ return 0;
+ }
+
#ifdef TARGET_PPC64
switch (env->excp_model) {
case POWERPC_EXCP_POWER7:
--
2.47.1Thanks for the quick fix.
I tested the patch and I am successfully able to boot the KVM guest.
Console Logs:
# b4 am 20250320043443.88829-1-npiggin@gmail.com --outdir patches
Analyzing 1 messages in the thread
Analyzing 0 code-review messages
Checking attestation on all messages, may take a moment...
---
✓ [PATCH] ppc/spapr: Fix RTAS stopped state
---
✓ Signed: DKIM/gmail.com
---
Total patches: 1
---
Link:
https://lore.kernel.org/qemu-devel/20250320043443.88829-1-npiggin@gmail.com
# git am patches/*.mbx
Applying: ppc/spapr: Fix RTAS stopped state
# /usr/bin/qemu-system-ppc64 -name avocado-vt-vm1 -machine
pseries,accel=kvm -m 32768 -smp 32,sockets=1,cores=32,threads=1
-nographic -device virtio-scsi-pci,id=scsi -drive
file=/home/kvmci/tests/data/avocado-vt/images/rhel8.0devel-ppc64le.qcow2,if=none,id=drive0,format=qcow2
-device scsi-hd,drive=drive0,bus=scsi.0 -netdev
bridge,id=net0,br=virbr0 -device virtio-net-pci,netdev=net0 -serial
pty -device virtio-balloon-pci -cpu host
QEMU 9.2.90 monitor - type 'help' for more information
char device redirected to /dev/pts/1 (label serial0)
(qemu)
(In another ssh session)
# screen /dev/pts/1
Preparing to boot Linux version 6.10.4-200.fc40.ppc64le
(mockbuild@c23cc4e677614c34bb22d54eeea4dc1f) (gcc (GCC) 14.2.1 20240801
(Red Hat 14.2.1-1), GNU ld version 2.41-37.fc40) #1 SMP Sun Aug 11
15:20:17 UTC 2024
Detected machine type: 0000000000000101
command line:
BOOT_IMAGE=(ieee1275/disk,msdos2)/vmlinuz-6.10.4-200.fc40.ppc64le
root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root crashkernel=1024M
Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
Calling ibm,client-architecture-support... done
memory layout at init:
memory_limit : 0000000000000000 (16 MB aligned)
alloc_bottom : 0000000008200000
alloc_top : 0000000030000000
alloc_top_hi : 0000000800000000
rmo_top : 0000000030000000
ram_top : 0000000800000000
found display : /pci@800000020000000/vga@0, opening... done
instantiating rtas at 0x000000002fff0000... done
prom_hold_cpus: skipped
copying OF device tree...
Building dt strings...
Building dt structure...
Device tree strings 0x0000000008210000 -> 0x0000000008210c0d
Device tree struct 0x0000000008220000 -> 0x0000000008230000
Quiescing Open Firmware ...
Booting Linux via __start() @ 0x0000000000440000 ...
[ 0.000000] random: crng init done
[ 0.000000] Reserving 1024MB of memory at 512MB for crashkernel
(System RAM: 32768MB)
[ 0.000000] radix-mmu: Page sizes from device-tree:
[ 0.000000] radix-mmu: Page size shift = 12 AP=0x0
[ 0.000000] radix-mmu: Page size shift = 16 AP=0x5
[ 0.000000] radix-mmu: Page size shift = 21 AP=0x1
[ 0.000000] radix-mmu: Page size shift = 30 AP=0x2
[ 0.000000] Activating Kernel Userspace Access Prevention
[ 0.000000] Activating Kernel Userspace Execution Prevention
[ 0.000000] radix-mmu: Mapped 0x0000000000000000-0x0000000003950000
with 64.0 KiB pages (exec)
[ 0.000000] radix-mmu: Mapped 0x0000000003950000-0x0000000800000000
with 64.0 KiB pages
[ 0.000000] lpar: Using radix MMU under hypervisor
[ 0.000000] Linux version 6.10.4-200.fc40.ppc64le
(mockbuild@c23cc4e677614c34bb22d54eeea4dc1f) (gcc (GCC) 14.2.1 20240801
(Red Hat 14.2.1-1), GNU ld version 2.41-37.fc40) #1 SMP Sun Aug 11
15:20:17 UTC 2024
...
...
** Boot Successful
Tested-by: Misbah Anjum N <misanjum@linux.ibm.com>
Thanks,
Misbah Anjum N
On 2025-03-20 10:04, Nicholas Piggin wrote:
> This change takes the CPUPPCState 'quiesced' field added for powernv
> hardware CPU core controls (used to stop and start cores), and extends
> it to spapr to model the "RTAS stopped" state. This prevents the
> schedulers attempting to run stopped CPUs unexpectedly, which can cause
> hangs and possibly other unexpected behaviour.
>
> The detail of the problematic situation is this:
>
> A KVM spapr guest boots with all secondary CPUs defined to be in the
> "RTAS stopped" state. In this state, the CPU is only responsive to the
> start-cpu RTAS call. This behaviour is modeled in QEMU with the
> start_powered_off feature, which sets ->halted on secondary CPUs at
> boot. ->halted=true looks like an idle / sleep / power-save state which
> typically is responsive to asynchronous interrupts, but spapr clears
> wake-on-interrupt bits in the LPCR SPR. This more-or-less works.
>
> Commit e8291ec16da8 ("target/ppc: fix timebase register reset state")
> recently caused the decrementer to expire sooner at boot, causing a
> decrementer exception on secondary CPUs in RTAS stopped state. This
> was not a problem on TCG, but KVM limits how a guest can modify LPCR,
> in
> particular it prevents the clearing of wake-on-interrupt bits, and so
> in
> the course of CPU register synchronisation, the LPCR as set by spapr to
> model the RTAS stopped state is overwritten with KVM's LPCR value, and
> that then causes QEMU's interrupt code to notice the expired
> decrementer
> exception, turn that into an interrupt, and set CPU_INTERRUPT_HARD.
>
> That causes the CPU to be kicked, and the KVM vCPU thread to loop
> calling kvm_cpu_exec(). kvm_cpu_exec() calls
> kvm_arch_process_async_events(), which on ppc just returns ->halted.
> This is still true, so it returns immediately with EXCP_HLT, and the
> vCPU never goes to sleep because qemu_wait_io_event() sees
> CPU_INTERRUPT_HARD is set. All this while the vCPU holds the bql. This
> causes the boot CPU to eventually lock up when it needs the bql.
>
> So make 'quiesced' represent the "RTAS stopped" state, and have it
> explicitly not respond to exceptions (interrupt conditions) rather than
> rely on machine register state to model that state. This matches the
> powernv quiesced state very well because it essentially turns off the
> CPU core via a side-band control unit.
>
> There are still issues with QEMU and KVM idea of LPCR diverging and
> that
> is quite ugly and fragile that should be fixed. spapr should
> synchronize
> its LPCR properly with KVM, and not try to use values that KVM does not
> support.
>
> Reported-by: Misbah Anjum N <misanjum@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/ppc/cpu.h | 11 +++++++++++
> hw/ppc/pnv_core.c | 6 +++++-
> hw/ppc/spapr_cpu_core.c | 6 ++++++
> hw/ppc/spapr_rtas.c | 5 ++++-
> target/ppc/excp_helper.c | 4 ++++
> 5 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index efab54a0683..3ee83517dcd 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1356,6 +1356,17 @@ struct CPUArchState {
> * special way (such as routing some resume causes to 0x100, i.e.
> sreset).
> */
> bool resume_as_sreset;
> +
> + /*
> + * On powernv, quiesced means the CPU has been stopped using PC
> direct
> + * control xscom registers.
> + *
> + * On spapr, quiesced means it is in the "RTAS stopped" state.
> + *
> + * The core halted/stopped variables aren't sufficient for this,
> because
> + * they can be changed with various side-band operations like qmp
> cont,
> + * powersave interrupts, etc.
> + */
> bool quiesced;
> #endif
>
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 99d9644ee38..a33977da188 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -248,21 +248,25 @@ static void pnv_core_power10_xscom_write(void
> *opaque, hwaddr addr,
>
> if (val & PPC_BIT(7 + 8 * i)) { /* stop */
> val &= ~PPC_BIT(7 + 8 * i);
> - cpu_pause(cs);
> env->quiesced = true;
> + ppc_maybe_interrupt(env);
> + cpu_pause(cs);
> }
> if (val & PPC_BIT(6 + 8 * i)) { /* start */
> val &= ~PPC_BIT(6 + 8 * i);
> env->quiesced = false;
> + ppc_maybe_interrupt(env);
> cpu_resume(cs);
> }
> if (val & PPC_BIT(4 + 8 * i)) { /* sreset */
> val &= ~PPC_BIT(4 + 8 * i);
> env->quiesced = false;
> + ppc_maybe_interrupt(env);
> pnv_cpu_do_nmi_resume(cs);
> }
> if (val & PPC_BIT(3 + 8 * i)) { /* clear maint */
> env->quiesced = false;
> + ppc_maybe_interrupt(env);
> /*
> * Hardware has very particular cases for where clear
> maint
> * must be used and where start must be used to resume
> a
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 0671d9e44b4..faf9170ba6b 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -37,6 +37,9 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
>
> cpu_reset(cs);
>
> + env->quiesced = true; /* set "RTAS stopped" state. */
> + ppc_maybe_interrupt(env);
> +
> /*
> * "PowerPC Processor binding to IEEE 1275" defines the initial
> MSR state
> * as 32bit (MSR_SF=0) with MSR_ME=1 and MSR_FP=1 in "8.2.1.
> Initial
> @@ -98,6 +101,9 @@ void spapr_cpu_set_entry_state(PowerPCCPU *cpu,
> target_ulong nip,
> CPU(cpu)->halted = 0;
> /* Enable Power-saving mode Exit Cause exceptions */
> ppc_store_lpcr(cpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
> +
> + env->quiesced = false; /* clear "RTAS stopped" state. */
> + ppc_maybe_interrupt(env);
> }
>
> /*
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 503d441b48e..78309dbb09d 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -110,7 +110,8 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU
> *cpu_,
> id = rtas_ld(args, 0);
> cpu = spapr_find_cpu(id);
> if (cpu != NULL) {
> - if (CPU(cpu)->halted) {
> + CPUPPCState *env = &cpu->env;
> + if (env->quiesced) {
> rtas_st(rets, 1, 0);
> } else {
> rtas_st(rets, 1, 2);
> @@ -215,6 +216,8 @@ static void rtas_stop_self(PowerPCCPU *cpu,
> SpaprMachineState *spapr,
> * For the same reason, set PSSCR_EC.
> */
> env->spr[SPR_PSSCR] |= PSSCR_EC;
> + env->quiesced = true; /* set "RTAS stopped" state. */
> + ppc_maybe_interrupt(env);
> cs->halted = 1;
> ppc_store_lpcr(cpu, env->spr[SPR_LPCR] & ~pcc->lpcr_pm);
> kvmppc_set_reg_ppc_online(cpu, 0);
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 44e19aacd8d..c941c89806e 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1951,6 +1951,10 @@ static int
> ppc_next_unmasked_interrupt(CPUPPCState *env)
> target_ulong lpcr = env->spr[SPR_LPCR];
> bool async_deliver;
>
> + if (unlikely(env->quiesced)) {
> + return 0;
> + }
> +
> #ifdef TARGET_PPC64
> switch (env->excp_model) {
> case POWERPC_EXCP_POWER7:
© 2016 - 2025 Red Hat, Inc.