[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 5/6] xen/arm: smccc: Add wrapper to automatically select the calling convention





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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.