Re: [Qemu-devel] [RFC qom-cpu v2 1/2] target-i386: Convert CPU definitions into X86CPU subclasses

[Eduardo Habkost]

On Tue, Jan 15, 2013 at 09:41:04AM +0100, Igor Mammedov wrote:
> On Mon, 10 Dec 2012 23:59:31 +0100
> Andreas Färber <address@hidden> wrote:
> 
> > TODO: sort classes for -cpu ?, generalize X86CPUListState, more testing
> > 
> > Signed-off-by: Andreas Färber <address@hidden>
> > Cc: Eduardo Habkost <address@hidden>
> > Cc: Igor Mammedov <address@hidden>
[...]
> The patch is just renaming of the current builtin_x86_defs
> into a bunch of functions and polluting X86CPUClass
> with fields from the former x86_def_t.
> object_new() still creates a dummy cpu instance whose defaults
> are still manually copied from X86CPUClass instead of x86_def_t.
> 

That's a good thing, isn't it? It means the patch is easier to review.
:-)

No patch alone will do everything we want, because we want to do a lot.
We need to do it one step at a time.

(BTW, why are you looking at this RFC instead of the more recent one,
that I have sent on Jan 4? [that's very similar to this one])


> What's the point in having dummy sub-classes?
> How it can help in your CPU re-factoring?

It will help us to unify the CPU creation/realization code that's
duplicated over all the architectures.

It will give libvirt an easy mechanism to list the available CPU models
that won't require parsing help output (using "qom-list-types" QMP
command).

> 
> 
> On the other hand converting features to static properties first and
> then converting X86CPU to sub-classes, yields already initialized to
> defaults sub-class completely removing notion of x86_def_t and
> not polluting X86CPUClass with redundant fields.

I wouldn't disagree with this approach in principle, but I believe our
main problem today is lack of reviewer bandwidth. I learned the hard way
that trying to clean up everything before implementing something will
make sure the code takes forever to be reviewed.


> For example see following patches on
> https://github.com/imammedo/qemu/commits/x86-cpu-classes.Jan142013
> 
> e9fd18f qdev: extend DEFINE_GENERIC_PROP() to support default values
> c65eca9 qdev: make qdev_prop_find_bit return non const so prop default value 
> could be modified
> 0311952 allow to expolit default value static props for model, family, 
> stepping & vendor props
> 8b3080e target-i386: add helpers to change default values of static 
> properties before object is created
> ed506d3 target-i386: prepare for subclasses to have its own instance of 
> static properties definitions
> a48e252 target-i386: declare subclass for qemu64 cpu model
> 9c556c2 target-i386: move cpu_x86_init() & cpu_x86_register() into it and 
> switch to subclasses. PS: implemended only for qemu64
> f5dbfe6 CPU_CLASS_NAME(qemu64) hack
> 00e15b8 target-i386: properties list are per subclass: do not set them in 
> superclass to avoid defaults set by subclass be over-written
> 
> -- 
> Regards,
>   Igor

-- 
Eduardo