From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45064) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SBQsR-0007lS-Bc for qemu-devel@nongnu.org; Sat, 24 Mar 2012 09:19:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SBQsP-0004sG-8U for qemu-devel@nongnu.org; Sat, 24 Mar 2012 09:19:26 -0400 Received: from mail-iy0-f173.google.com ([209.85.210.173]:46856) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SBQsP-0004s7-1M for qemu-devel@nongnu.org; Sat, 24 Mar 2012 09:19:25 -0400 Received: by iafj26 with SMTP id j26so6985666iaf.4 for ; Sat, 24 Mar 2012 06:19:22 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4F6CB292.4080906@suse.de> References: <1330893156-26569-1-git-send-email-afaerber@suse.de> <1331747617-7837-1-git-send-email-afaerber@suse.de> <1331747617-7837-12-git-send-email-afaerber@suse.de> <4F6CB292.4080906@suse.de> From: Blue Swirl Date: Sat, 24 Mar 2012 13:19:02 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC 11/12] target-sparc: QOM'ify CPU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Andreas_F=C3=A4rber?= Cc: qemu-devel@nongnu.org On Fri, Mar 23, 2012 at 17:27, Andreas F=C3=A4rber wrote= : > Am 14.03.2012 21:16, schrieb Blue Swirl: >> On Wed, Mar 14, 2012 at 17:53, Andreas F=C3=A4rber wr= ote: >>> diff --git a/target-sparc/cpu-qom.h b/target-sparc/cpu-qom.h >>> new file mode 100644 >>> index 0000000..15dcf84 >>> --- /dev/null >>> +++ b/target-sparc/cpu-qom.h > [...] >>> +/** >>> + * SPARCCPUClass: >>> + * @parent_reset: The parent class' reset handler. >>> + * >>> + * A SPARC CPU model. >>> + */ >>> +typedef struct SPARCCPUClass { >>> + =C2=A0 =C2=A0/*< private >*/ >>> + =C2=A0 =C2=A0CPUClass parent_class; >>> + =C2=A0 =C2=A0/*< public >*/ >>> + >>> + =C2=A0 =C2=A0void (*parent_reset)(CPUState *cpu); >>> + >>> + =C2=A0 =C2=A0target_ulong iu_version; >>> + =C2=A0 =C2=A0uint32_t fpu_version; >>> + =C2=A0 =C2=A0uint32_t mmu_version; >>> + =C2=A0 =C2=A0uint32_t mmu_bm; >>> + =C2=A0 =C2=A0uint32_t mmu_ctpr_mask; >>> + =C2=A0 =C2=A0uint32_t mmu_cxr_mask; >>> + =C2=A0 =C2=A0uint32_t mmu_sfsr_mask; >>> + =C2=A0 =C2=A0uint32_t mmu_trcr_mask; >>> + =C2=A0 =C2=A0uint32_t mxcc_version; >>> + =C2=A0 =C2=A0uint32_t features; >>> + =C2=A0 =C2=A0uint32_t nwindows; >>> + =C2=A0 =C2=A0uint32_t maxtl; >>> +} SPARCCPUClass; >>> + >>> +/** >>> + * SPARCCPU: >>> + * @env: Legacy CPU state. >>> + * >>> + * A SPARC CPU. >>> + */ >>> +typedef struct SPARCCPU { >>> + =C2=A0 =C2=A0/*< private >*/ >>> + =C2=A0 =C2=A0CPUState parent_obj; >>> + =C2=A0 =C2=A0/*< public >*/ >>> + >>> + =C2=A0 =C2=A0CPUSPARCState env; >>> + >>> + =C2=A0 =C2=A0target_ulong iu_version; >>> + =C2=A0 =C2=A0uint32_t fpu_version; >>> + =C2=A0 =C2=A0uint32_t mmu_version; >>> + =C2=A0 =C2=A0uint32_t features; >>> + =C2=A0 =C2=A0uint32_t nwindows; >>> +} SPARCCPU; >> >> The fields do not look correct at all, the same fields are in both >> structs. > > Formerly you had the model of an array of sparc_def_t structs, which you > would duplicate and then associate with CPUSPARCState, modifying the > duplicate. > SPARCCPUClass exists only once though. Therefore we cannot modify > classes based on command line parameters and must do so on the instance > instead. I have therefore duplicated some fields from class to instance > as you have noticed, initialized the object's value from the class' and > let any -cpu options modify the latter. > The same pattern has been used for arm and i386. On arm my v5 postpones > this by doing a bare-bones conversion for now; on x86 the only request > so far was to set the values via QOM properties. > >> Moreover Sparc32 and Sparc64 fields are mixed. Maybe I don't >> fully understand the conversion. > > Mixed fields likely means they were mixed in your original code. It is a > mostly mechanical conversion. I see, this is only for the sparc_def_t structure. >> Would it be possible to make a common parent class which is then >> specialized by Sparc32 and Sparc64 classes? There are many common >> fields but also many 32/64 specific ones. Also cpu_common.c, cpu32.c >> and cpu64.c to avoid #ifdeffery? > > That is possible, but I would ask that you do such split-ups later. > Discussions for arm and ppc have shown that the structure of subclasses > in lack of multi-inheritence can be a tricky and controversial issue and > requires a lot of target knowledge that I do not posses for sparc. > Also, restructuring target code, e.g., into multiple files is orthogonal > to the goal of QOM'ifying all target CPUs and doing cleanups in common co= de. > > If you think this is heading into a totally wrong direction due to some > in-progress work of yours, we could strip it down like target-arm v5, > but ATM I don't believe so. ;) No, the patch is fine as is. > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3= =BCrnberg