Re: [Xen-devel] [PATCH v7 10/10] x86/microcode: always collect_cpu_info() during boot

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.05.19 at 10:31, <chao.gao@xxxxxxxxx> wrote:
> From: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
> 
> Currently cpu_sig struct is not updated during boot when either:
> 
>     1. ucode_scan is set to false (e.g. no "ucode=scan" in cmdline)
>     2. initrd does not contain a microcode blob

I thought we'd already discussed this - "ucode=<number>" is not
covered by this.

> These will result in cpu_sig.rev being 0 which affects APIC's
> check_deadline_errata() and retpoline_safe() functions.
> 
> Fix this by getting ucode revision early during boot and SMP bring up.
> While at it.

While at it?

> Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> ---
> changes in v7:
> - rebase on patch 1~9

From the looks of it this doesn't depend on any of the earlier changes
(except the ucode_cpu_info -> cpu_sig change), and hence could go
in right away. Am I overlooking something? If not, all that's needed
would be clarifications of the description as per above.

> --- a/xen/arch/x86/microcode.c
> +++ b/xen/arch/x86/microcode.c
> @@ -590,6 +590,10 @@ int __init early_microcode_init(void)
>  
>      if ( microcode_ops )
>      {
> +        rc = microcode_ops->collect_cpu_info(&this_cpu(cpu_sig));
> +        if ( rc )
> +            return rc;
> +
>          if ( ucode_mod.mod_end || ucode_blob.size )
>              rc = early_microcode_parse_and_update_cpu();
>      }

Do we really need to bail on error here? I don't see anything wrong
with simply continuing. The caller doesn't care about the return
value anyway, so best effort would seem to be good enough.

Jan



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