Re: [Qemu-devel] [PATCH v3 13/16] target-arm: A64: Emulate the HVC insn

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Mon, Aug 04, 2014 at 03:24:42PM +0100, Peter Maydell wrote:
> On 4 August 2014 05:12, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Fri, Aug 01, 2014 at 03:21:08PM +0100, Peter Maydell wrote:
> >> On 17 June 2014 09:45, Edgar E. Iglesias <edgar.iglesias@gmail.com> 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