Re: [Qemu-devel] [PATCH] target-arm: Minimal implementation of performance counters

[Peter Maydell]

On 26 April 2011 11:23, Aurelien Jarno <address@hidden> wrote:
> On Mon, Apr 25, 2011 at 11:35:54PM +0100, Peter Maydell wrote:
>> On 25 April 2011 23:31, Aurelien Jarno <address@hidden> wrote:
>> > On Mon, Apr 25, 2011 at 10:59:52PM +0100, Peter Maydell wrote:
>> >> On 25 April 2011 22:09, Aurelien Jarno <address@hidden> wrote:
>> >> > Instead of having this complex test for all cp15 access, but only 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 different
>> 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:
 * 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()
 * (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
 * (b) is bad because it increases the set of helper functions accessing
global env
 * and therefore helpers which throw exceptions are a bad idea

For rationale for/arguing about (b) see the comment on this patch:
 http://patchwork.ozlabs.org/patch/94384/

-- PMM