[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] PowerPC code generation and the program counter
From: |
Edgar E. Iglesias |
Subject: |
Re: [Qemu-devel] PowerPC code generation and the program counter |
Date: |
Tue, 14 Sep 2010 21:33:03 +0200 |
User-agent: |
Mutt/1.5.20 (2009-06-14) |
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
> <address@hidden> 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 <address@hidden> 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