PowerPC sPAPRs CPUs start in the halted state, but generic QEMU code
assumes that CPUs start in the non-halted state. spapr_reset_vcpu()
attempts to rectify this by setting CPUState::halted to 1. But that's too
late for hotplugged CPUs in a machine configured with 2 or mor threads per
core.
By then, other parts of QEMU have already caused the vCPU to run in an
unitialized state a couple of times. For example, ppc_cpu_reset() calls
ppc_tlb_invalidate_all(), which ends up calling async_run_on_cpu(). This
kicks the new vCPU while it has CPUState::halted = 0, causing QEMU to issue
a KVM_RUN ioctl on the new vCPU before the guest is able to make the
start-cpu RTAS call to initialize its register state.
This doesn't seem to cause visible issues for regular guests, but on a
secure guest running under the Ultravisor it does. The Ultravisor relies on
being able to snoop on the start-cpu RTAS call to map vCPUs to guests, and
this issue causes it to see a stray vCPU that doesn't belong to any guest.
Fix by adding a starts_halted() method to the CPUState class, and making it
return 1 if the machine is an sPAPR guest.
Signed-off-by: Thiago Jung Bauermann <bauer...@linux.ibm.com>
---
hw/core/cpu.c | 8 +++++++-
hw/ppc/spapr_cpu_core.c | 5 -----
include/hw/core/cpu.h | 2 ++
target/ppc/translate_init.inc.c | 16 ++++++++++++++++
4 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 0f23409f1d..8f9a3335d5 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -259,7 +259,7 @@ static void cpu_common_reset(DeviceState *dev)
}
cpu->interrupt_request = 0;
- cpu->halted = 0;
+ cpu->halted = cc->starts_halted();
cpu->mem_io_pc = 0;
cpu->icount_extra = 0;
atomic_set(&cpu->icount_decr_ptr->u32, 0);
@@ -275,6 +275,11 @@ static void cpu_common_reset(DeviceState *dev)
}
}
+static uint32_t cpu_common_starts_halted(void)
+{
+ return 0;
+}
+
static bool cpu_common_has_work(CPUState *cs)
{
return false;
@@ -428,6 +433,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
k->cpu_exec_exit = cpu_common_noop;
k->cpu_exec_interrupt = cpu_common_exec_interrupt;
k->adjust_watchpoint_address = cpu_adjust_watchpoint_address;
+ k->starts_halted = cpu_common_starts_halted;
set_bit(DEVICE_CATEGORY_CPU, dc->categories);
dc->realize = cpu_common_realizefn;
dc->unrealize = cpu_common_unrealizefn;
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 26ad566f42..d0ad92240c 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -36,11 +36,6 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
cpu_reset(cs);
- /* All CPUs start halted. CPU0 is unhalted from the machine level
- * reset code and the rest are explicitly started up by the guest
- * using an RTAS call */
- cs->halted = 1;
-
env->spr[SPR_HIOR] = 0;
lpcr = env->spr[SPR_LPCR];
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index b3f4b79318..7c9cd67e8d 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -219,6 +219,8 @@ typedef struct CPUClass {
vaddr (*adjust_watchpoint_address)(CPUState *cpu, vaddr addr, int len);
void (*tcg_initialize)(void);
+ uint32_t (*starts_halted)(void);
+
/* Keep non-pointer data at the end to minimize holes. */
int gdb_num_core_regs;
bool gdb_stop_before_watchpoint;
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 49212bfd90..1dc1ebbdaf 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -36,6 +36,7 @@
#include "qapi/visitor.h"
#include "hw/qdev-properties.h"
#include "hw/ppc/ppc.h"
+#include "hw/ppc/spapr.h"
#include "mmu-book3s-v3.h"
#include "sysemu/qtest.h"
#include "qemu/cutils.h"
@@ -10646,6 +10647,20 @@ static void ppc_cpu_set_pc(CPUState *cs, vaddr value)
cpu->env.nip = value;
}
+static uint32_t ppc_cpu_starts_halted(void)
+{
+ SpaprMachineState *spapr =
+ (SpaprMachineState *) object_dynamic_cast(qdev_get_machine(),
+ TYPE_SPAPR_MACHINE);
+
+ /*
+ * In sPAPR, all CPUs start halted. CPU0 is unhalted from the machine level
+ * reset code and the rest are explicitly started up by the guest using an
+ * RTAS call.
+ */
+ return spapr != NULL;
+}
+
static bool ppc_cpu_has_work(CPUState *cs)
{
PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -10922,6 +10937,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void
*data)
#endif
cc->disas_set_info = ppc_disas_set_info;
+ cc->starts_halted = ppc_cpu_starts_halted;
dc->fw_name = "PowerPC,UNKNOWN";
}
