Re: [PATCH v2 07/23] x86/boot: Use RSTORSSP to establish SSP

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.09.2025 20:53, Andrew Cooper wrote:
> On 01/09/2025 4:41 pm, Jan Beulich wrote:
>> On 01.09.2025 17:33, Andrew Cooper wrote:
>>> On 01/09/2025 10:28 am, Jan Beulich wrote:
>>>> On 28.08.2025 17:03, Andrew Cooper wrote:
>>>>> @@ -908,7 +909,29 @@ static void __init noreturn reinit_bsp_stack(void)
>>>>>      if ( cpu_has_xen_shstk )
>>>>>      {
>>>>>          wrmsrl(MSR_S_CET, xen_msr_s_cet_value());
>>>>> -        asm volatile ("setssbsy" ::: "memory");
>>>>> +
>>>>> +        /*
>>>>> +         * IDT and FRED differ by a Supervisor Token on the shadow 
>>>>> stack, and
>>>>> +         * therefore by the value in MSR_PL0_SSP.
>>>> Beside not being overly relevant here afaict, is this last part of the 
>>>> sentence
>>>> actually correct? Patch 06 doesn't write different values into the MSR.
>>> It is correct, but also well hidden.
>>>
>>> #define MSR_FRED_SSP_SL0                    MSR_PL0_SSP
>>>
>>> I suppose I should should write MSR_PL0_SSP/MSR_FRED_SSP_SL0 here to
>>> highlight the logically different names for the two modes.
>> But the code following the comment doesn't access any MSR. That's what
>> first tripped me up. It was only then that I wasn't able to spot the two
>> different writes. Now that you point out the aliasing it becomes clear
>> that until patch 14 it is simply impossible to find that other write.
> 
> I suppose the MSR value isn't relevant now that neither paths write the
> value.  The first iteration had both writes here.
> 
> I guess I can drop that paragraph, and just have the second?

I'd keep everything up to the comma (plus the other paragraph of course).

Jan