On 14/08/2025 4:00 pm, Jan Beulich wrote: > On 08.08.2025 22:23, Andrew Cooper wrote: >> FRED and IDT differ by a Supervisor Token on the base of the shstk. This >> means that the value they load into MSR_PL0_SSP differs by 8. >> >> s3_resume() in particular has logic which is otherwise invariant of FRED >> mode, >> and must not clobber a FRED MSR_PL0_SSP with an IDT one. >> >> This also simplifies the AP path too. Updating reinit_bsp_stack() is >> deferred >> until later. >> >> No functional change. >> >> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> > I wonder why this was originally done in assembly in the first place, when > we aim at reducing tghe assembly code we have.
It took several iterations (and releases) to get the setup of the supervisor tokens correct. I can't even recall if we had load_system_tables() working like this when the first code went it. It may have been following a subsequent clean-up series. > >> --- a/xen/arch/x86/boot/x86_64.S >> +++ b/xen/arch/x86/boot/x86_64.S >> @@ -65,17 +65,11 @@ ENTRY(__high_start) >> or $(PRIMARY_SHSTK_SLOT + 1) * PAGE_SIZE - 8, %rdx >> >> /* >> - * Write a new supervisor token. Doesn't matter on boot, but for S3 >> - * resume this clears the busy bit. >> + * Write a new Supervisor Token. It doesn't matter the first time a >> + * CPU boots, but for S3 resume or CPU hot re-add, this clears the >> + * busy bit. >> */ >> wrssq %rdx, (%rdx) >> - >> - /* Point MSR_PL0_SSP at the token. */ >> - mov $MSR_PL0_SSP, %ecx >> - mov %edx, %eax >> - shr $32, %rdx >> - wrmsr >> - >> setssbsy > This is ending up a little odd: The comment says the write is to clear the > busy bit, when that's re-set immediately afterwards. That comment is about the wrssq. I suppose what isn't said is that setssbsy will fault if not. How about ", so SETSSBSY can set it again" ? ~Andrew
