Re: [PATCH v5 4/4] x86/microcode: Disable microcode update handler if DIS_MCU_UPDATE is set

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On Wed, Jul 05, 2023 at 12:51:47PM +0200, Jan Beulich wrote:
> > --- a/xen/arch/x86/cpu/microcode/intel.c
> > +++ b/xen/arch/x86/cpu/microcode/intel.c
> > @@ -385,6 +385,19 @@ static struct microcode_patch *cf_check 
> > cpu_request_microcode(
> >      return patch;
> >  }
> >  
> > +bool __init intel_can_load_microcode(void)
> > +{
> > +    uint64_t mcu_ctrl;
> > +
> > +    if ( !cpu_has_mcu_ctrl )
> > +        return true;
> > +
> > +    rdmsrl(MSR_MCU_CONTROL, mcu_ctrl);
> 
> While one would hope that feature bit and MSR access working come in
> matched pairs, I still wonder whether - just to be on the safe side -
> the caller wouldn't better avoid calling here when rev == ~0 (and
> hence we won't try to load ucode anyway). I would envision can_load's
> initializer to become this_cpu(cpu_sig).rev != ~0, with other logic
> adjusted as necessary in early_microcode_init().
> 
> Jan
We only know about the ucode revision after the collect_cpu_info() call,
and we can only make that call after the vendor-specific section that sets
the function pointers up (and calls intel_can_load_microcode()).

One could imagine turning can_load into a function pointer so that its
execution is deferred until after the revision check (and skipped
altogether if `rev==~0`).

Alejandro