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

Re: [PATCH] xen/x86: Work around Clang code generation bug with asm parameters



On 11.11.2020 13:45, Andrew Cooper wrote:
> Clang 9 and later don't handle the clobber of %r10 correctly in
> _hypercall64_4().  See https://bugs.llvm.org/show_bug.cgi?id=48122

Are you sure this is a bug? With ...

>  #define _hypercall64_4(type, hcall, a1, a2, a3, a4)                     \
>      ({                                                                  \
> -        long res, tmp__;                                                \
> -        register long _a4 asm ("r10") = ((long)(a4));                   \
> +        long res, _a1 = (long)(a1), _a2 = (long)(a2),                   \
> +            _a3 = (long)(a3);                                           \
> +        register long _a4 asm ("r10") = (long)(a4);                     \
>          asm volatile (                                                  \
>              "call hypercall_page + %c[offset]"                          \
> -            : "=a" (res), "=D" (tmp__), "=S" (tmp__), "=d" (tmp__),     \
> -              "=&r" (tmp__) ASM_CALL_CONSTRAINT                         \

... this we've requested "any register", while with ...

> -            : [offset] "i" (hcall * 32),                                \
> -              "1" ((long)(a1)), "2" ((long)(a2)), "3" ((long)(a3)),     \
> -              "4" (_a4)                                                 \

... this we've asked for that specific register to be initialized
from r10 (and without telling the compiler that r10 is going to
change).

Also, by what I would have hoped is convention meanwhile, the new
macro local variables' names shouldn't start with an underscore,
but instead perhaps end in one. But to be honest, despite knowing
of the latent (albeit highly hypothetical) issue, each time I
find myself making such a comment I'm one tiny step closer to
giving up.

Anyway, with at least title and description changed (or your view
clarified verbally)
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



 


Rackspace

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