RE: [PATCH 1/9] Hexagon (target/hexagon) Add support for v68/v69/v71/v73

[Taylor Simpson]

> -----Original Message-----
> From: Anton Johansson <anjo@rev.ng>
> Sent: Wednesday, April 26, 2023 1:06 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng; Brian Cain
> <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>
> Subject: Re: [PATCH 1/9] Hexagon (target/hexagon) Add support for
> v68/v69/v71/v73
> 
> On 4/26/23 04:30, Taylor Simpson wrote:
> > diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c index
> > ab40cfc283..8699db8c24 100644
> > --- a/target/hexagon/cpu.c
> > +++ b/target/hexagon/cpu.c
> > @@ -29,6 +29,22 @@ static void hexagon_v67_cpu_init(Object *obj)
> >   {
> >   }
> >
> > +static void hexagon_v68_cpu_init(Object *obj) { }
> > +
> > +static void hexagon_v69_cpu_init(Object *obj) { }
> > +
> > +static void hexagon_v71_cpu_init(Object *obj) { }
> > +
> > +static void hexagon_v73_cpu_init(Object *obj) { }
> > +
> >   static ObjectClass *hexagon_cpu_class_by_name(const char
> *cpu_model)
> >   {
> >       ObjectClass *oc;
> > @@ -382,6 +398,10 @@ static const TypeInfo hexagon_cpu_type_infos[] =
> {
> >           .class_init = hexagon_cpu_class_init,
> >       },
> >       DEFINE_CPU(TYPE_HEXAGON_CPU_V67,
> hexagon_v67_cpu_init),
> > +    DEFINE_CPU(TYPE_HEXAGON_CPU_V68,
> hexagon_v68_cpu_init),
> > +    DEFINE_CPU(TYPE_HEXAGON_CPU_V69,
> hexagon_v69_cpu_init),
> > +    DEFINE_CPU(TYPE_HEXAGON_CPU_V71,
> hexagon_v71_cpu_init),
> > +    DEFINE_CPU(TYPE_HEXAGON_CPU_V73,
> hexagon_v73_cpu_init),
> 
> The large spacing to hexagon_v*_cpu_init looks a bit odd.

I'll put them each on a single line with no line in between.

> 
> Also, do we need to provide a *_cpu_init() stub for each version? Seems
> from qom/object.c like we should be able to just default initialize it

I could point them all to a single function, but at some point, we'll want to 
execute only the instructions that are available an the specified version of 
the core.

> 
> Otherwise,
> 
> Reviewed-by: Anton Johansson <anjo@rev.ng>