qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Igor Mammedov
Subject: Re: [Qemu-devel] [PATCH 0/3] Move CPU model definitions to C
Date: Thu, 02 Aug 2012 13:44:12 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0

On 08/01/2012 11:53 PM, Andreas Färber wrote:
Am 01.08.2012 22:27, schrieb 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?

Because of the resulting second movement sketched below. Like I told you
some time ago I am seriously scared of loosing some tiny feature bit or
mixing up some TLAs and breaking things if I had to do that
touch-all-CPU-definitions refactoring myself. So I'd rather either avoid
it or have someone who knows that code and/or CPU better do it. So if
you touch it here that would've been a perfect fit. ;)
I could do this conversion later after as follow up series for 1.3


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

Just raising awareness. If that's what people want to do and there's
volunteers for refactoring and reviewing then fine with me. :-)

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?

http://patchwork.ozlabs.org/patch/146704/ ;-)
and the surrounding couple of series back then (Blue for sparc and Paul
for arm come to mind, cc'ed).

The key isssue is that class_init gets the class_data pointer (e.g.,
x86_def_t aka X86CPUInfo) but the initfn doesn't. Thus all data initfn
needs must either be stored into X86CPUClass (then there's two copies of
the data) or hardcoded per type in an initfn. For the -cpudef we get
arbitrary data so we'd need the former solution.

(To add to the confusion, for -cpu ...,x=y we will need to set QOM
properties on the QOM instance in the future, that's what my setters are
for that you have helped to review. Disappointing how little progress
we've made since then...)
That is thing I am working on lately, a 3 days stale state you could see at https://github.com/imammedo/qemu/tree/x86-cpu-properties.WIP I'm going to post completed series next week. I guess it's not candidate for 1.3 so no rush.

BTW: Are you going to host qom-next like last time while qemu is in freeze for 1.2?

Andreas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]