From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=58904 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OvbFe-0002gn-Ve for Qemu-devel@nongnu.org; Tue, 14 Sep 2010 15:33:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OvbFb-00009L-RL for Qemu-devel@nongnu.org; Tue, 14 Sep 2010 15:33:10 -0400 Received: from mail-ey0-f173.google.com ([209.85.215.173]:57136) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OvbFb-00009D-KB for Qemu-devel@nongnu.org; Tue, 14 Sep 2010 15:33:07 -0400 Received: by eyf18 with SMTP id 18so3811871eyf.4 for ; Tue, 14 Sep 2010 12:33:06 -0700 (PDT) Date: Tue, 14 Sep 2010 21:33:03 +0200 From: "Edgar E. Iglesias" Subject: Re: [Qemu-devel] PowerPC code generation and the program counter Message-ID: <20100914193303.GA16690@laped.lan> References: <20100914170501.GA16141@laped.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Stu Grossman , Qemu-devel@nongnu.org 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 > 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 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