On Thu, Jul 25, 2024 at 9:44 AM Jan Beulich <jbeul...@suse.com> wrote:
> On 25.07.2024 10:27, 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 or downgrade.
> > xc_microcode_update() refactored to accept flags and utilize
> xenpf_microcode_update2.
> >
> > Signed-off-by: Fouad Hilly <fouad.hi...@cloud.com>
> > Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
> > ---
> > [v6]
> > 1- Fix usage() output for -f option to be explicitly wrapped for 80
> character width
> > [v5]
> > 1- Update commit message.
> > 2- Re-phrase --force option description.
> > [v4]
> > 1- Add --force to xen-ucode options.
> > 2- Update xc_microcode_update() to accept and handle flags.
> > ---
> > tools/include/xenctrl.h | 3 ++-
> > tools/libs/ctrl/xc_misc.c | 12 +++++++-----
> > tools/misc/xen-ucode.c | 15 ++++++++++++---
> > 3 files changed, 21 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> > index 9ceca0cffc2f..2c4608c09ab0 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 2c9f337b86cb..688e540943b1 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";
> > @@ -79,7 +81,9 @@ static void usage(FILE *stream, const char *name)
> > "options:\n"
> > " -h, --help display this help\n"
> > " -s, --show-cpu-info show CPU information\n"
> > - "Usage: %s [microcode file | options]\n", name, name);
> > + " -f, --force skip certain checks; do not use
> unless\n"
> > + "you know exactly what you are doing\n"
>
> Did you look at the produced output? Imo you want to have
>
> " -f, --force skip certain checks; do not use
> unless\n"
> " you know exactly what you are doing\n"
>
> > + "Usage: %s [microcode file [-f,--force] | options]\n",
> name, name);
>
> At least
>
> "Usage: %s [microcode file [-f|--force] | options]\n", name,
> name);
>
> But: "options" now includes -f / --force, yet that on its own makes no
> sense.
> I think this needs further textual clarification to properly indicate what
> is
> valid to use and what is not.
>
Will be fixed in v7:
static void usage(FILE *stream, const char *name)
{
fprintf(stream,
"%s: Xen microcode updating tool\n"
"Usage: %s [options | microcode-file]\n"
"options:\n"
" -h, --help display this help\n"
" -s, --show-cpu-info show CPU information\n"
" -f, --force <microcode-file> skip certain checks; do not
\n"
" use unless you know exactly
\n"
" what you are doing\n",
name, name);
show_curr_cpu(stream);
}
>
> Jan
>
Thanks,
Fouad
