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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 03/31/2017 07:42 PM, Stefano Stabellini wrote:
> 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.
That's correct. I thought the name "SKIP_CHECK_PENDING_VSERROR" does not 
match the behavior here.
Cheers,

--
Julien Grall

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