Re: [PATCH v10 03/15] x86/alternatives: Disable LASS when patching kernel alternatives

[Sohil Mehta <sohil.mehta@intel.com>]

On 10/7/2025 9:55 AM, Edgecombe, Rick P wrote:
> It's not just used for alternatives anymore. bpf, kprobes, etc use it too. Maybe
> drop "alternatives" from the subject?
> 

Yeah, I was just being lazy. The file is still called alternatives.c and
that's probably what most folks are familiar with.

How about:
x86/text-patching: Disable LASS when patching kernel code

> 
> The above variant has "a barrier is implicit in alternative", is it not needed
> here too? Actually, not sure what that comment is trying to convey anyway.
> 

Yes, the same implication holds true for the LASS versions as well.
I assume it is to let users know that a separate memory barrier is not
needed to prevent the memory accesses following the STAC/CLAC
instructions from getting reordered.

I will add a similar note to the lass_clac()/stac() comments as well.

> Not a strong opinion, but the naming of stac()/clac() lass_stac()/lass_clac() is
> a bit confusing to me. stac/clac instructions now has LASS and SMAP behavior.
> Why keep the smap behavior implicit and give LASS a special variant?
> 
> The other odd aspect is that calling stac()/clac() is needed for LASS in some
> places too, right? But stac()/clac() depend on X86_FEATURE_SMAP not
> X86_FEATURE_LASS. A reader might wonder, why do we not need the lass variant
> there too.
> 
> I'd expect in the real world LASS won't be found without SMAP. Maybe it could be
> worth just improving the comment around stac()/clac() to include some nod that
> it is doing LASS stuff too, or that it relies on that USER mappings are only
> found in the lower half, but KERNEL mappings are not only found upper half.
> 

LASS (data access) depends on SMAP in the hardware as well as the
kernel. The STAC/CLAC instructions toggle LASS alongwith SMAP. One
option is to use the current stac()/clac() instruction for all cases.
However, that would mean unnecessary AC bit toggling during
text-patching on systems without LASS.

The code comments mainly describe how these helpers should be used,
rather than why they exist the way they do.

>> /* Use stac()/clac() when accessing userspace (_PAGE_USER)
>> mappings, regardless of location. */
>> 
>> /* Use lass_stac()/lass_clac() when accessing kernel mappings (!
>> _PAGE_USER) in the lower half of the address space. */
Does this look accurate? The difference is subtle. Also, there is some
potential for incorrect usage, but Dave would prefer to track them
separately.

I can add more explanation to the commit message. Any preferred wording?
Also, the separate patch that Dave recommended would help clarify things
as well.

>>  
>> +/*
>> + * Text poking creates and uses a mapping in the lower half of the
>> + * address space. Relax LASS enforcement when accessing the poking
>> + * address.
>> + *
>> + * Also, objtool enforces a strict policy of "no function calls within
>> + * AC=1 regions". Adhere to the policy by using inline versions of
>> + * memcpy()/memset() that will never result in a function call.
> 
> Is "Also, ..." here really a separate issue? What is the connection to
> lass_stac/clac()?
> 

The issues are interdependent. We need the STAC/CLAC because text poking
accesses special memory. We require the inline memcpy/memset because we
have now added the STAC/CLAC usage and objtool guards against the
potential misuse of STAC/CLAC.

Were you looking for any specific change to the wording?

>>  static void text_poke_memcpy(void *dst, const void *src, size_t len)
>>  {
>> -	memcpy(dst, src, len);
>> +	lass_stac();
>> +	__inline_memcpy(dst, src, len);
>> +	lass_clac();
>>  }
>>