From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45382) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbYiV-0007If-CJ for qemu-devel@nongnu.org; Mon, 04 Jun 2012 10:57:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SbYiN-0003iN-TM for qemu-devel@nongnu.org; Mon, 04 Jun 2012 10:57:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:10144) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbYiN-0003hp-Ka for qemu-devel@nongnu.org; Mon, 04 Jun 2012 10:57:03 -0400 Message-ID: <4FCCCCB2.9090103@redhat.com> Date: Mon, 04 Jun 2012 16:56:50 +0200 From: Igor Mammedov MIME-Version: 1.0 References: <1338329426-28118-1-git-send-email-imammedo@redhat.com> <1338329426-28118-9-git-send-email-imammedo@redhat.com> <4FC63B30.5040206@suse.de> In-Reply-To: <4FC63B30.5040206@suse.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH qom-next 08/12] target-i386: introduce cpu-model property for x86_cpu List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-15?Q?Andreas_F=E4rber?= Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, Eduardo Habkost , stefano.stabellini@eu.citrix.com, sw@weilnetz.de, mtosatti@redhat.com, qemu-devel@nongnu.org, agraf@suse.de, blauwirbel@gmail.com, jcmvbkbc@gmail.com, avi@redhat.com, jan.kiszka@siemens.com, anthony.perard@citrix.com, pbonzini@redhat.com, rth@twiddle.net On 05/30/2012 05:22 PM, Andreas F=E4rber wrote: > Am 30.05.2012 00:10, schrieb Igor Mammedov: >> it's probably intermidiate step till cpu modeled as >> sub-classes. After then we probably could drop it. >> >> However it still could be used for overiding default >> cpu subclasses definition, and probably renamed to >> something like 'features'. >> >> v2: >> - remove accidential tcg_* init code move >> >> Signed-off-by: Igor Mammedov >> --- >> cpu-defs.h | 2 +- >> hw/pc.c | 10 ---------- >> target-i386/cpu.c | 24 ++++++++++++++++++++++++ >> target-i386/helper.c | 17 ++++++++++++----- >> 4 files changed, 37 insertions(+), 16 deletions(-) > > For me this is still the big no-go in this series: > > * Moving the default cpu_model into cpu_x86_init() buys us nothing IMO, > it duplicates the property setting code and differs from all other > targets where the default CPU is the machine's decision. Agreed. > > * As you rightly point out, we are heading towards sub-classes and that > contradicts this two-step initialization. I don't see how this is an > intermediate step? It's not clear to me how sub-classes contradict with two-step initializat= ion, , could you elaborate more on this? About cpu_model property being an intermediate step, I think it is an easy and safe way to hide/isolate cpu implementation details in cpu.c and use only QOM interface to create cpus. Later with an introduction of sub-classes it could be reduced to feature override functionality and when cpu features are converted to proper properties then cpu-model property could be dropped altogether. There are 2 problems I am solving with cpu-model property: 1 - APIC should be created before cpu becomes run-able i.e. before x86_cpu_realize(). Now it's not so but cpu_reset() that is called after A= PIC is created kind of 'fixes' issue. Moving APIC [11/12] creation into prope= rty forces us to create it at the proper time. 2 - I'm not sure yet how to deal with APIC creation with sub-classes introduction. Should it be created in an each sub-class initfn /doubts: code duplication/ or in parent class /doubts: lack of APICless 4= 86 cpu model/? I could move APIC from cpu-model property in initfn right now and create it there and in cpu-model property destroy APIC if cpu_def doesn't have APIC feature /only 486cpu, may we ignore it?/ or if feature is asked to be disabled via feature flag in cpu-model string then just disable it as it's done for the rest of supported cpus in real hw. > I admit, I am the one to blame for not redoing the x86 CPU subclasses > patches yet - major issue being the built-in vs. -cpudef split: > Now that Eduardo refactored the config file reading, I wonder if we can > outsource the built-in CPUs to the config file? If so, then I would > appreciate one of you x86 experts to do that please (may need some > rebasing when the occasional features get added/tweaked). Then I can > base a series on top that initializes CPU subclasses in a consistent wa= y > rather than duplicating data for the builtins just to make -cpudef work > or creating different intermediate subclasses for both types. I'll ask Eduardo how I can help here. > > * To override CPU features you should think about how to set x86 CPU > features in a QOM way (think QMP), then we can design the infrastructur= e > for setting them through global properties (or whatever needed) around > it. I honestly don't know what the requirements are in practice, so I > can't make suggestions there without some more feedback. > > Regards, > Andreas > --=20 ----- Igor