From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mitchel Humpherys Subject: Re: [PATCH] iommu/arm-smmu: Only return IRQ_NONE if FSR is not set Date: Tue, 06 Oct 2015 13:40:33 -0700 Message-ID: References: <1443226325-28456-1-git-send-email-mitchelh@codeaurora.org> <20151005142402.GF8818@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20151005142402.GF8818-5wv7dgnIgG8@public.gmane.org> (Will Deacon's message of "Mon, 5 Oct 2015 15:24:03 +0100") 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" , Pratik Patel , Rohit Vaswani , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: iommu@lists.linux-foundation.org On Mon, Oct 05 2015 at 03:24:03 PM, Will Deacon wrote: > Hi Mitch, > > On Sat, Sep 26, 2015 at 01:12:05AM +0100, Mitchel Humpherys wrote: >> Currently we return IRQ_NONE from the context fault handler if the FSR >> doesn't actually have the fault bit set (some sort of miswired >> interrupt?) or if the client doesn't register an IOMMU fault handler. >> However, registering a client fault handler is optional, so telling the >> interrupt framework that the interrupt wasn't for this device if the >> client doesn't register a handler isn't exactly accurate. Fix this by >> returning IRQ_HANDLED even if the client doesn't register a handler. >> >> Signed-off-by: Mitchel Humpherys >> --- >> drivers/iommu/arm-smmu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index 48a39dfa9777..95560d447a54 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -653,7 +653,7 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev) >> dev_err_ratelimited(smmu->dev, >> "Unhandled context fault: iova=0x%08lx, fsynr=0x%x, cb=%d\n", >> iova, fsynr, cfg->cbndx); >> - ret = IRQ_NONE; >> + ret = IRQ_HANDLED; >> resume = RESUME_TERMINATE; > > Hmm, but if we haven't actually done anything to rectify the cause of the > fault, what means that we won't take it again immediately? I guess I'm not > understanding the use-case that triggered you to write this patch... Does returning IRQ_NONE actually prevent us from taking another interrupt (despite clearing the FSR below)? We definitely take more interrupts on our platform despite returning IRQ_NONE, but maybe we have something misconfigured... I thought that returning IRQ_NONE would simply affect spurious interrupt accounting (only disabling the interrupt if we took enough of them in close enough succession to flag a misbehaving device). As far as a valid use case, I can't think of one. I just thought we were messing up the spurious interrupt accounting. -Mitch -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project