Re: [Xen-devel] Resume from suspend to RAM broken when using early microcode updates

[Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>]

Jan Beulich:
>>>> On 11.04.18 at 14:11, <JBeulich@xxxxxxxx> wrote:
>>>>> On 11.04.18 at 14:01, <simon@xxxxxxxxxxxxxxxxxxxxxx> 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().

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel