| Message ID | 20240416091546.11622-6-fouad.hilly@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/xen-ucode: Introduce --force option | expand |
On Tue, Apr 16, 2024 at 10:15:46AM +0100, Fouad Hilly wrote: > 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. > > [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. > > Signed-off-by: Fouad Hilly <fouad.hilly@cloud.com> > --- > > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> You might want to move that tag before the '---' separation otherwise it wont be part of the commit message. `git am` discard every thing after the '---' line. Also add the tag before your SOB. > --- > tools/include/xenctrl.h | 3 ++- > tools/libs/ctrl/xc_misc.c | 13 +++++++++++-- > tools/misc/xen-ucode.c | 18 +++++++++++++----- > 3 files changed, 26 insertions(+), 8 deletions(-) > > diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c > index e3c1943e3633..4178fd2221ea 100644 > --- a/tools/misc/xen-ucode.c > +++ b/tools/misc/xen-ucode.c > @@ -24,7 +26,8 @@ static void usage(const char *name) > "Usage: %s [microcode file] [options]\n" Now, that usage line is wrong. The options needs to go before the file. That could be fix on the previous patch. With that usage line, we would want to run `./xen-ucode ucode.bin --force`, but I don't think that would work. > "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); > } > Thanks,
On Mon, Apr 22, 2024 at 6:57 PM Anthony PERARD <anthony.perard@cloud.com> wrote: > > On Tue, Apr 16, 2024 at 10:15:46AM +0100, Fouad Hilly wrote: > > 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. > > > > [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. > > > > Signed-off-by: Fouad Hilly <fouad.hilly@cloud.com> > > --- > > > > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > > You might want to move that tag before the '---' separation otherwise it > wont be part of the commit message. `git am` discard every thing after > the '---' line. Also add the tag before your SOB. Noted. > > > --- > > tools/include/xenctrl.h | 3 ++- > > tools/libs/ctrl/xc_misc.c | 13 +++++++++++-- > > tools/misc/xen-ucode.c | 18 +++++++++++++----- > > 3 files changed, 26 insertions(+), 8 deletions(-) > > > > diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c > > index e3c1943e3633..4178fd2221ea 100644 > > --- a/tools/misc/xen-ucode.c > > +++ b/tools/misc/xen-ucode.c > > @@ -24,7 +26,8 @@ static void usage(const char *name) > > "Usage: %s [microcode file] [options]\n" > > Now, that usage line is wrong. The options needs to go before the file. I am not sure what you mean "wrong"? from parsing perspective, both scenarios can be successfully executed: xen-ucode firmware-file --force xen-ucode --force firmware-file it becomes wrong if there is an argument expects the file as an input. > That could be fix on the previous patch. With that usage line, we would > want to run `./xen-ucode ucode.bin --force`, but I don't think that > would work. > > > "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); > > } > > > > Thanks, > > -- > Anthony PERARD Thanks, Fouad
On Tue, Apr 23, 2024 at 04:26:53PM +0100, Fouad Hilly wrote: > On Mon, Apr 22, 2024 at 6:57 PM Anthony PERARD <anthony.perard@cloud.com> wrote: > > On Tue, Apr 16, 2024 at 10:15:46AM +0100, Fouad Hilly wrote: > > > diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c > > > index e3c1943e3633..4178fd2221ea 100644 > > > --- a/tools/misc/xen-ucode.c > > > +++ b/tools/misc/xen-ucode.c > > > @@ -24,7 +26,8 @@ static void usage(const char *name) > > > "Usage: %s [microcode file] [options]\n" > > > > Now, that usage line is wrong. The options needs to go before the file. > I am not sure what you mean "wrong"? from parsing perspective, both > scenarios can be successfully executed: > xen-ucode firmware-file --force > xen-ucode --force firmware-file > it becomes wrong if there is an argument expects the file as an input. Oh, sorry, I didn't know that getopt() would look for options on all arguments, even when separated by nonoptions. I'm used to CLI programs that takes options first, then files, even if that might work with a different order. Cheers,
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index 2ef8b4e05422..49d2f19c0d77 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 5ecdfa2c7934..fbc17cefa82e 100644 --- a/tools/libs/ctrl/xc_misc.c +++ b/tools/libs/ctrl/xc_misc.c @@ -203,7 +203,8 @@ 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 = {}; @@ -215,7 +216,15 @@ int xc_microcode_update(xc_interface *xch, const void *buf, size_t len) memcpy(uc, buf, len); - platform_op.cmd = XENPF_microcode_update; + if ( flags > 0 ) + { + platform_op.cmd = XENPF_microcode_update2; + platform_op.u.microcode.flags = flags; + } + else + { + platform_op.cmd = XENPF_microcode_update; + } platform_op.u.microcode.length = len; set_xen_guest_handle(platform_op.u.microcode.data, uc); diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c index e3c1943e3633..4178fd2221ea 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"; @@ -24,7 +26,8 @@ static void usage(const char *name) "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,10 +109,10 @@ int main(int argc, char *argv[]) exit(1); } - if ( argc != 2 ) + if ( argc < 2 || argc > 3) goto ext_err; - while ( (opt = getopt_long(argc, argv, "hs", options, NULL)) != -1 ) + while ( (opt = getopt_long(argc, argv, "hsf", options, NULL)) != -1 ) { switch (opt) { @@ -117,12 +122,15 @@ 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; } } - filename = argv[1]; + filename = argv[optind]; fd = open(filename, O_RDONLY); if ( fd < 0 ) { @@ -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",
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. [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. Signed-off-by: Fouad Hilly <fouad.hilly@cloud.com> --- Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> --- tools/include/xenctrl.h | 3 ++- tools/libs/ctrl/xc_misc.c | 13 +++++++++++-- tools/misc/xen-ucode.c | 18 +++++++++++++----- 3 files changed, 26 insertions(+), 8 deletions(-)