diff mbox series

[v2,2/5] x86: Refactor microcode_update() hypercall with flags

Message ID 20240416091546.11622-3-fouad.hilly@cloud.com (mailing list archive)
State Superseded
Headers show
Series x86/xen-ucode: Introduce --force option | expand

Commit Message

Fouad Hilly April 16, 2024, 9:15 a.m. UTC 
Refactor microcode_update() hypercall by adding flags field.
Introduce XENPF_microcode_update2 hypercall to handle flags field.
struct xenpf_microcode_update updated to have uint32_t flags at
the end of the sturcture.

[v2]
1- Update message description to highlight interface change.
2- Removed extra empty lines.
3- removed unnecessary define.
4- Corrected long lines.
5- Removed ternary operator.
6- Introduced static ucode_update_flags, which will be used later to determine local ucode_force_flag.

Signed-off-by: Fouad Hilly <fouad.hilly@cloud.com>
---
 xen/arch/x86/cpu/microcode/core.c    | 14 +++++++++++---
 xen/arch/x86/include/asm/microcode.h |  3 ++-
 xen/arch/x86/platform_hypercall.c    | 12 +++++++++++-
 xen/include/public/platform.h        |  6 ++++++
 4 files changed, 30 insertions(+), 5 deletions(-)

Comments

Jan Beulich April 22, 2024, 2:18 p.m. UTC | #1 
On 16.04.2024 11:15, Fouad Hilly wrote:
> Refactor microcode_update() hypercall by adding flags field.
> Introduce XENPF_microcode_update2 hypercall to handle flags field.
> struct xenpf_microcode_update updated to have uint32_t flags at
> the end of the sturcture.
> 
> [v2]
> 1- Update message description to highlight interface change.
> 2- Removed extra empty lines.
> 3- removed unnecessary define.
> 4- Corrected long lines.
> 5- Removed ternary operator.
> 6- Introduced static ucode_update_flags, which will be used later to determine local ucode_force_flag.

Non-style comments on v1 have remained un-addressed. Specifically, to
give an example, while 1 says you now highlight the interface change,
the request was to explain why changing an existing structure is okay
(hint: it likely isn't, as the structure size changes for compat [aka
32-bit] callers).

I'm not going to give the same comments again; I'll rather expect you to
respond to them by either adjustments to the patch (or its description),
or by verbal replies.

Jan
Fouad Hilly April 23, 2024, 2:43 p.m. UTC | #2 
On Mon, Apr 22, 2024 at 3:18 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 16.04.2024 11:15, Fouad Hilly wrote:
> > Refactor microcode_update() hypercall by adding flags field.
> > Introduce XENPF_microcode_update2 hypercall to handle flags field.
> > struct xenpf_microcode_update updated to have uint32_t flags at
> > the end of the sturcture.
> >
> > [v2]
> > 1- Update message description to highlight interface change.
> > 2- Removed extra empty lines.
> > 3- removed unnecessary define.
> > 4- Corrected long lines.
> > 5- Removed ternary operator.
> > 6- Introduced static ucode_update_flags, which will be used later to determine local ucode_force_flag.
>
> Non-style comments on v1 have remained un-addressed. Specifically, to
> give an example, while 1 says you now highlight the interface change,
> the request was to explain why changing an existing structure is okay
> (hint: it likely isn't, as the structure size changes for compat [aka
> 32-bit] callers).

I see your point now, I will keep the stable ABI as is.
>
> I'm not going to give the same comments again; I'll rather expect you to
> respond to them by either adjustments to the patch (or its description),
> or by verbal replies.
I will respond to your V1 comment on the previous email to keep things inlined
>
> Jan

Thanks,

Fouad
diff mbox series

Patch

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 1c9f66ea8a0f..99b651d8c3a1 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -40,6 +40,8 @@ 
 #include <asm/processor.h>
 #include <asm/setup.h>
 
+#include <public/platform.h>
+
 #include "private.h"
 
 /*
@@ -100,6 +102,8 @@  static bool ucode_in_nmi = true;
 
 bool __read_mostly opt_ucode_allow_same;
 
+static unsigned int ucode_update_flags = 0;
+
 /* Protected by microcode_mutex */
 static struct microcode_patch *microcode_cache;
 
@@ -580,6 +584,7 @@  static long cf_check microcode_update_helper(void *data)
     struct ucode_buf *buffer = data;
     unsigned int cpu, updated;
     struct microcode_patch *patch;
