[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Mon, Aug 05, 2019 at 12:11:01PM +0000, Jan Beulich wrote:
>On 01.08.2019 12:22, Chao Gao wrote:
>> @@ -439,12 +440,37 @@ static int do_microcode_update(void *patch)
>>       return ret;
>>   }
>>   
>> +static int microcode_nmi_callback(const struct cpu_user_regs *regs, int cpu)
>> +{
>> +    bool print = false;
>> +
>> +    /* The first thread of a core is to load an update. Don't block it. */
>> +    if ( cpu == cpumask_first(per_cpu(cpu_sibling_mask, cpu)) )
>> +        return 0;
>> +
>> +    if ( loading_state == LOADING_ENTERED )
>> +    {
>> +        cpumask_set_cpu(cpu, &cpu_callin_map);
>> +        printk(XENLOG_DEBUG "CPU%u enters %s\n", smp_processor_id(), 
>> __func__);
>
>Here  and ...
>
>> +        print = true;
>> +    }
>> +
>> +    while ( loading_state == LOADING_ENTERED )
>> +        rep_nop();
>> +
>> +    if ( print )
>> +        printk(XENLOG_DEBUG "CPU%u exits %s\n", smp_processor_id(), 
>> __func__);
>
>... here - why smp_processor_id() when you can use "cpu"? And what
>use is __func__ here?
>
>The rep_nop() above also presumably wants to be cpu_relax() again.
>
>But on the whole I was really hoping for more aggressive disabling
>of NMI handling, more like (but of course not quite as heavy as)
>the crash path wiring the IDT entry to trap_nop().

Hi Jan,

I agree with you that it should be more aggressive. This patch is
problematic considering there is a lot of code before reaching this
callback (especially, SPEC_CTRL_ENTRY_FROM_INTR_IST which may write
MSR_SPEC_CTRL).

In my mind, we have two options to solve the issue:
1. Wire the IDT entry to trap_nop() like the crash path.

2. Enhance this patch: A thread which is not going to load an update
is forced to send an #NMI to itself to enter the callback, do
busy-loop until completion of loading ucode on all cores.
With this method, no #NMI delivery, or MSR write would happen on this
threads during microcode update.

Thanks
Chao

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.