On 28/08/18 16:50, Volodymyr Babchuk wrote:
Hi Julien,
Hi,
On 28.08.18 18:27, Julien Grall wrote:
Hi Volodymyr,
On 28/08/18 16:10, Volodymyr Babchuk wrote:
On 28.08.18 17:43, Julien Grall wrote:
[...]
I have looked at cpus_have_const_cap() and haven't found good way
to optimize it with the current infrastructure in Xen. Feel free
to suggest improvement.
Another thing: maybe it is worth to branch to 1.0 code and leave
1.1 in a straight path of execution? This will save you one more
instruction for SMCCC 1.1 call.
I am not sure to understand your suggestion here. Could you expand?
+#define arm_smccc_smc(...) \
+ do { \
+ if ( cpus_have_const_cap(ARM_SMCCC_1_1) ) \
+ arm_smccc_1_1_smc(__VA_ARGS__); \
+ else \
+ arm_smccc_1_0_smc(__VA_ARGS__); \
+ } while ( 0 )
However, why SMCCC 1.1 should be in the straight path of execution?
It is easier to read - no negation in if().
I can do that. I will also probably add an unlikely in
cpus_have_const_cap(...).
That would be great.
Also, I think, branch predictor would be happy. At least, if the
following is true:
" If the branch information is not contained in the BTAC, static
branch prediction is used, whereby we assume the branch will be taken
if the branch is a conditional backwards branch or not taken if the
branch is a conditional forwards branch." [1]
The Arm Arm does not provide any details on the branch predictor
implementation. An implementer is free to do whatever he wants and
could design a branch predicator doing the opposite as this statement.
Generally speaking - yes. But, AFAIK, most static predictors behave in
the way described above. Anyways, as you pointed below, better to leave
hint for the compiler.
Spectre would not have existed if the branch predictor was so easy ;).
You also can't assume how the compiler will compile the code, it may
end up to generate the else branch first because it is predicted to be
taken more often. This is why GCC provide __builtin_expect (commonly
used as unlikely/likely) to influence the compiler choice for branch
prediction.
Yes, this is the good point. So, you can add likely/unlikely not only in
cpus_have_const_cap(...) but also in #define arm_smccc_smc(...)
There are no need to have likely/unlikely in arm_smccc_smc if it is
already present in cpus_have_const_cap.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
