lkml.org 
[lkml]   [2022]   [Aug]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86/microcode/intel: Allow late loading only if a min rev is specified
On Mon, Aug 29, 2022 at 10:31:22PM +0200, Borislav Petkov wrote:
> On Mon, Aug 29, 2022 at 06:04:36PM +0000, Ashok Raj wrote:
> > @@ -886,7 +905,7 @@ static bool is_blacklisted(unsigned int cpu)
> > }
> >
> > static enum ucode_state request_microcode_fw(int cpu, struct device *device,
> > - bool refresh_fw)
> > + bool late_loading)
> > {
>
> Until the refresh_fw's function hasn't been clarified:
>
> https://lore.kernel.org/all/YwaBim3Xt3Il3KXm@zn.tnic/
>

I don't know exactly what you mean.

But I suppose, you mean what refresh_hw is supposed to mean from the existing code?

refresh_hw seems to imply when to update the copy of the microcode from
the filesystem. Also seems to imply late loading.

its used in the following places.

1. During reload_store() where this is exclicitly due to echo 1 > reload

tmp_ret = microcode_ops->request_microcode_fw(bsp, &microcode_pdev->dev, true);

Here passing true makes sense since you are going to do a full
refresh on all CPUs via late loading.


2. microcode_update_cpu() -> microcode_init_cpu()->request_microcode_fw(false)

Early loading from resume. So we would use the microcode cache to load
from.

3. mc_device_add() -> microcode_init_cpu(true)->request_microcode_fw(true)

This seems like normal CPU hot-add, I'm not sure if refresh_fw=true is
valid. A new CPU should also use from the cache, but not a full reload
from filesystem. This could end up with new cpu with an updated ucode
and older with something that was loaded earlier. Sort of what was fixed
in:

commit 7189b3c11903667808029ec9766a6e96de5012a5 (tag: x86_microcode_for_v5.13)


intel.c doesn't seem to use this parameter at all today. But judging from

amd.c: request_microcode_amd()

/* reload ucode container only on the boot cpu */
if (!refresh_fw || !bsp)
return UCODE_OK;

Seems like it does the right job, since you would refresh only if
refresh_fw=true and its the bsp. Hence it seems immute to the
mc_device_add() bug.

1. Fix mc_device_add() to set refresh_fw() to false.
2. We can change the parameter to late_loading as you had alluded in
previous post.
3. Use that to distinguish if we need to enforce the
microcode_sanity_check() to validate min_rev.

Let me know if this is what you meant.

Cheers,
Ashok

\
 
 \ /
  Last update: 2022-08-30 00:43    [W:0.052 / U:0.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site