+    bool ucode_force_flag = ucode_update_flags == XENPF_UCODE_FLAG_FORCE_SET;
 
     /* cpu_online_map must not change during update */
     if ( !get_cpu_maps() )
@@ -633,12 +638,12 @@  static long cf_check microcode_update_helper(void *data)
                                   microcode_cache);
 
         if ( result != NEW_UCODE &&
-             !(opt_ucode_allow_same && result == SAME_UCODE) )
+             !((opt_ucode_allow_same || ucode_force_flag) && result == SAME_UCODE) )
         {
             spin_unlock(&microcode_mutex);
             printk(XENLOG_WARNING
                    "microcode: couldn't find any newer%s revision in the provided blob!\n",
-                   opt_ucode_allow_same ? " (or the same)" : "");
+                   (opt_ucode_allow_same || ucode_force_flag) ? " (or the same)" : "");
             microcode_free_patch(patch);
             ret = -ENOENT;
 
@@ -708,7 +713,8 @@  static long cf_check microcode_update_helper(void *data)
     return ret;
 }
 
-int microcode_update(XEN_GUEST_HANDLE(const_void) buf, unsigned long len)
+int microcode_update(XEN_GUEST_HANDLE(const_void) buf,
+                     unsigned long len, unsigned int flags)
 {
     int ret;
     struct ucode_buf *buffer;
@@ -731,6 +737,8 @@  int microcode_update(XEN_GUEST_HANDLE(const_void) buf, unsigned long len)
     }
     buffer->len = len;
 
+    ucode_update_flags = flags;
+
     /*
      * Always queue microcode_update_helper() on CPU0.  Most of the logic
      * won't care, but the update of the Raw CPU policy wants to (re)run on
diff --git a/xen/arch/x86/include/asm/microcode.h b/xen/arch/x86/include/asm/microcode.h
index 8f59b20b0289..57c08205d475 100644
--- a/xen/arch/x86/include/asm/microcode.h
+++ b/xen/arch/x86/include/asm/microcode.h
@@ -22,7 +22,8 @@  struct cpu_signature {
 DECLARE_PER_CPU(struct cpu_signature, cpu_sig);
 
 void microcode_set_module(unsigned int idx);
-int microcode_update(XEN_GUEST_HANDLE(const_void) buf, unsigned long len);
+int microcode_update(XEN_GUEST_HANDLE(const_void) buf,
+                     unsigned long len, unsigned int flags);
 int early_microcode_init(unsigned long *module_map,
                          const struct multiboot_info *mbi);
 int microcode_init_cache(unsigned long *module_map,
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 95467b88ab64..3b29ede8b316 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -311,7 +311,17 @@  ret_t do_platform_op(
 
         guest_from_compat_handle(data, op->u.microcode.data);
 
-        ret = microcode_update(data, op->u.microcode.length);
+        ret = microcode_update(data, op->u.microcode.length, 0);
+        break;
+    }
+
+    case XENPF_microcode_update2:
+    {
+        XEN_GUEST_HANDLE(const_void) data;
+
+        guest_from_compat_handle(data, op->u.microcode.data);
+
+        ret = microcode_update(data, op->u.microcode.length, op->u.microcode.flags);
         break;
     }
 
diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
index 15777b541690..cc19b2956b46 100644
--- a/xen/include/public/platform.h
+++ b/xen/include/public/platform.h
@@ -99,6 +99,9 @@  struct xenpf_microcode_update {
     /* IN variables. */
     XEN_GUEST_HANDLE(const_void) data;/* Pointer to microcode data */
     uint32_t length;                  /* Length of microcode data. */
+    uint32_t flags;                   /* Flags to be passed with ucode. */
+/* Force to skip microcode version check when set */
+#define XENPF_UCODE_FLAG_FORCE_SET     1
 };
 typedef struct xenpf_microcode_update xenpf_microcode_update_t;
 DEFINE_XEN_GUEST_HANDLE(xenpf_microcode_update_t);
@@ -624,6 +627,9 @@  struct xenpf_ucode_revision {
 typedef struct xenpf_ucode_revision xenpf_ucode_revision_t;
 DEFINE_XEN_GUEST_HANDLE(xenpf_ucode_revision_t);
 
+/* Hypercall to microcode_update with flags */
+#define XENPF_microcode_update2    66
+
 /*
  * ` enum neg_errnoval
  * ` HYPERVISOR_platform_op(const struct xen_platform_op*);