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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 09/04/2026 9:13 am, Jan Beulich wrote:
> On 08.04.2026 18:58, Andrew Cooper wrote:
>> On 08/04/2026 1:22 pm, Jan Beulich wrote:
>>> We will want to use that value for call trace generation, and likely
>>> also to eliminate the somewhat fragile shadow stack searching done in
>>> fixup_exception_return(). For those purposes, guest-only entry points do
>>> not need to record that value.
>>>
>>> To keep the saving code simple, record our own SSP that corresponds to
>>> an exception frame, pointing to the top of the shadow stack counterpart
>>> of what the CPU has saved on the regular stack. Consuming code can then
>>> work its way from there.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> For PUSH_AND_CLEAR_GPRS and POP_GPRS, putting the new field right next to
>>> the error code isn't entirely nice; putting it ahead of %r15 would entail
>>> other changes, though. An option may be to not make SSP handling part of
>>> the macros in the first place. Thoughts?
>> I have a firm dislike for SAVE/RESTORE_ALL, both for their substantial
>> complexity/inefficiency, and mixing of unrelated tasks.
>>
>> I have several series trying to purge them.  I suppose I really ought to
>> try and finish this off properly.
>>
>> While classing SSP as a "register" is probably fine, the ssp= parameter
>> (and particular it's asymmetric nature) is on the wrong side of the
>> "complex" argument IMO.
>>
>>> For POP_GPRS, does it really matter that it doesn't alter EFLAGS? 
>> Yes.  The SYSCALL fix for one (reviewed, but waiting on final testing
>> before I commit).
>>
>> Then the VT-x code when swapped to use POP_GPRS.
>>
>>
>> To take a step back, you say that putting it ahead of %r15 would entail
>> other changes.  What changes?
> SAVE_ALL's initial ADD, RESTORE_ALL's final SUB,

Ok, this problem goes away if they're purged.

I guess I should refresh and repost my series.

> and then the hunt for
> anything which may simply assume UREGS_r15 to be 0.

I highly doubt we've got anything like this.  (And even if we do, it
wants fixing, not using as an argument against doing this the nicer way.)

>  If UREGS_r<xyz> were
> ordered by register number, I would have considered putting it where
> %rsp nominally would go, but without that putting it somewhere in the
> middle feels rather arbitrary.
>
>> The resulting asm would be far cleaner.
> I agree.
>
>>   It would be an rdssp;push on
>> one side, and a pop into any register on the other side.  Furthermore,
>> given that the ssp= doesn't exclude storing it for some user frames,
>> just store it for all.  It's one push/pop into a hot cacheline, and
>> makes a substantial reduction in complexity.
> I'm having significant reservations against that. I use the 0 put there
> in subsequent patches, to identify absence of that data being available.

Well, that's not safe then.

You've already got non-zero values there on entry-from-PV because
there's no CPL check gating RDSPP the common IDT paths.

~Andrew