It's not unnecessary (as the earlier commit claimed): The integrated
assembler of Clang up to 11 complains about an "invalid operand for
instruction".

Fixes: b42cf31d1165 ("x86: use alternative_input() in cache_flush()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -286,7 +286,7 @@ void cache_flush(const void *addr, unsig
          * + prefix than a clflush + nop, and hence the prefix is added instead
          * of letting the alternative framework fill the gap by appending nops.
          */
-        alternative_input("ds clflush %[p]",
+        alternative_input("ds; clflush %[p]",
                           "data16 clflush %[p]", /* clflushopt */
                            X86_FEATURE_CLFLUSHOPT,
                            [p] "m" (*(const char *)(addr)));

On 08/10/2024 5:00 pm, Jan Beulich wrote:
> It's not unnecessary (as the earlier commit claimed): The integrated
> assembler of Clang up to 11 complains about an "invalid operand for
> instruction".
>
> Fixes: b42cf31d1165 ("x86: use alternative_input() in cache_flush()")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/flushtlb.c
> +++ b/xen/arch/x86/flushtlb.c
> @@ -286,7 +286,7 @@ void cache_flush(const void *addr, unsig
>           * + prefix than a clflush + nop, and hence the prefix is added instead
>           * of letting the alternative framework fill the gap by appending nops.
>           */
> -        alternative_input("ds clflush %[p]",
> +        alternative_input("ds; clflush %[p]",

/* Clang-IAS < 12 needs the semicolon */  which can probably fit on the
end of the line.

Or we stop supporting such old versions of Clang/LLVM.

~Andrew

On 08.10.2024 18:37, Andrew Cooper wrote:
> On 08/10/2024 5:00 pm, Jan Beulich wrote:
>> It's not unnecessary (as the earlier commit claimed): The integrated
>> assembler of Clang up to 11 complains about an "invalid operand for
>> instruction".
>>
>> Fixes: b42cf31d1165 ("x86: use alternative_input() in cache_flush()")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/flushtlb.c
>> +++ b/xen/arch/x86/flushtlb.c
>> @@ -286,7 +286,7 @@ void cache_flush(const void *addr, unsig
>>           * + prefix than a clflush + nop, and hence the prefix is added instead
>>           * of letting the alternative framework fill the gap by appending nops.
>>           */
>> -        alternative_input("ds clflush %[p]",
>> +        alternative_input("ds; clflush %[p]",
> 
> /* Clang-IAS < 12 needs the semicolon */  which can probably fit on the
> end of the line.

I've made it "Semicolon for Clang-IAS < 12" to actually fit on the line.

I wonder whether I can take the reply as "ack with that change"?

> Or we stop supporting such old versions of Clang/LLVM.

As indicated in reply to Roger's proposal, that would leave me without
any way to test with at least some Clang versions (unless I got into the
business of also building my own Clang binaries). IOW - I could live
with such a move, but I'd prefer us not to be overly aggressive there.

Jan

On 09/10/2024 7:04 am, Jan Beulich wrote:
> On 08.10.2024 18:37, Andrew Cooper wrote:
>> On 08/10/2024 5:00 pm, Jan Beulich wrote:
>>> It's not unnecessary (as the earlier commit claimed): The integrated
>>> assembler of Clang up to 11 complains about an "invalid operand for
>>> instruction".
>>>
>>> Fixes: b42cf31d1165 ("x86: use alternative_input() in cache_flush()")
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> --- a/xen/arch/x86/flushtlb.c
>>> +++ b/xen/arch/x86/flushtlb.c
>>> @@ -286,7 +286,7 @@ void cache_flush(const void *addr, unsig
>>>           * + prefix than a clflush + nop, and hence the prefix is added instead
>>>           * of letting the alternative framework fill the gap by appending nops.
>>>           */
>>> -        alternative_input("ds clflush %[p]",
>>> +        alternative_input("ds; clflush %[p]",
>> /* Clang-IAS < 12 needs the semicolon */  which can probably fit on the
>> end of the line.
> I've made it "Semicolon for Clang-IAS < 12" to actually fit on the line.
>
> I wonder whether I can take the reply as "ack with that change"?

Yeah, that's good enough.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
>> Or we stop supporting such old versions of Clang/LLVM.
> As indicated in reply to Roger's proposal, that would leave me without
> any way to test with at least some Clang versions (unless I got into the
> business of also building my own Clang binaries).

Or using the upstream provided builds.  (Seriously - this is something
the LLVM community completely outshine the GCC/Binutils community.)

Or using Gitlab CI.

~Andrew