From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
To: qemu-ppc@nongnu.org
Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>,
Eduardo Habkost <ehabkost@redhat.com>,
qemu-devel@nongnu.org, David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH] cpu: Add starts_halted() method
Date: Tue, 7 Jul 2020 17:43:33 -0300 [thread overview]
Message-ID: <20200707204333.261506-1-bauerman@linux.ibm.com> (raw)
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 <bauerman@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";
}
next reply other threads:[~2020-07-07 20:49 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-07 20:43 Thiago Jung Bauermann [this message]
2020-07-07 21:49 ` [PATCH] cpu: Add starts_halted() method Eduardo Habkost
2020-07-07 23:28 ` Thiago Jung Bauermann
2020-07-08 8:38 ` Philippe Mathieu-Daudé
2020-07-08 10:00 ` David Gibson
2020-07-08 13:14 ` Peter Maydell
2020-07-08 15:25 ` Eduardo Habkost
2020-07-08 15:32 ` Peter Maydell
2020-07-08 16:03 ` Eduardo Habkost
2020-07-08 17:09 ` Peter Maydell
2020-07-08 17:36 ` Eduardo Habkost
2020-07-08 20:11 ` Peter Maydell
2020-07-08 21:32 ` Eduardo Habkost
2020-07-09 3:05 ` Thiago Jung Bauermann
2020-07-09 3:26 ` Thiago Jung Bauermann
2020-07-09 10:24 ` Philippe Mathieu-Daudé
2020-07-10 20:02 ` Thiago Jung Bauermann
2020-07-10 20:17 ` Eduardo Habkost
[not found] ` <87k0zdm63s.fsf@linaro.org>
2020-07-10 20:16 ` Thiago Jung Bauermann
2020-07-11 17:55 ` Alex Bennée
2020-07-08 16:45 ` Philippe Mathieu-Daudé
2020-07-08 21:39 ` Eduardo Habkost
2020-07-09 5:11 ` Philippe Mathieu-Daudé
2020-07-09 9:54 ` Greg Kurz
2020-07-09 10:18 ` Philippe Mathieu-Daudé
2020-07-09 10:55 ` Greg Kurz
2020-07-09 12:21 ` Philippe Mathieu-Daudé
2020-07-09 13:13 ` Greg Kurz
2020-07-09 13:19 ` Philippe Mathieu-Daudé
2020-07-09 13:40 ` Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200707204333.261506-1-bauerman@linux.ibm.com \
--to=bauerman@linux.ibm.com \
--cc=david@gibson.dropbear.id.au \
--cc=ehabkost@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).