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.
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.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
