Re: [Xen-devel] [PATCH 13/18] xen/arm: Replace do_trap_guest_serror with new helpers

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 13 Mar 2017, Wei Chen wrote:
> We have introduced two helpers to handle the guest/hyp SErrors:
> do_trap_guest_serror and do_trap_guest_hyp_serror. These handlers
> can take the role of do_trap_guest_serror and reduce the assembly
> code in the same time. So we use these two helpers to replace it
> and drop it now.
> 
> Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  xen/arch/arm/arm32/traps.c      |  5 +----
>  xen/arch/arm/arm64/entry.S      | 36 +++---------------------------------
>  xen/arch/arm/traps.c            | 15 ---------------
>  xen/include/asm-arm/processor.h |  2 --
>  4 files changed, 4 insertions(+), 54 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
> index 4176f0e..5bc5f64 100644
> --- a/xen/arch/arm/arm32/traps.c
> +++ b/xen/arch/arm/arm32/traps.c
> @@ -62,10 +62,7 @@ asmlinkage void do_trap_prefetch_abort(struct 
> cpu_user_regs *regs)
>  
>  asmlinkage void do_trap_data_abort(struct cpu_user_regs *regs)
>  {
> -    if ( VABORT_GEN_BY_GUEST(regs) )
> -        do_trap_guest_error(regs);
> -    else
> -        do_unexpected_trap("Data Abort", regs);
> +    do_trap_hyp_serror(regs);
>  }
>  
>  /*
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 113e1c3..8d5a890 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -178,40 +178,10 @@ hyp_error_invalid:
>          invalid BAD_ERROR
>  
>  hyp_error:
> -        /*
> -         * Only two possibilities:
> -         * 1) Either we come from the exit path, having just unmasked
> -         *    PSTATE.A: change the return code to an EL2 fault, and
> -         *    carry on, as we're already in a sane state to handle it.
> -         * 2) Or we come from anywhere else, and that's a bug: we panic.
> -         */
>          entry   hyp=1
>          msr     daifclr, #2
> -
> -        /*
> -         * The ELR_EL2 may be modified by an interrupt, so we have to use the
> -         * saved value in cpu_user_regs to check whether we come from 1) or
> -         * not.
> -         */
> -        ldr     x0, [sp, #UREGS_PC]
> -        adr     x1, abort_guest_exit_start
> -        cmp     x0, x1
> -        adr     x1, abort_guest_exit_end
> -        ccmp    x0, x1, #4, ne
>          mov     x0, sp
> -        mov     x1, #BAD_ERROR
> -
> -        /*
> -         * Not equal, the exception come from 2). It's a bug, we have to
> -         * panic the hypervisor.
> -         */
> -        b.ne    do_bad_mode
> -
> -        /*
> -         * Otherwise, the exception come from 1). It happened because of
> -         * the guest. Crash this guest.
> -         */
> -        bl      do_trap_guest_error
> +        bl      do_trap_hyp_serror
>          exit    hyp=1
>  
>  /* Traps taken in Current EL with SP_ELx */
> @@ -267,7 +237,7 @@ guest_error:
>          entry   hyp=0, compat=0
>          msr     daifclr, #2
>          mov     x0, sp
> -        bl      do_trap_guest_error
> +        bl      do_trap_guest_serror
>          exit    hyp=0, compat=0
>  
>  guest_sync_compat:
> @@ -309,7 +279,7 @@ guest_error_compat:
>          entry   hyp=0, compat=1
>          msr     daifclr, #2
>          mov     x0, sp
> -        bl      do_trap_guest_error
> +        bl      do_trap_guest_serror
>          exit    hyp=0, compat=1
>  
>  ENTRY(return_to_new_vcpu32)
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 48cfc8e..44a0281 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2899,21 +2899,6 @@ asmlinkage void do_trap_hypervisor(struct 
> cpu_user_regs *regs)
>      }
>  }
>  
> -asmlinkage void do_trap_guest_error(struct cpu_user_regs *regs)
> -{
> -    enter_hypervisor_head(regs);
> -
> -    /*
> -     * Currently, to ensure hypervisor safety, when we received a
> -     * guest-generated vSerror/vAbort, we just crash the guest to protect
> -     * the hypervisor. In future we can better handle this by injecting
> -     * a vSerror/vAbort to the guest.
> -     */
> -    gdprintk(XENLOG_WARNING, "Guest(Dom-%u) will be crashed by vSError\n",
> -             current->domain->domain_id);
> -    domain_crash_synchronous();
> -}
> -
>  asmlinkage void do_trap_hyp_serror(struct cpu_user_regs *regs)
>  {
>      enter_hypervisor_head(regs);
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 885dbca..afad78c 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -707,8 +707,6 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
>  int call_smc(register_t function_id, register_t arg0, register_t arg1,
>               register_t arg2);
>  
> -void do_trap_guest_error(struct cpu_user_regs *regs);
> -
>  void do_trap_hyp_serror(struct cpu_user_regs *regs);
>  
>  void do_trap_guest_serror(struct cpu_user_regs *regs);
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel
> 

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