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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

~Andrew