Re: [Qemu-devel] [PATCH 0/3] Move CPU model definitions to C

[Eduardo Habkost]

On Wed, Aug 01, 2012 at 10:04:50PM +0200, Andreas Färber wrote:
> Am 01.08.2012 20:45, schrieb Eduardo Habkost:
> > This makes the change we discussed on the latest KVM conf call[1], moving 
> > the
> > existing cpudefs from cpus-x86_64.conf to the C code.
> > 
> > The config file data was converted to C using a script, available at:
> > https://gist.github.com/3229602
> > 
> > Except by the extra square brackets around the CPU model names (indicating 
> > they
> > are built-in models), the output of "-cpu ?dump" is exactly the same before 
> > and
> > after applying this series.
> > 
> > [1] http://article.gmane.org/gmane.comp.emulators.kvm.devel/95328
> > 
> > Eduardo Habkost (3):
> >   i386: add missing CPUID_* constants
> >   move CPU models from cpus-x86_64.conf to C
> >   eliminate cpus-x86_64.conf file
> 
> If we do this, we will need to refactor the C code again for CPU
> subclasses. Can't we (you) do that in one go then? :-)

Why again? The refactor for classes would be a one-time mechanical
process, won't it?

Anyway, I wouldn't do it in a single step. I prefer doing things one
small step at a time.


> 
> I thought there was a broad consensus not to go my declarative
> X86CPUInfo way but to initialize CPUs imperatively as done for ARM.
> That would mean transforming your
> 
> +    {
> +        .family = 6,
> +        .model = 2,
> ...
> 
> into an initfn doing
> 
> -    {
> +static void conroe_initfn(Object *obj)
> +{
> +    X86CPU *cpu = X86_CPU(obj);
> +    CPUX86State *env = &cpu->env;
> +
> -        .family = 6,
> +    env->family = 6;
> -        .model = 2,
> +    env->model = 2;
> ...
> 
> or so (not all being as nicely aligned).

Really? I am surprised that this is the consensus. Why would one want to
transform machine-friendly data into a large set of opaque functions
that look all the same and contains exactly the same data inside it, but
in a too verbose, machine-unfriendly and refactor-unfriendly way? It
doesn't make sense to me.

I will look for previous discussions to try to understand the reason for
that (was this discussed on qemu-devel?). Do you have any pointers
handy?


> 
> Unfortunately the move of the CPU definitions from config file to C does
> not eliminate the issue that users can still specify CPU models in their
> own config files or from the command line, right? Doesn't that mean that
> either we need to keep all CPU definitions declarative as done here and
> live with any field duplication between X86CPUInfo and X86CPUClass, or
> have a special intermediate subclass UserX86CPUClass with such fields
> for user-specified -cpudef models?
> Assuming, dropping -cpudef for 1.2 is still not an option.

I would be happy to drop support for cpudef config sections completely.
The only problem is compatibility, but personally I wouldn't mind
breaking it (I don't think many users have been writing their own
cpudefs).

Anyway, if we keep supporting it for compatibility (it could be simply
one separated CPU subclass that uses the config file data), at least we
don't have the need for versioning/compatibility mechanisms for
user-provided cpudefs.

> 
> Regards,
> Andreas
> 

-- 
Eduardo