Re: [PATCH v3 2/5] x86/microcode/intel: Allow a late-load only if a min rev is specified

[Ashok Raj <ashok.raj@intel.com>]

On Wed, Aug 24, 2022 at 09:52:42PM +0200, Borislav Petkov wrote:
> On Tue, Aug 23, 2022 at 12:08:27AM +0000, Ashok Raj wrote:
> > Correct, but print_err parameter is 0 when called from scan_microcode() and 1
> > when called from generic_load_microcode().
> 
> Well, scan_microcode() gets called from save_microcode_in_initrd() which
> is fs_initcall and if we had to be really precise, print_err being 0
> there is wrong.
> 
> Because at fs_initcall time we can very well print error messages. But
> that print_err thing is an old relic so will have to get fixed some
> other day.

Well, the code hasn't changed since 2016, and possibly they migrated from
another file. 
> 
> > We do min_rev enforcement only when print_err is set.
> 
> That's wrong - you need to do min_rev enforcement only when you're
> loading microcode late. I.e., to paste from my previous mail:

True, if this hasn't been used for soo long, I was hoping to simply rename
the variable as late_load, and repurpose it.. 

As you mention we do have some good opportunity to perform some cleanups
here, and could address at that time.

If you feel compelled to turn the print on early boot, I could flip it and
send it along with my other changes? Let me know if you prefer that.


And I'll pursue what you said below. I still like the
microcode_sanity_check(), it sort of falls in that category. I can add
another parameter passing all the way from the request_fw... come through
all the other interceptors and land in the same spot.

The microcode_sanity_check() was nicely isolated Intel only function and
didn't need to perform surgery where it wasn't necessary :-).. 

Good bang for the buck :-)
> 
> "So you'd have to pass down the fact that you're doing late loading from
> request_microcode_fw().
> 
> Now, I'm staring at that ugly refresh_fw bool arg in that function and
> I *think* I did it 10 years ago because it shouldn't try to load from
> the fs when it is resuming because there might not be a fs yet... or
> something to that effect.
> 
> tglx might have a better idea how to check we're in the ->starting
> notifier..."
> 
> IOW, we're going to have to do something like
> 
> ->request_microcode_fw(, ... late_loading=true)

Sure, I'll check with how Thomas prefers it. 
> 
> and I wanted to reuse that refresh_fw arg instead of adding another
> one...
> 
> HTH.

YTH!

Cheers,
Ashok