On 19/10/26, Brian Cain wrote: > Let the hexagon-sim-style machines their QDSP6 subsystem, and with it the > l2vic and QTimer. > > The config table's subsystem base is updated to align with the chosen > address. > > Signed-off-by: Brian Cain <[email protected]> > --- > MAINTAINERS | 1 + > include/hw/hexagon/hexagon.h | 4 ++ > hw/hexagon/hexagon_dsp.c | 62 +++++++++++++++++++++++- > tests/qtest/hexagon-config-test.c | 78 +++++++++++++++++++++++++++++++ > tests/qtest/meson.build | 3 +- > 5 files changed, 146 insertions(+), 2 deletions(-) > create mode 100644 tests/qtest/hexagon-config-test.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 04df9db77dc..f1fbc971fed 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1386,6 +1386,7 @@ F: include/hw/misc/qcom-qdsp6ss-clk.h > F: configs/devices/hexagon-softmmu/default.mak > F: docs/system/hexagon/ > F: tests/functional/hexagon/ > +F: tests/qtest/hexagon-config-test.c > F: tests/qtest/qcom-dsp-test.c > F: docs/devel/hexagon-sys.rst > > diff --git a/include/hw/hexagon/hexagon.h b/include/hw/hexagon/hexagon.h > index 7d15a9e1eb0..32acc2aa26b 100644 > --- a/include/hw/hexagon/hexagon.h > +++ b/include/hw/hexagon/hexagon.h > @@ -44,6 +44,10 @@ typedef enum { > > #define HEXAGON_CFGSPACE_ENTRIES (128) > > +/* Offsets of q6ss cfg peripherals from the subsystem base. */ > +#define HEXAGON_L2VIC_OFFSET 0x10000 > +#define HEXAGON_QTIMER_OFFSET 0x21000 > + > union hexagon_config_table { > struct { > /* Base address of L2TCM space */ > diff --git a/hw/hexagon/hexagon_dsp.c b/hw/hexagon/hexagon_dsp.c > index 9f6f34ef9be..1f92a8adccf 100644 > --- a/hw/hexagon/hexagon_dsp.c > +++ b/hw/hexagon/hexagon_dsp.c > @@ -17,6 +17,7 @@ > #include "hw/hexagon/hex-subsys.h" > #include "hw/core/loader.h" > #include "qapi/error.h" > +#include "qapi/visitor.h" > #include "qemu/error-report.h" > #include "qemu/log.h" > #include "elf.h" > @@ -35,6 +36,7 @@ > #include "machine_cfg_v81qa_1.h.inc" > > #define TYPE_HEXAGON_DSP_MACHINE "hexagon-dsp-machine" > +#define HEXAGON_DSP_SUBSYSTEM_BASE 0xfc900000 > OBJECT_DECLARE_SIMPLE_TYPE(HexagonDspMachineState, HEXAGON_DSP_MACHINE) > > struct HexagonDspMachineState { > @@ -42,6 +44,8 @@ struct HexagonDspMachineState { > > hwaddr isdb_secure_flag; > hwaddr isdb_trusted_flag; > + uint32_t subsystem_base; > + struct hexagon_machine_config config; > }; > > static HexagonDspMachineState *current_dms; > @@ -107,6 +111,50 @@ static void do_cpu_reset(void *opaque) > cpu_reset(cs); > } > > +static void hexagon_sim_get_subsystem_base(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > + HexagonDspMachineState *dms = HEXAGON_DSP_MACHINE(obj); > + uint32_t value = dms->subsystem_base; > + > + visit_type_uint32(v, name, &value, errp); > +} > + > +static void hexagon_sim_set_subsystem_base(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > + HexagonDspMachineState *dms = HEXAGON_DSP_MACHINE(obj); > + uint32_t value; > + > + if (!visit_type_uint32(v, name, &value, errp)) { > + return; > + } > + if (!value || !QEMU_IS_ALIGNED(value, 64 * KiB)) { > + error_setg(errp, "subsystem-base must be nonzero and 64 KiB > aligned"); > + return; > + } > + > + dms->subsystem_base = value; > +} > + > +static void add_dsp_properties(ObjectClass *oc) > +{ > + object_class_property_add(oc, "subsystem-base", "uint32", > + hexagon_sim_get_subsystem_base, > + hexagon_sim_set_subsystem_base, NULL, NULL); > + object_class_property_set_description(oc, "subsystem-base", > + "Simulator subsystem base > address"); > +} > + > +static void hexagon_dsp_init(Object *obj) > +{ > + HexagonDspMachineState *dms = HEXAGON_DSP_MACHINE(obj); > + > + dms->subsystem_base = HEXAGON_DSP_SUBSYSTEM_BASE; > +} > + > static void hexagon_common_init(MachineState *machine, Rev_t rev, > const struct hexagon_machine_config *m_cfg) > { > @@ -121,7 +169,13 @@ static void hexagon_common_init(MachineState *machine, > Rev_t rev, > > machine->enable_graphics = 0; > > - hex_subsys_create(hms, m_cfg, rev); > + dms->config = *m_cfg; > + dms->config.csr_base = dms->subsystem_base; > + dms->config.l2vic_base = dms->subsystem_base + HEXAGON_L2VIC_OFFSET; > + dms->config.qtmr_region = dms->subsystem_base + HEXAGON_QTIMER_OFFSET;
Since the only requirement on `subsystem_base` is that it's non-zero and 64 KiB aligned, this addition can overflow with e.g. `subsystem-base=0xffff0000`, adding an upper-bounds check to the set function could be nice to avoid any footguns in the future. > + dms->config.cfgtable.subsystem_base = dms->subsystem_base >> 16; > + > + hex_subsys_create(hms, &dms->config, rev); > > g_autofree HexagonCPU **cpus = g_new(HexagonCPU *, machine->smp.cpus); > > @@ -183,6 +237,7 @@ static void v66g_1024_init(ObjectClass *oc, const void > *data) > mc->is_default = true; > mc->default_cpu_type = TYPE_HEXAGON_CPU_V66; > mc->default_cpus = 4; > + add_dsp_properties(oc); > } -- Anton Johansson rev.ng Labs Srl.
