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

Re: [Xen-devel] [PATCH v2 03/12] x86: infrastructure to allow converting certain indirect calls to direct ones



Hi Jan,

On 08/29/2018 03:02 PM, Jan Beulich wrote:
+#define alternative_vcall2(func, arg1, arg2) ({           \
+    ALT_CALL_ARG(arg1, 1);                                \
+    ALT_CALL_ARG(arg2, 2);                                \

I believe this code has the same issue Stefano recently discovered on the SMCCC.

Using explicit register variable will not reserve the register. So if arg* is a function call, the register you have just assigned will get clobbered (see [1]).

The solution to this is evaluating all the arguments before declaring the variable with explicit registers. See the patch [2] for an example.


+    ALT_CALL_NO_ARG3;                                     \
+    (void)sizeof(func(arg1, arg2));                       \
+    (void)alternative_callN(2, int, func);                \
+})

[...]

--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -66,6 +66,10 @@
#define ROUNDUP(x, a) (((x) + (a) - 1) & ~((a) - 1)) +#define count_va_arg_(dot, a1, a2, a3, a4, a5, a6, a7, a8, x, ...) x > +#define count_va_arg(args...) \
+    count_va_arg_(., ## args, 8, 7, 6, 5, 4, 3, 2, 1, 0)

We have a similar function in SMCCC where only the arguments except the first one (Function ID) and last one (Result pointer). I believe different context will require different way to count argument in order to keep the code readable.

So I am not entirely sure if there are a benefit to have this function in common.

Cheers,

[1] https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html#Local-Register-Variables

[2] https://lists.xen.org/archives/html/xen-devel/2018-08/msg02139.html

--
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®.