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

[Julien Grall <julien.grall@arm.com>]

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