Re: [Xen-devel] [Patch v3 2/2] x86/microcode: Synchronize late microcode loading

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.05.18 at 15:25, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 16/05/18 14:10, Jan Beulich wrote:
>>> +static int do_microcode_update(void *_info)
>>> +{
>>> +    struct microcode_info *info = _info;
>>> +    unsigned int cpu = smp_processor_id();
>>> +    int ret;
>>> +
>>> +    ret = wait_for_cpus(&info->cpu_in, MICROCODE_DEFAULT_TIMEOUT);
>>> +    if ( ret )
>>> +        return ret;
>>> +
>>> +    /*
>>> +     * Logical threads which set the first bit in cpu_sibling_mask can do
>>> +     * the update. Other sibling threads just await the completion of
>>> +     * microcode update.
>>> +     */
>>> +    if ( !cpumask_test_and_set_cpu(
>>> +                cpumask_first(per_cpu(cpu_sibling_mask, cpu)), 
>>> &info->cpus) )
>>> +        ret = microcode_update_cpu(info->buffer, info->buffer_size);
>>> +    /*
>>> +     * Increase the wait timeout to a safe value here since we're 
>>> serializing
>>> +     * the microcode update and that could take a while on a large number 
>>> of
>>> +     * CPUs. And that is fine as the *actual* timeout will be determined by
>>> +     * the last CPU finished updating and thus cut short
>>> +     */
>>> +    if ( wait_for_cpus(&info->cpu_out, MICROCODE_DEFAULT_TIMEOUT *
>>> +                                       nr_phys_cpus) )
>> I remain unconvinced that this is a safe thing to do on a huge system with
>> guests running (even Dom0 alone would seem risky enough). I continue to
>> hope for comments from others, in particular Andrew, here. At the very
>> least I think you should taint the hypervisor when making it here.
> 
> I see nothing in this patch which prevents a deadlock against the time
> calibration rendezvous.  It think its fine to pause the time calibration
> rendezvous while performing this update.

If there's a problem here, wouldn't that be a general one with
stop_machine()?

> Also, what is the purpose of serialising the updates while all pcpus are
> in rendezvous?  Surely at that point the best option is to initiate an
> update on all processors which don't have an online sibling thread with
> a lower thread id.

I've suggested that before.

Jan



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