Re: [Xen-devel] [PATCH v11 6/7] microcode: rendezvous CPUs in NMI handler and load ucode

[Chao Gao <chao.gao@intel.com>]

On Fri, Sep 27, 2019 at 03:55:00PM +0200, Jan Beulich wrote:
>On 27.09.2019 15:53, Chao Gao wrote:
>> On Fri, Sep 27, 2019 at 12:19:22PM +0200, Jan Beulich wrote:
>>> On 26.09.2019 15:53, Chao Gao wrote:
>>>> @@ -420,14 +498,23 @@ static int control_thread_fn(const struct microcode_patch *patch)
>>>>          return ret;
>>>>      }
>>>>  
>>>> -    /* Let primary threads load the given ucode update */
>>>> -    set_state(LOADING_ENTER);
>>>> -
>>>> +    /* Control thread loads ucode first while others are in NMI handler. */
>>>>      ret = microcode_ops->apply_microcode(patch);
>>>>      if ( !ret )
>>>>          atomic_inc(&cpu_updated);
>>>>      atomic_inc(&cpu_out);
>>>>  
>>>> +    if ( ret == -EIO )
>>>> +    {
>>>> +        printk(XENLOG_ERR
>>>> +               "Late loading aborted: CPU%u failed to update ucode\n", cpu);
>>>> +        set_state(LOADING_EXIT);
>>>> +        return ret;
>>>> +    }
>>>> +
>>>> +    /* Let primary threads load the given ucode update */
>>>> +    set_state(LOADING_ENTER);
>>>
>>> While the description goes to some lengths to explain this ordering of
>>> updates, I still don't really see the point: How is it better for the
>>> control CPU to have updated its ucode early and then hit an NMI before
>>> the other CPUs have even started updating, than the other way around
>>> in the opposite case?
>> 
>> We want to be conservative here. If an ucode is to update something
>> shared by a whole socket, for the latter case, control thread may
>> be accessing things that are being updating by the ucode loading on
>> other cores. It is not safe, just like sibling thread isn't expected
>> to access features exposed by the old ucode when primary thread is
>> loading ucode.
>
>Ah yes, considering a socket-wide effect didn't occur to me (although
>it should have). So if you mention this aspect in the description, I
>think I'm going to be fine with the change in this regard. Yet (as so
>often) this raises another question: What about "secondary" sockets?
>Shouldn't we entertain a similar two-step approach there then?

No. The two-step approach is because control thread cannot call
self_nmi() in case of triggering unknown_nmi_error() and what is done
in the main NMI handler isn't well controlled. All cores on other
sockets will rendezvous in NMI handler. It means every core's behavior
on other sockets is well controlled.

Thanks
Chao

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