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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 14 Jul 2016, Julien Grall wrote:
> 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.

all right

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