From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andreas Herrmann Subject: Re: [PATCH 4/7] iommu/arm-smmu: Check for num_context_irqs > 0 to avoid divide by zero exception Date: Wed, 25 Sep 2013 12:50:10 +0200 Message-ID: <20130925105010.GA4845@alberich> References: <1380035221-11576-1-git-send-email-andreas.herrmann@calxeda.com> <1380035221-11576-5-git-send-email-andreas.herrmann@calxeda.com> <20130924154048.GE20774@mudshark.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20130924154048.GE20774-MRww78TxoiP5vMa5CHWGZ34zcgK1vI+I0E9HWUfgJXw@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Will Deacon Cc: "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , Rob Herring , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: iommu@lists.linux-foundation.org T24gVHVlLCBTZXAgMjQsIDIwMTMgYXQgMTE6NDA6NDhBTSAtMDQwMCwgV2lsbCBEZWFjb24gd3Jv dGU6Cj4gT24gVHVlLCBTZXAgMjQsIDIwMTMgYXQgMDQ6MDY6NThQTSArMDEwMCwgQW5kcmVhcyBI ZXJybWFubiB3cm90ZToKPiA+IFdpdGggdGhlIHJpZ2h0IChvciB3cm9uZzstKSBkZWZpbml0aW9u IG9mIHYxIFNNTVUgbm9kZSBpbiBEVEIgaXQgaXMKPiA+IHBvc3NpYmxlIHRvIHRyaWdnZXIgYSBk aXZpc2lvbiBieSB6ZXJvIGluIGFybV9zbW11X2luaXRfZG9tYWluX2NvbnRleHQKPiA+IChpZiBu dW1iZXIgb2YgY29udGV4dCBpcnFzIGlzIDApOgo+ID4gCj4gPiAgICAgICAgaWYgKHNtbXUtPnZl cnNpb24gPT0gMSkgewo+ID4gICAgICAgICAgICAgICAgcm9vdF9jZmctPmlycHRuZHggPSBhdG9t aWNfaW5jX3JldHVybigmc21tdS0+aXJwdG5keCk7Cj4gPiAg4oaSICAgICAgICAgICAgIHJvb3Rf Y2ZnLT5pcnB0bmR4ICU9IHNtbXUtPm51bV9jb250ZXh0X2lycXM7Cj4gPiAgICAgICAgfSBlbHNl IHsKPiA+IAo+ID4gQXZvaWQgdGhpcyBieSBjaGVja2luZyBmb3IgbnVtX2NvbnRleHRfaXJxcyA+ IDAgYmVmb3JlIHRyeWluZyB0bwo+ID4gYXNzaWduIGludGVycnVwdHMgdG8gY29udGV4dHMuCj4g PiAKPiA+IFNpZ25lZC1vZmYtYnk6IEFuZHJlYXMgSGVycm1hbm4gPGFuZHJlYXMuaGVycm1hbm5A Y2FseGVkYS5jb20+Cj4gPiAtLS0KPiA+ICBkcml2ZXJzL2lvbW11L2FybS1zbW11LmMgfCAgIDMx ICsrKysrKysrKysrKysrKysrLS0tLS0tLS0tLS0tLS0KPiA+ICAxIGZpbGUgY2hhbmdlZCwgMTcg aW5zZXJ0aW9ucygrKSwgMTQgZGVsZXRpb25zKC0pCj4gPiAKPiA+IGRpZmYgLS1naXQgYS9kcml2 ZXJzL2lvbW11L2FybS1zbW11LmMgYi9kcml2ZXJzL2lvbW11L2FybS1zbW11LmMKPiA+IGluZGV4 IGY1YTg1NmUuLjBkZmQyNTUgMTAwNjQ0Cj4gPiAtLS0gYS9kcml2ZXJzL2lvbW11L2FybS1zbW11 LmMKPiA+ICsrKyBiL2RyaXZlcnMvaW9tbXUvYXJtLXNtbXUuYwo+ID4gQEAgLTgyOCwyMSArODI4 LDI0IEBAIHN0YXRpYyBpbnQgYXJtX3NtbXVfaW5pdF9kb21haW5fY29udGV4dChzdHJ1Y3QgaW9t bXVfZG9tYWluICpkb21haW4sCj4gPiAgCQlyZXR1cm4gcmV0Owo+ID4gIAo+ID4gIAlyb290X2Nm Zy0+Y2JuZHggPSByZXQ7Cj4gPiAtCWlmIChzbW11LT52ZXJzaW9uID09IDEpIHsKPiA+IC0JCXJv b3RfY2ZnLT5pcnB0bmR4ID0gYXRvbWljX2luY19yZXR1cm4oJnNtbXUtPmlycHRuZHgpOwo+ID4g LQkJcm9vdF9jZmctPmlycHRuZHggJT0gc21tdS0+bnVtX2NvbnRleHRfaXJxczsKPiA+IC0JfSBl bHNlIHsKPiA+IC0JCXJvb3RfY2ZnLT5pcnB0bmR4ID0gcm9vdF9jZmctPmNibmR4Owo+ID4gLQl9 Cj4gPiAgCj4gPiAtCWlycSA9IHNtbXUtPmlycXNbc21tdS0+bnVtX2dsb2JhbF9pcnFzICsgcm9v dF9jZmctPmlycHRuZHhdOwo+ID4gLQlyZXQgPSByZXF1ZXN0X2lycShpcnEsIGFybV9zbW11X2Nv bnRleHRfZmF1bHQsIElSUUZfU0hBUkVELAo+ID4gLQkJCSAgImFybS1zbW11LWNvbnRleHQtZmF1 bHQiLCBkb21haW4pOwo+ID4gLQlpZiAoSVNfRVJSX1ZBTFVFKHJldCkpIHsKPiA+IC0JCWRldl9l cnIoc21tdS0+ZGV2LCAiZmFpbGVkIHRvIHJlcXVlc3QgY29udGV4dCBJUlEgJWQgKCV1KVxuIiwK PiA+IC0JCQlyb290X2NmZy0+aXJwdG5keCwgaXJxKTsKPiA+IC0JCXJvb3RfY2ZnLT5pcnB0bmR4 ID0gLTE7Cj4gPiAtCQlnb3RvIG91dF9mcmVlX2NvbnRleHQ7Cj4gPiArCWlmIChzbW11LT5udW1f Y29udGV4dF9pcnFzKSB7Cj4gCj4gQ2FuIHdlIG1vdmUgdGhpcyBjaGVjayB0byBwcm9iZSB0aW1l LCB0byBhdm9pZCByZS1ldmFsdWF0aW5nIGl0IGV2ZXJ5IHRpbWUKPiB3ZSBpbml0aWFsaXNlIGEg bmV3IGRvbWFpbj8KClllcywgSSdsbCBtb3ZlIHRoaXMgY2hlY2sgYW5kIGlzc3VlIGFuIGVycm9y IG1lc3NhZ2Ugd2hlbiB0aGVyZSBpcyBub3QKYXQgbGVhc3Qgb25lIGNvbnRleHQgaW50ZXJydXB0 IGF2YWlsYWJsZS4KCkFuZHJlYXMKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KaW9tbXUgbWFpbGluZyBsaXN0CmlvbW11QGxpc3RzLmxpbnV4LWZvdW5kYXRp b24ub3JnCmh0dHBzOi8vbGlzdHMubGludXhmb3VuZGF0aW9uLm9yZy9tYWlsbWFuL2xpc3RpbmZv L2lvbW11 From mboxrd@z Thu Jan 1 00:00:00 1970 From: andreas.herrmann@calxeda.com (Andreas Herrmann) Date: Wed, 25 Sep 2013 12:50:10 +0200 Subject: [PATCH 4/7] iommu/arm-smmu: Check for num_context_irqs > 0 to avoid divide by zero exception In-Reply-To: <20130924154048.GE20774@mudshark.cambridge.arm.com> References: <1380035221-11576-1-git-send-email-andreas.herrmann@calxeda.com> <1380035221-11576-5-git-send-email-andreas.herrmann@calxeda.com> <20130924154048.GE20774@mudshark.cambridge.arm.com> Message-ID: <20130925105010.GA4845@alberich> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Sep 24, 2013 at 11:40:48AM -0400, Will Deacon wrote: > On Tue, Sep 24, 2013 at 04:06:58PM +0100, Andreas Herrmann wrote: > > With the right (or wrong;-) definition of v1 SMMU node in DTB it is > > possible to trigger a division by zero in arm_smmu_init_domain_context > > (if number of context irqs is 0): > > > > if (smmu->version == 1) { > > root_cfg->irptndx = atomic_inc_return(&smmu->irptndx); > > ? root_cfg->irptndx %= smmu->num_context_irqs; > > } else { > > > > Avoid this by checking for num_context_irqs > 0 before trying to > > assign interrupts to contexts. > > > > Signed-off-by: Andreas Herrmann > > --- > > drivers/iommu/arm-smmu.c | 31 +++++++++++++++++-------------- > > 1 file changed, 17 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > > index f5a856e..0dfd255 100644 > > --- a/drivers/iommu/arm-smmu.c > > +++ b/drivers/iommu/arm-smmu.c > > @@ -828,21 +828,24 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain, > > return ret; > > > > root_cfg->cbndx = ret; > > - if (smmu->version == 1) { > > - root_cfg->irptndx = atomic_inc_return(&smmu->irptndx); > > - root_cfg->irptndx %= smmu->num_context_irqs; > > - } else { > > - root_cfg->irptndx = root_cfg->cbndx; > > - } > > > > - irq = smmu->irqs[smmu->num_global_irqs + root_cfg->irptndx]; > > - ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED, > > - "arm-smmu-context-fault", domain); > > - if (IS_ERR_VALUE(ret)) { > > - dev_err(smmu->dev, "failed to request context IRQ %d (%u)\n", > > - root_cfg->irptndx, irq); > > - root_cfg->irptndx = -1; > > - goto out_free_context; > > + if (smmu->num_context_irqs) { > > Can we move this check to probe time, to avoid re-evaluating it every time > we initialise a new domain? Yes, I'll move this check and issue an error message when there is not at least one context interrupt available. Andreas