Re: [Xen-devel] [PATCH 14/27] xen/arm: traps: Improve logging for data/prefetch abort fault

[Julien Grall <julien.grall@xxxxxxx>]

On 08/22/2017 06:20 PM, Andre Przywara wrote:
> Hi,
Hi Andre,


> On 14/08/17 15:24, Julien Grall wrote:
> > Walk the hypervisor page table for data/prefetch abort fault to help
> > diagnostics error in the page tables.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > ---
> >   xen/arch/arm/traps.c | 19 +++++++++++++++++++
> >   1 file changed, 19 insertions(+)
> >
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index 819bdbc69e..dac4e54fa7 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -2967,7 +2967,26 @@ asmlinkage void do_trap_hyp_sync(struct cpu_user_regs 
> > *regs)
> >           do_trap_brk(regs, hsr);
> >           break;
> >   #endif
> > +    case HSR_EC_DATA_ABORT_CURR_EL:
> > +    case HSR_EC_INSTR_ABORT_CURR_EL:
> > +    {
> > +        bool is_data = (hsr.ec == HSR_EC_DATA_ABORT_CURR_EL);
> > +        const char *fault = (is_data) ? "Data Abort" : "Instruction Abort";
> > +
> > +        printk("%s Trap. Syndrome=%#x\n", fault, hsr.iss);
> > +        /*
> > +         * FAR may not be valid for a Synchronous External abort other
> > +         * than translation table walk.
> > +         */
> > +        if ( hsr.xabt.fsc != FSC_SEA || !hsr.xabt.fnv )
> This is quite hard to read. Would the DeMorgan'ed version be better?
>            if ( hsr.xabt.fsc == FSC_SEA && hsr.xabt.fnv )
>                printk ....
>             else
>                dump_hyp_walk ...
Indeed it is better. I will use that.

Cheers,

> > +            dump_hyp_walk(get_hfar(is_data));
> > +        else
> > +            printk("Invalid FAR, don't walk the hypervisor tables\n");
> Nit: "not walking" sounds less ambiguous.
>
> > +        do_unexpected_trap(fault, regs);
> >   
> > +        break;
> > +    }
> >       default:
> >           printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x 
> > Syndrome=0x%"PRIx32"\n",
> >                  hsr.bits, hsr.ec, hsr.len, hsr.iss);
> Ignoring the nits above:
I will fix both and keep the reviewed-by if you don't mind.

> Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>
Cheers,

--
Julien Grall

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