Re: [PATCH v2 1/6] x86: replace __ASM_{CL,ST}AC

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.10.2020 13:04, Andrew Cooper wrote:
> On 28/09/2020 13:29, Jan Beulich wrote:
>> --- a/xen/arch/x86/arch.mk
>> +++ b/xen/arch/x86/arch.mk
>> @@ -20,6 +20,7 @@ $(call as-option-add,CFLAGS,CC,"rdrand %
>>  $(call as-option-add,CFLAGS,CC,"rdfsbase %rax",-DHAVE_AS_FSGSBASE)
>>  $(call as-option-add,CFLAGS,CC,"xsaveopt (%rax)",-DHAVE_AS_XSAVEOPT)
>>  $(call as-option-add,CFLAGS,CC,"rdseed %eax",-DHAVE_AS_RDSEED)
>> +$(call as-option-add,CFLAGS,CC,"clac",-DHAVE_AS_CLAC_STAC)
> 
> Kconfig

I know that's your view, and you know I disagree. I don't see the
thread I had started to have lead to any consensus.

>> --- /dev/null
>> +++ b/xen/include/asm-x86/asm-defns.h
>> @@ -0,0 +1,9 @@
>> +#ifndef HAVE_AS_CLAC_STAC
>> +.macro clac
>> +    .byte 0x0f, 0x01, 0xca
>> +.endm
>> +
>> +.macro stac
>> +    .byte 0x0f, 0x01, 0xcb
>> +.endm
>> +#endif
>> --- a/xen/include/asm-x86/asm_defns.h
>> +++ b/xen/include/asm-x86/asm_defns.h
> 
> We cannot have two files which differ by the vertical positioning of a dash.

Why "cannot"? One is the helper of the other, so them being named almost
identically is quite sensible imo (and no-one is supposed to include the
new one directly). In any event I'd at most see this be "we don't want to".

> How about asm-insn.h for the former, seeing as that is what it contains.

Until "x86: fold indirect_thunk_asm.h into asm-defns.h", where it starts
to be more than just plain insn replacements. And I suspect more non-insn
macros will appear over time. I'd have suggested asm-macros.h in case the
present name really can't be reached consensus on, but we have a
(generated) file of this name already.

Jan