Re: [PATCH v2 4/9] media: iris: migrate firmware loading to qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] media: iris: unzeroed carveout scanned by
  iris_detect_firmware()
- [Low] media: iris: missing kernel-doc entry for pas_ctx in struct
  iris_core
--

commit 7ebe09aa487374b06a38922ce80ef4d57f96da6a
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

media: iris: migrate firmware loading to qcom_mdt_pas_load() and qcom_pas_prepare_and_auth_reset()

This commit embeds a struct qcom_pas_context in struct iris_core and
migrates iris firmware loading and authentication to qcom_mdt_pas_load()
and qcom_pas_prepare_and_auth_reset().

> diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
> index 3e4cf652bf39a..ac73bda072f85 100644
> --- a/drivers/media/platform/qcom/iris/iris_core.h
> +++ b/drivers/media/platform/qcom/iris/iris_core.h

[ ... ]

> @@ -129,6 +130,7 @@ struct iris_core {
>  	struct platform_inst_fw_cap		inst_fw_caps_dec[INST_FW_CAP_MAX];
>  	struct platform_inst_fw_cap		inst_fw_caps_enc[INST_FW_CAP_MAX];
>  	struct dma_iova_state			*iova_state;
> +	struct qcom_pas_context			pas_ctx;
>  };
>  
>  int iris_core_init(struct iris_core *core);

[Severity: Low]
This isn't a bug, but struct iris_core has a kernel-doc comment block
that lacks an @pas_ctx entry, which causes a W=1 kernel-doc warning.

> diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
> index 21f57d37da187..b701a7f43514b 100644
> --- a/drivers/media/platform/qcom/iris/iris_firmware.c
> +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
> @@ -201,21 +201,20 @@ static int iris_load_fw_to_memory(struct iris_core *core)
>  		goto err_release_fw;
>  	}
>  
> -	mem_virt = memremap(mem_phys, res_size, MEMREMAP_WC);
> -	if (!mem_virt) {
> -		ret = -ENOMEM;
> -		goto err_release_fw;
> -	}
> -
> -	memset(mem_virt, 0, res_size);
> -
> -	ret = qcom_mdt_load(dev, firmware, fw_name,
> -			    IRIS_PAS_ID, mem_virt, mem_phys, res_size, NULL);
> +	qcom_pas_context_init(&core->pas_ctx, dev, IRIS_PAS_ID, mem_phys,
> +			      res_size);
> +
> +	ret = qcom_mdt_pas_load(&core->pas_ctx, firmware, fw_name, NULL);
> +	if (!ret) {
> +		mem_virt = memremap(mem_phys, res_size, MEMREMAP_WC);
> +		if (!mem_virt) {
> +			ret = -ENOMEM;
> +			goto err_release_fw;
> +		}
>  
> -	if (!ret)
>  		iris_detect_firmware(core, fw_name, mem_virt, res_size, desc);
> -
> -	memunmap(mem_virt);
> +		memunmap(mem_virt);
> +	}

[Severity: Medium]
Can iris_detect_firmware() scan uninitialized carveout memory now that
memset(mem_virt, 0, res_size) is removed?

qcom_mdt_pas_load() calls qcom_mdt_load_no_init(), which only writes each
PT_LOAD segment and zeroes its BSS tail, leaving inter-segment gaps and
the trailing [fw_size, res_size) portion of the reserved memory region
untouched.  When a dual-generation platform uses a DT firmware-name
override to load a Gen1 image, iris_detect_firmware() ->
iris_detect_gen2_from_fwdata() scans the entire res_size mapping and can
match stale bytes in the unwritten carveout (for example after a warm
reboot or kexec), misclassifying the Gen1 firmware as Gen2 and selecting
incompatible HFI operations and buffer-sizing callbacks.  Commit
a9ff61527805 ("media: iris: Add Gen2 firmware autodetect and fallback")
added the memset() specifically to avoid scanning stale data across
res_size.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113185009.157893-1-mukesh.ojha@oss.qualcomm.com?part=4