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@lists.xen.org
https://lists.xen.org/xen-devel
