linux-kernel - Re: [PATCH] accel/ivpu: Add missing MODULE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <832c27d0-a2ec-46df-afcd-be74b8108358@linux.intel.com>
Date: Mon, 8 Jul 2024 09:42:56 +0200
From: Jacek Lawrynowicz <jacek.lawrynowicz@...ux.intel.com>
To: "Alexander F. Lent" <lx@...derlent.com>,
 Stanislaw Gruszka <stanislaw.gruszka@...ux.intel.com>,
 Oded Gabbay <ogabbay@...nel.org>, Jeffrey Hugo <quic_jhugo@...cinc.com>,
 Daniel Vetter <daniel.vetter@...ll.ch>,
 Andrzej Kacprowski <andrzej.kacprowski@...ux.intel.com>,
 Krystian Pradzynski <krystian.pradzynski@...ux.intel.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] accel/ivpu: Add missing MODULE_FIRMWARE metadata

Hi, thanks for the commit.

On 05.07.2024 13:14, Alexander F. Lent wrote:
> Modules that load firmware from various paths at runtime must declare
> those paths at compile time, via the MODULE_FIRMWARE macro, so that the
> firmware paths are included in the module's metadata.
> 
> The accel/ivpu driver loaded firmware but lacked this metadata,
> preventing dracut from correctly locating firmware files.
> 
> Fixes: 9ab43e95f922 ("accel/ivpu: Switch to generation based FW names")
> Fixes: 02d5b0aacd05 ("accel/ivpu: Implement firmware parsing and booting")
> Signed-off-by: Alexander F. Lent <lx@...derlent.com>
> ---
>  drivers/accel/ivpu/ivpu_fw.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
> index 1457300828bf..51792a00b7e3 100644
> --- a/drivers/accel/ivpu/ivpu_fw.c
> +++ b/drivers/accel/ivpu/ivpu_fw.c
> @@ -48,16 +48,26 @@ static char *ivpu_firmware;
>  module_param_named_unsafe(firmware, ivpu_firmware, charp, 0644);
>  MODULE_PARM_DESC(firmware, "NPU firmware binary in /lib/firmware/..");
>  
> +#define IVPU_FW_PATH_37XX_BIN		"vpu_37xx.bin"
> +#define IVPU_FW_PATH_37XX_V0_BIN	"intel/vpu/vpu_37xx_v0.0.bin"
> +#define IVPU_FW_PATH_40XX_BIN		"vpu_40xx.bin"
> +#define IVPU_FW_PATH_40XX_V0_BIN	"intel/vpu/vpu_40xx_v0.0.bin"
> +
>  static struct {
>  	int gen;
>  	const char *name;
>  } fw_names[] = {
> -	{ IVPU_HW_37XX, "vpu_37xx.bin" },
> -	{ IVPU_HW_37XX, "intel/vpu/vpu_37xx_v0.0.bin" },
> -	{ IVPU_HW_40XX, "vpu_40xx.bin" },
> -	{ IVPU_HW_40XX, "intel/vpu/vpu_40xx_v0.0.bin" },
> +	{ IVPU_HW_37XX, IVPU_FW_PATH_37XX_BIN },
> +	{ IVPU_HW_37XX, IVPU_FW_PATH_37XX_V0_BIN },
> +	{ IVPU_HW_40XX, IVPU_FW_PATH_40XX_BIN },
> +	{ IVPU_HW_40XX, IVPU_FW_PATH_40XX_V0_BIN },
>  };
>  
> +MODULE_FIRMWARE(IVPU_FW_PATH_37XX_BIN);
> +MODULE_FIRMWARE(IVPU_FW_PATH_37XX_V0_BIN);
> +MODULE_FIRMWARE(IVPU_FW_PATH_40XX_BIN);
> +MODULE_FIRMWARE(IVPU_FW_PATH_40XX_V0_BIN);
Include only V0 variants, FW names without version are used for development and they wont be included in any distribution.
I would also skip macros and just use literal strings as macros add a lot of lines that don't increase readability.

Regards,
Jacek