From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40422) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XEK0A-0006JP-Iq for qemu-devel@nongnu.org; Mon, 04 Aug 2014 11:16:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XEK01-000852-9k for qemu-devel@nongnu.org; Mon, 04 Aug 2014 11:16:42 -0400 Received: from mail-pa0-x231.google.com ([2607:f8b0:400e:c03::231]:34133) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XEK00-00084R-SD for qemu-devel@nongnu.org; Mon, 04 Aug 2014 11:16:33 -0400 Received: by mail-pa0-f49.google.com with SMTP id hz1so10191864pad.36 for ; Mon, 04 Aug 2014 08:16:32 -0700 (PDT) Date: Tue, 5 Aug 2014 01:15:33 +1000 From: "Edgar E. Iglesias" Message-ID: <20140804151533.GA27221@zapo.iiNet> References: <1402994746-8328-1-git-send-email-edgar.iglesias@gmail.com> <1402994746-8328-14-git-send-email-edgar.iglesias@gmail.com> <20140804041228.GW13735@toto> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v3 13/16] target-arm: A64: Emulate the HVC insn List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Rob Herring , Peter Crosthwaite , Fabian Aggeler , QEMU Developers , Alexander Graf , Blue Swirl , John Williams , Greg Bellows , Paolo Bonzini , Alex =?iso-8859-1?Q?Benn=E9e?= , Christoffer Dall , Richard Henderson On Mon, Aug 04, 2014 at 03:24:42PM +0100, Peter Maydell wrote: > On 4 August 2014 05:12, Edgar E. Iglesias wrote: > > On Fri, Aug 01, 2014 at 03:21:08PM +0100, Peter Maydell wrote: > >> On 17 June 2014 09:45, Edgar E. Iglesias wrote: > >> > + case 2: > >> > + if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_pl == 0) { > >> > + unallocated_encoding(s); > >> > + break; > >> > + } > >> > + tmp = tcg_const_i32(syn_aa64_hvc(imm16)); > >> > + gen_a64_set_pc_im(s->pc); > >> > >> (This set_pc_im is unnecessary.) > >> > >> > + gen_helper_hvc(cpu_env, tmp); > >> > >> This means that exceptions due to HVC are going to be > >> runtime-detected and cause us to go through and retranslate > >> the TB to determine the guest PC. Maybe we should do: > >> > >> /* This helper will raise EXCP_UDEF if HVC is not permitted */ > >> gen_helper_hvc_access_check(cpu_env); > >> /* Common case: HVC causes EXCP_HVC */ > >> gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16)); > >> > >> Then you only get the overhead of retranslating the TB in the > >> unexpected case where the guest does something dumb and > >> executes an HVC that UNDEFs. > > > > That doesn't match my understanding of what will happen with this kind > > of exception raising. I think the set_pc_im matters and there won't > > be any retranslation of TBs to figure out the guest PC. > > Sorry, yes; you're right and I was wrong -- we only retranslate > where we call cpu_restore_state(), which is done only where > tlb_fill() is going to raise an exception. > > (I think I need to think a bit about how I'm currently implementing > architectural debug singlestep, since at the moment I assume > that you can tell at translate time whether something is going > to be a valid SMC/HVC/SVC or not, and so whether or not to > advance the singlestep state machine. Maybe I can defer that > to exception entry...) Aha, I see. THere is actually another twist to this code that I found while testing more. The UDEFs and SMC route to EL2 case should raise the exception on the SMC/HVC itself, while the success case should raise it with ELR pointing ahead of the SMC/HVC insn. My first thought was to fixup the PC in the helper but a split helper approach might be OK aswell if it helps your debug implementation. I'll look more at it tomorrow. Thanks, Edgar > > thanks > -- PMM