Re: [PATCH v5 01/16] x86/cpu: Enumerate the LASS feature bits

From: Sohil Mehta
Date: Tue Oct 29 2024 - 17:47:13 EST



>> +/*
>> + * The CLAC/STAC instructions toggle enforcement of X86_FEATURE_SMAP.
>> + * Add dedicated lass_*() variants for cases that are necessitated by

It would be useful to know when such a situation is necessitated. For
example, text_poke_mem* doesn't get flagged by SMAP but only by LASS. I
guess the answer is related to paging but it would be useful to describe
it in a commit message or a comment.

I am imagining a scenario where someone needs to use one of these
stac()/clac() pairs but isn't sure which one to use. Both of them would
seem to work but one is better suited than other.

>> + * LASS (X86_FEATURE_LASS) enforcement, which helps readability and
>> + * avoids AC flag flipping on CPUs that don't support LASS.
>> + */
>
> Maybe add a new line here? The comment is for the group of helpers, not
> for clac() specifically.
>

Also, it might be better to move the common text "/* Note: a barrier is
implicit in alternative() */" to the above comment as well.

Repeating it 4 times makes it unnecessarily distracting to read the code.

>> static __always_inline void clac(void)
>> {
>> /* Note: a barrier is implicit in alternative() */
>> @@ -39,6 +45,18 @@ static __always_inline void stac(void)
>> alternative("", __ASM_STAC, X86_FEATURE_SMAP);
>> }
>>
>> +static __always_inline void lass_clac(void)
>> +{
>> + /* Note: a barrier is implicit in alternative() */
>> + alternative("", __ASM_CLAC, X86_FEATURE_LASS);
>> +}
>> +
>> +static __always_inline void lass_stac(void)
>> +{
>> + /* Note: a barrier is implicit in alternative() */
>> + alternative("", __ASM_STAC, X86_FEATURE_LASS);
>> +}
>> +