Re: [Xen-devel] [PATCH v9 15/15] microcode: block #NMI handling when loading an ucode

[Chao Gao <chao.gao@xxxxxxxxx>]

On Thu, Aug 29, 2019 at 02:11:10PM +0200, Jan Beulich wrote:
>On 27.08.2019 06:52, Chao Gao wrote:
>> On Mon, Aug 26, 2019 at 04:07:59PM +0800, Chao Gao wrote:
>>> On Fri, Aug 23, 2019 at 09:46:37AM +0100, Sergey Dyasli wrote:
>>>> On 19/08/2019 02:25, Chao Gao wrote:
>>>>> register an nmi callback. And this callback does busy-loop on threads
>>>>> which are waiting for loading completion. Control threads send NMI to
>>>>> slave threads to prevent NMI acceptance during ucode loading.
>>>>>
>>>>> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
>>>>> ---
>>>>> Changes in v9:
>>>>>  - control threads send NMI to all other threads. Slave threads will
>>>>>  stay in the NMI handling to prevent NMI acceptance during ucode
>>>>>  loading. Note that self-nmi is invalid according to SDM.
>>>>
>>>> To me this looks like a half-measure: why keep only slave threads in
>>>> the NMI handler, when master threads can update the microcode from
>>>> inside the NMI handler as well?
>>>
>>> No special reason. Because the issue we want to address is that slave
>>> threads might go to handle NMI and access MSRs when master thread is
>>> loading ucode. So we only keep slave threads in the NMI handler.
>>>
>>>>
>>>> You mention that self-nmi is invalid, but Xen has self_nmi() which is
>>>> used for apply_alternatives() during boot, so can be trusted to work.
>>>
>>> Sorry, I meant using self shorthand to send self-nmi. I tried to use
>>> self shorthand but got APIC error. And I agree that it is better to
>>> make slave thread call self_nmi() itself.
>>>
>>>>
>>>> I experimented a bit with the following approach: after loading_state
>>>> becomes LOADING_CALLIN, each cpu issues a self_nmi() and rendezvous
>>>> via cpu_callin_map into LOADING_ENTER to do a ucode update directly in
>>>> the NMI handler. And it seems to work.
>>>>
>>>> Separate question is about the safety of this approach: can we be sure
>>>> that a ucode update would not reset the status of the NMI latch? I.e.
>>>> can it cause another NMI to be delivered while Xen already handles one?
>>>
>>> Ashok, what's your opinion on Sergey's approach and his concern?
>> 
>> I talked with Ashok. We think your approach is better. I will follow
>> your approach in v10. It would be much helpful if you post your patch
>> so that I can just rebase it onto other patches.
>
>Doing the actual ucode update inside an NMI handler seems rather risky
>to me. Even if Ashok confirmed it would not be an issue on past and
>current Intel CPUs - what about future ones, or ones from other vendors?

Will confirm these with Ashok.

Thanks
Chao

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