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/all/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/all/tests/qtest/test-hmp --tap -p /mips64el/hmp/boston TAP version 14 # random seed: R02S0d3b1a4f1aef5198107851bdee539e7d # Start of mips64el tests # Start of hmp tests # starting QEMU: exec ./qemu-system-mips64el -qtest unix:/tmp/qtest-530181.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-530181.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -run-with exit-with-parent=on -S -M boston -accel qtest main_cpu_reset: dbg mips_gcr_reset: dbg mps_reset_exit: dbg ok 1 /mips64el/hmp/boston # End of hmp tests # End of mips64el tests 1..1 Cc: Peter Maydell <[email protected]> Message-ID: <[email protected]> Signed-off-by: Alex Bennée <[email protected]> --- v2 - use proper 3-phase reset --- include/hw/mips/cps.h | 14 +++++++++++++- hw/mips/cps.c | 26 +++++++++++++++++--------- hw/misc/mips_cmgcr.c | 1 - 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/include/hw/mips/cps.h b/include/hw/mips/cps.h index 878b4d819f4..1084a10de0f 100644 --- a/include/hw/mips/cps.h +++ b/include/hw/mips/cps.h @@ -30,7 +30,7 @@ #include "qom/object.h" #define TYPE_MIPS_CPS "mips-cps" -OBJECT_DECLARE_SIMPLE_TYPE(MIPSCPSState, MIPS_CPS) +OBJECT_DECLARE_TYPE(MIPSCPSState, MIPSCPSClass, MIPS_CPS) struct MIPSCPSState { SysBusDevice parent_obj; @@ -48,6 +48,18 @@ struct MIPSCPSState { Clock *clock; }; +/* + * MIPSCPSClass: + * @parent_phases: The parent class' reset phase handlers. + * + * A Coherent Processing System model. + */ +struct MIPSCPSClass { + SysBusDeviceClass parent_class; + + ResettablePhases parent_phases; +}; + qemu_irq get_cps_irq(MIPSCPSState *cps, int pin_number); #endif diff --git a/hw/mips/cps.c b/hw/mips/cps.c index 620ee972f8f..23918147276 100644 --- a/hw/mips/cps.c +++ b/hw/mips/cps.c @@ -55,6 +55,17 @@ static void main_cpu_reset(void *opaque) cpu_reset(cs); } +static void mps_reset_exit(Object *obj, ResetType type) +{ + MIPSCPSState *s = MIPS_CPS(obj); + 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 +76,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 +154,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 +166,6 @@ 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)); } static const Property mips_cps_properties[] = { @@ -176,8 +178,13 @@ static const Property mips_cps_properties[] = { static void mips_cps_class_init(ObjectClass *klass, const void *data) { DeviceClass *dc = DEVICE_CLASS(klass); + MIPSCPSClass *mcs = MIPS_CPS_CLASS(klass); + ResettableClass *rc = RESETTABLE_CLASS(klass); dc->realize = mips_cps_realize; + + resettable_class_set_parent_phases(rc, NULL, NULL, mps_reset_exit, + &mcs->parent_phases); device_class_set_props(dc, mips_cps_properties); } @@ -187,6 +194,7 @@ static const TypeInfo mips_cps_info = { .instance_size = sizeof(MIPSCPSState), .instance_init = mips_cps_init, .class_init = mips_cps_class_init, + .class_size = sizeof(MIPSCPSClass), }; static void mips_cps_register_types(void) 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
