Re: [PATCH 1/3] x86: drop NOP_DS_PREFIX

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.02.2022 11:54, Andrew Cooper wrote:
> On 23/02/2022 10:12, Jan Beulich wrote:
>> This wasn't really necessary to introduce: The binutils change
>> permitting use of standalone "ds" (and "cs") in 64-bit code predates
>> the minimum binutils version we support.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks.

> I was never a fan of NOP_DS_PREFIX.  Far too verbose for what it's doing.
> 
>> ---
>> In fact we could patch _just_ the opcode prefix in flush_area_local().
>>
>> --- a/xen/arch/x86/flushtlb.c
>> +++ b/xen/arch/x86/flushtlb.c
>> @@ -247,8 +247,7 @@ unsigned int flush_area_local(const void
>>          {
>>              alternative("", "sfence", X86_FEATURE_CLFLUSHOPT);
>>              for ( i = 0; i < sz; i += c->x86_clflush_size )
>> -                alternative_input(".byte " __stringify(NOP_DS_PREFIX) ";"
>> -                                  " clflush %0",
>> +                alternative_input("ds; clflush %0",
> 
> Binutils appears to be happy with "ds clflush", i.e. treating it like a
> proper prefix on the instruction.  Drop the semicolon at the same time?

I'd rather not. A clever assembler may eliminate the prefix as redundant
when the base register isn't stack or frame pointer. In 64-bit mode an
assembler might even decide to eliminate all non-standalone segment
overrides using the pre-386 segment registers.

Jan