Re: [Xen-devel] [PATCH 12/18] xen/arm: Introduce new helpers to handle guest/hyp SErrors

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 13 Mar 2017, Wei Chen wrote:
> Currently, ARM32 and ARM64 has different SError exception handlers.
> These handlers include lots of code to check SError handle options
> and code to distinguish guest-generated SErrors from hypervisor
> SErrors.
> 
> The new helpers: do_trap_guest_serror and do_trap_hyp_serror are
> wrappers of __do_trap_serror with constant guest/hyp parameters.
> __do_trap_serror moves the option checking code and SError checking
> code from assembly to C source. This will make the code become more
> readable and avoid placing check code in too many places.
> 
> These two helpers only handle the following 3 types of SErrors:
> 1) Guest-generated SError and had been delivered in EL1 and then
>    been forwarded to EL2.
> 2) Guest-generated SError but hadn't been delivered in EL1 before
>    trapping to EL2. This SError would be caught in EL2 as soon as
>    we just unmasked the PSTATE.A bit.
> 3) Hypervisor generated native SError, that would be a bug.
> 
> In the new helpers, we have used the function "inject_vabt_exception"
> which was disabled by "#if 0" before. Now, we can remove the "#if 0"
> to make this function to be available.
> 
> Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  xen/arch/arm/traps.c            | 69 
> +++++++++++++++++++++++++++++++++++++++--
>  xen/include/asm-arm/processor.h |  4 +++
>  2 files changed, 71 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 053b7fc..48cfc8e 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -646,7 +646,6 @@ static void inject_dabt_exception(struct cpu_user_regs 
> *regs,
>  #endif
>  }
>  
> -#if 0
>  /* Inject a virtual Abort/SError into the guest. */
>  static void inject_vabt_exception(struct cpu_user_regs *regs)
>  {
> @@ -676,7 +675,59 @@ static void inject_vabt_exception(struct cpu_user_regs 
> *regs)
>  
>      current->arch.hcr_el2 |= HCR_VA;
>  }
> -#endif
> +
> +/*
> + * SError exception handler. We only handle the following 3 types of SErrors:
> + * 1) Guest-generated SError and had been delivered in EL1 and then
> + *    been forwarded to EL2.
> + * 2) Guest-generated SError but hadn't been delivered in EL1 before
> + *    trapping to EL2. This SError would be caught in EL2 as soon as
> + *    we just unmasked the PSTATE.A bit.
> + * 3) Hypervisor generated native SError, that would be a bug.
> + *
> + * A true parameter "guest" means that the SError is type#1 or type#2.
> + */
> +static void __do_trap_serror(struct cpu_user_regs *regs, bool guest)
> +{
> +    /*
> +     * Only "DIVERSE" option needs to distinguish the guest-generated SErrors
> +     * from hypervisor SErrors.
> +     */
> +    if ( serrors_op == SERRORS_DIVERSE )
> +    {
> +        /* Forward the type#1 and type#2 SErrors to guests. */
> +        if ( guest )
> +            return inject_vabt_exception(regs);
> +
> +        /* Type#3 SErrors will panic the whole system */
> +        goto crash_system;
> +    }
> +
> +    /*
> +     * The "FORWARD" option will forward all SErrors to the guests, except
> +     * idle domain generated SErrors.
> +     */
> +    if ( serrors_op == SERRORS_FORWARD )
> +    {
> +        /*
> +         * Because the idle domain doesn't have the ability to handle the
> +         * SErrors, we have to crash the whole system while we get a SError
> +         * generated by idle domain.
> +         */
> +        if ( is_idle_vcpu(current) )
> +            goto crash_system;
> +
> +        return inject_vabt_exception(regs);
> +    }
> +
> +crash_system:
> +    /* Three possibilities to crash the whole system:
> +     * 1) "DIVERSE" option with Hypervisor generated SErrors.
> +     * 2) "FORWARD" option with Idle Domain generated SErrors.
> +     * 3) "PANIC" option with all SErrors.
> +     */
> +    do_unexpected_trap("SError", regs);
> +}
>  
>  struct reg_ctxt {
>      /* Guest-side state */
> @@ -2863,6 +2914,20 @@ asmlinkage void do_trap_guest_error(struct 
> cpu_user_regs *regs)
>      domain_crash_synchronous();
>  }
>  
> +asmlinkage void do_trap_hyp_serror(struct cpu_user_regs *regs)
> +{
> +    enter_hypervisor_head(regs);
> +
> +    __do_trap_serror(regs, VABORT_GEN_BY_GUEST(regs));
> +}
> +
> +asmlinkage void do_trap_guest_serror(struct cpu_user_regs *regs)
> +{
> +    enter_hypervisor_head(regs);
> +
> +    __do_trap_serror(regs, true);
> +}
> +
>  asmlinkage void do_trap_irq(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 148cc6f..885dbca 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -709,6 +709,10 @@ int call_smc(register_t function_id, register_t arg0, 
> register_t arg1,
>  
>  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);
> +
>  /* Functions for pending virtual abort checking window. */
>  void abort_guest_exit_start(void);
>  void abort_guest_exit_end(void);
> -- 
> 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