Skip to contentSkip to conversation

iommu/arm-smmu: Do not access non-existing S2CR registers

PATCH Superseded Olav Haugan 2 participants 3 messages Aug 18, 2014 – Aug 22, 2014

Subsystems (2)
Arm Qualcomm mailing list IOMMU

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Olav Haugan

author
#0 lore raw

PATCH v3 1/1 iommu/arm-smmu: Do not access non-existing S2CR registers

The number of S2CR registers is not properly set when stream matching is not supported. Fix this and add check that we do not try to access outside of the number of S2CR regisrers.

Signed-off-by Olav Haugan <[email protected]>
---
 drivers/iommu/arm-smmu.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
drivers/iommu/arm-smmu.c +17 -7
index 9fd8754d..a050f7a 100644
@@ -548,9 +548,16 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
 	master->of_node			= masterspec->np;
 	master->cfg.num_streamids	= masterspec->args_count;
 
-	for (i = 0; i < master->cfg.num_streamids; ++i)
+	for (i = 0; i < master->cfg.num_streamids; ++i) {
+		if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) &&
+		     (masterspec->args[i] >= smmu->num_mapping_groups)) {
+			dev_err(dev,
+				"stream ID for master device %s greater than maximum allowed (%d)\n",
+				masterspec->np->name, smmu->num_mapping_groups);
+			return -ENOSPC;
+		}
 		master->cfg.streamids[i] = masterspec->args[i];
-
+	}
 	return insert_smmu_master(smmu, master);
 }
 

@@ -1734,7 +1741,7 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
 
 	/* Mark all SMRn as invalid and all S2CRn as bypass */
 	for (i = 0; i < smmu->num_mapping_groups; ++i) {
-		writel_relaxed(~SMR_VALID, gr0_base + ARM_SMMU_GR0_SMR(i));
+		writel_relaxed(0, gr0_base + ARM_SMMU_GR0_SMR(i));
 		writel_relaxed(S2CR_TYPE_BYPASS,
 			gr0_base + ARM_SMMU_GR0_S2CR(i));
 	}

@@ -1875,6 +1882,9 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
 		dev_notice(smmu->dev,
 			   "\tstream matching with %u register groups, mask 0x%x",
 			   smmu->num_mapping_groups, mask);
+	} else {
+		smmu->num_mapping_groups = (id >> ID0_NUMSIDB_SHIFT) &
+					   ID0_NUMSIDB_MASK;
 	}
 	smmu->idr[0] = id;
 

@@ -2011,6 +2021,10 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 		smmu->irqs[i] = irq;
 	}
 
+	err = arm_smmu_device_cfg_probe(smmu);
+	if (err)
+		return err;
+
 	i = 0;
 	smmu->masters = RB_ROOT;
 	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",

@@ -2027,10 +2041,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	}
 	dev_notice(dev, "registered %d master devices\n", i);
 
-	err = arm_smmu_device_cfg_probe(smmu);
-	if (err)
-		goto out_put_masters;
-
 	parse_driver_options(smmu);
 
 	if (smmu->version > 1 &&
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#1 lore raw

PATCH v3 1/1 iommu/arm-smmu: Do not access non-existing S2CR registers

Hi Olav,

Some really minor comments, but this looks good.

Quoted 14 lines · Olav Haugan

The number of S2CR registers is not properly set when stream matching is not supported. Fix this and add check that we do not try to access outside of the number of S2CR regisrers.

Signed-off-by Olav Haugan <[email protected]>
---
 drivers/iommu/arm-smmu.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
drivers/iommu/arm-smmu.c +0 -0
index 9fd8754d..a050f7a 100644

drivers/iommu/arm-smmu.c static int register_smmu_master(struct arm_smmu_device *smmu, · line 553

Quoted 7 lines
drivers/iommu/arm-smmu.c +3 -1
@@ -548,9 +548,16 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
 	master->of_node			= masterspec->np;
 	master->cfg.num_streamids	= masterspec->args_count;
 
-	for (i = 0; i < master->cfg.num_streamids; ++i)
+	for (i = 0; i < master->cfg.num_streamids; ++i) {
+		if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) &&
+		     (masterspec->args[i] >= smmu->num_mapping_groups)) {

Can we stick masterspec->args[i] into a loop-local streamid variable, please?

drivers/iommu/arm-smmu.c static int register_smmu_master(struct arm_smmu_device *smmu, · line 557

Quoted 4 lines
drivers/iommu/arm-smmu.c +4 -0
+			dev_err(dev,
+				"stream ID for master device %s greater than maximum allowed (%d)\n",
+				masterspec->np->name, smmu->num_mapping_groups);
+			return -ENOSPC;

-ERANGE might be better.

Will

author
#2 lore raw

PATCH v3 1/1 iommu/arm-smmu: Do not access non-existing S2CR registers

Quoted 30 lines · Will Deacon

Hi Olav,

Some really minor comments, but this looks good.

Quoted 14 lines · Olav Haugan

The number of S2CR registers is not properly set when stream matching is not supported. Fix this and add check that we do not try to access outside of the number of S2CR regisrers.

Signed-off-by Olav Haugan <[email protected]>
---
 drivers/iommu/arm-smmu.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
drivers/iommu/arm-smmu.c +0 -0
index 9fd8754d..a050f7a 100644

drivers/iommu/arm-smmu.c static int register_smmu_master(struct arm_smmu_device *smmu, · line 553

Quoted 7 lines
drivers/iommu/arm-smmu.c +3 -1
@@ -548,9 +548,16 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
 	master->of_node			= masterspec->np;
 	master->cfg.num_streamids	= masterspec->args_count;
 
-	for (i = 0; i < master->cfg.num_streamids; ++i)
+	for (i = 0; i < master->cfg.num_streamids; ++i) {
+		if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) &&
+		     (masterspec->args[i] >= smmu->num_mapping_groups)) {

Can we stick masterspec->args[i] into a loop-local streamid variable, please?

Quoted 7 lines

drivers/iommu/arm-smmu.c static int register_smmu_master(struct arm_smmu_device *smmu, · line 557

Quoted 4 lines
drivers/iommu/arm-smmu.c +4 -0
+			dev_err(dev,
+				"stream ID for master device %s greater than maximum allowed (%d)\n",
+				masterspec->np->name, smmu->num_mapping_groups);
+			return -ENOSPC;

-ERANGE might be better.

Sure, will post v4 with these changes.

Thanks,

Olav

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation