Re: [PATCH 3/3] net: ipa: Add IPA v5.1 data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Driver probe failure on SM8450 due to intentional SMEM size mismatch.
- [High] Overlapping IPA local memory regions will cause silent data corruption between filter tables and routing statistics.
- [Medium] The AP_MODEM_RX endpoint is incorrectly assigned to the DST_UL resource group, causing resource starvation and unused DST_DL resources.
--

commit 6da897f459f64d1618e077cb44111c9397ad478b
Author: Esteban Urrutia <esteuwu@proton.me>

net: ipa: Add IPA v5.1 data

Add the required ipa_data-v5.1.c file for IPA v5.1 along with changes
that declare IPA v5.1 support. This version of IPA is used in both SM8450
and SM8475 SoCs.

> diff --git a/drivers/net/ipa/data/ipa_data-v5.1.c b/drivers/net/ipa/data/ipa_data-v5.1.c
> new file mode 100644
> index 0000000000000..85b21efa1224d
> --- /dev/null
> +++ b/drivers/net/ipa/data/ipa_data-v5.1.c
[ ... ]
> +	[IPA_ENDPOINT_AP_MODEM_RX] = {
> +		.ee_id		= GSI_EE_AP,
> +		.channel_id	= 1,
> +		.endpoint_id	= 23,
> +		.toward_ipa	= false,
> +		.channel = {
> +			.tre_count	= 256,
> +			.event_count	= 256,
> +			.tlv_count	= 9,
> +		},
> +		.endpoint = {
> +			.config = {
> +				.resource_group	= IPA_RSRC_GROUP_DST_UL,

[Severity: Medium]
Will assigning this downlink endpoint to the UL resource group cause
resource starvation?

Since this is the AP_MODEM_RX endpoint, should it be assigned to
IPA_RSRC_GROUP_DST_DL instead?

Later in the ipa_resource_dst[] configuration, 5 data sectors are allocated
to IPA_RSRC_GROUP_DST_DL. If no endpoints are assigned to the DL group,
those sectors will remain unused while AP_MODEM_RX competes with AP_LAN_RX
for the limited DST_UL resources.

[ ... ]
> +/* IPA-resident memory region data for an SoC having IPA v5.1 */
> +static const struct ipa_mem ipa_mem_local_data[] = {
[ ... ]
> +	{
> +		.id		= IPA_MEM_STATS_TETHERING,
> +		.offset		= 0x2910,
> +		.size		= 0x03c0,
> +		.canary_count	= 0,
> +	},
> +	{
> +		.id		= IPA_MEM_AP_V4_FILTER,
> +		.offset		= 0x29b8,
> +		.size		= 0x0188,
> +		.canary_count	= 2,
> +	},
> +	{
> +		.id		= IPA_MEM_AP_V6_FILTER,
> +		.offset		= 0x2b40,
> +		.size		= 0x0228,
> +		.canary_count	= 0,
> +	},
> +	{
> +		.id		= IPA_MEM_STATS_FILTER_ROUTE,
> +		.offset		= 0x2cd0,
> +		.size		= 0x0ba0,
> +		.canary_count	= 2,
> +	},

[Severity: High]
Do these memory region offsets overlap and lead to data corruption?

IPA_MEM_STATS_TETHERING starts at 0x2910 and has a size of 0x03c0, which
means it extends up to 0x2cd0.

However, IPA_MEM_AP_V4_FILTER begins at 0x29b8, placing it entirely inside
the tethering region. Similarly, IPA_MEM_AP_V6_FILTER starts at 0x2b40,
which overlaps both the end of the tethering region and the start of
IPA_MEM_STATS_FILTER_ROUTE.

Could hardware updates to the routing and filter stats corrupt the AP
filter tables here?

[ ... ]
> +/* Memory configuration data for an SoC having IPA v5.1 */
> +static const struct ipa_mem_data ipa_mem_data = {
> +	.local_count	= ARRAY_SIZE(ipa_mem_local_data),
> +	.local		= ipa_mem_local_data,
> +	.imem_addr	= 0x146a8000,
> +	.imem_size	= 0x00002000,
> +	/*
> +	 * While this value is 0xb000 on SM8450 and 0x9000 on SM8475,
> +	 * it has been left set to 0x9000 for compatibility with SM8475
> +	 */
> +	.smem_size	= 0x00009000,

[Severity: High]
Will this hardcoded SMEM size cause the driver probe to fail on SM8450?

When the SM8450 bootloader or modem pre-allocates 0xb000 bytes,
qcom_smem_get() will return that actual size.

Looking at ipa_smem_init() in drivers/net/ipa/ipa_mem.c:

	if (ret && actual != size) {
		dev_err(dev, "SMEM item has size %zu, expected %zu\n",
			actual, size);
		return -EINVAL;
	}

Because this strictly validates the returned size against
ipa->mem_data->smem_size, setting this to 0x9000 for SM8475 compatibility
would cause the size check to fail and abort the driver probe.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-sm8450-ipa-v1-0-532f0299f96e@proton.me?part=3