Re: [PATCH v2 1/2] target/ppc: Fix env->quiesced migration

[Harsh Prateek Bora]

One minor comment below:

<snip>

On 05/01/26 2:30 pm, Harsh Prateek Bora wrote:

diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index d72e5ecb94..ba63a7debb 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -6,6 +6,7 @@
  #include "mmu-hash64.h"
  #include "migration/cpu.h"
  #include "qapi/error.h"
+#include "qemu/error-report.h"

I do not see any error reporting being added in the patch below.
Does this header inclusion need to be removed ?

regards,
Harsh

  #include "kvm_ppc.h"
  #include "power8-pmu.h"
  #include "system/replay.h"

@@ -257,6 +258,45 @@ 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 (this)

+         * QEMU will have put the secondary vcpus in stopped state,

+         * waiting for the start-cpu RTAS call. That call will never come +         * if the source cpus were already running. Try to infer the cpus

+         * state and set env->quiesced accordingly.
+         *
+         * env->quiesced = true  ==> the cpu is waiting to start
+         * env->quiesced = false ==> the cpu is running (unless halted)
+         */
+
+        /*
+         * Halted _could_ mean quiesced, but it could also be cede,
+         * confer_self, power management, etc.
+         */
+        if (CPU(cpu)->halted) {
+            PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+            /*
+             * Both the PSSCR_EC bit and LPCR PM bits set at cpu reset
+             * and rtas_stop and cleared at rtas_start, it's a good
+             * heuristic.
+             */
+            if ((env->spr[SPR_PSSCR] & PSSCR_EC) &&
+                (env->spr[SPR_LPCR] & pcc->lpcr_pm)) {
+                env->quiesced = true;
+            } else {
+                env->quiesced = false;
+            }
+        } else {
+            /*
+             * Old QEMU sets halted during rtas_stop_self. Not halted,
+             * therefore definitely not quiesced.
+             */
+            env->quiesced = false;
+        }
+    }
+
      post_load_update_msr(env);
      if (tcg_enabled()) {

@@ -649,6 +689,28 @@ static const VMStateDescription vmstate_reservation = {

      }
  };
+static bool rtas_stopped_needed(void *opaque)
+{
+    PowerPCCPU *cpu = opaque;
+
+    return cpu->rtas_stopped_state;
+}
+
+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 +777,7 @@ const VMStateDescription vmstate_ppc_cpu = {
          &vmstate_tlbmas,
          &vmstate_compat,
          &vmstate_reservation,
+        &vmstate_rtas_stopped,
          NULL
      }
  };