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 <afaerber@suse.de> wrote:

...

TODO: sort classes for -cpu ?, generalize X86CPUListState, more testing

Signed-off-by: Andreas Färber <afaerber@suse.de> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> [...] 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