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 <peter.maydell@linaro.org>
Message-ID: <20260108143423.1378674-9-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
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
On Thu, 19 Feb 2026 at 17:18, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> 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
This works because we call reset hooks in the order
they're registered, and clearly the qemu_register_reset()
in mips_cps_realize() gets in first.
To actually get the three-phase ordering so that the
'exit' phase hook you add in this patch definitely
runs after the 'hold' phase of the CPU, you would need
to stop using cpu_reset() and instead use
use qemu_register_resettable(OBJ(cpu));.
> ok 1 /mips64el/hmp/boston
> # End of hmp tests
> # End of mips64el tests
> 1..1
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Message-ID: <20260108143423.1378674-9-alex.bennee@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> 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));
It still seems odd to me that we are messing with the GCR device
in this CPS reset method. The TYPE_MIPS_GCR device (in hw/misc/mips_cmgcr.c)
is the one that owns these memory regions and it has code that
adjusts their addresses at runtime based on guest register writes.
It would seem more logical to have the CPs code just set things up
(e.g. map them at address 0) in realize, and then have the GCR
device's reset (exit phase) set the address/enabled of the MRs
according to their required state at reset.
thanks
-- PMM
On 2/19/26 9:18 AM, Alex Bennée wrote: > 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 <peter.maydell@linaro.org> > Message-ID: <20260108143423.1378674-9-alex.bennee@linaro.org> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > --- > 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(-) > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
© 2016 - 2026 Red Hat, Inc.