Re: Re [PATCH] x86/CET: Fix build following c/s 43b98e7190

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.06.2020 13:44, Andrew Cooper wrote:
> On 03/06/2020 10:50, Jan Beulich wrote:
>> On 02.06.2020 19:15, Andrew Cooper wrote:
>>> On 02/06/2020 15:21, Jan Beulich wrote:
>>>>> OSSTest reports:
>>>>>
>>>>>   x86_64.S: Assembler messages:
>>>>>   x86_64.S:57: Error: no such instruction: `setssbsy'
>>>>>   /home/osstest/build.150510.build-amd64/xen/xen/Rules.mk:183: recipe for 
>>>>> target 'head.o' failed
>>>>>   make[4]: Leaving directory 
>>>>> '/home/osstest/build.150510.build-amd64/xen/xen/arch/x86/boot'
>>>>>   make[4]: *** [head.o] Error 1
>>>>>
>>>>> All use of CET instructions, even those inside alternative blocks, needs 
>>>>> to be
>>>>> behind CONFIG_XEN_SHSTK, as it indicates suitable toolchain support.
>>>>>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>> That's quite a bit of #ifdef-ary here. Simple (operand-less) insns
>>>> like SETSSBSY could easily be made available via .byte directives.
>>>> Would you be amenable to to ack-ing a patch to replace some of the
>>>> #ifdef-s (at least the ones at the lstar, cstar, and sysenter
>>>> entry points), after 4.14?
>>> Yeah - that was a bit of a mess in the end.  (But given the
>>> circumstances, and that I've got past form typo'ing the SETSSBSY opcode,
>>> it probably was the right move even in hindsight).
>>>
>>> Reducing it to .byte should be fine so long as some form of /* setssbsy
>>> */ comment appears.
>> Sure.
>>
>>> One other option would be to introduce a SETSSBSY macro, but that hides
>>> the alternative so is something I'd prefer to avoid.
>> With this you mean you'd rather not see us go the CLAC/STAC route?
>> I was instead thinking of a pure assembly macro named "setssbsy".
>> In fact we could switch the CLAC/STAC ugliness to some such, if we
>> end up being happy with the model.
> 
> The thing about the current STAC / CLAC is that, as written, they give
> the impression of being unconditional.  This is poor in terms of code
> clarity.
> 
> Furthermore, making them indistinguishable from the plain instructions
> is definitely a no-go, because then we've got assembly source (again,
> which appears unconditional) which doesn't match its disassembly (the
> backing nops) - at least with the macros in upper case, it is obvious
> that something is up, even if you have to searching for why.
> 
> If we went for pure assembly macros with an alt_ or maybe_ prefix, then
> that would be reasonable.  It looks as close to regular instruction as
> possible, but also makes it explicitly clear that it is conditional.

That wasn't the plan - I didn't mean to hide the alternatives aspect.
I wanted to have simple "setssbsy", "clac", and "stac" macros expanding
to just the insns, getting instantiated when !HAVE_AS_<whatever>. This
would make assembly code look descriptive no matter what assembler
capabilities there are (for these operand-less insns, that is, not in
general).

Jan