[PATCH 0/3] memory: tegra: UBSAN fix and cleanups

[PATCH 0/3] memory: tegra: UBSAN fix and cleanups

[Sumit Gupta]

This series fixes an UBSAN warning in the Tegra MC ICC aggregate
path and removes two pieces of related dead code.

- Patch 1: Sets the EMC provider's aggregate hook to
  icc_std_aggregate, instead of borrowing the MC's aggregate hook.
- Patch 2: Drops tegra264_mc_icc_aggregate() as its only check
  duplicates the one in tegra264_mc_icc_set().
- Patch 3: Drops a dead 'if (mc)' check inside the CPU-cluster
  branch of tegra234_mc_icc_aggregate().


Sumit Gupta (3):
  memory: tegra186-emc: stop borrowing MC aggregate hook for EMC
  memory: tegra264: drop redundant tegra264_mc_icc_aggregate()
  memory: tegra234: drop dead NULL check in tegra234_mc_icc_aggregate()

 drivers/memory/tegra/tegra186-emc.c |  4 +---
 drivers/memory/tegra/tegra234.c     |  6 ++----
 drivers/memory/tegra/tegra264.c     | 17 +----------------
 3 files changed, 4 insertions(+), 23 deletions(-)

-- 
2.34.1

[PATCH 1/3] memory: tegra186-emc: stop borrowing MC aggregate hook for EMC

[Sumit Gupta]

tegra186_emc_interconnect_init() copies the MC's ICC aggregate hook
into the EMC provider.  That hook (tegra234_mc_icc_aggregate /
tegra264_mc_icc_aggregate) uses container_of() to recover 'mc',
which is only valid when the icc_provider is embedded in struct
tegra_mc.  For an EMC node the provider is embedded in struct
tegra186_emc, so 'mc' points into unrelated memory.

