From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59270) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S8FA0-0007YF-NH for qemu-devel@nongnu.org; Thu, 15 Mar 2012 14:12:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S8F9u-0002bw-EM for qemu-devel@nongnu.org; Thu, 15 Mar 2012 14:12:24 -0400 Received: from cantor2.suse.de ([195.135.220.15]:50285 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S8F9u-0002bk-4Z for qemu-devel@nongnu.org; Thu, 15 Mar 2012 14:12:18 -0400 Message-ID: <4F6230FF.8090307@suse.de> Date: Thu, 15 Mar 2012 19:12:15 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1331761376-20362-1-git-send-email-afaerber@suse.de> <1331761376-20362-4-git-send-email-afaerber@suse.de> <87r4wthmbx.fsf@ginnungagap.bsc.es> In-Reply-To: <87r4wthmbx.fsf@ginnungagap.bsc.es> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v5 03/43] monitor: Don't access registers through CPUState List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?TGx1w61zIFZpbGFub3Zh?= Cc: Luiz Capitulino , qemu-devel@nongnu.org, Markus Armbruster Am 15.03.2012 17:15, schrieb Llu=C3=ADs Vilanova: > Andreas F=C3=A4rber writes: >=20 >> Use CPUX86State etc. instead (hand-converted). >=20 > Is there any reason not to move this into CPU${arch}State and instead p= rovide > virtual methods in CPUState? Yes, a very simple one: This is one of the prerequisite patches to having a QOM CPUState struct we can place such virtual methods in. Originally, not even reset was in there, but there were comments that my series was getting too long, so I moved it out of the ARM series and squashed it into what is now patch 43. > For example (a CPUState argument is implicit on all methods): >=20 > int reg_get_number(const char *regname); > const char* reg_get_name(int regnum); >=20 > /* register validity for a specific instantiation must be chekced s= omehow */ > bool reg_valid(int regnum); >=20 > size_t reg_get_size(int regnum); >=20 > /* multiple sizes must be handled somehow */ > /* precondition: get_reg_validity(regnum) =3D=3D true */ > void reg_get_value(int regnum, void *buf); > void reg_set_value(int regnum, void *buf); >=20 >=20 > This way, target-specific code would be moved where it belongs, partial= ly > merging (for example) code from both monitor.c, gdbstub.c and the targe= t > itself. That is definitely the direction I want to go with QOM CPUs. I haven't dug into gdbstub or monitor too deeply yet, beyond looking for ways to make the CPUState -> CPUArchState conversion as small as possible. In this case, I am not sure why the original authors chose not to place the code into target-*. Maybe those reasons no longer apply. Anyway, once all CPUs are converted - or if you're adventurous based on my qom-cpu-wip branch - I'd be happy to review patches moving more stuff into CPUState. :) Moving properties from CPUArchState into CPUState seemed fairly mechanical, so I attacked the more challenging problem of statically calculated offsets for TCG's icount first. I'd also envision some of the #defines we have flying around in cpu.h to turn into read-only CPU properties, like page size or number of MMU modes, so that the CPU becomes more self-describing and doesn't pollute the global namespace. > The downside is that using pointers to set/get the value is pretty > uncomfortable. We can add cpu_...(CPUState, ...) wrapper functions, like cpu_reset(). Cheers, Andreas --=20 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=BC= rnberg