Re: [PATCH v3 17/19] xen/arm: Isolate the SError between the context switch of 2 vCPUs

[Julien Grall <julien.grall@arm.com>]

Hi Wei,

On 31/03/17 14:07, 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

NIT: s/guranatee/guarantee/

s/serror/Serror/

> be caught by current vCPU, otherwise it will be caught by next vCPU and
> be forwarded to this wrong vCPU.
>
> So we have to synchronize SError before switch to next vCPU. But this is
> only required by "FORWARD" option. In this case we added a new flag
> SKIP_CTXT_SWITCH_SERROR_SYNC in cpu_hwcaps to skip synchronizing SError
> in context switch for other options. In the meantime, we don't need to
> export serror_op accessing to other source files.
>
> Because we have umasked the Abort/SError bit in previous patch, we have
> to disable Abort/SError before doing context switch as we have done for
> IRQ.
>
> Signed-off-by: Wei Chen <Wei.Chen@arm.com>
>
> ---
> v2->v3:
> 1. Use the macro instead of the function to synchronize SErrors.
> 2. Disable Abort/SError before doing context switch.
> ---
>  xen/arch/arm/domain.c            | 12 ++++++++++++
>  xen/arch/arm/traps.c             |  3 +++
>  xen/include/asm-arm/cpufeature.h |  3 ++-
>  3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 69c2854..e1a620a 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -29,6 +29,7 @@
>  #include <asm/cpufeature.h>
>  #include <asm/vfp.h>
>  #include <asm/procinfo.h>
> +#include <asm/alternative.h>
>
>  #include <asm/gic.h>
>  #include <asm/vgic.h>
> @@ -312,6 +313,17 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
>
>      local_irq_disable();
>
> +    /*
> +     * If the SErrors option is "FORWARD", we have to prevent forwarding

SErrors is the option. So it should be in lowercase.

> +     * serror to wrong vCPU. So before context switch, we have to use the

NIT : s/serror/Serror/

> +     * synchronize_serror to guarantee that the pending serror would be

s/synchronize_serror/macro SYNCRONIZE_SERROR/

With the grammar nits fixed:

Reviewed-by: Julien Grall <julien.grall@arm.com>

Unless that are more comments from Stefano, this should be fixable 
whilst committing.

Cheers,

-- 
Julien Grall

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