This stayed harmless until commit faafd6ca7e6e ("memory: tegra:
make icc_set_bw return zero if BWMGR not supported") added an
unconditional read of mc->bwmgr_mrq_supported at the top of the
hook.  UBSAN catches the stray load on every EMC aggregation:

  UBSAN: invalid-load in drivers/memory/tegra/tegra234.c:1104:9
  load of value 112 is not a valid value for type '_Bool'

No functional impact in practice, since the hook's only other mc
dereference (mc->num_channels) sits inside a
TEGRA_ICC_MC_CPU_CLUSTER* branch that EMC nodes never enter.

Fix this by setting the EMC provider's aggregate hook to
icc_std_aggregate, instead of borrowing the MC's hook.  The MC
providers continue using their own aggregate hooks, where
container_of() correctly resolves to struct tegra_mc.

Reported-by: Jon Hunter <jonathanh@nvidia.com>
Fixes: 9a38cb27668e ("memory: tegra: Add interconnect support for DRAM scaling in Tegra234")
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/memory/tegra/tegra186-emc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
index 03ebab6fbe68..f71265b303b9 100644
--- a/drivers/memory/tegra/tegra186-emc.c
+++ b/drivers/memory/tegra/tegra186-emc.c
@@ -258,15 +258,13 @@ static int tegra186_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *pe
 
 static int tegra186_emc_interconnect_init(struct tegra186_emc *emc)
 {
-	struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
-	const struct tegra_mc_soc *soc = mc->soc;
 	struct icc_node *node;
 	int err;
 
 	emc->provider.dev = emc->dev;
 	emc->provider.set = tegra186_emc_icc_set_bw;
 	emc->provider.data = &emc->provider;
-	emc->provider.aggregate = soc->icc_ops->aggregate;
+	emc->provider.aggregate = icc_std_aggregate;
 	emc->provider.xlate = tegra186_emc_of_icc_xlate;
 	emc->provider.get_bw = tegra186_emc_icc_get_init_bw;
 
-- 
2.34.1

[PATCH 2/3] memory: tegra264: drop redundant tegra264_mc_icc_aggregate()

[Sumit Gupta]

tegra264_mc_icc_aggregate() does nothing on top of icc_std_aggregate
except an early return on !mc->bwmgr_mrq_supported.
tegra264_mc_icc_set() already returns early on the same condition,
before reading any of the aggregated avg_bw / peak_bw values.  This
makes the early return in aggregate() redundant.  So, set
tegra264_mc_icc_ops.aggregate to icc_std_aggregate and drop the
helper.

Reported-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/memory/tegra/tegra264.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/memory/tegra/tegra264.c b/drivers/memory/tegra/tegra264.c
index e43ef14da1ee..bad7d61165b0 100644
--- a/drivers/memory/tegra/tegra264.c
+++ b/drivers/memory/tegra/tegra264.c
@@ -295,21 +295,6 @@ static int tegra264_mc_icc_set(struct icc_node *src, struct icc_node *dst)
 	return ret;
 }
 
-static int tegra264_mc_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
-				     u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
-{
-	struct icc_provider *p = node->provider;
-	struct tegra_mc *mc = icc_provider_to_tegra_mc(p);
-
-	if (!mc->bwmgr_mrq_supported)
-		return 0;
-
-	*agg_avg += avg_bw;
-	*agg_peak = max(*agg_peak, peak_bw);
-
-	return 0;
-}
-
 static int tegra264_mc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
 {
 	*avg = 0;
@@ -626,7 +611,7 @@ static const irq_handler_t tegra264_mc_irq_handlers[8] = {
 
 static const struct tegra_mc_icc_ops tegra264_mc_icc_ops = {
 	.xlate = tegra_mc_icc_xlate,
-	.aggregate = tegra264_mc_icc_aggregate,
+	.aggregate = icc_std_aggregate,
 	.get_bw = tegra264_mc_icc_get_init_bw,
 	.set = tegra264_mc_icc_set,
 };
-- 
2.34.1

[PATCH 3/3] memory: tegra234: drop dead NULL check in tegra234_mc_icc_aggregate()

[Sumit Gupta]

In tegra234_mc_icc_aggregate(), the 'if (mc)' check inside the
CPU-cluster branch is always true.  'mc' was already dereferenced
via 'mc->bwmgr_mrq_supported' a few lines above, so if it were NULL
the function would have faulted there.  Drop the redundant check.

Reported-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/memory/tegra/tegra234.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/memory/tegra/tegra234.c b/drivers/memory/tegra/tegra234.c
index 9fbd34d4abe0..982476b67d5a 100644
--- a/drivers/memory/tegra/tegra234.c
+++ b/drivers/memory/tegra/tegra234.c
@@ -1106,10 +1106,8 @@ static int tegra234_mc_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
 
 	if (node->id == TEGRA_ICC_MC_CPU_CLUSTER0 ||
 	    node->id == TEGRA_ICC_MC_CPU_CLUSTER1 ||
-	    node->id == TEGRA_ICC_MC_CPU_CLUSTER2) {
-		if (mc)
-			peak_bw = peak_bw * mc->num_channels;
-	}
+	    node->id == TEGRA_ICC_MC_CPU_CLUSTER2)
+		peak_bw = peak_bw * mc->num_channels;
 
 	*agg_avg += avg_bw;
 	*agg_peak = max(*agg_peak, peak_bw);
-- 
2.34.1

Re: [PATCH 0/3] memory: tegra: UBSAN fix and cleanups

[Jon Hunter]

On 27/05/2026 15:01, Sumit Gupta wrote:
> This series fixes an UBSAN warning in the Tegra MC ICC aggregate
> path and removes two pieces of related dead code.
> 
> - Patch 1: Sets the EMC provider's aggregate hook to
>    icc_std_aggregate, instead of borrowing the MC's aggregate hook.
> - Patch 2: Drops tegra264_mc_icc_aggregate() as its only check
>    duplicates the one in tegra264_mc_icc_set().
> - Patch 3: Drops a dead 'if (mc)' check inside the CPU-cluster
>    branch of tegra234_mc_icc_aggregate().
> 
> 
> Sumit Gupta (3):
>    memory: tegra186-emc: stop borrowing MC aggregate hook for EMC
>    memory: tegra264: drop redundant tegra264_mc_icc_aggregate()
>    memory: tegra234: drop dead NULL check in tegra234_mc_icc_aggregate()
> 
>   drivers/memory/tegra/tegra186-emc.c |  4 +---
>   drivers/memory/tegra/tegra234.c     |  6 ++----
>   drivers/memory/tegra/tegra264.c     | 17 +----------------
>   3 files changed, 4 insertions(+), 23 deletions(-)
> 

For the series ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

-- 
nvpublic

Re: [PATCH 1/3] memory: tegra186-emc: stop borrowing MC aggregate hook for EMC

[Krzysztof Kozlowski]

On 27/05/2026 16:01, Sumit Gupta wrote:
> tegra186_emc_interconnect_init() copies the MC's ICC aggregate hook
> into the EMC provider.  That hook (tegra234_mc_icc_aggregate /
> tegra264_mc_icc_aggregate) uses container_of() to recover 'mc',
> which is only valid when the icc_provider is embedded in struct
> tegra_mc.  For an EMC node the provider is embedded in struct
> tegra186_emc, so 'mc' points into unrelated memory.
> 
> This stayed harmless until commit faafd6ca7e6e ("memory: tegra:
> make icc_set_bw return zero if BWMGR not supported") added an
> unconditional read of mc->bwmgr_mrq_supported at the top of the
> hook.  UBSAN catches the stray load on every EMC aggregation:
> 
>   UBSAN: invalid-load in drivers/memory/tegra/tegra234.c:1104:9
>   load of value 112 is not a valid value for type '_Bool'
> 
> No functional impact in practice, since the hook's only other mc
> dereference (mc->num_channels) sits inside a
> TEGRA_ICC_MC_CPU_CLUSTER* branch that EMC nodes never enter.
> 
> Fix this by setting the EMC provider's aggregate hook to
> icc_std_aggregate, instead of borrowing the MC's hook.  The MC
> providers continue using their own aggregate hooks, where
> container_of() correctly resolves to struct tegra_mc.
> 
> Reported-by: Jon Hunter <jonathanh@nvidia.com>

I assume these reports were offlist. Otherwise this has a valid
checkpatch warning.

Best regards,
Krzysztof

Re: [PATCH 0/3] memory: tegra: UBSAN fix and cleanups

[Krzysztof Kozlowski]

On Wed, 27 May 2026 19:31:24 +0530, Sumit Gupta wrote:
> This series fixes an UBSAN warning in the Tegra MC ICC aggregate
> path and removes two pieces of related dead code.
> 
> - Patch 1: Sets the EMC provider's aggregate hook to
>   icc_std_aggregate, instead of borrowing the MC's aggregate hook.
> - Patch 2: Drops tegra264_mc_icc_aggregate() as its only check
>   duplicates the one in tegra264_mc_icc_set().
> - Patch 3: Drops a dead 'if (mc)' check inside the CPU-cluster
>   branch of tegra234_mc_icc_aggregate().
> 
> [...]

Applied, thanks!

[1/3] memory: tegra186-emc: stop borrowing MC aggregate hook for EMC
      https://git.kernel.org/krzk/linux-mem-ctrl/c/2e05f3d6005d9aa3e2e423d2471f290d9ccbe3d2
[2/3] memory: tegra264: drop redundant tegra264_mc_icc_aggregate()
      https://git.kernel.org/krzk/linux-mem-ctrl/c/e23d87a69e827b60fb985236a0984bacb3b68a19
[3/3] memory: tegra234: drop dead NULL check in tegra234_mc_icc_aggregate()
      https://git.kernel.org/krzk/linux-mem-ctrl/c/b97f7dceb8adb2b05d556469afc6fb54947ef61c

Best regards,
-- 
Krzysztof Kozlowski <krzk@kernel.org>

Re: [PATCH 1/3] memory: tegra186-emc: stop borrowing MC aggregate hook for EMC

[Jon Hunter]

On 09/06/2026 16:01, Krzysztof Kozlowski wrote:
> On 27/05/2026 16:01, Sumit Gupta wrote:
>> tegra186_emc_interconnect_init() copies the MC's ICC aggregate hook
>> into the EMC provider.  That hook (tegra234_mc_icc_aggregate /
>> tegra264_mc_icc_aggregate) uses container_of() to recover 'mc',
>> which is only valid when the icc_provider is embedded in struct
>> tegra_mc.  For an EMC node the provider is embedded in struct
>> tegra186_emc, so 'mc' points into unrelated memory.
>>
>> This stayed harmless until commit faafd6ca7e6e ("memory: tegra:
>> make icc_set_bw return zero if BWMGR not supported") added an
>> unconditional read of mc->bwmgr_mrq_supported at the top of the
>> hook.  UBSAN catches the stray load on every EMC aggregation:
>>
>>    UBSAN: invalid-load in drivers/memory/tegra/tegra234.c:1104:9
>>    load of value 112 is not a valid value for type '_Bool'
>>
>> No functional impact in practice, since the hook's only other mc
>> dereference (mc->num_channels) sits inside a
>> TEGRA_ICC_MC_CPU_CLUSTER* branch that EMC nodes never enter.
>>
>> Fix this by setting the EMC provider's aggregate hook to
>> icc_std_aggregate, instead of borrowing the MC's hook.  The MC
>> providers continue using their own aggregate hooks, where
>> container_of() correctly resolves to struct tegra_mc.
>>
>> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> 
> I assume these reports were offlist. Otherwise this has a valid
> checkpatch warning.


Yes some of our internal testing flagged this and I had asked Sumit to 
take a look.

Cheers
Jon

-- 
nvpublic

Re: [PATCH 1/3] memory: tegra186-emc: stop borrowing MC aggregate hook for EMC

[Krzysztof Kozlowski]

On 09/06/2026 17:07, Jon Hunter wrote:
> 
> On 09/06/2026 16:01, Krzysztof Kozlowski wrote:
>> On 27/05/2026 16:01, Sumit Gupta wrote:
>>> tegra186_emc_interconnect_init() copies the MC's ICC aggregate hook
>>> into the EMC provider.  That hook (tegra234_mc_icc_aggregate /
>>> tegra264_mc_icc_aggregate) uses container_of() to recover 'mc',
>>> which is only valid when the icc_provider is embedded in struct
>>> tegra_mc.  For an EMC node the provider is embedded in struct
>>> tegra186_emc, so 'mc' points into unrelated memory.
>>>
>>> This stayed harmless until commit faafd6ca7e6e ("memory: tegra:
>>> make icc_set_bw return zero if BWMGR not supported") added an
>>> unconditional read of mc->bwmgr_mrq_supported at the top of the
>>> hook.  UBSAN catches the stray load on every EMC aggregation:
>>>
>>>    UBSAN: invalid-load in drivers/memory/tegra/tegra234.c:1104:9
>>>    load of value 112 is not a valid value for type '_Bool'
>>>
>>> No functional impact in practice, since the hook's only other mc
>>> dereference (mc->num_channels) sits inside a
>>> TEGRA_ICC_MC_CPU_CLUSTER* branch that EMC nodes never enter.
>>>
>>> Fix this by setting the EMC provider's aggregate hook to
>>> icc_std_aggregate, instead of borrowing the MC's hook.  The MC
>>> providers continue using their own aggregate hooks, where
>>> container_of() correctly resolves to struct tegra_mc.
>>>
>>> Reported-by: Jon Hunter <jonathanh@nvidia.com>
>>
>> I assume these reports were offlist. Otherwise this has a valid
>> checkpatch warning.
> 
> 
> Yes some of our internal testing flagged this and I had asked Sumit to 
> take a look.
> 

Ack, thanks!

Best regards,
Krzysztof