On 14.08.2025 22:09, Andrew Cooper wrote:
> On 14/08/2025 4:11 pm, Jan Beulich wrote:
>> On 08.08.2025 22:23, Andrew Cooper wrote:
>>> Under FRED, SETSSBSY is unavailable, and we want to be setting up FRED prior
>>> to setting up shadow stacks. As we still need Supervisor Tokens in IDT
>>> mode,
>>> we need mode-specific logic to establish SSP.
>>>
>>> In FRED mode, write a Restore Token, RSTORSSP it, and discard the resulting
>>> Previous-SSP token.
>>>
>>> No change outside of FRED mode.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
>> Why is it that in patch 17 you could use identical code, but here you can't?
>
> This caught me out at first too.
>
> For S3, we're going from "no shadow stack" to "back to where we were on
> an active shadow stack". All we need to do is get saved_ssp back into
> the SSP register.
>
> Here, we're going from "no shadow stack" to "on a good, empty, shadow
> stack". For FRED we only need to load a value into SSP, but in IDT mode
> we must also arrange to create a busy Supervisor Token on the base of
> the stack.
>
> We could in principle conditionally write a busy supervisor token, then
> unconditionally RSTORSSP, but that's even more complicated to follow IMO.
Why would the write need to be conditional? Can't we write what effectively
is already there? Or is it more a safety measure to avoid the write when
it's supposed to be unnecessary, to avoid papering over bugs?
>>> @@ -912,10 +913,30 @@ static void __init noreturn reinit_bsp_stack(void)
>>>
>>> if ( cpu_has_xen_shstk )
>>> {
>>> - wrmsrl(MSR_PL0_SSP,
>>> - (unsigned long)stack + (PRIMARY_SHSTK_SLOT + 1) * PAGE_SIZE
>>> - 8);
>> Does this removal perhaps belong elsewhere, especially with "No change
>> outside of FRED mode" in the description?
>
> This is the "Updating reinit_bsp_stack() is deferred until later." note
> in the previous patch.
>
> This hunk was illegible without the split, although I have to admit that
> I can't quite remember why now.
Hmm, if it is to stay like this, would you mind adding a respective remark
also in the description here?
Jan
