On Wed, Apr 11, 2018 at 01:12:51PM +0100, Andrew Cooper wrote: > On 11/04/18 13:01, Simon Gaiser 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) > > Judging by what is going on, it wants to be between tboot_s3_error() and > the done label. > > We only need to restore microcode if we successfully went into S3. The > done and enable_cpu labels are only used by paths which don't need to > restore microcode. > > OTOH, you should check the return value and panic if restoration > failed. As you've seen, the system won't survive trying to blindly > continue resuming. > > > > >> That said, I would have expected a backtrace complaining about > >> a GP fault if we had hit the use of IBRS/IBPB before the microcode was > >> reloaded. > > Yeah, not sure what's happening here. I don't get any output from after > > local_irq_restore(flags). If you have some ideas for more debug output I > > can easily test it. > > In hindsight, I am. We take a #GP fault because of a bad MSR, and at > the head of the exception handler try to use the same bad MSR. It will > repeatedly fault until hitting a guard page (or other read-only page), > at which point we take a double fault, and suffer a #GP yet again. > Taking a #DF will reset the stack to a moderately sane value, and the > system will livelock taking faults. > > This is an unfortunate consequence of having $MAGIC in the exception > handlers.
We can just disable IBRS before going to sleep? > > ~Andrew > > _______________________________________________ > Xen-devel mailing list > Xenemail@example.com > https://lists.xenproject.org/mailman/listinfo/xen-devel _______________________________________________ Xen-devel mailing list Xenfirstname.lastname@example.org https://lists.xenproject.org/mailman/listinfo/xen-devel