On Sun, May 15, 2011 at 01:01:40AM +0300, Blue Swirl wrote: > On Sun, May 15, 2011 at 12:32 AM, Aurelien Jarno <aurel...@aurel32.net> wrote: > > On Fri, May 06, 2011 at 03:32:27PM +0100, Peter Maydell wrote: > >> On 26 April 2011 11:23, Aurelien Jarno <aurel...@aurel32.net> wrote: > >> > On Mon, Apr 25, 2011 at 11:35:54PM +0100, Peter Maydell wrote: > >> >> On 25 April 2011 23:31, Aurelien Jarno <aurel...@aurel32.net> wrote: > >> >> > On Mon, Apr 25, 2011 at 10:59:52PM +0100, Peter Maydell wrote: > >> >> >> On 25 April 2011 22:09, Aurelien Jarno <aurel...@aurel32.net> 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() > > > > 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(). > > > >> * (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/ > >> > > > > 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. >
That's true for the fastpath, but the slowpath really need to have access to the env register (for example to access mem_io_pc, mem_io_vaddr, iotlb). Also looking at the softmmu code, it seems to be possible to call cpu_loop_exit() without env, using cpu_single_env instead. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net