On 21/04/2020 08:24, Jan Beulich wrote:
> On 20.04.2020 16:59, Andrew Cooper wrote:
>> @@ -46,24 +36,13 @@ void restore_rest_processor_state(void)
>> /* Restore full CR4 (inc MCE) now that the IDT is in place. */
>> write_cr4(mmu_cr4_features);
>>
>> - /* Recover syscall MSRs */
>> - wrmsrl(MSR_LSTAR, saved_lstar);
>> - wrmsrl(MSR_CSTAR, saved_cstar);
>> - wrmsrl(MSR_STAR, XEN_MSR_STAR);
>> - wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK);
>> + /* (re)initialise SYSCALL/SYSENTER state, amongst other things. */
>> + percpu_traps_init();
> Without tweaks to subarch_percpu_traps_init() this assumes that,
> now and going forward, map_domain_page() will work reliably at
> this early point of resume. I'm not opposed, i.e.
> Acked-by: Jan Beulich <jbeul...@suse.com>
I think this reasonable to expect, and robust going forward.
We are properly in d[IDLE]v0 context when it comes to pagetables, and
there is nothing interesting between this point and coming fully back
online.
That said, I could easily move the call to later in the resume path for
even more certainty.
diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index 3ad7dfc9a3..d5a468eddd 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -297,6 +297,8 @@ static int enter_state(u32 state)
ci->spec_ctrl_flags |= (default_spec_ctrl_flags & SCF_ist_wrmsr);
spec_ctrl_exit_idle(ci);
+ /* (re)initialise SYSCALL/SYSENTER state, amongst other things. */
+ percpu_traps_init();
+
done:
spin_debug_enable();
local_irq_restore(flags);
In fact - I prefer this, because it works towards one low priority goal
of deleting {save,restore}_rest_processor_state() which I've still got a
pending series for.
Would your ack still stand if I tweak the patch in this way?
> but it would feel less fragile if the redundant re-writing of
> the stubs would be skipped in the BSP resume case (I didn't
> check whether it's also redundant for APs, but I suspect it's
> not, as the stub pages may get allocated anew).
I don't really agree. Symmetry (even if it is expected to be redundant)
is much more easy to reason about in terms of robustness. S3 is not a
fastpath.
~Andrew
