Re: [PATCH v2 69/70] x86/efi: Disable CET-IBT around Runtime Services calls

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.02.2022 00:00, Andrew Cooper wrote:
> On 15/02/2022 16:53, Jan Beulich wrote:
>> On 14.02.2022 13:51, Andrew Cooper wrote:
>>> --- a/xen/common/efi/runtime.c
>>> +++ b/xen/common/efi/runtime.c
>>> @@ -21,6 +21,7 @@ struct efi_rs_state {
>>>    * don't strictly need that.
>>>    */
>>>   unsigned long __aligned(32) cr3;
>>> +    unsigned long msr_s_cet;
>>>  #endif
>>>  };
>> The latest with the next addition here we will probably want to ...
>>
>>> @@ -113,6 +114,19 @@ struct efi_rs_state efi_rs_enter(void)
>> ... no longer have this be the function's return type.
> 
> So about this.
> 
> why aren't we using __attribute__((force_align_arg_pointer)) ?  It
> exists in at least GCC 4.1 and Clang 6.

Perhaps first and foremost because this is the first time I encounter
this attribute, despite it having been around for so long. However,
Clang 6 would be a little too high for the main box I have a Clang
installed on - that's Clang 5 only (and, afaict, no option to upgrade
without also upgrading the distro, while I'd also like to avoid having
to also build myself Clang binaries; maybe sooner or later that's
going to be unavoidable, though). While from binary searching its
libraries it looks to know of that attribute, it still doesn't accept
its use.

The other issue I see is that using it would be fragile: We cannot
afford to forget putting the attribute on any of the relevant
functions. Whereas the present model makes it impossible to miss
any instance.

Finally the attribute's interaction with -mpreferred-stack-boundary=
isn't spelled out anywhere. It looks to behave sanely on gcc 11, but
who knows whether this has always been the case.

Jan

> We're way way overdue bumping the minimum toolchain versions, and Clang
> 3.5=>6 is still very obsolete minimum version.  This way, we're not
> depending on some very subtle ABI mechanics to try and keep the stack
> properly aligned.
> 
> ~Andrew