On 15/08/2025 9:52 am, Jan Beulich wrote: > On 14.08.2025 21:37, Andrew Cooper wrote: >> On 14/08/2025 4:00 pm, Jan Beulich wrote: >>> On 08.08.2025 22:23, Andrew Cooper wrote: >>>> --- 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" ? > That would improve things, but then it's still unclear to me why we need to > go through this. If the busy bit is already set, why clear it just to set > it again.
Because the behaviour of SETSSBSY is along the lines of: if ( test_and_set(busy) ) SSP = token; else #GP It really is quite an ugly instruction, and I'm not sorry to see it go. > Or perhaps asked differently: Wouldn't we be better off if we > cleared the busy bit when a CPU went offline? That's actually quite hard. The shadow stack is in use until the CPU takes an INIT, so ought to have it's busy bit set. In practice, clearing the primary shstk busy bit is probably safe to do when we're certain we'll never return to PV context again. But this is still a larger change (needs wrss() in every play dead) and more fragile than just clearing it before we're about to use the stack fresh. ~Andrew
