Re: [PATCH v2 5/5] x86: Add --force option to xen-ucode to override microcode version check

[Fouad Hilly <fouad.hilly@xxxxxxxxx>]

On Mon, Apr 22, 2024 at 6:57 PM Anthony PERARD <anthony.perard@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >
> > Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> 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