Re: [PATCH v2] xen: Work around Clang-IAS macro \@ expansion bug

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.02.2023 21:36, Andrew Cooper wrote:
> https://github.com/llvm/llvm-project/issues/60792
> 
> It turns out that Clang-IAS does not expand \@ uniquely in a translaition
> unit, and the XSA-426 change tickles this bug:
> 
>   <instantiation>:4:1: error: invalid symbol redefinition
>   .L1_fill_rsb_loop:
>   ^
>   make[3]: *** [Rules.mk:247: arch/x86/acpi/cpu_idle.o] Error 1
> 
> Extend DO_OVERWRITE_RSB with an optional parameter so C callers can mix %= in
> too, which Clang does seem to expand properly.
> 
> Fixes: 63305e5392ec ("x86/spec-ctrl: Mitigate Cross-Thread Return Address 
> Predictions")
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

A little hesitantly
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> --- a/xen/arch/x86/include/asm/spec_ctrl.h
> +++ b/xen/arch/x86/include/asm/spec_ctrl.h
> @@ -83,7 +83,7 @@ static always_inline void spec_ctrl_new_guest_context(void)
>      wrmsrl(MSR_PRED_CMD, PRED_CMD_IBPB);
>  
>      /* (ab)use alternative_input() to specify clobbers. */
> -    alternative_input("", "DO_OVERWRITE_RSB", X86_BUG_IBPB_NO_RET,
> +    alternative_input("", "DO_OVERWRITE_RSB xu=%=", X86_BUG_IBPB_NO_RET,

Especially with there possibly appearing more cases where we need to
add such, wrapping the extra parameter in a C macro continues to
seem better to me, for having a minimal level of documentation (I
has CLANG in the suggested name for exactly this purpose) right at
the place of use. The way you have it you force readers to go look
up the assembler macro and read through the commentary there in order
to find any explanation for the oddity.

Jan