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

[Jan Beulich <JBeulich@xxxxxxxx>]

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(). Andrew, I'm
curious to learn what you're thinking would be best here.

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