Re: [Xen-devel] [PATCH v2 16/19] xen/arm: Introduce a helper to synchronize SError

[Julien Grall <julien.grall@xxxxxxx>]

On 30/03/17 19:32, Julien Grall wrote:
> On 30/03/17 19:28, Julien Grall wrote:
> > Hi Wei,
> >
> > On 30/03/17 10:13, Wei Chen wrote:
> > > +void synchronize_serror(void)
> > Sorry for been late in the party. Looking at the way you use the
> > function, you execute depending on the behavior chosen by the user when
> > an SErrors happen. This behavior will not change at runtime, so always
> > checking the option chosen in the hot path does not sound very efficient.
> >
> > I would recommend to look at ALTERNATIVE and streamline (dsb sy, isb).
> > I.e
> >
> > ALTERNATIVE("dsb sy; isb", "nop; nop", ...) or the invert depending of
> > the place.
> To complete what I was suggestion, you could define:
>
> /* Synchronize SError unless the feature is selected */
> #define SYNCHRONIZE_SERROR(feat) ALTERNATIVE("dsb sy; isb", "nop; nop");
Or even:

/*
 * Synchronize SError unless the feature is selected.
 * This is relying on the SErrors are currently unmasked.
 */
#define SYNCHRONIZE_SERROR(feat)                                  \
        do {                                                      \
          ASSERT(cpus_have_cap(feat) && local_abort_is_enabled());\
          ALTERNATIVE("dsb sy; isb", "nop; nop");             \
        while (0)

The ASSERT is here to check that we have abort enabled. Otherwise, doing 
the synchronization would be pointless.
Note that the function local_abort_is_enabled is not implemented. But it 
is easy to write it. Looking how local_irq_is_enabled was implemented.
Cheers,

--
Julien Grall

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