The commit referenced (from QEMU 10.0) has changed the way the pseries machine marks a cpu as quiesced. Previously, the cpu->halted value from QEMU common cpu code was (incorrectly) used. With the fix, the env->quiesced variable starts being used, which improves on the original situation, but also causes a side effect after migration:
The env->quiesced is set at reset and never migrated, which causes the destination QEMU to stop delivering interrupts and hang the machine. To fix the issue from this point on, start migrating the env->quiesced value. For QEMU versions < 10.0, sending the new element on the stream would cause migration to be aborted, so add the appropriate compatibility property to omit the new subsection. Independently of this patch, all migrations from QEMU versions < 10.0 will result in a hang since the older QEMU never migrates env->quiesced. This is bad because it leaves machines already running on the old QEMU without a migration path into newer versions. As a workaround, clear env->quiesced in the new QEMU whenever cpu->halted is also clear. This assumes rtas_stop_self() always sets both flags at the same time. Migrations during secondaries bringup (i.e. before rtas-start-cpu) will still cause a hang, but those are early enough that requiring reboot would not be unreasonable. Note that this was tested with -cpu power9 and -machine ic-mode=xive due to another bug affecting migration of XICS guests. Tested both forward and backward migration and savevm/loadvm from 9.2 and 10.0. Reported-by: Fabian Vogt <fv...@suse.de> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3079 Fixes: fb802acdc8b ("ppc/spapr: Fix RTAS stopped state") Signed-off-by: Fabiano Rosas <faro...@suse.de> --- The choice of PowerPCCPU to hold the compat property is dubious. This only affects pseries, but it seems like a layering violation to access SpaprMachine from target/ppc/, suggestions welcome. --- hw/core/machine.c | 1 + target/ppc/cpu.h | 1 + target/ppc/cpu_init.c | 7 +++++++ target/ppc/machine.c | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+) diff --git a/hw/core/machine.c b/hw/core/machine.c index bd47527479..ea83c0876b 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -42,6 +42,7 @@ GlobalProperty hw_compat_10_0[] = { { "vfio-pci", "x-migration-load-config-after-iter", "off" }, { "ramfb", "use-legacy-x86-rom", "true"}, { "vfio-pci-nohotplug", "use-legacy-x86-rom", "true" }, + { "powerpc64-cpu", "rtas-stopped-state", "false" }, }; const size_t hw_compat_10_0_len = G_N_ELEMENTS(hw_compat_10_0); diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 6b90543811..8ff453024b 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1470,6 +1470,7 @@ struct ArchCPU { void *machine_data; int32_t node_id; /* NUMA node this CPU belongs to */ PPCHash64Options *hash64_opts; + bool rtas_stopped_state; /* Those resources are used only during code translation */ /* opcode handlers */ diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index a0e77f2673..4380c6eb14 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -55,6 +55,11 @@ /* #define PPC_DEBUG_SPR */ /* #define USE_APPLE_GDB */ +static const Property powerpc_cpu_properties[] = { + DEFINE_PROP_BOOL("rtas-stopped-state", PowerPCCPU, + rtas_stopped_state, true), +}; + static inline void vscr_init(CPUPPCState *env, uint32_t val) { /* Altivec always uses round-to-nearest */ @@ -7525,6 +7530,8 @@ static void ppc_cpu_class_init(ObjectClass *oc, const void *data) &pcc->parent_unrealize); pcc->pvr_match = ppc_pvr_match_default; + device_class_set_props(dc, powerpc_cpu_properties); + resettable_class_set_parent_phases(rc, NULL, ppc_cpu_reset_hold, NULL, &pcc->parent_phases); diff --git a/target/ppc/machine.c b/target/ppc/machine.c index d72e5ecb94..8797233ebe 100644 --- a/target/ppc/machine.c +++ b/target/ppc/machine.c @@ -257,6 +257,23 @@ static int cpu_post_load(void *opaque, int version_id) ppc_store_sdr1(env, env->spr[SPR_SDR1]); } + if (!cpu->rtas_stopped_state) { + /* + * The source QEMU doesn't have fb802acdc8 and still uses halt + * + PM bits in LPCR to implement RTAS stopped state. The new + * QEMU will have put the newly created vcpus in that state, + * waiting for the start-cpu RTAS call. Clear the quiesced + * flag if possible, otherwise the newly-loaded machine will + * hang indefinitely due to quiesced state ignoring + * interrupts. + */ + + if (!CPU(cpu)->halted) { + /* not halted, so definitely not in RTAS stopped state */ + env->quiesced = 0; + } + } + post_load_update_msr(env); if (tcg_enabled()) { @@ -649,6 +666,28 @@ static const VMStateDescription vmstate_reservation = { } }; +static bool rtas_stopped_needed(void *opaque) +{ + PowerPCCPU *cpu = opaque; + + return cpu->rtas_stopped_state && !cpu->env.quiesced; +} + +static const VMStateDescription vmstate_rtas_stopped = { + .name = "cpu/rtas_stopped", + .version_id = 1, + .minimum_version_id = 1, + .needed = rtas_stopped_needed, + .fields = (const VMStateField[]) { + /* + * "RTAS stopped" state, independent of halted state. For QEMU + * < 10.0, this is taken from cpu->halted at cpu_post_load() + */ + VMSTATE_BOOL(env.quiesced, PowerPCCPU), + VMSTATE_END_OF_LIST() + } +}; + #ifdef TARGET_PPC64 static bool bhrb_needed(void *opaque) { @@ -715,6 +754,7 @@ const VMStateDescription vmstate_ppc_cpu = { &vmstate_tlbmas, &vmstate_compat, &vmstate_reservation, + &vmstate_rtas_stopped, NULL } }; -- 2.35.3
