Re: [PATCH RFC 4/4] arm64: dts: qcom: sdm670-google: Add modemsmem

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing probe dependency between the `modemsmem` platform device and the modem remoteproc driver.
- [High] Probe fails if the SMEM item is already allocated by the bootloader (-EEXIST).
- [High] Uninitialized SMEM memory is exposed to the modem because several fields are not initialized.
--

commit f684325f1bc358462d3e26fd3838d75a70d04ce2
Author: Richard Acayan <mailingradian@gmail.com>

arm64: dts: qcom: sdm670-google: Add modemsmem

The Pixel 3a modem needs the modemsmem client. Add it in the device
tree so the modem can properly boot.

> diff --git a/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi b/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
> index b4854801a5f5e..4604c4eb2192a 100644
> --- a/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm670-google-common.dtsi
> @@ -66,6 +66,11 @@ key-vol-up {
>  		};
>  	};
>  
> +	modemsmem {
> +		compatible = "google,modemsmem";
> +		qcom,smem-id = <134>;
> +	};
> +

[Severity: High]
Is there a missing probe dependency between the modemsmem platform device
and the modem remoteproc driver?

Because this node is defined as a standalone platform device with no
parent-child relationship or fw_devlink dependency (like a phandle) to the
modem node, the kernel driver core will probe them in an unconstrained order.

If remoteproc probes first and starts the modem firmware, will the modem
driver crash or fail to boot if it attempts to read SMEM item 134 before it
is allocated and populated?

[Severity: High]
Does this node addition trigger a probe failure on warm reboots?

Looking at modemsmem_probe() in drivers/soc/qcom/modemsmem.c:

	ret = qcom_smem_alloc(QCOM_SMEM_HOST_ANY, smem_id, sizeof(*info));
	if (ret)
		return dev_err_probe(dev, ret, "Could not allocate modem smem\n");

If the requested SMEM item was already allocated (e.g., by the bootloader
or persisting across a warm reboot), qcom_smem_alloc() returns -EEXIST.

Because modemsmem_probe() treats any non-zero return value as a fatal error
instead of tolerating -EEXIST, will this prevent the modem from receiving
updated SoC information?

[Severity: High]
Does allocating this SMEM item expose uninitialized memory to the modem?

Looking at write_socinfo() in drivers/soc/qcom/modemsmem.c:

	target->version = cpu_to_le32(MODEM_SMEM_VERSION);
	target->major_id = cpu_to_le32(PLAT_VER_TO_MAJOR_ID(plat_ver));
	target->minor_id = cpu_to_le32(PLAT_VER_TO_MINOR_ID(plat_ver));
	target->platform = socinfo->hw_plat;
	target->subtype = socinfo->hw_plat_subtype;

qcom_smem_alloc() allocates memory from the SMEM region but does not
zero-initialize it. Since only 5 of the 8 fields in struct modem_smem_info
are assigned, the remaining fields (modem_flag, efs_magic, ftm_magic) are
left untouched.

Could this lead to unpredictable behavior if the modem firmware reads and
processes these uninitialized garbage values?

>  	reserved-memory {
>  		#address-cells = <2>;
>  		#size-cells = <2>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-modemsmem-v1-0-a3634b553841@ixit.cz?part=4