Re: [PATCH v2] x86/altcall: further refine clang workaround

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.07.2024 14:47, Roger Pau Monné wrote:
> On Mon, Jul 29, 2024 at 02:41:23PM +0200, Jan Beulich wrote:
>> On 29.07.2024 14:00, Roger Pau Monné wrote:
>>> On Mon, Jul 29, 2024 at 12:47:09PM +0200, Jan Beulich wrote:
>>>> On 29.07.2024 12:30, Roger Pau Monne wrote:
>>>>> --- a/xen/arch/x86/include/asm/alternative.h
>>>>> +++ b/xen/arch/x86/include/asm/alternative.h
>>>>> @@ -183,13 +183,13 @@ extern void alternative_branches(void);
>>>>>   * https://github.com/llvm/llvm-project/issues/12579
>>>>>   * https://github.com/llvm/llvm-project/issues/82598
>>>>>   */
>>>>> -#define ALT_CALL_ARG(arg, n)                                            \
>>>>> -    register union {                                                    \
>>>>> -        typeof(arg) e[sizeof(long) / sizeof(arg)];                      \
>>>>> -        unsigned long r;                                                \
>>>>> -    } a ## n ## _ asm ( ALT_CALL_arg ## n ) = {                         \
>>>>> -        .e[0] = ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })\
>>>>> -    }
>>>>> +#define ALT_CALL_ARG(arg, n)                                             
>>>>> \
>>>>> +     register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n ) = ({   
>>>>> \
>>>>> +         unsigned long tmp = 0;                                          
>>>>> \
>>>>> +         *(typeof(arg) *)&tmp = (arg);                                   
>>>>> \
>>>>> +         BUILD_BUG_ON(sizeof(arg) > sizeof(void *));                     
>>>>> \
>>>>
>>>> With this, even more so than before, I think the type of tmp would better
>>>> be void * (or the BUILD_BUG_ON() be made use unsigned long, yet I consider
>>>> that less desirable).
>>>
>>> Won't using void * be uglier, as we then need to cast the last tmp
>>> statement to unsigned long?
>>
>> Only if we stick to using unsigned long for a ## n ## _. Afaics there's
>> nothing wrong with making that void *, too.
> 
> Right, but then for consistency I would also like to make r{10,11}_
> void *, and ALT_CALL_NO_ARG(), which might be too much.
> 
> My preference is likely to keep it at unsigned long, and adjust the
> BUILD_BUG_ON(), unless you have a strong opinion to change it to void
> * (and possibly the rest of the register variables).

That's okay if you prefer it that way; I said "less desirable" and I really
don't mean any more than that.

Jan