Re: [Xen-devel] [PATCH 3/3] xen/arm: Don't crash the domain on invalid HVC immediate

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 30 Jan 2018, Julien Grall wrote:
> domain_crash_synchronous() should only be used when something went wrong
> in Xen. It is better to inject to the guest as it will be in better
                                                              ^ a better


> position to provide helpful information (stack trace...).
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> 
> ---
> 
>     We potentially want to return -1 instead. This would make Xen more
>     future-proof if we decide to implement the other HVC immediate.
> ---
>  xen/arch/arm/traps.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 843adf4959..18da45dff3 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1473,14 +1473,17 @@ static void do_debug_trap(struct cpu_user_regs *regs, 
> unsigned int code)
>  #endif
>  
>  static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
> -                              unsigned long iss)
> +                              const union hsr hsr)
>  {
>      arm_hypercall_fn_t call = NULL;
>  
>      BUILD_BUG_ON(NR_hypercalls < ARRAY_SIZE(arm_hypercall_table) );
>  
> -    if ( iss != XEN_HYPERCALL_TAG )
> -        domain_crash_synchronous();
> +    if ( hsr.iss != XEN_HYPERCALL_TAG )
> +    {
> +        gprintk(XENLOG_WARNING, "Invalid HVC imm 0x%x\n", hsr.iss);
> +        return inject_undef_exception(regs, hsr);

It looks a bit weird, given that inject_undef_exception doesn't return
anything. This is just code style so anyway:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> +    }
>  
>      if ( *nr >= ARRAY_SIZE(arm_hypercall_table) )
>      {
> @@ -2150,7 +2153,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
>          if ( hsr.iss == 0 )
>              return do_trap_hvc_smccc(regs);
>          nr = regs->r12;
> -        do_trap_hypercall(regs, &nr, hsr.iss);
> +        do_trap_hypercall(regs, &nr, hsr);
>          regs->r12 = (uint32_t)nr;
>          break;
>      }
> @@ -2164,7 +2167,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
>  #endif
>          if ( hsr.iss == 0 )
>              return do_trap_hvc_smccc(regs);
> -        do_trap_hypercall(regs, &regs->x16, hsr.iss);
> +        do_trap_hypercall(regs, &regs->x16, hsr);
>          break;
>      case HSR_EC_SMC64:
>          /*
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel