There's no point using alternative_io() when there are no outputs.

No functional change.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -286,11 +286,10 @@ 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_io("ds; clflush %[p]",
-                       "data16 clflush %[p]", /* clflushopt */
-                       X86_FEATURE_CLFLUSHOPT,
-                       /* no outputs */,
-                       [p] "m" (*(const char *)(addr)));
+        alternative_input("ds; clflush %[p]",
+                          "data16 clflush %[p]", /* clflushopt */
+                           X86_FEATURE_CLFLUSHOPT,
+                           [p] "m" (*(const char *)(addr)));
     }
 
     alternative_2("",

On 30/09/2024 4:09 pm, Jan Beulich wrote:
> There's no point using alternative_io() when there are no outputs.
>
> No functional change.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/flushtlb.c
> +++ b/xen/arch/x86/flushtlb.c
> @@ -286,11 +286,10 @@ 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_io("ds; clflush %[p]",
> -                       "data16 clflush %[p]", /* clflushopt */
> -                       X86_FEATURE_CLFLUSHOPT,
> -                       /* no outputs */,
> -                       [p] "m" (*(const char *)(addr)));
> +        alternative_input("ds; clflush %[p]",

Drop the ; as you're altering the line anyway?

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

However, switching to ds ds will avoid a trailing nop, so will be
marginally better.

> +                          "data16 clflush %[p]", /* clflushopt */
> +                           X86_FEATURE_CLFLUSHOPT,
> +                           [p] "m" (*(const char *)(addr)));
>      }
>  
>      alternative_2("",

On 30.09.2024 17:27, Andrew Cooper wrote:
> On 30/09/2024 4:09 pm, Jan Beulich wrote:
>> There's no point using alternative_io() when there are no outputs.
>>
>> No functional change.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/flushtlb.c
>> +++ b/xen/arch/x86/flushtlb.c
>> @@ -286,11 +286,10 @@ 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_io("ds; clflush %[p]",
>> -                       "data16 clflush %[p]", /* clflushopt */
>> -                       X86_FEATURE_CLFLUSHOPT,
>> -                       /* no outputs */,
>> -                       [p] "m" (*(const char *)(addr)));
>> +        alternative_input("ds; clflush %[p]",
> 
> Drop the ; as you're altering the line anyway?

Oh, sure.

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

Thanks.

> However, switching to ds ds will avoid a trailing nop, so will be
> marginally better.

I don't think I understand, which may or may not be due to me not being
sure whether "ds ds" is a typo. What trailing NOP are you seeing? The
"ds" that's there already covers for the sole trailing NOP that would
otherwise result due to the "data16" prefix.

Jan

On 30/09/2024 4:39 pm, Jan Beulich wrote:
> On 30.09.2024 17:27, Andrew Cooper wrote:
>> On 30/09/2024 4:09 pm, Jan Beulich wrote:
>>> There's no point using alternative_io() when there are no outputs.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> --- a/xen/arch/x86/flushtlb.c
>>> +++ b/xen/arch/x86/flushtlb.c
>>> @@ -286,11 +286,10 @@ 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_io("ds; clflush %[p]",
>>> -                       "data16 clflush %[p]", /* clflushopt */
>>> -                       X86_FEATURE_CLFLUSHOPT,
>>> -                       /* no outputs */,
>>> -                       [p] "m" (*(const char *)(addr)));
>>> +        alternative_input("ds; clflush %[p]",
>> Drop the ; as you're altering the line anyway?
> Oh, sure.
>
>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Thanks.
>
>> However, switching to ds ds will avoid a trailing nop, so will be
>> marginally better.
> I don't think I understand, which may or may not be due to me not being
> sure whether "ds ds" is a typo. What trailing NOP are you seeing? The
> "ds" that's there already covers for the sole trailing NOP that would
> otherwise result due to the "data16" prefix.

Oh of course.  I can't count.  Sorry for the noise.

~Andrew