Re: [Xen-devel] [PATCH] xen/arm32: Distinguish guest SError from Xen data aborts

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Stefano,

On 2017/5/4 5:56, Stefano Stabellini wrote:
> On Wed, 3 May 2017, Wei Chen wrote:
>> ARM32 doesn't have an exception similar to hyp_sync of ARM64 to catch
>> the synchronous data abort (For example, a NULL pointer has been referenced).
>> Hence the SError and sync data abort will be caught by the same data abort
>> exception.
>>
>> Since commit "3f16c8cb" we treat all data aborts caught by this excetpion
>> as SError. This means, we will forward Xen synchronous data abort to guest,
>> if the serror_op=FORWARD. This is obviously incorrect. But we don't have
>> any method to distinguish SError from Xen data aborts.
>>
>> But we can distinguish guest generated SError from Xen data aborts. So we
>> want to change the policy to handle data aborts for ARM32:
>> 1. If this data abort is guest generated SError, we will handle this data
>>    abort follow the SError handle option setting.
>> 2. If this data abort is synchronous data abort or Xen generate SError, we
>>    will PANIC the whole system.
>>
>> Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
>> ---
>>  xen/arch/arm/arm32/traps.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
>> index 5bc5f64..1e17ae7 100644
>> --- a/xen/arch/arm/arm32/traps.c
>> +++ b/xen/arch/arm/arm32/traps.c
>> @@ -62,7 +62,10 @@ asmlinkage void do_trap_prefetch_abort(struct 
>> cpu_user_regs *regs)
>>
>>  asmlinkage void do_trap_data_abort(struct cpu_user_regs *regs)
>>  {
>> -    do_trap_hyp_serror(regs);
>> +    if ( VABORT_GEN_BY_GUEST(regs) )
>> +        do_trap_guest_serror(regs);
>> +    else
>> +        do_unexpected_trap("Data Abort", regs);
>>  }
>
> The consequence of this is that any Xen generated SErrors will just end
> with do_unexpected_trap instead of do_trap_hyp_serror. The result is
> that they won't be forwarded to the guest when serror_op=FORWARD (while
> the result is the same for serror_op=DIVERSE, except for the error
> message printed).
>
> I guess it's the best compromise we can shoot for.
>
> Please add an in-code comment, like this:
>
>   /*
>    * We cannot distinguish Xen SErrors from synchronous data aborts. We
>    * want to avoid treating any Xen synchronous aborts as SErrors and
>    * forwarding them to the guest. Instead, crash the system in all
>    * cases when the abort comes from Xen. Even if they are Xen SErrors
>    * it would be a reasonable thing to do, and the default behavior with
>    * serror_op == DIVERSE.
>    */
>

I will add above comment to code and send a new version.

Thanks.


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