Re: [PATCH v8 10/18] media: iris: skip PIPE if it is not supported by the platform

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Omitting the PIPE capability causes a division-by-zero kernel panic in `iris_vpu3x_vpu4x_calculate_frequency()`.
- [Medium] `iris_set_stage()` lacks the fallback check for unsupported capabilities that was added to `iris_set_pipe()`.
--

--- Patch [10]: [PATCH v8 10/18] media: iris: skip PIPE if it is not supported by the platform ---
commit 47a09938f9dc9bf17faca2d850a92320d738ffe8
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

media: iris: skip PIPE if it is not supported by the platform

AR50Lt doesn't support HFI_PROPERTY_PARAM_WORK_ROUTE. Tables for AR50LT
won't have corresponding entry in the capability tables. Let
iris_set_pipe() silently skip propgramming the property if there is no
corresponding capability.

> diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c
> index 10e33b8a73f60..33a34573391a4 100644
> --- a/drivers/media/platform/qcom/iris/iris_ctrls.c
> +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c
> @@ -534,6 +534,9 @@ int iris_set_pipe(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id)
>  	u32 work_route = inst->fw_caps[PIPE].value;
>  	u32 hfi_id = inst->fw_caps[cap_id].hfi_id;
>  
> +	if (!hfi_id)
> +		return 0;
> +

[Severity: Critical]
This isn't a bug introduced by this patch, but by allowing platforms to
legitimately omit the PIPE capability, could this lead to a division by zero
panic in iris_vpu3x_vpu4x_calculate_frequency()?

When the PIPE capability is missing (like in inst_fw_cap_sm8550_enc),
inst->fw_caps[PIPE].value is initialized to 0. During clock scaling,
iris_vpu3x_vpu4x_calculate_frequency() in iris_vpu_common.c unconditionally
uses this value:

drivers/media/platform/qcom/iris/iris_vpu_common.c:iris_vpu3x_vpu4x_calculate_frequency() {
    ...
    vpp_cycles = mult_frac(mbs_per_second, caps->mb_cycles_vpp,
                           (u32)inst->fw_caps[PIPE].value);
    ...
}

Will this trigger a kernel panic if a stream is started on a platform where
PIPE is intentionally absent?

[Severity: Medium]
This is also a pre-existing issue, but should iris_set_stage() in
iris_ctrls.c include the exact same hfi_id check?

In iris_common.c, iris_alloc_and_queue_input_int_bufs() calls both
iris_set_stage() and iris_set_pipe(). Since iris_set_stage() lacks this
check, if a platform omits the STAGE capability, it will call
session_set_property() with an hfi_id of 0:

drivers/media/platform/qcom/iris/iris_ctrls.c:iris_set_stage() {
    ...
    u32 hfi_id = inst->fw_caps[cap_id].hfi_id;

    return hfi_ops->session_set_property(inst, hfi_id, ...);
    ...
}

Would this pass an invalid property ID and potentially crash the firmware?

>  	return hfi_ops->session_set_property(inst, hfi_id,
>  					     HFI_HOST_FLAGS_NONE,
>  					     iris_get_port_info(inst, cap_id),
>  					     HFI_PAYLOAD_U32,
>  					     &work_route, sizeof(u32));
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-iris-ar50lt-v8-0-6597c08cea6f@oss.qualcomm.com?part=10