From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56781) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwtpM-0006oz-11 for qemu-devel@nongnu.org; Thu, 02 Aug 2012 07:44:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwtpF-00066U-O4 for qemu-devel@nongnu.org; Thu, 02 Aug 2012 07:44:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:24360) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwtpF-00066N-FT for qemu-devel@nongnu.org; Thu, 02 Aug 2012 07:44:21 -0400 Message-ID: <501A680C.5090403@redhat.com> Date: Thu, 02 Aug 2012 13:44:12 +0200 From: Igor Mammedov MIME-Version: 1.0 References: <1343846728-8611-1-git-send-email-ehabkost@redhat.com> <50198BE2.4050005@suse.de> <20120801202758.GC3702@otherpad.lan.raisama.net> <5019A56F.2000605@suse.de> In-Reply-To: <5019A56F.2000605@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 0/3] Move CPU model definitions to C List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Andreas_F=E4rber?= Cc: Peter Maydell , Eduardo Habkost , qemu-devel@nongnu.org, Blue Swirl , Paul Brook , Anthony Liguori On 08/01/2012 11:53 PM, Andreas F=E4rber wrote: > Am 01.08.2012 22:27, schrieb Eduardo Habkost: >> On Wed, Aug 01, 2012 at 10:04:50PM +0200, Andreas F=E4rber wrote: >>> Am 01.08.2012 20:45, schrieb Eduardo Habkost: >>>> This makes the change we discussed on the latest KVM conf call[1], m= oving 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 (indi= cating 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 yo= u > 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 avoi= d > 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 =3D 6, >>> + .model =3D 2, >>> ... >>> >>> into an initfn doing >>> >>> - { >>> +static void conroe_initfn(Object *obj) >>> +{ >>> + X86CPU *cpu =3D X86_CPU(obj); >>> + CPUX86State *env =3D &cpu->env; >>> + >>> - .family =3D 6, >>> + env->family =3D 6; >>> - .model =3D 2, >>> + env->model =3D 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, b= ut >> 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 f= or >> 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 o= f > 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=3Dy we will need to set QOM > properties on the QOM instance in the future, that's what my setters ar= e > 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=20 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=20 for 1.3 so no rush. BTW: Are you going to host qom-next like last time while qemu is in=20 freeze for 1.2? > > Andreas >