Re: [PATCH 04/18] target: Declare FOO_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h'

[LIU Zhiwei]

On 2023/10/11 11:21, Philippe Mathieu-Daudé wrote:
> Hi Zhiwei,
>
> On 11/10/23 04:51, LIU Zhiwei wrote:
> > On 2023/10/10 17:28, Philippe Mathieu-Daudé wrote:
> > > Hegerogeneous code needs access to the FOO_CPU_TYPE_NAME()
> > > macro to resolve target CPU types.
> >
> > Hi Philippe,
> >
> > I don't understand why should we use FOO_CPU_TYPE_NAME macro to 
> > resolve target CPU types? In my opinion, we should pass the
> > CPU typename from command line for heterogeneous case.
> >
> > Could you make it clearer how should we use FOO_CPU_TYPE_NAME macro 
> > to resolve target CPU types in heterogeneous case?
>
> To be honest I start to feel a bit lost with the "cpu
> resolving type" design.
>
> We are not quite there yet to "create from command line"
> or "create from QMP", so I'm prototyping in plain C.
> One of my test is:
>
>   #include "target/arm/cpu-qom.h"
>   #include "target/hexagon/cpu-qom.h"
>   ...
>
>   static void
>   my_machine3_init((MachineState *machine)
>   {
>     CPUState cpu[2];
>     ...
>
>     cpu[0] = CPU(object_new(ARM_CPU_TYPE_NAME("cortex-a72")));
>     cpu[1] = CPU(object_new(HEXAGON_CPU_TYPE_NAME("v68")));
>     ...
>   }
>
> The machine code need access to the per-target
> FOO_CPU_TYPE_NAME() macros. 

I see what you mean. It works if we will pass the cpu model instead of 
cpu typename to the machine state(Not yet).

Acked-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Zhiwei

> I'm not sure what each macro
> expands to is considered stable, so IIUC I can't inline and use:
>
>     cpu[0] = CPU(object_new("cortex-a72-arm-cpu"));
>     cpu[1] = CPU(object_new("v68"-hexagon-cpu));
>
> That said, maybe I'm mistaken.
>
> Kinda related discussion with Gavin/Igor:
> https://lore.kernel.org/qemu-devel/35653f53-a977-02ea-28f6-6fe85b1efd5a@redhat.com/ 
>
> (related to 
> https://lore.kernel.org/qemu-devel/20230907003553.1636896-1-gshan@redhat.com/).
>
> > Thanks,
> > Zhiwei
> >
> > > Move the declaration
> > > (along with the required FOO_CPU_TYPE_SUFFIX) to "cpu-qom.h".
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > ---
> > >   target/alpha/cpu-qom.h   | 5 ++++-
> > >   target/alpha/cpu.h       | 2 --
> > >   target/avr/cpu-qom.h     | 5 ++++-
> > >   target/avr/cpu.h         | 2 --
> > >   target/cris/cpu-qom.h    | 5 ++++-
> > >   target/cris/cpu.h        | 2 --
> > >   target/i386/cpu-qom.h    | 3 +++
> > >   target/i386/cpu.h        | 2 --
> > >   target/m68k/cpu-qom.h    | 5 ++++-
> > >   target/m68k/cpu.h        | 2 --
> > >   target/mips/cpu-qom.h    | 3 +++
> > >   target/mips/cpu.h        | 2 --
> > >   target/rx/cpu-qom.h      | 5 ++++-
> > >   target/rx/cpu.h          | 2 --
> > >   target/s390x/cpu-qom.h   | 5 ++++-
> > >   target/s390x/cpu.h       | 2 --
> > >   target/sh4/cpu-qom.h     | 5 ++++-
> > >   target/sh4/cpu.h         | 2 --
> > >   target/sparc/cpu-qom.h   | 5 ++++-
> > >   target/sparc/cpu.h       | 2 --
> > >   target/tricore/cpu-qom.h | 5 +++++
> > >   target/tricore/cpu.h     | 2 --
> > >   target/xtensa/cpu-qom.h  | 5 ++++-
> > >   target/xtensa/cpu.h      | 2 --
> > >   24 files changed, 47 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/target/alpha/cpu-qom.h b/target/alpha/cpu-qom.h
> > > index 1f200724b6..d596d1b69f 100644
> > > --- a/target/alpha/cpu-qom.h
> > > +++ b/target/alpha/cpu-qom.h
> > > @@ -1,5 +1,5 @@
> > >   /*
> > > - * QEMU Alpha CPU
> > > + * QEMU Alpha CPU QOM header (target agnostic)
> > >    *
> > >    * Copyright (c) 2012 SUSE LINUX Products GmbH
> > >    *
> > > @@ -27,6 +27,9 @@
> > >   OBJECT_DECLARE_CPU_TYPE(AlphaCPU, AlphaCPUClass, ALPHA_CPU)
> > > +#define ALPHA_CPU_TYPE_SUFFIX "-" TYPE_ALPHA_CPU
> > > +#define ALPHA_CPU_TYPE_NAME(model) model ALPHA_CPU_TYPE_SUFFIX
> > > +
> > >   /**
> > >    * AlphaCPUClass:
> > >    * @parent_realize: The parent class' realize handler.
> > > diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
> > > index e2a467ec17..ba0d9e3468 100644
> > > --- a/target/alpha/cpu.h
> > > +++ b/target/alpha/cpu.h
> > > @@ -428,8 +428,6 @@ enum {
> > >   void alpha_translate_init(void);
> > > -#define ALPHA_CPU_TYPE_SUFFIX "-" TYPE_ALPHA_CPU
> > > -#define ALPHA_CPU_TYPE_NAME(model) model ALPHA_CPU_TYPE_SUFFIX
> > >   #define CPU_RESOLVING_TYPE TYPE_ALPHA_CPU
> > >   void alpha_cpu_list(void);
> [...]