[v10,07/16] microcode/amd: call svm_host_osvw_init() in common code
diff mbox series

Message ID 1568272949-1086-8-git-send-email-chao.gao@intel.com
State New
Headers show
Series
  • improve late microcode loading
Related show

Commit Message

Chao Gao Sept. 12, 2019, 7:22 a.m. UTC
Introduce a vendor hook, .end_update_percpu, 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>
---
Changes in v10:
 - rename end_update to end_update_percpu.
 - use #ifdef rather than #if and frame the implementation with

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    | 25 ++++++++++++-------------
 xen/include/asm-x86/microcode.h |  1 +
 3 files changed, 22 insertions(+), 14 deletions(-)

Comments

Jan Beulich Sept. 12, 2019, 12:34 p.m. UTC | #1
On 12.09.2019 09:22, Chao Gao wrote:
> Introduce a vendor hook, .end_update_percpu, 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>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit with one further remark:

> @@ -613,29 +609,32 @@ static int cpu_request_microcode(const void *buf, size_t bufsize)
>      return error;
>  }
>  
> +#ifdef CONFIG_HVM
>  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_percpu(void)
> +{
> +    svm_host_osvw_init();
> +}

I don't see the need for this wrapper, you could ...

> +#endif
> +
>  static const struct microcode_ops microcode_amd_ops = {
>      .cpu_request_microcode            = cpu_request_microcode,
>      .collect_cpu_info                 = collect_cpu_info,
>      .apply_microcode                  = apply_microcode,
> +#ifdef CONFIG_HVM
>      .start_update                     = start_update,
> +    .end_update_percpu                = end_update_percpu,

... use svm_host_osvw_init here directly.

Jan

Patch
diff mbox series

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 89a8d2b..5c82a2d 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -276,6 +276,9 @@  static long do_microcode_update(void *_info)
     if ( error )
         info->error = error;
 
+    if ( microcode_ops->end_update_percpu )
+        microcode_ops->end_update_percpu();
+
     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);
@@ -376,7 +379,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_percpu )
+            microcode_ops->end_update_percpu();
+
+        return rc;
     }
     else
         return -ENOMEM;
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 1d27c71..c96a3b3 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -600,10 +600,6 @@  static int cpu_request_microcode(const void *buf, size_t bufsize)
     free_patch(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
@@ -613,29 +609,32 @@  static int cpu_request_microcode(const void *buf, size_t bufsize)
     return error;
 }
 
+#ifdef CONFIG_HVM
 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_percpu(void)
+{
+    svm_host_osvw_init();
+}
+#endif
+
 static const struct microcode_ops microcode_amd_ops = {
     .cpu_request_microcode            = cpu_request_microcode,
     .collect_cpu_info                 = collect_cpu_info,
     .apply_microcode                  = apply_microcode,
+#ifdef CONFIG_HVM
     .start_update                     = start_update,
+    .end_update_percpu                = end_update_percpu,
+#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 f2a5ea4..b0eee0e 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_percpu)(void);
     void (*free_patch)(void *mc);
     bool (*match_cpu)(const struct microcode_patch *patch);
     enum microcode_match_result (*compare_patch)(