Re: [PATCH v2 06/12] target/arm: register separate max-v8 CPU types for arm and aarch64

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Nov 05, 2026 at 09:25:39PM +0800, Yonggang Luo wrote:
> On Thursday, November 5, 2026, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Thu, Nov 05, 2026 at 07:14:35PM +0800, Yonggang Luo wrote:
> > > On Thu, Nov 5, 2026 at 6:55 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > >
> > > > On Sun, Nov 01, 2026 at 12:40:01AM +0800, Yonggang Luo wrote:
> > > > > max-v8 was one TypeInfo whose instance initializer was chosen from the
> > > > > selected target at registration. Register both initializers, and let
> > > > > target_arm() and target_aarch64() select which type is available.
> > > > >
> > > > > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
> > > > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > > > > ---
> > > > >  target/arm/cpu.c            |  1 +
> > > > >  target/arm/cpu.h            |  1 +
> > > > >  target/arm/tcg/cpu-max-v8.c | 16 ++++++++++++----
> > > > >  3 files changed, 14 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > > > > index 510fc9eeb3b..6eb55242980 100644
> > > > > --- a/target/arm/cpu.c
> > > > > +++ b/target/arm/cpu.c
> > > > > @@ -2695,6 +2695,7 @@ void arm_cpu_register(const ARMCPUInfo *info)
> > > > >          .instance_init = arm_cpu_instance_init,
> > > > >          .class_init = info->class_init ?: cpu_register_class_init,
> > > > >          .class_data = info,
> > > > > +        .is_available = info->is_available,
> > > > >      };
> > > > >
> > > > >      type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
> > > > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > > > > index add993ef101..3240a8633ee 100644
> > > > > --- a/target/arm/cpu.h
> > > > > +++ b/target/arm/cpu.h
> > > > > @@ -1203,6 +1203,7 @@ typedef struct ARMCPUInfo {
> > > > >      const char *deprecation_note;
> > > > >      void (*initfn)(Object *obj);
> > > > >      void (*class_init)(ObjectClass *oc, const void *data);
> > > > > +    bool (*is_available)(void);
> > > > >  } ARMCPUInfo;
> > > > >
> > > > >  /**
> > > > > diff --git a/target/arm/tcg/cpu-max-v8.c b/target/arm/tcg/cpu-max-v8.c
> > > > > index 65f851f2006..e819d07e6ce 100644
> > > > > --- a/target/arm/tcg/cpu-max-v8.c
> > > > > +++ b/target/arm/tcg/cpu-max-v8.c
> > > > > @@ -121,10 +121,18 @@ void aarch32_max_v8_tcg_initfn(Object *obj)
> > > > >
> > > > >  static void cpu_max_v8_register_type(void)
> > > > >  {
> > > > > -    static const ARMCPUInfo max_v8[2] = {
> > > > > -        { .name = "max-v8", .initfn = aarch32_max_v8_tcg_initfn },
> > > > > -        { .name = "max-v8", .initfn = aarch64_max_v8_tcg_initfn },
> > > > > +    static const ARMCPUInfo max_v8_aarch32 = {
> > > > > +        .name = "max-v8",
> > > > > +        .initfn = aarch32_max_v8_tcg_initfn,
> > > > > +        .is_available = target_arm,
> > > > >      };
> > > > > -    arm_cpu_register(&max_v8[target_aarch64()]);
> > > > > +    static const ARMCPUInfo max_v8_aarch64 = {
> > > > > +        .name = "max-v8",
> > > > > +        .initfn = aarch64_max_v8_tcg_initfn,
> > > > > +        .is_available = target_aarch64,
> > > > > +    };
> > > > > +
> > > > > +    arm_cpu_register(&max_v8_aarch32);
> > > > > +    arm_cpu_register(&max_v8_aarch64);
> > > > >  }
> > > > >  type_init(cpu_max_v8_register_type)
> > > >
> > > > Conceptually these are two separate CPUs one 32-bit featureset
> > > > and one 64-bit featureset, but both given the same name. The
> > > > current code registers them only in the respective arm and aarch64
> > > > target binaries, but this CPU is an oddity overall.
> > >
> > > This is the one I did before
> > > https://lore.kernel.org/qemu-devel/20261013195606.152-1-luoyonggang@gmail.com/
> > >
> > > target/arm: defer max-v8 initializer selection until CPU creation
> > > but rejected, and I think it's fine.
> >
> > So that essentially matches the historical semantics where one
> > CPU class magically changes its behaviour based on what target
> > is running.  So in that sense, it works, and is no worse than
> > the semantics we've had for 'max' models forever.
> >
> > Splitting into 2 distinct types is worthwhile if we want to
> > make it possible for the aarch64 emulator to support a runtime
> > choice of both 32-bit and 64-bit "max" models.
> >
> > This patch doesn't allow that though, as it is creating two
> > classes with the same name and filtering which is registered.
> > If we don't intend to allow 64-bit target to ever use a 32-bit
> > 'max' CPU, then your original patch is better IMHO.
> 
> 
> Hi Daniel, this is a prepare to allow creating two classes with
> the same name, the is_available will be removed later.

Sending patches to implement an approach which is already planned
to be removed is rather making life hard on reviewers.

> 
> Maybe I should directly replace is_available with TypeBucket to get
> things be clear to review,
> but that is explicitly rejected by Pierrick
> 
> >
> >
> > > > More generally the aarch64 emulator includes all the 32-bit CPUs
> > > > that the arm emulator includes, and just adds extra 64-bit CPUs.
> > > > IOW, excluding the 32-bit max-v8 CPU is a special case - if it
> > > > were not for the clashing name, it would be included like all the
> > > > other 32-bit CPUs.
> > > >
> > > > Thus rather than using is_available to continue to exclude it,
> > > > we must instead resolve the naming problem, such that both
> > > > models can be included concurrently.
> > >
> > > I have already implemented the TypeBucket over targets/arch_mask that works
> > > properly. That same QOM name will have multiple TypeImpl.
> >
> > I'm not sure what you mean by TypeBucket, but my general thought is that
> > at the QOM class/type naming level everything must always be unique,
> > such that we have a single object modelk across QEMU.
> >
> > Any ambiguitity & its resolution (if needed) should live outside core
> > QOM typework, such as with aliases in machines/CPUs.
> 
> Yeap, it's a design choice, both renaming the QOM type with
> MachineClass::targets;
> or using TypeBucket in object.c(so all QOM types do not need rename
> including "virt" machines)
> is implemented by me, it's just finally what we want to do.

I don't know what you mean with this TypeBucket concept, can you
share more info ?

IMHO, renaming the non-unique QOM types is inescapable. From a POV of
introspecting a binary, we need to get to a single uniform type
hierarchy across QEMU where the classes are all statically defined
and unique.  This complements the work to replace instance properties
with class properties, to also give us unambiguously introspectable
types.


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|