From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60906) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QLMuR-0005tO-F4 for qemu-devel@nongnu.org; Sat, 14 May 2011 18:02:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QLMuQ-0006Wx-5d for qemu-devel@nongnu.org; Sat, 14 May 2011 18:02:03 -0400 Received: from mail-qy0-f173.google.com ([209.85.216.173]:43368) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QLMuP-0006Wt-V1 for qemu-devel@nongnu.org; Sat, 14 May 2011 18:02:02 -0400 Received: by qyk36 with SMTP id 36so926017qyk.4 for ; Sat, 14 May 2011 15:02:01 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20110514213247.GB13600@volta.aurel32.net> References: <1303401708-5419-1-git-send-email-peter.maydell@linaro.org> <20110425210953.GH21831@volta.aurel32.net> <20110425223146.GG6181@volta.aurel32.net> <20110426102307.GA20532@volta.aurel32.net> <20110514213247.GB13600@volta.aurel32.net> From: Blue Swirl Date: Sun, 15 May 2011 01:01:40 +0300 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] target-arm: Minimal implementation of performance counters List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: Peter Maydell , patches@linaro.org, Paul Brook , qemu-devel@nongnu.org On Sun, May 15, 2011 at 12:32 AM, Aurelien Jarno wro= te: > On Fri, May 06, 2011 at 03:32:27PM +0100, Peter Maydell wrote: >> On 26 April 2011 11:23, Aurelien Jarno wrote: >> > On Mon, Apr 25, 2011 at 11:35:54PM +0100, Peter Maydell wrote: >> >> On 25 April 2011 23:31, Aurelien Jarno wrote: >> >> > On Mon, Apr 25, 2011 at 10:59:52PM +0100, Peter Maydell wrote: >> >> >> On 25 April 2011 22:09, Aurelien Jarno wrot= e: >> >> >> > Instead of having this complex test for all cp15 access, but onl= y for >> >> >> > catching a few access to performance registers, wouldn't it make= more >> >> >> > sense to have this test and an exception triggering directly in >> >> >> > helper.c? >> >> >> >> >> >> That was what my first design did, but in discussions on IRC >> >> >> with Paul Brook he basically said that you can't generate an >> >> >> exception in the helper routine, you have to either generate >> >> >> runtime code to do the test or throw away the TBs. Unfortunately >> >> >> I forget the exact rationale, so I've cc'd Paul to remind me :-) >> >> > >> >> > This is something strange, plenty of targets are raising exceptions= from >> >> > helpers without any problem. >> >> >> >> You'd at minimum need to move the cp15 helper functions to a differen= t >> >> file, they're currently in helper.c which doesn't get compiled >> >> with access to the global 'env' register. >> >> > I agree, but it's something that has to be done sooner or later anyway= . >> >> I just spoke with Paul on IRC about this. In summary: >> =C2=A0* for a helper to cause an exception then it has (a) to make sure = CPU >> state (pc, condflags) is sync'd before the call to the helper and (b) >> the helper has to be in a file with access to global env, because it >> needs to call cpu_loop_exit() > > I don't think (a) is true. It is possible to use the same way as for > load/store operations, that is call cpu_restore_state() before calling > cpu_loop_exit(). > >> =C2=A0* (a) is a bit fragile because it's easy to forget and if TCG gets >> cleverer things might break in a hard-to-track down way >> =C2=A0* (b) is bad because it increases the set of helper functions acce= ssing >> global env >> =C2=A0* and therefore helpers which throw exceptions are a bad idea >> >> For rationale for/arguing about (b) see the comment on this patch: >> =C2=A0http://patchwork.ozlabs.org/patch/94384/ >> > > If you don't really like having env as a fixed host registers (recent > patch series from Blue Swirl seems to want to get rid of this), it is > possible to pass env as an argument of the helper to do that. > > What ever solution is used, we need env for the load/store functions, Currently this is true, but actually qemu_ld/st only need a pointer to the TLB. If the address is a constant, some of the TLB calculations could be performed at translation time. This would need a very different approach to qemu_ld/st than current one.