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

[Peter Maydell <peter.maydell@linaro.org>]

On 25 April 2011 22:09, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Thu, Apr 21, 2011 at 05:01:48PM +0100, Peter Maydell wrote:

>> +                tb_flush(env);
>
> If you flush all tbs, you also have to ensure that on the translate.c
> side, this is the last instruction of the tb. Otherwise, the rest of the
> TB will be executed with the wrong access rights.

This is OK, because we can't get here unless we're in privileged
mode (PMUSERENR is never writable in user mode), and changing
PMUSERENR doesn't affect the access rights for privileged mode.
And a switch into user mode will be a change of TB anyway.

(Compare the handling of the TEECR, which also doesn't need to change
TB after a tb_flush(), for the same reasons.)

> 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 :-)

On the subject of complexity: I have vague plans for overhauling
the cp15 support code anyway, so you can effectively register
handler functions for the cp15 registers you care about rather
than having to have one enormous function full of nested case
statements. You could then have the access checking code not
so wildly far away from the register read/write implementation.
(Plus we need support for banked cp15 registers at some point.)

-- PMM