Thread (43 messages) 43 messages, 3 authors, 2013-10-08
STALE4627d

[PATCH 4/9] iommu/arm-smmu: Check for num_context_irqs > 0 to avoid divide by zero exception

From: Will Deacon <hidden>
Date: 2013-09-27 10:23:07
Also in: linux-iommu
Subsystem: iommu subsystem, the rest · Maintainers: Joerg Roedel, Will Deacon, Linus Torvalds

On Fri, Sep 27, 2013 at 10:03:48AM +0100, Andreas Herrmann wrote:
On Fri, Sep 27, 2013 at 04:41:54AM -0400, Will Deacon wrote:
quoted
On Thu, Sep 26, 2013 at 11:36:16PM +0100, Andreas Herrmann wrote:
quoted
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 4307fbc..de9dd60 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1822,7 +1822,11 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 			 num_irqs, smmu->num_global_irqs);
 		smmu->num_global_irqs = num_irqs;
 	}
-	smmu->num_context_irqs = num_irqs - smmu->num_global_irqs;
Why are you deleting this line?
Because I felt it's redundant in some cases and erroneously I thought
it could be bogus if num_irqs < num_global_irqs.
Of course the latter is wrong, as num_global_irqs is corrected two
lines above.

Now I think it's always redundant. num_context_irqs is only
incremented here

               if (num_irqs > smmu->num_global_irqs)
                        smmu->num_context_irqs++;
Yes, I think you're right. This leaves us in a situation where getting the
#global-interrupts property wrong will trigger a warning followed
immediately by a failure to probe. That means we could simply augment the
current check and make it fatal instead:

--->8
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 2931921..03ffbae 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1799,12 +1799,11 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
                        smmu->num_context_irqs++;
        }
 
-       if (num_irqs < smmu->num_global_irqs) {
+       if (num_irqs <= smmu->num_global_irqs) {
                dev_warn(dev, "found %d interrupts but expected at least %d\n",
-                        num_irqs, smmu->num_global_irqs);
-               smmu->num_global_irqs = num_irqs;
+                        num_irqs, smmu->num_global_irqs + 1);
+               return -ENODEV;
        }
-       smmu->num_context_irqs = num_irqs - smmu->num_global_irqs;
 
        smmu->irqs = devm_kzalloc(dev, sizeof(*smmu->irqs) * num_irqs,
                                  GFP_KERNEL);
8<---

What do you think?

Will
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help