Re: [Xen-devel] [PATCH v3 18/19] xen/arm: Prevent slipping hypervisor SError to guest

[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're returning from trap. If the
> > SError handle option is "DIVERSE", we have to prevent slipping this
> > hypervisor SError to guest. So we have to use the dsb/isb to guarantee
> > that the pending hypervisor SError would be caught in hypervisor before
> > return to guest.
> > 
> > In previous patch, we will set SKIP_CHECK_PENDING_VSERROR to cpu_hwcaps
> > when option is NOT "DIVERSE". This means we can use the alternative to
> > skip synchronizing SErrors for other SErrors handle options.
> > 
> > Because we have umasked the Abort/SError bit in previous patch. We have
> > to disable the Abort/SError before returning to guest as we have done
> > for IRQ.
> > 
> > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > 
> > ---
> > v2->v3:
> > 1. Use alternative instead of check serror_op to skip sychronizing SErrors
> >    while option is NOT "DIVERSE".
> > 2. Disable Abort/SError before returning to guest.
> > ---
> >  xen/arch/arm/traps.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index 2c610c4..8f1a0cd 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -2936,6 +2936,19 @@ asmlinkage void leave_hypervisor_tail(void)
> >          local_irq_disable();
> >          if (!softirq_pending(smp_processor_id())) {
> >              gic_inject();
> > +
> > +            /*
> > +             * If the SErrors handle option is "DIVERSE", we have to
> > prevent
> > +             * slipping the hypervisor SError to guest. In this option,
> > before
> > +             * returning from trap, we have to synchronize SErrors to
> > guarantee
> > +             * that the pending SError would be caught in hypervisor.
> > +             *
> > +             * If option is NOT "DIVERSE", SKIP_CHECK_PENDING_VSERROR will
> > be
> > +             * set to cpu_hwcaps. This means we can use the alternative to
> > skip
> > +             * synchronizing SErrors for other SErrors handle options.
> > +             */
> > +            SYNCHRONIZE_SERROR(SKIP_CHECK_PENDING_VSERROR);
> 
> The feature name SKIP_CHECK_PENDING_VSERROR does not make sense here because
> we are not synchronizing guest SError.
> 
> I am happy to re-use the same feature, but this would need to rename. Maybe
> SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT?

Just for clarity, you are suggest to use the same cpu_hwcaps feature
here and in patch #8, as Wei is doing, but we a different name. That's
fine by me.


> > +
> >              return;
> >          }
> >          local_irq_enable();
> > 
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

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