Re: [PATCH v2 14/16] x86/fsgsbase: Split out __{rd,wr}gskern() helpers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.08.2025 22:20, Andrew Cooper wrote:
> On 19/08/2025 2:01 pm, Jan Beulich wrote:
>> On 15.08.2025 22:41, Andrew Cooper wrote:
>>> Right now they're inline in {read,write}_gs_shadow(), but we're going to 
>>> need
>>> to use these elsewhere to support FRED.
>> But why "kern"? We're not dealing with GS in kernel / user terms, but in
>> real / shadow ones.
> 
> Because it's a common name that also has the property of aligning nicely
> when used beside GS_BASE.
> 
> But fine, I'll rename it.
> 
>>  I'm also not quite happy with the double leading
>> underscores, fwiw.
> 
> Consistency with the similar logic.

Yet making for more changes later, once we mean to strictly follow the
respective Misra rule.

>>> --- a/xen/arch/x86/include/asm/fsgsbase.h
>>> +++ b/xen/arch/x86/include/asm/fsgsbase.h
>>> @@ -32,6 +32,17 @@ static inline unsigned long __rdgsbase(void)
>>>      return base;
>>>  }
>>>  
>>> +static inline unsigned long __rdgskern(void)
>>> +{
>>> +    unsigned long base;
>>> +
>>> +    asm_inline volatile ( "swapgs\n\t"
>>> +                          "rdgsbase %0\n\t"
>>> +                          "swapgs" : "=r" (base) );
>> Again strictly speaking "=&r", if already you open-code rdgsbase() now.
> 
> As before, why?   There are no inputs to be clobbered, early or otherwise.

Hence why I said "strictly speaking". Inputs or not imo doesn't matter in
such a decision; that merely makes using the early-clobber form benign.
The sole criteria by which I think one ought to go is whether a register
is altered solely by the last (often: only) insn. IOW it's again more to
prevent setting a bad precedent. Here things are easy to see, so a
hypothetical future change isn't at much of a risk of breaking things. The
more complex asm()-s get, the easier it is to overlook a missing early-
clobber when making some change.

Jan