Re: [PATCH] firmware_loader: Use init_utsname()->release

From: Luis Chamberlain
Date: Thu Feb 22 2024 - 10:53:38 EST


On Thu, Feb 22, 2024 at 02:58:19PM +0000, John Garry wrote:
> Instead of using UTS_RELEASE, use init_utsname()->release, which means
> that we don't need to rebuild the code just for the git head commit
> changing.
>
> Since UTS_RELEASE is used for fw_path[] and this points to const data,
> append init_utsname()->release dynamically to an intermediate string.
>
> The check for appending uts release is if the string in fw_path[] ends
> in '/'. Since fw_path[] may include a path from the kernel command line
> in fw_path_para, and it would be valid for this string to end in '/',
> check for fw_path_para when appending uts release.
>
> Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
> ---
> Note: As mentioned by Masahiro in [0], when CONFIG_MODVERSIONS=y it
> could be possible for a driver to be built as a module with a different
> kernel baseline and so use a different UTS_RELEASE from the baseline. So
> now using init_utsname()->release could lead to a change in behaviour
> in this driver. However, considering the nature of this driver and how it
> would not make sense to build as module against a different tree, this

would not make sense to build it as an external module against against a
different tree, so this change should not have any effect to users.

> change should be ok.
>
> [0] https://lore.kernel.org/lkml/CAK7LNAQ_r5yUjNpOppLkDBQ12sDxBYQTvRZGn1ng8D1POfZr_A@xxxxxxxxxxxxxx/

Thanks for doing this, could you send a v2 with the "Note:" removed but
instead include that blurb as part of the commit log as it is important
information to retain.

Could you also test the selftests to ensure nothing is broken ?

/tools/testing/selftests/firmware/fw_run_tests.sh

Reviewed-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>

Luis