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

From: Stefano Stabellini <sstabellini@kernel.org>
To: Wei Chen <Wei.Chen@arm.com>
Cc: sstabellini@kernel.org, steve.capper@arm.com,
	xen-devel@lists.xen.org, Kaly.Xin@arm.com, julien.grall@arm.com,
	nd@arm.com
Subject: Re: [PATCH v2] xen/arm32: Distinguish guest SError from Xen data aborts
Date: Thu, 4 May 2017 11:32:34 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.10.1705041130590.9240@sstabellini-ThinkPad-X260> (raw)
In-Reply-To: <1493868469-5054-1-git-send-email-Wei.Chen@arm.com>

On Thu, 4 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@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> v1->v2:
> Add an in-code comment to describe this change.
> 
> ---
>  xen/arch/arm/arm32/traps.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
> index 5bc5f64..48baa64 100644
> --- a/xen/arch/arm/arm32/traps.c
> +++ b/xen/arch/arm/arm32/traps.c
> @@ -62,7 +62,18 @@ 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);
> +    /*
> +     * 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.
> +     */
> +    if ( VABORT_GEN_BY_GUEST(regs) )
> +        do_trap_guest_serror(regs);
> +    else
> +        do_unexpected_trap("Data Abort", regs);
>  }
>  
>  /*
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 

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