On Tue, Sep 14, 2010 at 05:48:30PM +0000, Blue Swirl wrote: > On Tue, Sep 14, 2010 at 5:05 PM, Edgar E. Iglesias > <edgar.igles...@gmail.com> wrote: > > On Tue, Sep 14, 2010 at 04:10:27PM +0000, Blue Swirl wrote: > >> On Mon, Sep 13, 2010 at 4:51 AM, Stu Grossman <stu.gross...@gmail.com> > >> wrote: > >> > I've been using qemu-12.4 to trace accesses to non-existent addresses, > >> > but I've > >> > found that the PC is incorrect when cpu_abort() is called from within the > >> > unassigned memory helper routines (unassigned_mem_read[bwl] and > >> > unassigned_mem_write[bwl]). Even nearby instructions (plus or minus 10 > >> > instructions or so) don't match the type of load or store being done, so > >> > this > >> > isn't a PC being current_instr+4 kind of thing. > >> > >> If PC is not updated to match the value at the access instruction, it > >> will point to the last instruction that did update PC, or start of the > >> translation block (TB). > >> > >> > I ended up modifying the GEN_LD* and GEN_ST* macros (in > >> > target-ppc/translate.c) > >> > to call gen_update_nip(ctx, ctx->nip - 4). This fixed the above > >> > problem, which > >> > has helped enormously. > >> > > >> > Since I'm not a qemu expert, I was wondering about several things: > >> > > >> > 1) Was it really necessary to add gen_update_nip to the load and > >> > store > >> > instructions in order to get the correct PC? Could the > >> > correct PC > >> > have been derived some other way, without a runtime cost for > >> > all > >> > basic loads and stores? > >> > >> This is the way used by Sparc. There save_state() updates PC, NPC and > >> forces lazy flag calculation. > > > > Hi! > > > > There might be reasons you might need that logic in SPARC, but in general > > I dont think the PC needs to be up to date at generated load/stores for > > qemu to cope with MMU exceptions. > > Maybe the reason is NPC and the flags, or unassigned access problem.
The lazy condition-code flags evaluation is a bit problematic at load/stores. For CRIS (which implements the lazy CC flag optimization) I avoided evaluating the flags at every load/store by instead putting the evaluation in the slow path (target-cris/op_helper.c:tlb_fill()). The generated code now only has to make sure that the condition-code bookkeeping is up to date (i.e cc_op, operands etc). It's a bit of a hack and I'm not 100% sure it's the right thing to do, but it seems to be working fine :) > >> It may be possible to avoid updating the state, if TB generation was > >> limited to allow only one instruction which can update the state per > >> TB. But shorter TBs will also decrease performance, so the trade-off > >> should be evaluated. > >> > >> > 2) As the current code lacks that fix, the basic load and store > >> > instructions will save an incorrect PC if an exception occurs. > >> > If > >> > so, how come nobody noticed this before? I think that > >> > exceptions > >> > would have srr0 pointing at the last instruction which called > >> > gen_update_nip. So when the target returns from a data > >> > exception, > >> > it might re-execute some instructions. Possibly harmless, but > >> > could > >> > lead to subtle bugs... > >> > >> Yes. Also, page fault handlers are not interested in the exact > >> location, only the page. Because we ensure that TBs will never cross > >> page boundaries, the page will be correct. > > > > > > I'm not sure I follow you here, but in general, MMU exception handlers > > need to know the exact address of the instruction that caused the exception. > > Right, I was in a severe state of confusion. :) Cheers
