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

[Julien Grall <julien.grall@arm.com>]

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,

-- 
Julien Grall

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