Re: [Qemu-devel] [RFC] target-i386: register a class for each CPU model

[Andreas Färber]

Am 12.11.2012 22:48, schrieb Eduardo Habkost:
> This creates the following class hierarchy:
> 
> - TYPE_X86_CPU ("<arch>-cpu")
>  - TYPE_X86_DEFCPU "<arch>-cpu-predefined": abstract base class for the
>    predefined CPU models
>    - "<arch>-cpu-model-<model>": a class for each predefined CPU model
>  - TYPE_X86_HOST_CPU ("<arch>-cpu-model-host"): class for the "-cpu host" CPU
>    model

Although KVM was invented in Israel, we usually use left-to-right
naming, i.e. foo-<arch>-cpu. :)

> 
> On TARGET_X86_64, "<arch>" is "x86_64", on TARGET_I386, "<arch>" is
> "i386".
> 
> The trick here is to replace exactly what's implemented in the
> cpu_x86_find_cpudef() logic and nothing else. So, the only difference in
> relation to the previous code is that instead of looking at the CPU model 
> table
> on cpu_x86_find_cpudef(), we just use the right CPU class, that will fill the
> X86CPUDefinition struct on a ->init_cpudef() method.
> 
> The init_cpudef() method was added jut to not require us to kill the
> X86CPUDefinition array in the same patch. But the X86CPUDefinition
> struct may eventually go away, and all the default-value initialization
> for each CPU model can become just different defaults set on
> instance_init() or class_init().
> 
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
> I am using those class names because I don't want the CPU model names to live
> in the same namespace as all device names. I want to avoid doing the same
> mistake that was done in the arm code, that registers a QOM class named "any".
> Some CPU model names, like "qemu64", don't make any sense unless you already
> know it is a CPU model name.
> 
> As an alternative to the complex naming above, I was considering simply using
> "cpu-<model>" as the class name. I don't know what others think.

To be honest, this is not what I had in mind, it is still pretty close
to my original throw at x86 CPU subclasses.
As you mention there's two more modern alternatives, instance_init per
model or class_init per model. I thought I heard someone voice a need to
inspect classes rather than objects, so that would speak for class_init,
meaning we would need to convert
    .ext2_features = foo | bar | baz,
to
    xcc->ext2_features = foo | bar | baz;
without loosing any of those stupid flags. :-/
The upside is that in a class_init we can call any KVM ioctl we need, as
long as the class is not accessed before KVM initialization. The
previous review comment of not duplicating values between definition and
class would be resolved by only having them in the class.
The QOM properties that I had already prepared for this would then be
called after instantiating the CPU for handling the extra -cpu arguments
only. More qdev or whatever CPU properties are then only needed for
versioning of changing machine defaults and later for
inspecting/changing things via QMP.

It would be possible to do something like this patch as an intermediate
step to get there, but then we would be changing the hierarchy back and
forth... hmmm.
Your new header cleanup series looks good, so does part of the init
cleanups series, I'll review it in-depth the coming week.

Maybe we can find a way to convert -cpu host before we dive into the big
mess? And maybe we can generalize my experimental SuperH CPU name->class
mapping to the CPU base class? (just thinking loud)

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg