[4/5] x86/ucode: Drop ops->free_patch()

Commit Message

Andrew Cooper April 2, 2020, 10:19 a.m. UTC

With the newly cleaned up vendor logic, each struct microcode_patch is a
trivial object in memory with no dependent allocations.

This is unlikely to change moving forwards, and function pointers are
expensive in the days of retpoline.  Move the responsibility to xfree() back
to common code.  If the need does arise in the future, we can consider
reintroducing the hook.

Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: Jan Beulich <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Roger Pau Monné <[e-mail]>
---
 xen/arch/x86/cpu/microcode/amd.c     | 6 ------
 xen/arch/x86/cpu/microcode/core.c    | 4 ++--
 xen/arch/x86/cpu/microcode/intel.c   | 6 ------
 xen/arch/x86/cpu/microcode/private.h | 5 +----
 4 files changed, 3 insertions(+), 18 deletions(-)

Comments

Jan Beulich April 3, 2020, 1:44 p.m. UTC | #1

On 02.04.2020 12:19, Andrew Cooper wrote:
> With the newly cleaned up vendor logic, each struct microcode_patch is a
> trivial object in memory with no dependent allocations.
> 
> This is unlikely to change moving forwards, and function pointers are
> expensive in the days of retpoline.  Move the responsibility to xfree() back
> to common code.  If the need does arise in the future, we can consider
> reintroducing the hook.
> 
> Signed-off-by: Andrew Cooper <[e-mail]>

Reviewed-by: Jan Beulich <[e-mail]>

Yet with the given argumentation, ...

> --- a/xen/arch/x86/cpu/microcode/core.c
> +++ b/xen/arch/x86/cpu/microcode/core.c
> @@ -243,9 +243,9 @@ static struct microcode_patch *parse_blob(const char *buf, size_t len)
>      return NULL;
>  }
>  
> -static void microcode_free_patch(struct microcode_patch *microcode_patch)
> +static void microcode_free_patch(struct microcode_patch *patch)
>  {
> -    microcode_ops->free_patch(microcode_patch);
> +    xfree(patch);
>  }

... drop this wrapper as well? (R-b would also cover this)

Jan

Patch

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 0ca0e9a038..e23bdef6f2 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -188,11 +188,6 @@  static enum microcode_match_result microcode_fits(
     return NEW_UCODE;
 }
 
-static void free_patch(struct microcode_patch *patch)
-{
-    xfree(patch);
-}
-
 static enum microcode_match_result compare_header(
     const struct microcode_patch *new, const struct microcode_patch *old)
 {
@@ -418,6 +413,5 @@  const struct microcode_ops amd_ucode_ops = {
     .start_update                     = start_update,
     .end_update_percpu                = svm_host_osvw_init,
 #endif
-    .free_patch                       = free_patch,
     .compare_patch                    = compare_patch,
 };
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index b3e5913d49..53e447ea9a 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -243,9 +243,9 @@  static struct microcode_patch *parse_blob(const char *buf, size_t len)
     return NULL;
 }
 
-static void microcode_free_patch(struct microcode_patch *microcode_patch)
+static void microcode_free_patch(struct microcode_patch *patch)
 {
-    microcode_ops->free_patch(microcode_patch);
+    xfree(patch);
 }
 
 /* Return true if cache gets updated. Otherwise, return false */
diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c
index 9cb077b583..29745ed55f 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -245,11 +245,6 @@  static enum microcode_match_result microcode_update_match(
     return mc->rev > cpu_sig->rev ? NEW_UCODE : OLD_UCODE;
 }
 
-static void free_patch(struct microcode_patch *patch)
-{
-    xfree(patch);
-}
-
 static enum microcode_match_result compare_patch(
     const struct microcode_patch *new, const struct microcode_patch *old)
 {
@@ -356,6 +351,5 @@  const struct microcode_ops intel_ucode_ops = {
     .cpu_request_microcode            = cpu_request_microcode,
     .collect_cpu_info                 = collect_cpu_info,
     .apply_microcode                  = apply_microcode,
-    .free_patch                       = free_patch,
     .compare_patch                    = compare_patch,
 };
diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h
index d31bcf14b1..878f8d805f 100644
--- a/xen/arch/x86/cpu/microcode/private.h
+++ b/xen/arch/x86/cpu/microcode/private.h
@@ -25,7 +25,7 @@  struct microcode_ops {
      *
      * If one is found, allocate and return a struct microcode_patch
      * encapsulating the appropriate microcode patch.  Does not alias the
-     * original buffer.
+     * original buffer.  Must be suitable to be freed with a single xfree().
      *
      * If one is not found, (nothing matches the current CPU), return NULL.
      * Also may return ERR_PTR(-err), e.g. bad container, out of memory.
@@ -56,9 +56,6 @@  struct microcode_ops {
      */
     void (*end_update_percpu)(void);
 
-    /* Free a patch previously allocated by cpu_request_microcode(). */
-    void (*free_patch)(struct microcode_patch *patch);
-
     /*
      * Given two patches, are they both applicable to the current CPU, and is
      * new a higher revision than old?