| Message ID | 1466601669-25398-5-git-send-email-julien.grall@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@arm.com> > --- > 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. > paddr_t gpa; > const struct npfec npfec = { > @@ -2451,9 +2451,9 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs, > return; /* Try again */ > } > > - switch ( dabt.dfsc & 0x3f ) > + switch ( dabt.dfsc & ~FSC_LL_MASK ) > { > - case FSC_FLT_PERM ... FSC_FLT_PERM + 3: > + case FSC_FLT_PERM: > { > const struct npfec npfec = { > .read_access = !dabt.write, same here > -- > 1.9.1 >
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@arm.com> >> --- >> 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,
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@arm.com> > > > --- > > > 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
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: { paddr_t gpa; const struct npfec npfec = { @@ -2451,9 +2451,9 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs, return; /* Try again */ } - switch ( dabt.dfsc & 0x3f ) + switch ( dabt.dfsc & ~FSC_LL_MASK ) { - case FSC_FLT_PERM ... FSC_FLT_PERM + 3: + case FSC_FLT_PERM: { const struct npfec npfec = { .read_access = !dabt.write,
The fault status we care are all the form BBBBxx where xx is the lookup level that gave the fault. We can simply the code by masking the 2 least significant bits. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/traps.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)