Maruks reported in [1] a problem with QMP introspection with qemu-system-riscv64 that happens after querying a profile CPU. QEMU crashes hitting an abort during satp_mode logic that, somehow, is being triggered via profile code.
For every profile CPU that goes through init we're setting an 'enabled' flag in the static RISCVProfile object. This sounded like an okay idea back when profiles were introduced, but now we've realized that QMP will querying CPUs without exiting the QEMU process while also going multiple times through cpu_init() and friends. The result is that we're in a situation where we have runtime information being stored in static arrays that persists between CPU definitions being introspected. A proper fix is to get rid of profile->enabled and turn profiles into cpu->cfg flags like we do with every other extension. Unfortunately we've been using profile->enabled for lots of things and a redesign would be in order, one which I'm not comfortable doing right now - we're too close to the QEMU 11.2 code freeze for such a change. For now let's silver tape a fix in cpu_init() to reset profile->enabled for all available profiles. We'll do a proper redesign in the next release cycle. [1] https://lore.kernel.org/qemu-devel/[email protected]/ Reported-by: Markus Armbruster <[email protected]> Signed-off-by: Daniel Henrique Barboza <[email protected]> --- target/riscv/cpu.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 08a4d14ea1..46b74302db 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1550,6 +1550,25 @@ static void riscv_cpu_init(Object *obj) cpu->env.vext_ver = VEXT_VERSION_1_00_0; cpu->cfg.max_satp_mode = -1; + /* + * The way we're handling profiles does not cope well with QMP + * instropection. We have a static array that stores RISCVCPUProfiles + * objs that hold runtime state for the lifetime of the QEMU process. + * Thus setting profile->enabled once, e.g. when instropecting a + * profile CPU, will leave the flag set for subsequent intropections. + * + * For now, clear all 'enabled' flags to avoid misleading QMP w.r.t + * profiles being set. + * + * FIXME: a better long term solution is to remove profile->enabled + * and just use regular cpu->cfg flags, avoiding static variables + * storing dynamic state. This will help us not only with QMP but + * also single-binary support. + */ + for (int i = 0; riscv_profiles[i] != NULL; i++) { + riscv_profiles[i]->enabled = false; + } + if (mcc->def->profile) { mcc->def->profile->enabled = true; } -- 2.43.0
