Re: [Xen-devel] [RFC 01/22] xen/arm: do_trap_instr_abort_guest: Move the IPA computation out of the switch

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 16/08/2016 01:21, Stefano Stabellini wrote:
> On Thu, 28 Jul 2016, Julien Grall wrote:
> > A follow-up patch will add more case to the switch that will require the
> > IPA. So move the computation out of the switch.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > ---
> >  xen/arch/arm/traps.c | 36 ++++++++++++++++++------------------
> >  1 file changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index 683bcb2..46e0663 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -2403,35 +2403,35 @@ static void do_trap_instr_abort_guest(struct 
> > cpu_user_regs *regs,
> >      int rc;
> >      register_t gva = READ_SYSREG(FAR_EL2);
> >      uint8_t fsc = hsr.iabt.ifsc & ~FSC_LL_MASK;
> > +    paddr_t gpa;
> > +
> > +    if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
> > +        gpa = get_faulting_ipa(gva);
> > +    else
> > +    {
> > +        /*
> > +         * Flush the TLB to make sure the DTLB is clear before
> > +         * doing GVA->IPA translation. If we got here because of
> > +         * an entry only present in the ITLB, this translation may
> > +         * still be inaccurate.
> > +         */
> > +        flush_tlb_local();
> > +
> > +        rc = gva_to_ipa(gva, &gpa, GV2M_READ);
> > +        if ( rc == -EFAULT )
> > +            return; /* Try again */
> The issue with this is that now for any cases that don't require a gpa
> if gva_to_ipa fails we wrongly return -EFAULT.
Well, stage-1 fault is prioritized over stage-2 fault (see B3.12.3 in 
ARM DDI 0406C.b), so gva_to_ipa should never fail unless someone is 
playing with the stage-1 page table at the same time or because of an 
erratum (see 834220). In both case, we should replay the instruction to 
let the processor injecting the correct fault.
FWIW, this is already what we do for the data abort handler.

> I suggest having two switches or falling through from the first case to
> the second.
I am not sure to understand your suggestion. Could you detail it?

Regards,

--
Julien Grall

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