qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] PowerPC code generation and the program counter


From: Blue Swirl
Subject: Re: [Qemu-devel] PowerPC code generation and the program counter
Date: Tue, 14 Sep 2010 17:48:30 +0000

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.

>> 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.

>> >
>> >        Thanks, Stu
>> >
>> > Here's the patch if anybody is interested:
>>
>> Please resubmit with git send-email, with Signed-off-by line and a
>> short description. I think it should be applied.
>
>
> I don't think the patch is needed.
>
> IIUC:
> When a load/store in QEMU causes a memory abort, QEMU will retranslate the
> current TB in a "search" mode that creates side tables that make it possible
> to map host PC to guest PC and restore the state. See translate-all.c:
> cpu_restore_state().
>
> For cases when that logic is not applied, the translator needs to generate
> code to put the required state up to date. For example for exceptions that
> the translator itself generates.
>
> Accesses to unmapped addresses might be a case that is not handled today.
> IMO, we should treat it similary to a memory abort and cpu_restore_state()
> in the unmapped access exception code path.

If this indeed is the case, maybe the fix will be generic and also
Sparc won't need to save state.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]