Re: [Xen-devel] [PATCH 4/9] xen/arm: traps: Simplify the switch in do_trap_*_abort_guest

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 14/07/16 12:12, Stefano Stabellini wrote:
> On Wed, 22 Jun 2016, Julien Grall wrote:
> > The fault status we care are all the form BBBBxx where xx is the lookup
>                                   ^ in the form of
>
> > level that gave the fault. We can simply the code by masking the 2 least
>                                         ^ simplify
>
> > significant bits.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > ---
> >   xen/arch/arm/traps.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index 2e84b5a..8a3fac0 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -2388,9 +2388,9 @@ static void do_trap_instr_abort_guest(struct 
> > cpu_user_regs *regs,
> >       int rc;
> >       register_t gva = READ_SYSREG(FAR_EL2);
> >
> > -    switch ( hsr.iabt.ifsc & 0x3f )
> > +    switch ( hsr.iabt.ifsc & ~FSC_LL_MASK )
> >       {
> > -    case FSC_FLT_PERM ... FSC_FLT_PERM + 3:
> > +    case FSC_FLT_PERM:
> >       {
> This is a good improvement in code readability. I would go one step
> further and replace the switch with a simple if.
I would prefer to keep the switch case here. The patch #7 adds another 
case for do_trap_data_abort_guest because we should not emulate MMIO for 
any kind of fault as it is done today.
Also, I have got more fixes coming up which require the switch here.

Cheers,

--
Julien Grall

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