Re: [PATCH v5 3/4] x86: Read MSR_ARCH_CAPS immediately after early_microcode_init()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.06.2023 17:26, Alejandro Vallejo wrote:
> @@ -324,9 +324,10 @@ void __init early_cpu_init(void)
>       case X86_VENDOR_SHANGHAI: this_cpu = &shanghai_cpu_dev; break;
>       case X86_VENDOR_HYGON:    this_cpu = &hygon_cpu_dev;    break;
>       default:
> -             printk(XENLOG_ERR
> -                    "Unrecognised or unsupported CPU vendor '%.12s'\n",
> -                    c->x86_vendor_id);
> +             if (verbose)
> +                     printk(XENLOG_ERR
> +                            "Unrecognised or unsupported CPU vendor 
> '%.12s'\n",
> +                            c->x86_vendor_id);

Just as a remark:

        if (!verbose)
                break;

would have been less of a delta and keeping all lines within the 80
chars limit.

> @@ -340,10 +341,11 @@ void __init early_cpu_init(void)
>       c->x86_capability[FEATURESET_1d] = edx;
>       c->x86_capability[FEATURESET_1c] = ecx;
>  
> -     printk(XENLOG_INFO
> -            "CPU Vendor: %s, Family %u (%#x), Model %u (%#x), Stepping %u 
> (raw %08x)\n",
> -            x86_cpuid_vendor_to_str(c->x86_vendor), c->x86, c->x86,
> -            c->x86_model, c->x86_model, c->x86_mask, eax);
> +     if (verbose)
> +             printk(XENLOG_INFO
> +                    "CPU Vendor: %s, Family %u (%#x), Model %u (%#x), 
> Stepping %u (raw %08x)\n",
> +                    x86_cpuid_vendor_to_str(boot_cpu_data->x86_vendor), 
> c->x86, c->x86,
> +                    c->x86_model, c->x86_model, c->x86_mask, eax);

Since rearrangement to limit line length isn't really possible here,
the last two lines need re-flowing to stay within limits.

> --- a/xen/arch/x86/cpu/microcode/core.c
> +++ b/xen/arch/x86/cpu/microcode/core.c
> @@ -886,5 +886,11 @@ int __init early_microcode_init(unsigned long 
> *module_map,
>      if ( ucode_mod.mod_end || ucode_blob.size )
>          rc = early_microcode_update_cpu();
>  
> +    /*
> +     * MSR_ARCH_CAPS may have appeared after the microcode update. Reload
> +     * boot_cpu_data if so because they are needed in tsx_init().
> +     */
> +    early_cpu_init(false);

I think the comment would better talk of ARCH_CAPS as an example of what
may newly appear with a ucode update.

With at least the middle item taken care of (which I'd be happy to
do while committing)
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan