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

Re: [Xen-devel] [PATCH 1/2] x86/microcode: Synchronize late microcode loading



On Fri, Mar 30, 2018 at 02:59:00PM +0800, Chao Gao wrote:
> From: Gao Chao <chao.gao@xxxxxxxxx>
> 
> This patch is to backport microcode improvement patches from linux
> kernel. Below are the original patches description:
> 
>     commit a5321aec6412b20b5ad15db2d6b916c05349dbff
>     Author: Ashok Raj <ashok.raj@xxxxxxxxx>
>     Date:   Wed Feb 28 11:28:46 2018 +0100
> 
>       x86/microcode: Synchronize late microcode loading
> 
>       Original idea by Ashok, completely rewritten by Borislav.
> 
>       Before you read any further: the early loading method is still the
>       preferred one and you should always do that. The following patch is
>       improving the late loading mechanism for long running jobs and cloud use
>       cases.
> 
>       Gather all cores and serialize the microcode update on them by doing it
>       one-by-one to make the late update process as reliable as possible and
>       avoid potential issues caused by the microcode update.
> 
>       [ Borislav: Rewrite completely. ]
> 
>       Co-developed-by: Borislav Petkov <bp@xxxxxxx>
>       Signed-off-by: Ashok Raj <ashok.raj@xxxxxxxxx>
>       Signed-off-by: Borislav Petkov <bp@xxxxxxx>
>       Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>       Tested-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
>       Tested-by: Ashok Raj <ashok.raj@xxxxxxxxx>

The tested by tags were good for linux upstream. Can you make sure
you add your name under tested-by for xen?

>       Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
>       Cc: Arjan Van De Ven <arjan.van.de.ven@xxxxxxxxx>
>       Link: https://lkml.kernel.org/r/20180228102846.13447-8-bp@xxxxxxxxx
> 
> +static int do_microcode_update(void *_info)
> +{
> +    struct microcode_info *info = _info;
> +    int error, ret = 0;
> +
> +    error = __wait_for_cpus(&info->cpu_in, USEC_PER_SEC);
> +    if ( error )
> +    {
> +        ret = -EBUSY;
> +        return ret;
> +    }
> +
> +    error = microcode_update_cpu(info->buffer, info->buffer_size);
> +    if ( error && !ret )
> +        ret = error;

Isn't this redundant? can ret become non-zero in the check above?

> +    /*
> +     * 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
> +     */
> +    error = __wait_for_cpus(&info->cpu_out, USEC_PER_SEC * 
> num_online_cpus());
> +    if (error)
> +        panic("Timeout when finishing updating microcode");
> +    info->error = ret;
> +    return ret;
>  }
>  



_______________________________________________
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®.