Re: [PATCH v6 3/4] x86/ucode: Introduce --force option to xen-ucode

[Fouad Hilly <fouad.hilly@xxxxxxxxx>]

On Mon, Aug 19, 2024 at 10:47 AM Anthony PERARD <anthony.perard@xxxxxxxxxx> wrote:

On Mon, Aug 19, 2024 at 09:56:57AM +0100, Fouad Hilly wrote:
> On Thu, Jul 25, 2024 at 9:44 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> > On 25.07.2024 10:27, Fouad Hilly wrote:
> > > @@ -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"

If I recall correctly, "--force" doesn't take any argument, so this
usage is misleading. One could be tempted to execute `./xen-ucode
-fmicrocode` or event `./xen-ucode --force -microcode` and expect it to
work with files "microcode" or "-microcode" but instead I think getopt()
is just going to return an error.

Instead of writing "--force <microcode-file>", could you change the help
text, with something like "skip certain checks when applying microcode"?

Sure, can be done 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              skip certain checks when applying\n"

            "                           microcode; do not use unless you know\n"

            "                           exactly what you are doing\n",

            name, name);

    show_curr_cpu(stream);

} 

>             "                                   use unless you know exactly
> \n"
>             "                                   what you are doing\n",
>             name, name);
>     show_curr_cpu(stream);

Cheers,

--

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Thanks,

Fouad