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

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On Fri, Jun 23, 2023 at 09:33:56AM +0200, Jan Beulich wrote:
> On 22.06.2023 19:42, Alejandro Vallejo wrote:
> > --- a/xen/arch/x86/cpu/microcode/core.c
> > +++ b/xen/arch/x86/cpu/microcode/core.c
> > @@ -885,5 +885,18 @@ 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 relevant fields in boot_cpu_data if so because they are
> > +     * needed in tsx_init().
> > +     */
> > +    if ( boot_cpu_data.cpuid_level >= 7 )
> > +        boot_cpu_data.x86_capability[FEATURESET_7d0]
> > +            = cpuid_count_edx(7, 0);
> > +    if ( cpu_has_arch_caps )
> > +        rdmsr(MSR_ARCH_CAPABILITIES,
> > +              boot_cpu_data.x86_capability[FEATURESET_m10Al],
> > +              boot_cpu_data.x86_capability[FEATURESET_m10Ah]);
> > +
> >      return rc;
> >  }
> 
> Did you consider simply calling early_cpu_init() a 2nd time, and then
> perhaps from setup.c and only if ucode load didn't report an error?
> There's a printk() in there which will want avoiding on the 2nd pass,
> but otherwise this would look more future-proof to me.
> 
> Jan
I had, but avoiding the internal printk() was annoying. I've simply created
a boolean "verbosity" flag on the new version for early_cpu_init() and
called it at the end of early_microcode_init().

Alejandro