[PATCH] hw/misc/mips_itu: Make MIPSITUState target agnostic

[PATCH] hw/misc/mips_itu: Make MIPSITUState target agnostic

[Philippe Mathieu-Daudé]

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(-)

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(),
 };
 
-- 
2.41.0

Re: [PATCH] hw/misc/mips_itu: Make MIPSITUState target agnostic

[Richard Henderson]

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~