| Message ID | 20230918073023.3846-1-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/misc/mips_itu: Make MIPSITUState target agnostic | expand |
On 9/18/23 09:30, Philippe Mathieu-Daudé wrote: > @@ -530,9 +530,12 @@ static void mips_itu_realize(DeviceState *dev, Error **errp) > if (!s->cpu0) { > error_setg(errp, "Missing 'cpu[0]' property"); > return; > + } else if (!object_dynamic_cast(OBJECT(s->cpu0), TYPE_MIPS_CPU)) { > + error_setg(errp, "MIPS ITU expects a MIPS CPU"); > + return; > } > > - env = &s->cpu0->env; > + env = &MIPS_CPU(s->cpu0)->env; > if (env->saarp) { > s->saar = env->CP0_SAAR; > }... > @@ -563,7 +566,7 @@ static Property mips_itu_properties[] = { > ITC_FIFO_NUM_MAX), > DEFINE_PROP_UINT32("num-semaphores", MIPSITUState, num_semaphores, > ITC_SEMAPH_NUM_MAX), > - DEFINE_PROP_LINK("cpu[0]", MIPSITUState, cpu0, TYPE_MIPS_CPU, MIPSCPU *), > + DEFINE_PROP_LINK("cpu[0]", MIPSITUState, cpu0, TYPE_CPU, CPUState *), > DEFINE_PROP_END_OF_LIST(), I think you want to keep TYPE_MIPS_CPU here. If you do that, I believe you could remove the dynamic_cast check above and let the MIPS_CPU macro assert, because the link cannot be set with an incorrect type. r~
diff --git a/include/hw/misc/mips_itu.h b/include/hw/misc/mips_itu.h index 35218b2d14..7917524987 100644 --- a/include/hw/misc/mips_itu.h +++ b/include/hw/misc/mips_itu.h @@ -73,7 +73,7 @@ struct MIPSITUState { /* SAAR */ uint64_t *saar; - MIPSCPU *cpu0; + CPUState *cpu0; }; /* Get ITC Configuration Tag memory region. */ diff --git a/hw/misc/mips_itu.c b/hw/misc/mips_itu.c index 0eda302db4..2b46b132fb 100644 --- a/hw/misc/mips_itu.c +++ b/hw/misc/mips_itu.c @@ -530,9 +530,12 @@ static void mips_itu_realize(DeviceState *dev, Error **errp) if (!s->cpu0) { error_setg(errp, "Missing 'cpu[0]' property"); return; + } else if (!object_dynamic_cast(OBJECT(s->cpu0), TYPE_MIPS_CPU)) { + error_setg(errp, "MIPS ITU expects a MIPS CPU"); + return; } - env = &s->cpu0->env; + env = &MIPS_CPU(s->cpu0)->env; if (env->saarp) { s->saar = env->CP0_SAAR; } @@ -563,7 +566,7 @@ static Property mips_itu_properties[] = { ITC_FIFO_NUM_MAX), DEFINE_PROP_UINT32("num-semaphores", MIPSITUState, num_semaphores, ITC_SEMAPH_NUM_MAX), - DEFINE_PROP_LINK("cpu[0]", MIPSITUState, cpu0, TYPE_MIPS_CPU, MIPSCPU *), + DEFINE_PROP_LINK("cpu[0]", MIPSITUState, cpu0, TYPE_CPU, CPUState *), DEFINE_PROP_END_OF_LIST(), };
When prototyping a heterogenous machine including the ITU, we get: include/hw/misc/mips_itu.h:76:5: error: unknown type name 'MIPSCPU' MIPSCPU *cpu0; ^ MIPSCPU is declared in the target specific "cpu.h" header, but we don't want to include it, because "cpu.h" is target specific and its inclusion taints all files including "mips_itu.h", which become target specific too. Avoid that by using the common CPUState structure. Add an extra QOM check to ensure the CPU is of type MIPS. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- RFC: ITU isn't relevant for the heterogeneous machines we are interested to model, but I wanted something relatively easy to start the discussion. I'm not really happy about this because we lose the QOM type check on the linked ArchCPU (thus I had to add it manually in the device realize() handler). But I guess this is the compromise we have to accept to include headers declaring target-specific devices in a heterogeneous container. --- include/hw/misc/mips_itu.h | 2 +- hw/misc/mips_itu.c | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-)