diff mbox series

[v3,5/5] Add --force option to xen-ucode to override microcode version check

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

Commit Message

Fouad Hilly April 30, 2024, 12:47 p.m. UTC
Introduce --force option to xen-ucode to force skipping microcode version
check, which allows the user to update x86 microcode even if both versions
are the same.
xc_microcode_update() refactored to accept flags and utilize xenpf_microcode_update2

Signed-off-by: Fouad Hilly <fouad.hilly@cloud.com>

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
[v3]
1- Message description updated.
2- xc_microcode_update() uses xenpf_microcode_update2 to pass flags.

[v2]
1- Changed data type from uint32_t to unsigned int.
2- Corrected line length.
3- Removed XENPF_UCODE_FLAG_FORCE_NOT_SET.
4- Corrected indentations.
5- Changed command line options to have the file name as first argument when applicable.
6- --force option doesn't require an argument anymore.
7- Used optint to access filename in argv.
 tools/include/xenctrl.h   |  3 ++-
 tools/libs/ctrl/xc_misc.c | 12 +++++++-----
 tools/misc/xen-ucode.c    | 16 ++++++++++++----
 3 files changed, 21 insertions(+), 10 deletions(-)

Comments

Jan Beulich May 6, 2024, 9:39 a.m. UTC | #1
On 30.04.2024 14:47, Fouad Hilly wrote:
> @@ -21,10 +23,11 @@ static const char   amd_id[] = "AuthenticAMD";
>  static void usage(const char *name)
>  {
>      printf("%s: Xen microcode updating tool\n"
> -           "Usage: %s [<microcode file> | Options]\n"
> +           "Usage: %s [microcode file] [options]\n"
>             "options:\n"
>             "  -h, --help            display this help and exit\n"
> -           "  -s, --show-cpu-info   show CPU information and exit\n",
> +           "  -s, --show-cpu-info   show CPU information and exit\n"
> +           "  -f, --force           force to skip micorocde version check\n",

Besides (nit) the typo, isn't "version check" too broad here? Elsewhere in the
series you mention Intel's "min_rev" addition. _That_ kind of version check
probably shouldn't be permitted to skip? In which case maybe better "certain
version checking" or some such?

Jan
Fouad Hilly May 9, 2024, 2:31 p.m. UTC | #2
On Mon, May 6, 2024 at 10:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 30.04.2024 14:47, Fouad Hilly wrote:
> > @@ -21,10 +23,11 @@ static const char   amd_id[] = "AuthenticAMD";
> >  static void usage(const char *name)
> >  {
> >      printf("%s: Xen microcode updating tool\n"
> > -           "Usage: %s [<microcode file> | Options]\n"
> > +           "Usage: %s [microcode file] [options]\n"
> >             "options:\n"
> >             "  -h, --help            display this help and exit\n"
> > -           "  -s, --show-cpu-info   show CPU information and exit\n",
> > +           "  -s, --show-cpu-info   show CPU information and exit\n"
> > +           "  -f, --force           force to skip micorocde version check\n",
>
> Besides (nit) the typo, isn't "version check" too broad here? Elsewhere in the
> series you mention Intel's "min_rev" addition. _That_ kind of version check
> probably shouldn't be permitted to skip? In which case maybe better "certain
> version checking" or some such?
I will remove "min_rev" mention from this patch series. The main
reason for this change is to allow developers to downgrade microcode.
>
> Jan
diff mbox series

Patch

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 499685594427..7fb409bc6dc4 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -1171,7 +1171,8 @@  typedef uint32_t xc_node_to_node_dist_t;
 int xc_physinfo(xc_interface *xch, xc_physinfo_t *info);
 int xc_cputopoinfo(xc_interface *xch, unsigned *max_cpus,
                    xc_cputopo_t *cputopo);
-int xc_microcode_update(xc_interface *xch, const void *buf, size_t len);
+int xc_microcode_update(xc_interface *xch, const void *buf,
+                        size_t len, unsigned int flags);
 int xc_get_cpu_version(xc_interface *xch, struct xenpf_pcpu_version *cpu_ver);
 int xc_get_ucode_revision(xc_interface *xch,
                           struct xenpf_ucode_revision *ucode_rev);
diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c
index 50282fd60dcc..6a60216bda03 100644
--- a/tools/libs/ctrl/xc_misc.c
+++ b/tools/libs/ctrl/xc_misc.c
@@ -203,11 +203,12 @@  int xc_physinfo(xc_interface *xch,
     return 0;
 }
 
-int xc_microcode_update(xc_interface *xch, const void *buf, size_t len)
+int xc_microcode_update(xc_interface *xch, const void *buf,
+                        size_t len, unsigned int flags)
 {
     int ret;
     struct xen_platform_op platform_op = {};
-    DECLARE_HYPERCALL_BUFFER(struct xenpf_microcode_update, uc);
+    DECLARE_HYPERCALL_BUFFER(struct xenpf_microcode_update2, uc);
 
     uc = xc_hypercall_buffer_alloc(xch, uc, len);
     if ( uc == NULL )
@@ -215,9 +216,10 @@  int xc_microcode_update(xc_interface *xch, const void *buf, size_t len)
 
     memcpy(uc, buf, len);
 
-    platform_op.cmd = XENPF_microcode_update;
-    platform_op.u.microcode.length = len;
-    set_xen_guest_handle(platform_op.u.microcode.data, uc);
+    platform_op.cmd = XENPF_microcode_update2;
+    platform_op.u.microcode2.length = len;
+    platform_op.u.microcode2.flags = flags;
+    set_xen_guest_handle(platform_op.u.microcode2.data, uc);
 
     ret = do_platform_op(xch, &platform_op);
 
diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c
index d95f967f021b..b3c9a2bfe5c8 100644
--- a/tools/misc/xen-ucode.c
+++ b/tools/misc/xen-ucode.c
@@ -13,6 +13,8 @@ 
 #include <xenctrl.h>
 #include <getopt.h>
 
+#include <xen/platform.h>
+
 static xc_interface *xch;
 
 static const char intel_id[] = "GenuineIntel";
@@ -21,10 +23,11 @@  static const char   amd_id[] = "AuthenticAMD";
 static void usage(const char *name)
 {
     printf("%s: Xen microcode updating tool\n"
-           "Usage: %s [<microcode file> | Options]\n"
+           "Usage: %s [microcode file] [options]\n"
            "options:\n"
            "  -h, --help            display this help and exit\n"
-           "  -s, --show-cpu-info   show CPU information and exit\n",
+           "  -s, --show-cpu-info   show CPU information and exit\n"
+           "  -f, --force           force to skip micorocde version check\n",
            name, name);
 }
 
@@ -89,10 +92,12 @@  int main(int argc, char *argv[])
     size_t len;
     struct stat st;
     int opt;
+    uint32_t ucode_flag = 0;
 
     static const struct option options[] = {
         {"help", no_argument, NULL, 'h'},
         {"show-cpu-info", no_argument, NULL, 's'},
+        {"force", no_argument, NULL, 'f'},
         {NULL, no_argument, NULL, 0}
     };
 
@@ -104,7 +109,7 @@  int main(int argc, char *argv[])
         exit(1);
     }
 
-    while ( (opt = getopt_long(argc, argv, "hs", options, NULL)) != -1 )
+    while ( (opt = getopt_long(argc, argv, "hsf", options, NULL)) != -1 )
     {
         switch (opt)
         {
@@ -114,6 +119,9 @@  int main(int argc, char *argv[])
         case 's':
             show_curr_cpu(stdout);
             exit(EXIT_SUCCESS);
+        case 'f':
+            ucode_flag = XENPF_UCODE_FLAG_FORCE_SET;
+            break;
         default:
             goto ext_err;
         }
@@ -146,7 +154,7 @@  int main(int argc, char *argv[])
         exit(1);
     }
 
-    ret = xc_microcode_update(xch, buf, len);
+    ret = xc_microcode_update(xch, buf, len, ucode_flag);
     if ( ret )
     {
         fprintf(stderr, "Failed to update microcode. (err: %s)\n",