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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.
   */

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