Currently the cpu_reset() in mips_cpu_realizefn() hides an implicit sequencing requirement when setting gcr_base. Without it we barf because we end up setting the region between 0x0-0x000000001fbfffff which trips over a qtest that accesses the GCR during "memsave 0 4096 /dev/null".
By moving to the reset phase we have to drop the property lest we are admonished for "Attempting to set...after it was realized" but there doesn't seem to be a need to expose the property anyway. NB: it would be safer if I could guarantee the place in the reset tree but I haven't quite grokked how to do that yet. Currently I see this sequence when testing: ➜ env MALLOC_PERTURB_=43 G_TEST_DBUS_DAEMON=/home/alex/lsrc/qemu.git/tests/dbus-vmstate-daemon.sh UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-mips64el SPEED=thorough MESON_TEST_ITERATION=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 PYTHON=/home/alex/lsrc/qemu.git/builds/debug/pyvenv/bin/python3 QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 G_TEST_SLOW=1 RUST_BACKTRACE=1 /home/alex/lsrc/qemu.git/builds/debug/tests/qtest/test-hmp --tap -p /mips64el/hmp/boston TAP version 14 # random seed: R02S89554f0dc696ece515363e554b13b7f9 # Start of mips64el tests # Start of hmp tests # starting QEMU: exec ./qemu-system-mips64el -qtest unix:/tmp/qtest-883372.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-883372.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -run-with exit-with-parent=on -S -M boston -accel qtest mips_cpu_reset_hold: dbg mips_gcr_init: 0x5600f2160050 - 0 main_cpu_reset: dbg mips_cpu_reset_hold: dbg mps_reset: 000000001fbf8000 mips_cpu_reset_hold: dbg ok 1 /mips64el/hmp/boston # End of hmp tests # End of mips64el tests 1..1 Signed-off-by: Alex Bennée <[email protected]> Cc: Peter Maydell <[email protected]> --- hw/mips/cps.c | 22 +++++++++++++--------- hw/misc/mips_cmgcr.c | 1 - 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/hw/mips/cps.c b/hw/mips/cps.c index 620ee972f8f..c91243599e0 100644 --- a/hw/mips/cps.c +++ b/hw/mips/cps.c @@ -55,6 +55,18 @@ static void main_cpu_reset(void *opaque) cpu_reset(cs); } +static void mps_reset(void *opaque) +{ + DeviceState *dev = opaque; + MIPSCPSState *s = MIPS_CPS(dev); + hwaddr gcr_base; + + /* Global Configuration Registers - only valid once the CPU has been reset */ + gcr_base = MIPS_CPU(first_cpu)->env.CP0_CMGCRBase << 4; + memory_region_add_subregion(&s->container, gcr_base, + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gcr), 0)); +} + static bool cpu_mips_itu_supported(CPUMIPSState *env) { bool is_mt = (env->CP0_Config5 & (1 << CP0C5_VP)) || ase_mt_available(env); @@ -65,7 +77,6 @@ static bool cpu_mips_itu_supported(CPUMIPSState *env) static void mips_cps_realize(DeviceState *dev, Error **errp) { MIPSCPSState *s = MIPS_CPS(dev); - target_ulong gcr_base; bool itu_present = false; if (!clock_get(s->clock)) { @@ -144,16 +155,11 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(&s->container, 0, sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gic), 0)); - /* Global Configuration Registers */ - gcr_base = MIPS_CPU(first_cpu)->env.CP0_CMGCRBase << 4; - object_initialize_child(OBJECT(dev), "gcr", &s->gcr, TYPE_MIPS_GCR); object_property_set_uint(OBJECT(&s->gcr), "num-vp", s->num_vp, &error_abort); object_property_set_int(OBJECT(&s->gcr), "gcr-rev", 0x800, &error_abort); - object_property_set_int(OBJECT(&s->gcr), "gcr-base", gcr_base, - &error_abort); object_property_set_link(OBJECT(&s->gcr), "gic", OBJECT(&s->gic.mr), &error_abort); object_property_set_link(OBJECT(&s->gcr), "cpc", OBJECT(&s->cpc.mr), @@ -161,9 +167,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) if (!sysbus_realize(SYS_BUS_DEVICE(&s->gcr), errp)) { return; } - - memory_region_add_subregion(&s->container, gcr_base, - sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gcr), 0)); + qemu_register_reset(mps_reset, s); } static const Property mips_cps_properties[] = { diff --git a/hw/misc/mips_cmgcr.c b/hw/misc/mips_cmgcr.c index 3e262e828bc..9e1c8d26ea5 100644 --- a/hw/misc/mips_cmgcr.c +++ b/hw/misc/mips_cmgcr.c @@ -214,7 +214,6 @@ static const VMStateDescription vmstate_mips_gcr = { static const Property mips_gcr_properties[] = { DEFINE_PROP_UINT32("num-vp", MIPSGCRState, num_vps, 1), DEFINE_PROP_INT32("gcr-rev", MIPSGCRState, gcr_rev, 0x800), - DEFINE_PROP_UINT64("gcr-base", MIPSGCRState, gcr_base, GCR_BASE_ADDR), DEFINE_PROP_LINK("gic", MIPSGCRState, gic_mr, TYPE_MEMORY_REGION, MemoryRegion *), DEFINE_PROP_LINK("cpc", MIPSGCRState, cpc_mr, TYPE_MEMORY_REGION, -- 2.47.3
