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

From: Chao Gao <chao.gao@intel.com>
To: Sergey Dyasli <sergey.dyasli@citrix.com>
Cc: "Ashok Raj" <ashok.raj@intel.com>, "Wei Liu" <wl@xen.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	xen-devel@lists.xenproject.org,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v9 15/15] microcode: block #NMI handling when loading an ucode
Date: Tue, 27 Aug 2019 12:52:01 +0800	[thread overview]
Message-ID: <20190827045159.GA28509@gao-cwp> (raw)
In-Reply-To: <20190826080759.GB11910@gao-cwp>

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@intel.com>
>>> ---
>>> 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?

Hi Sergey,

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.

Thanks
Chao

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