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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 31 Mar 2017, Julien Grall wrote:
> 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@xxxxxxx>
> > 
> > ---
> > 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@xxxxxxx>
> 
> Unless that are more comments from Stefano, this should be fixable whilst
> committing.

Yes, it is true, but I prefer for Wei to resend the series with all the
Reviewed-by and little remaming nits. Less work for me :-)

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

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