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
