Re: [PATCH 5/5] x86/microcode: Handle NMI's during microcode update.

[Andrew Cooper]

On 14/08/2022 03:54, Ashok Raj wrote:
> On Sat, Aug 13, 2022 at 05:13:13PM -0700, Andy Lutomirski wrote:
>>
>> On Sat, Aug 13, 2022, at 3:38 PM, Ashok Raj wrote:
>>> Microcode updates need a guarantee that the thread sibling that is waiting
>>> for the update to finish on the primary core will not execute any
>>> instructions until the update is complete. This is required to guarantee
>>> any MSR or instruction that's being patched will be executed before the
>>> update is complete.
>>>
>>> After the stop_machine() rendezvous, an NMI handler is registered. If an
>>> NMI were to happen while the microcode update is not complete, the
>>> secondary thread will spin until the ucode update state is cleared.
>>>
>>> Couple of choices discussed are:
>>>
>>> 1. Rendezvous inside the NMI handler, and also perform the update from
>>>    within the handler. This seemed too risky and might cause instability
>>>    with the races that we would need to solve. This would be a difficult
>>>    choice.
>> I prefer choice 1.  As I understand it, Xen has done this for a while to good effect.
>>
>> If I were implementing this, I would rendezvous via stop_machine as usual.  Then I would set a flag or install a handler indicating that we are doing a microcode update, send NMI-to-self, and rendezvous in the NMI handler and do the update.
> Well, that is exactly what I did for the first attempt. The code looked so
> beautiful in the eyes of the creator :-) but somehow I couldn't get it to
> not lock up.

So the way we do this in Xen is to rendezvous in stop machine, then have
only the siblings self-NMI.  The primary threads don't need to be in NMI
context, because the WRMSR to trigger the update *is* atomic with NMIs.

However, you do need to make sure that the NMI wait loop knows not to
wait for primary threads, otherwise you can deadlock when taking an NMI
on a primary thread between setting up the NMI handler and actually
issuing the update.

~Andrew