Jan Beulich: >>>> On 11.04.18 at 14:11, wrote: >>>>> On 11.04.18 at 14:01, wrote: >>> Andrew Cooper: >>>> On 11/04/18 12:48, Simon Gaiser wrote: >>>>> Hi, >>>>> >>>>> when I use early microcode loading with the microcode update with the >>>>> BTI mitigations, resuming from suspend to RAM is broken. >>>>> >>>>> Based on added logging to enter_state() (from power.c) it doesn't >>>>> survive the local_irq_restore(flags) call (at least a printk() after the >>>>> call doesn't output anything on the serial console). >>>>> >>>>> I guess that some irq handler tries to use IBRS/IBPB. But the microcode >>>>> is only loaded later. >>>>> >>>>> If I simply move the microcode_resume_cpu(0) directly before the >>>>> local_irq_restore(flags) everything seems to work fine. But I'm not sure >>>>> if this has unintended consequences. >>>>> >>>>> I tested the above with Xen 4.8.3 from Qubes which includes the BTI and >>>>> microcode patches from staging-4.8. AFAICS there are no commits which >>>>> changes the affected code or other commits which sound relevant so this >>>>> probably affected also all the newer branches. >>>> >>>> S3 support is a very unloved area of the hypervisor. >>>> >>>> Yes - we definitely need to get microcode reloaded before interrupts are >>>> enabled. >>> >>> Do you see any problems with simply moving microcode_resume_cpu(0) >>> directly before the local_irq_restore(flags) call? (I'm not familiar >>> with the code at all and (early) resume handling sounds like something >>> which is easy to break in non obvious ways) >> >> Yes, there would be a problem: microcode_resume_cpu() >> spin_lock()-s almost first thing, and this would break our >> (simplistic) lock checking. Putting it also ahead of >> spin_debug_enable() should work otoh. >> >> Once at it, cpufreq_add_cpu() should be moved ahead of the >> enable_cpu label as well, as cpufreq_del_cpu() wasn't called >> yet at the point of the only goto to that label. > > And I think console_end_sync() want to be moved earlier then > as well. Where exactly? console_end_sync() seems to match the position of console_start_sync().