Re: [PATCH v4 1/4] x86: record SSP at non-guest entry points

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.10.2026 11:13, Jan Beulich wrote:
> On 16.10.2026 19:12, Andrew Cooper wrote:
>> On 27/07/2026 11:26 am, Jan Beulich wrote:
>>> --- a/xen/arch/x86/include/asm/asm_defns.h
>>> +++ b/xen/arch/x86/include/asm/asm_defns.h
>>> @@ -220,9 +220,11 @@ static always_inline void stac(void)
>>>  
>>>  #ifdef __ASSEMBLER__
>>>  /*
>>> - * Push and clear GPRs
>>> + * Push and clear GPRs.
>>> + *
>>> + * Use sites may override ssp to 0. It should never be overridden to 1.
>>>   */
>>> -.macro PUSH_AND_CLEAR_GPRS
>>> +.macro PUSH_AND_CLEAR_GPRS ssp=IS_ENABLED(CONFIG_XEN_SHSTK)
>>>          push  %rdi
>>>          xor   %edi, %edi
>>>          push  %rsi
>>> @@ -233,6 +235,9 @@ static always_inline void stac(void)
>>>          xor   %ecx, %ecx
>>>          push  %rax
>>>          xor   %eax, %eax
>>> + .if \ssp
>>> +        rdsspq %rcx
>>> + .endif
>>>          push  %r8
>>>          xor   %r8d, %r8d
>>>          push  %r9
>>> @@ -259,12 +264,18 @@ static always_inline void stac(void)
>>>          xor   %r14d, %r14d
>>>          push  %r15
>>>          xor   %r15d, %r15d
>>> +#ifdef CONFIG_XEN_SHSTK
>>> +        push  %rcx
>>> +#endif
>>>  .endm
>>
>> I don't like the ssp= parameter here.  It's invasive at may callsites,
>> doesn't 0 the value in all Ring3 frames anyway, and the asymmetry makes
>> the construct fragile.  Furthermore, the manual register scheduling
>> really isn't necessary on CPUs where it's not a NOP.
>>
>> I suggest having this:
>>
>> +#ifdef CONFIG_XEN_SHSTK
>> +        rdsspq %rcx
>> +        push  %rcx
>> +#endif
>>
>> always filling in the value.  rdsspq is pretty close to free (frankly, I
>> would be surprised if it wasn't subject to mov elimination in the
>> pipeline, given how tightly coupled SSP needs to be to RSP for CALL/RET
>> uops to work).
> 
> I can make that change, but I didn't code it that way from the start as
> there continues to be no evidence of what you say (no matter how
> plausible it reads): None of the latency / throughput documents I can
> find lists RDSSP.

Oh, also: You implicitly indicate then that the code size increase for those
entry sequences then isn't a problem either, in your opinion. I can accept
that, sure, yet my personal view is that it isn't helpful to have unnecessary
insns anywhere, no matter how cheap they may be. They're never zero cost.

Jan