| Message ID | 1564654971-31328-3-git-send-email-chao.gao@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | improve late microcode loading | expand |
On 01/08/2019 11:22, Chao Gao wrote: > From: Sergey Dyasli <sergey.dyasli@citrix.com> > > Currently cpu_sig struct is not updated during boot if no microcode blob > is specified by "ucode=[<interger>| scan]". > > It will result in cpu_sig.rev being 0 which affects APIC's > check_deadline_errata() and retpoline_safe() functions. > > Fix this by getting ucode revision early during boot and SMP bring up. > While at it, protect early_microcode_update_cpu() for cases when > microcode_ops is NULL. > > Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com> > Signed-off-by: Chao Gao <chao.gao@intel.com> > --- > Changes in v8: > - refine description. > - Jan asked if we could drop the call of collect_cpu_info() from > microcode_update_cpu(). In theory, yes, but should be placed later > in the series. Because there is an error path (__microcode_fini_cpu()) > in which cpu_sig.rev is cleared, it is hard to make things right > in all cases without removing the error path (which is done by following > patches). Considering it is a good fix, put it here so that it can > be merged without following patches. Ok. Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, and this does really want backporting.
On 01.08.2019 12:22, Chao Gao wrote: > --- a/xen/arch/x86/microcode.c > +++ b/xen/arch/x86/microcode.c > @@ -383,10 +383,15 @@ static struct notifier_block microcode_percpu_nfb = { > > int __init early_microcode_update_cpu(bool start_update) > { > + unsigned int cpu = smp_processor_id(); > + struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); > int rc = 0; > void *data = NULL; > size_t len; > > + if ( !microcode_ops ) > + return -ENOSYS; For posterity (realizing the patch has gone in already) - I'm not convinced this should be an error condition, but the error produced here is being ignored anyway afaics (i.e. the function could as well return void). In no case is this an appropriate use of ENOSYS: This isn't even on a hypercall path. Jan
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index 4163f50..421d57e 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -383,10 +383,15 @@ static struct notifier_block microcode_percpu_nfb = { int __init early_microcode_update_cpu(bool start_update) { + unsigned int cpu = smp_processor_id(); + struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); int rc = 0; void *data = NULL; size_t len; + if ( !microcode_ops ) + return -ENOSYS; + if ( ucode_blob.size ) { len = ucode_blob.size; @@ -397,6 +402,9 @@ int __init early_microcode_update_cpu(bool start_update) len = ucode_mod.mod_end; data = bootstrap_map(&ucode_mod); } + + microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig); + if ( data ) { if ( start_update && microcode_ops->start_update ) @@ -413,6 +421,8 @@ int __init early_microcode_update_cpu(bool start_update) int __init early_microcode_init(void) { + unsigned int cpu = smp_processor_id(); + struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu); int rc; rc = microcode_init_intel(); @@ -425,6 +435,8 @@ int __init early_microcode_init(void) if ( microcode_ops ) { + microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig); + if ( ucode_mod.mod_end || ucode_blob.size ) rc = early_microcode_update_cpu(true);