Re: [PATCH v2 03/14] x86/shstk: Introduce Supervisor Shadow Stack support

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 29/05/2020 16:51, Anthony PERARD wrote:
> On Fri, May 29, 2020 at 01:59:55PM +0200, Jan Beulich wrote:
>> On 28.05.2020 20:10, Andrew Cooper wrote:
>>> On 28/05/2020 11:25, Jan Beulich wrote:
>>>> On 27.05.2020 21:18, Andrew Cooper wrote:
>>>>> --- a/xen/scripts/Kconfig.include
>>>>> +++ b/xen/scripts/Kconfig.include
>>>>> @@ -31,6 +31,10 @@ cc-option = $(success,$(CC) -Werror $(CLANG_FLAGS) 
>>>>> $(1) -E -x c /dev/null -o /de
>>>>>  # Return y if the linker supports <flag>, n otherwise
>>>>>  ld-option = $(success,$(LD) -v $(1))
>>>>>  
>>>>> +# $(as-instr,<instr>)
>>>>> +# Return y if the assembler supports <instr>, n otherwise
>>>>> +as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x 
>>>>> assembler -o /dev/null -)
>>>> Is this actually checking the right thing in the clang case?
>>> Appears to work correctly for me.  (Again, its either fine, or need
>>> bugfixing anyway for 4.14, and raising with Linux as this is unmodified
>>> upstream code like the rest of Kconfig.include).
>> This answer made me try it out: On a system with clang 5 and
>> binutils 2.32 "incsspd       %eax" translates fine with
>> -no-integrated-as but doesn't without. The previously mentioned
>> odd use of CLANG_FLAGS, perhaps together with the disconnect
>> from where we establish whether to use -no-integrated-as in the
>> first place (arch.mk; I have no idea whether the CFLAGS
>> determined would be usable by the kconfig invocation, nor how
>> to solve the chicken-and-egg problem there if this is meant to
>> work that way), may be the reason for this. Cc-ing Anthony once
>> again ...
> I've just prepare/sent a patch which should fix this CLANG_FLAGS issue
> and allows to tests the assembler in Kconfig.
>
> See: [XEN PATCH] xen/build: introduce CLANG_FLAGS for testing other CFLAGS

Thanks.  I've checked carefully, and this is an improvement from before.

Therefore I have acked and included the patch.

However, I think there is a further problem.  When -no-integrated-as
does get passed down, I don't see Clang falling back to using the Gnu
assember, so I suspect we have a further plumbing problem.  (It only
affects Clang 5.0 and earlier, so obsolete toolchains these days).

~Andrew