Re: [Xen-devel] xen/arm: Software Step ARMv8 - PC stuck on instruction

[Julien Grall <julien.grall@xxxxxxx>]

On 03/08/17 11:46, James Morse wrote:
> Hi Florian,
>
> On 03/08/17 11:16, Florian Jakobsmeier wrote:
> > This is the current version:
> I'm not familiar with Xen, so forgive my annotations:
>
> > asmlinkage void leave_hypervisor_tail(void)
> > >  {
> > > +    /*This methode will be called after the 'guest_entry' macro in
> > > /arch/arm64/entry.S set guest registers
> > > +    Check single_step_enabled flag in domain struct here and set needed
> > > registers
> > > +    */
> > > +
> > > +    struct vcpu *v = current;
> > > +
> > > +    if ( unlikely(v->domain->arch.monitor.singlestep_enabled ) )
> > > +    {
> > > +        if(!(guest_cpu_user_regs()->cpsr & 0b1000))
> This tests SPSR.M[4], which is set for exits from AARCH32.
>
>
> > > +        {
> > > +            WRITE_SYSREG(READ_SYSREG(MDSCR_EL1) | 0x1, MDSCR_EL1);
> Sets MDSCR.EL1.SS to enable the state machine.
>
>
> > > +            WRITE_SYSREG(READ_SYSREG(MDCR_EL2)  | HDCR_TDE, MDCR_EL2);
> Sets TDE to trap debug exceptions to EL2 from lower exception levels. I'm
> surprised this isn't always set for Xen. Do you allow guests to use the debug
> features for their own purposes?
No. We already trap debug exceptions to EL2 by default and MDCR_EL2 
should already be configured correctly for that.
> > > +            guest_cpu_user_regs()->cpsr = guest_cpu_user_regs()->cpsr |
> > > 0x200000;
> Sets SPSR.SS to suppress the step exception in the guest until its executed an
> instruction.
>
>
> > > +            WRITE_SYSREG( READ_SYSREG(DAIF) & ~0x200, DAIF);
> Here I'm confused. This looks like you are clearing PSTATE.D from the DAIF
> register for EL2. This means debug exceptions are unmasked for exceptions from
> Xen at EL2.
>
> If you ERET with PSTATE.D clear the SPSR.SS bit will be discarded instead. I
> thin this is your bug. The rules in 'D2.12.4 Entering the active-not-pending
> state' require 'Debug exceptions are disabled from the current Exception level'.
>
> You've set MDSCR_EL1.SS, if you also have MDSCR_EL1.KDE set you will start
> single-stepping Xen once the register writes take effect, (which may be before
> or after this:)
>
> > > +            isb();
> What are you synchronising here? (Unless you want to single-step Xen I suspect
> you don't need this at all.)
Xen is been over cautious with isb at the moment :). I think this one is 
not necessary because you will affect EL1/EL0 context and it will be 
synchronized on the eret.
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel