From patchwork Mon Aug 22 13:42:24 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Philippe Brucker X-Patchwork-Id: 9293589 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 23E31607F0 for ; Mon, 22 Aug 2016 13:47:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 185EC28847 for ; Mon, 22 Aug 2016 13:47:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0CB47289BF; Mon, 22 Aug 2016 13:47:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 5E89D28847 for ; Mon, 22 Aug 2016 13:47:01 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bbpXc-00071l-J2; Mon, 22 Aug 2016 13:45:28 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bbpVI-0004Bv-G0 for linux-arm-kernel@lists.infradead.org; Mon, 22 Aug 2016 13:43:18 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5746A4FD; Mon, 22 Aug 2016 06:44:21 -0700 (PDT) Received: from e106794-lin.cambridge.arm.com (e106794-lin.cambridge.arm.com [10.1.210.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 8DC113F215; Mon, 22 Aug 2016 06:42:42 -0700 (PDT) From: Jean-Philippe Brucker To: iommu@lists.linux-foundation.org, linux-arm-kernel@lists.infradead.org Subject: [PATCH] iommu/arm-smmu: Fix event queues synchronization Date: Mon, 22 Aug 2016 14:42:24 +0100 Message-Id: <20160822134224.18574-1-jean-philippe.brucker@arm.com> X-Mailer: git-send-email 2.9.2 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160822_064305_049348_79AB5AB7 X-CRM114-Status: GOOD ( 15.45 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: robin.murphy@arm.com, joro@8bytes.org, will.deacon@arm.com MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP SMMUv3 only sends interrupts for event queues (EVTQ and PRIQ) when they transition from empty to non-empty. At the moment, if the SMMU adds new items to a queue before the event thread finished consuming a previous batch, the driver ignores any new item. The queue is then stuck in non-empty state and all subsequent events will be lost. As an example, consider the following flow, where (P, C) is the SMMU view of producer/consumer indices, and (p, c) the driver view. P C | p c 1. SMMU appends a PPR to the PRI queue, 1 0 | 0 0 sends an MSI 2. PRIQ handler is called. 1 0 | 1 0 3. SMMU appends a PPR to the PRI queue. 2 0 | 1 0 4. PRIQ thread removes the first element. 2 1 | 1 1 5. PRIQ thread believes that the queue is empty, goes into idle indefinitely. To avoid this, always synchronize the producer index and drain the queue once before leaving an event handler. In order to prevent races on the local producer index, move all event queue handling into the threads. Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/arm-smmu-v3.c | 150 +++++++++++++++++++------------------------- 1 file changed, 66 insertions(+), 84 deletions(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 1cd526d..a9219e6 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1164,36 +1164,66 @@ static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev) struct arm_smmu_queue *q = &smmu->evtq.q; u64 evt[EVTQ_ENT_DWORDS]; - while (!queue_remove_raw(q, evt)) { - u8 id = evt[0] >> EVTQ_0_ID_SHIFT & EVTQ_0_ID_MASK; + do { + while (!queue_remove_raw(q, evt)) { + u8 id = evt[0] >> EVTQ_0_ID_SHIFT & EVTQ_0_ID_MASK; - dev_info(smmu->dev, "event 0x%02x received:\n", id); - for (i = 0; i < ARRAY_SIZE(evt); ++i) - dev_info(smmu->dev, "\t0x%016llx\n", - (unsigned long long)evt[i]); - } + dev_info(smmu->dev, "event 0x%02x received:\n", id); + for (i = 0; i < ARRAY_SIZE(evt); ++i) + dev_info(smmu->dev, "\t0x%016llx\n", + (unsigned long long)evt[i]); + + } + + /* + * Not much we can do on overflow, so scream and pretend we're + * trying harder. + */ + if (queue_sync_prod(q) == -EOVERFLOW) + dev_err(smmu->dev, "EVTQ overflow detected -- events lost\n"); + } while (!queue_empty(q)); /* Sync our overflow flag, as we believe we're up to speed */ q->cons = Q_OVF(q, q->prod) | Q_WRP(q, q->cons) | Q_IDX(q, q->cons); return IRQ_HANDLED; } -static irqreturn_t arm_smmu_evtq_handler(int irq, void *dev) -{ - irqreturn_t ret = IRQ_WAKE_THREAD; - struct arm_smmu_device *smmu = dev; - struct arm_smmu_queue *q = &smmu->evtq.q; - - /* - * Not much we can do on overflow, so scream and pretend we're - * trying harder. - */ - if (queue_sync_prod(q) == -EOVERFLOW) - dev_err(smmu->dev, "EVTQ overflow detected -- events lost\n"); - else if (queue_empty(q)) - ret = IRQ_NONE; +static void arm_smmu_handle_ppr(struct arm_smmu_device *smmu, u64 *evt) +{ + u32 sid, ssid; + u16 grpid; + bool ssv, last; + + sid = evt[0] >> PRIQ_0_SID_SHIFT & PRIQ_0_SID_MASK; + ssv = evt[0] & PRIQ_0_SSID_V; + ssid = ssv ? evt[0] >> PRIQ_0_SSID_SHIFT & PRIQ_0_SSID_MASK : 0; + last = evt[0] & PRIQ_0_PRG_LAST; + grpid = evt[1] >> PRIQ_1_PRG_IDX_SHIFT & PRIQ_1_PRG_IDX_MASK; + + dev_info(smmu->dev, "unexpected PRI request received:\n"); + dev_info(smmu->dev, + "\tsid 0x%08x.0x%05x: [%u%s] %sprivileged %s%s%s access at iova 0x%016llx\n", + sid, ssid, grpid, last ? "L" : "", + evt[0] & PRIQ_0_PERM_PRIV ? "" : "un", + evt[0] & PRIQ_0_PERM_READ ? "R" : "", + evt[0] & PRIQ_0_PERM_WRITE ? "W" : "", + evt[0] & PRIQ_0_PERM_EXEC ? "X" : "", + evt[1] & PRIQ_1_ADDR_MASK << PRIQ_1_ADDR_SHIFT); + + if (last) { + struct arm_smmu_cmdq_ent cmd = { + .opcode = CMDQ_OP_PRI_RESP, + .substream_valid = ssv, + .pri = { + .sid = sid, + .ssid = ssid, + .grpid = grpid, + .resp = PRI_RESP_DENY, + }, + }; - return ret; + arm_smmu_cmdq_issue_cmd(smmu, &cmd); + } } static irqreturn_t arm_smmu_priq_thread(int irq, void *dev) @@ -1202,63 +1232,19 @@ static irqreturn_t arm_smmu_priq_thread(int irq, void *dev) struct arm_smmu_queue *q = &smmu->priq.q; u64 evt[PRIQ_ENT_DWORDS]; - while (!queue_remove_raw(q, evt)) { - u32 sid, ssid; - u16 grpid; - bool ssv, last; - - sid = evt[0] >> PRIQ_0_SID_SHIFT & PRIQ_0_SID_MASK; - ssv = evt[0] & PRIQ_0_SSID_V; - ssid = ssv ? evt[0] >> PRIQ_0_SSID_SHIFT & PRIQ_0_SSID_MASK : 0; - last = evt[0] & PRIQ_0_PRG_LAST; - grpid = evt[1] >> PRIQ_1_PRG_IDX_SHIFT & PRIQ_1_PRG_IDX_MASK; - - dev_info(smmu->dev, "unexpected PRI request received:\n"); - dev_info(smmu->dev, - "\tsid 0x%08x.0x%05x: [%u%s] %sprivileged %s%s%s access at iova 0x%016llx\n", - sid, ssid, grpid, last ? "L" : "", - evt[0] & PRIQ_0_PERM_PRIV ? "" : "un", - evt[0] & PRIQ_0_PERM_READ ? "R" : "", - evt[0] & PRIQ_0_PERM_WRITE ? "W" : "", - evt[0] & PRIQ_0_PERM_EXEC ? "X" : "", - evt[1] & PRIQ_1_ADDR_MASK << PRIQ_1_ADDR_SHIFT); - - if (last) { - struct arm_smmu_cmdq_ent cmd = { - .opcode = CMDQ_OP_PRI_RESP, - .substream_valid = ssv, - .pri = { - .sid = sid, - .ssid = ssid, - .grpid = grpid, - .resp = PRI_RESP_DENY, - }, - }; + do { + while (!queue_remove_raw(q, evt)) + arm_smmu_handle_ppr(smmu, evt); - arm_smmu_cmdq_issue_cmd(smmu, &cmd); - } - } + if (queue_sync_prod(q) == -EOVERFLOW) + dev_err(smmu->dev, "PRIQ overflow detected -- requests lost\n"); + } while (!queue_empty(q)); /* Sync our overflow flag, as we believe we're up to speed */ q->cons = Q_OVF(q, q->prod) | Q_WRP(q, q->cons) | Q_IDX(q, q->cons); return IRQ_HANDLED; } -static irqreturn_t arm_smmu_priq_handler(int irq, void *dev) -{ - irqreturn_t ret = IRQ_WAKE_THREAD; - struct arm_smmu_device *smmu = dev; - struct arm_smmu_queue *q = &smmu->priq.q; - - /* PRIQ overflow indicates a programming error */ - if (queue_sync_prod(q) == -EOVERFLOW) - dev_err(smmu->dev, "PRIQ overflow detected -- requests lost\n"); - else if (queue_empty(q)) - ret = IRQ_NONE; - - return ret; -} - static irqreturn_t arm_smmu_cmdq_sync_handler(int irq, void *dev) { /* We don't actually use CMD_SYNC interrupts for anything */ @@ -1291,15 +1277,11 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void *dev) if (active & GERROR_MSI_GERROR_ABT_ERR) dev_warn(smmu->dev, "GERROR MSI write aborted\n"); - if (active & GERROR_MSI_PRIQ_ABT_ERR) { + if (active & GERROR_MSI_PRIQ_ABT_ERR) dev_warn(smmu->dev, "PRIQ MSI write aborted\n"); - arm_smmu_priq_handler(irq, smmu->dev); - } - if (active & GERROR_MSI_EVTQ_ABT_ERR) { + if (active & GERROR_MSI_EVTQ_ABT_ERR) dev_warn(smmu->dev, "EVTQ MSI write aborted\n"); - arm_smmu_evtq_handler(irq, smmu->dev); - } if (active & GERROR_MSI_CMDQ_ABT_ERR) { dev_warn(smmu->dev, "CMDQ MSI write aborted\n"); @@ -2207,10 +2189,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) /* Request interrupt lines */ irq = smmu->evtq.q.irq; if (irq) { - ret = devm_request_threaded_irq(smmu->dev, irq, - arm_smmu_evtq_handler, + ret = devm_request_threaded_irq(smmu->dev, irq, NULL, arm_smmu_evtq_thread, - 0, "arm-smmu-v3-evtq", smmu); + IRQF_ONESHOT, + "arm-smmu-v3-evtq", smmu); if (ret < 0) dev_warn(smmu->dev, "failed to enable evtq irq\n"); } @@ -2235,10 +2217,10 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu) if (smmu->features & ARM_SMMU_FEAT_PRI) { irq = smmu->priq.q.irq; if (irq) { - ret = devm_request_threaded_irq(smmu->dev, irq, - arm_smmu_priq_handler, + ret = devm_request_threaded_irq(smmu->dev, irq, NULL, arm_smmu_priq_thread, - 0, "arm-smmu-v3-priq", + IRQF_ONESHOT, + "arm-smmu-v3-priq", smmu); if (ret < 0) dev_warn(smmu->dev,