Re: [Xen-devel] [PATCH 16/18] xen/arm: Isolate the SError between the context switch of 2 vCPUs

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 13 Mar 2017, Wei Chen wrote:
> If there is a pending SError while we are doing context switch, if the
> SError handle option is "FORWARD", We have to guranatee this serror to
> be caught by current vCPU, otherwise it will be caught by next vCPU and
> be forwarded to this wrong vCPU.
> 
> We don't want to export serror_op accessing to other source files and
> use serror_op every place, so we add a helper to synchronize SError for
> context switching. The synchronize_serror has been used in this helper,
> so the "#if 0" can be removed.
> 
> Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> ---
>  xen/arch/arm/domain.c           |  2 ++
>  xen/arch/arm/traps.c            | 14 ++++++++++++--
>  xen/include/asm-arm/processor.h |  2 ++
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 69c2854..a547fcd 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -312,6 +312,8 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>  
>      local_irq_disable();
>  
> +    prevent_forward_serror_to_next_vcpu();
> +
>      set_current(next);
>  
>      prev = __context_switch(prev, next);
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index ee7865b..b8c8389 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2899,7 +2899,6 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs 
> *regs)
>      }
>  }
>  
> -#if 0
>  static void synchronize_serror(void)
>  {
>      /* Synchronize against in-flight ld/st. */
> @@ -2908,7 +2907,18 @@ static void synchronize_serror(void)
>      /* A single instruction exception window */
>      isb();
>  }
> -#endif
> +
> +/*
> + * If the SErrors option is "FORWARD", we have to prevent forwarding
> + * serror to wrong vCPU. So before context switch, we have to use the
> + * synchronize_serror to guarantee that the pending serror would be
> + * caught by current vCPU.
> + */
> +void prevent_forward_serror_to_next_vcpu(void)
> +{
> +    if ( serrors_op == SERRORS_FORWARD )
> +        synchronize_serror();
> +}

I dislike introducing so many 2 lines functions. I would get rid of
prevent_forward_serror_to_next_vcpu and just add

   if ( serrors_op == SERRORS_FORWARD )
       synchronize_serror();

to context_switch, and I would make synchronize_serror a static inline.


>  asmlinkage void do_trap_hyp_serror(struct cpu_user_regs *regs)
>  {
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index afad78c..3b43234 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -711,6 +711,8 @@ void do_trap_hyp_serror(struct cpu_user_regs *regs);
>  
>  void do_trap_guest_serror(struct cpu_user_regs *regs);
>  
> +void prevent_forward_serror_to_next_vcpu(void);
> +
>  /* 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