| Message ID | 1566177928-19114-9-git-send-email-chao.gao@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | improve late microcode loading | expand |
On Mon, Aug 19, 2019 at 09:25:21AM +0800, Chao Gao wrote: > Introduce a vendor hook, .end_update, for svm_host_osvw_init(). > The hook function is called on each cpu after loading an update. > It is a preparation for spliting out apply_microcode() from > cpu_request_microcode(). > > Note that svm_host_osvm_init() should be called regardless of the > result of loading an update. > > Signed-off-by: Chao Gao <chao.gao@intel.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks.
On 19.08.2019 03:25, Chao Gao wrote: > --- a/xen/arch/x86/microcode_amd.c > +++ b/xen/arch/x86/microcode_amd.c > @@ -594,10 +594,6 @@ static int cpu_request_microcode(const void *buf, size_t bufsize) > xfree(mc_amd); > > out: > -#if CONFIG_HVM > - svm_host_osvw_init(); > -#endif > - > /* > * In some cases we may return an error even if processor's microcode has > * been updated. For example, the first patch in a container file is loaded > @@ -609,27 +605,28 @@ static int cpu_request_microcode(const void *buf, size_t bufsize) > > static int start_update(void) > { > -#if CONFIG_HVM > /* > - * We assume here that svm_host_osvw_init() will be called on each cpu (from > - * cpu_request_microcode()). > - * > - * Note that if collect_cpu_info() returns an error then > - * cpu_request_microcode() will not invoked thus leaving OSVW bits not > - * updated. Currently though collect_cpu_info() will not fail on processors > - * supporting OSVW so we will not deal with this possibility. > + * svm_host_osvw_init() will be called on each cpu by calling '.end_update' > + * in common code. > */ > svm_host_osvw_reset(); > -#endif > > return 0; > } > > +static void end_update(void) > +{ > + svm_host_osvw_init(); > +} > + > static const struct microcode_ops microcode_amd_ops = { > .cpu_request_microcode = cpu_request_microcode, > .collect_cpu_info = collect_cpu_info, > .apply_microcode = apply_microcode, > +#if CONFIG_HVM I realize it was wrong in the old code as well, but please use #ifdef instead of #if. > .start_update = start_update, > + .end_update = end_update, > +#endif With this there'll be two warnings about unused functions when !HVM - you need to frame the implementations with an #ifdef as well. Jan
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c index 3b87c72..c9401a7 100644 --- a/xen/arch/x86/microcode.c +++ b/xen/arch/x86/microcode.c @@ -277,6 +277,9 @@ static long do_microcode_update(void *_info) if ( error ) info->error = error; + if ( microcode_ops->end_update ) + microcode_ops->end_update(); + info->cpu = cpumask_next(info->cpu, &cpu_online_map); if ( info->cpu < nr_cpu_ids ) return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info); @@ -377,7 +380,12 @@ int __init early_microcode_update_cpu(bool start_update) if ( rc ) return rc; - return microcode_update_cpu(data, len); + rc = microcode_update_cpu(data, len); + + if ( microcode_ops->end_update ) + microcode_ops->end_update(); + + return rc; } else return -ENOMEM; diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c index dd3821c..b85fb04 100644 --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -594,10 +594,6 @@ static int cpu_request_microcode(const void *buf, size_t bufsize) xfree(mc_amd); out: -#if CONFIG_HVM - svm_host_osvw_init(); -#endif - /* * In some cases we may return an error even if processor's microcode has * been updated. For example, the first patch in a container file is loaded @@ -609,27 +605,28 @@ static int cpu_request_microcode(const void *buf, size_t bufsize) static int start_update(void) { -#if CONFIG_HVM /* - * We assume here that svm_host_osvw_init() will be called on each cpu (from - * cpu_request_microcode()). - * - * Note that if collect_cpu_info() returns an error then - * cpu_request_microcode() will not invoked thus leaving OSVW bits not - * updated. Currently though collect_cpu_info() will not fail on processors - * supporting OSVW so we will not deal with this possibility. + * svm_host_osvw_init() will be called on each cpu by calling '.end_update' + * in common code. */ svm_host_osvw_reset(); -#endif return 0; } +static void end_update(void) +{ + svm_host_osvw_init(); +} + static const struct microcode_ops microcode_amd_ops = { .cpu_request_microcode = cpu_request_microcode, .collect_cpu_info = collect_cpu_info, .apply_microcode = apply_microcode, +#if CONFIG_HVM .start_update = start_update, + .end_update = end_update, +#endif .free_patch = free_patch, .compare_patch = compare_patch, .match_cpu = match_cpu, diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h index 35223eb..c8d2c4f 100644 --- a/xen/include/asm-x86/microcode.h +++ b/xen/include/asm-x86/microcode.h @@ -24,6 +24,7 @@ struct microcode_ops { int (*collect_cpu_info)(struct cpu_signature *csig); int (*apply_microcode)(void); int (*start_update)(void); + void (*end_update)(void); void (*free_patch)(void *mc); bool (*match_cpu)(const struct microcode_patch *patch); enum microcode_match_result (*compare_patch)(
Introduce a vendor hook, .end_update, for svm_host_osvw_init(). The hook function is called on each cpu after loading an update. It is a preparation for spliting out apply_microcode() from cpu_request_microcode(). Note that svm_host_osvm_init() should be called regardless of the result of loading an update. Signed-off-by: Chao Gao <chao.gao@intel.com> --- Changes in v9: - call .end_update in early loading path - on AMD side, initialize .{start,end}_update only if "CONFIG_HVM" is true. --- xen/arch/x86/microcode.c | 10 +++++++++- xen/arch/x86/microcode_amd.c | 23 ++++++++++------------- xen/include/asm-x86/microcode.h | 1 + 3 files changed, 20 insertions(+), 14 deletions(-)