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] [thread-next>] [day] [month] [year] [list]
Message-ID: <6e25398f-9a3d-4cad-a66a-ebe43a723843@amd.com>
Date:   Sun, 10 Dec 2023 09:20:16 +0530
From:   Venkata Prasad Potturu <venkataprasad.potturu@....com>
To:     Cristian Ciocaltea <cristian.ciocaltea@...labora.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Peter Ujfalusi <peter.ujfalusi@...ux.intel.com>,
        Bard Liao <yung-chuan.liao@...ux.intel.com>,
        Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
        Daniel Baluta <daniel.baluta@....com>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        Alper Nebi Yasak <alpernebiyasak@...il.com>,
        Syed Saba Kareem <Syed.SabaKareem@....com>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        Marian Postevca <posteuca@...ex.one>,
        Vijendar Mukunda <Vijendar.Mukunda@....com>,
        V sujith kumar Reddy <Vsujithkumar.Reddy@....com>,
        Mastan Katragadda <Mastan.Katragadda@....com>,
        Ajit Kumar Pandey <AjitKumar.Pandey@....com>
Cc:     linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
        sound-open-firmware@...a-project.org, kernel@...labora.com
Subject: Re: [PATCH 09/11] ASoC: SOF: amd: Compute file paths on firmware load


On 12/10/23 02:23, Cristian Ciocaltea wrote:
> Commit 6c393ebbd74a ("ASoC: SOF: core: Implement IPC version fallback if
> firmware files are missing") changed the order of some operations and
> the firmware paths are not available anymore at snd_sof_probe() time.
>
> Precisely, fw_filename_prefix is set by sof_select_ipc_and_paths() via
>
>    plat_data->fw_filename_prefix = out_profile.fw_path;
>
> but sof_init_environment() which calls this function was moved from
> snd_sof_device_probe() to sof_probe_continue(). Moreover,
> snd_sof_probe() was moved from sof_probe_continue() to
> sof_init_environment(), but before the call to
> sof_select_ipc_and_paths().
>
> The problem here is that amd_sof_acp_probe() uses fw_filename_prefix to
> compute fw_code_bin and fw_data_bin paths, and because the field is not
> yet initialized, the paths end up containing (null):
>
> snd_sof_amd_vangogh 0000:04:00.5: Direct firmware load for (null)/sof-vangogh-code.bin failed with error -2
> snd_sof_amd_vangogh 0000:04:00.5: sof signed firmware code bin is missing
> snd_sof_amd_vangogh 0000:04:00.5: error: failed to load DSP firmware -2
> snd_sof_amd_vangogh: probe of 0000:04:00.5 failed with error -2
>
> Move usage of fw_filename_prefix right before request_firmware() calls
> in acp_sof_load_signed_firmware().
>
> Fixes: 6c393ebbd74a ("ASoC: SOF: core: Implement IPC version fallback if firmware files are missing")
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
> ---
>   sound/soc/sof/amd/acp-loader.c | 32 ++++++++++++++++++++++++++------
>   sound/soc/sof/amd/acp.c        |  7 ++-----
>   2 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/sound/soc/sof/amd/acp-loader.c b/sound/soc/sof/amd/acp-loader.c
> index e05eb7a86dd4..d2d21478399e 100644
> --- a/sound/soc/sof/amd/acp-loader.c
> +++ b/sound/soc/sof/amd/acp-loader.c
> @@ -267,29 +267,49 @@ int acp_sof_load_signed_firmware(struct snd_sof_dev *sdev)
>   {
>   	struct snd_sof_pdata *plat_data = sdev->pdata;
>   	struct acp_dev_data *adata = plat_data->hw_pdata;
> +	const char *fw_filename;
>   	int ret;
>   
> -	ret = request_firmware(&sdev->basefw.fw, adata->fw_code_bin, sdev->dev);
> +	fw_filename = kasprintf(GFP_KERNEL, "%s/%s",
> +				plat_data->fw_filename_prefix,
> +				adata->fw_code_bin);
File path already saved in adata->fw_code_bin in amd_sof_acp_probe function.
No need to get it again.
> +	if (!fw_filename)
> +		return -ENOMEM;
> +
> +	ret = request_firmware(&sdev->basefw.fw, fw_filename, sdev->dev);
>   	if (ret < 0) {
> +		kfree(fw_filename);
>   		dev_err(sdev->dev, "sof signed firmware code bin is missing\n");
>   		return ret;
>   	} else {
> -		dev_dbg(sdev->dev, "request_firmware %s successful\n", adata->fw_code_bin);
> +		dev_dbg(sdev->dev, "request_firmware %s successful\n", fw_filename);
>   	}
> +	kfree(fw_filename);
> +
>   	ret = snd_sof_dsp_block_write(sdev, SOF_FW_BLK_TYPE_IRAM, 0,
> -				      (void *)sdev->basefw.fw->data, sdev->basefw.fw->size);
> +				      (void *)sdev->basefw.fw->data,
> +				      sdev->basefw.fw->size);
> +
> +	fw_filename = kasprintf(GFP_KERNEL, "%s/%s",
> +				plat_data->fw_filename_prefix,
> +				adata->fw_data_bin);
> +	if (!fw_filename)
> +		return -ENOMEM;
>   
> -	ret = request_firmware(&adata->fw_dbin, adata->fw_data_bin, sdev->dev);
> +	ret = request_firmware(&adata->fw_dbin, fw_filename, sdev->dev);
>   	if (ret < 0) {
> +		kfree(fw_filename);
>   		dev_err(sdev->dev, "sof signed firmware data bin is missing\n");
>   		return ret;
>   
>   	} else {
> -		dev_dbg(sdev->dev, "request_firmware %s successful\n", adata->fw_data_bin);
> +		dev_dbg(sdev->dev, "request_firmware %s successful\n", fw_filename);
>   	}
> +	kfree(fw_filename);
>   
>   	ret = snd_sof_dsp_block_write(sdev, SOF_FW_BLK_TYPE_DRAM, 0,
> -				      (void *)adata->fw_dbin->data, adata->fw_dbin->size);
> +				      (void *)adata->fw_dbin->data,
> +				      adata->fw_dbin->size);
>   	return ret;
>   }
>   EXPORT_SYMBOL_NS(acp_sof_load_signed_firmware, SND_SOC_SOF_AMD_COMMON);
> diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c
> index 1e9840ae8938..87c5c71eac68 100644
> --- a/sound/soc/sof/amd/acp.c
> +++ b/sound/soc/sof/amd/acp.c
> @@ -479,7 +479,6 @@ EXPORT_SYMBOL_NS(amd_sof_acp_resume, SND_SOC_SOF_AMD_COMMON);
>   int amd_sof_acp_probe(struct snd_sof_dev *sdev)
>   {
>   	struct pci_dev *pci = to_pci_dev(sdev->dev);
> -	struct snd_sof_pdata *plat_data = sdev->pdata;
>   	struct acp_dev_data *adata;
>   	const struct sof_amd_acp_desc *chip;
>   	const struct dmi_system_id *dmi_id;
> @@ -547,8 +546,7 @@ int amd_sof_acp_probe(struct snd_sof_dev *sdev)
>   	dmi_id = dmi_first_match(acp_sof_quirk_table);
>   	if (dmi_id && dmi_id->driver_data) {
>   		adata->fw_code_bin = devm_kasprintf(sdev->dev, GFP_KERNEL,
> -						    "%s/sof-%s-code.bin",
> -						    plat_data->fw_filename_prefix,
> +						    "sof-%s-code.bin",
>   						    chip->name);
>   		if (!adata->fw_code_bin) {
>   			ret = -ENOMEM;
> @@ -556,8 +554,7 @@ int amd_sof_acp_probe(struct snd_sof_dev *sdev)
>   		}
>   
>   		adata->fw_data_bin = devm_kasprintf(sdev->dev, GFP_KERNEL,
> -						    "%s/sof-%s-data.bin",
> -						    plat_data->fw_filename_prefix,
> +						    "sof-%s-data.bin",
>   						    chip->name);
>   		if (!adata->fw_data_bin) {
>   			ret = -ENOMEM;

Powered by blists - more mailing lists