From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45397) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QG7Mc-000764-JK for qemu-devel@nongnu.org; Sat, 30 Apr 2011 06:25:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QG7Mb-0002kI-3T for qemu-devel@nongnu.org; Sat, 30 Apr 2011 06:25:26 -0400 Received: from mail-iy0-f173.google.com ([209.85.210.173]:34996) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QG7Mb-0002k1-0Y for qemu-devel@nongnu.org; Sat, 30 Apr 2011 06:25:25 -0400 Received: by iym10 with SMTP id 10so4450908iym.4 for ; Sat, 30 Apr 2011 03:25:23 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: Date: Sat, 30 Apr 2011 11:25:21 +0100 Message-ID: From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] xtensa: new target architecture for qemu List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Max Filippov , qemu-devel@nongnu.org On 30 April 2011 10:06, Blue Swirl wrote: > On Sat, Apr 30, 2011 at 11:24 AM, Max Filippov wrote: >> Is it worth keeping patches as small as they are now (as they only >> reflect development chronology), or maybe it's better to consolidate >> them on some other basis? > > Small patches are nicer for review. A couple of other patch series structuring hints: * avoid having a patch which implements X and then a later patch which fixes a bug in X * make sure everything compiles at all points in the series, not just at the end; usually "enable target in configure" comes last >> What is the reason for that? DisasContext itself has a pointer to env. >> Can it be used instead? > > DisasContext shouldn't have that pointer. Some of the decisions during > translation are made based on CPU model/features which don't change > during execution. Other decisions should be made based on the current > CPU state, however usually here we should not rely on env but on > tb_flags. Not using env during disas_insn tries to enforce that. > Please take a look at other targets how this should be handled. Some targets are better choices to copy than others. target-arm passes CPUState around way too much still (it only relies on things it is OK to, but it would be nicer not to have CPUState quite so conveniently available.) I think the ideal is that none of the functions implementing decode logic (in your case disas_xtensa_insn()) are directly passed a CPUState pointer. The reason for all this is that the CPUState you get during translation may not be the state of the CPU at the start of the code you are translating, so if you rely on bits of it that can change you'll run into problems. > I'd put CPU doc pointers to translate.c and chipset pointers to > corresponding device file. We also have a wiki page for CPU docs: http://wiki.qemu.org/Documentation/ISAManuals -- PMM