From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 035FAC3F2D1 for ; Wed, 4 Mar 2020 14:08:44 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A961E20848 for ; Wed, 4 Mar 2020 14:08:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="ziaLQQMT" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A961E20848 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 3105F6B0003; Wed, 4 Mar 2020 09:08:43 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2C1186B0005; Wed, 4 Mar 2020 09:08:43 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 188CC6B0006; Wed, 4 Mar 2020 09:08:43 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0059.hostedemail.com [216.40.44.59]) by kanga.kvack.org (Postfix) with ESMTP id F17F46B0003 for ; Wed, 4 Mar 2020 09:08:42 -0500 (EST) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id A5DF7180AD806 for ; Wed, 4 Mar 2020 14:08:42 +0000 (UTC) X-FDA: 76557860484.16.woman45_86349f8fd683a X-HE-Tag: woman45_86349f8fd683a X-Filterd-Recvd-Size: 6959 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by imf28.hostedemail.com (Postfix) with ESMTP for ; Wed, 4 Mar 2020 14:08:42 +0000 (UTC) Received: by mail-wr1-f66.google.com with SMTP id v4so2547710wrs.8 for ; Wed, 04 Mar 2020 06:08:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=TdUvMeA1bV5hMUNi9Uc9NGFwMrBhsdhhT6ZmBGFQFek=; b=ziaLQQMTnCJ7YSxd5sV95qsFJ5sMZH3rtvhOIq0q24OYVAeSN/juNgdJKUXVth7dmC +1cTjWWP/8vKz1/4KANL8PnAFbMAGOTkEIQvePO7fVK5ahr5EsdFhICYSslnU6Q/U3YP eaY+UNIzqcl1QfHqCD3sojdDlJ5DGbhtfT0Zz1unRHzAIht9z91VeCVs9QSOLqrFrm47 yYvlbSX/dMxmGH7JaXW06sQkDqq+gICORzdo63QS61gI1f3znBgEbBwZYbXIUzNXW34m 9I41VXjpTFujhXGbhcU+sCmTSxEf+GPlgeOR7llOnK3kpBjcJOHvuvpYAM84w3umrZVW WibQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=TdUvMeA1bV5hMUNi9Uc9NGFwMrBhsdhhT6ZmBGFQFek=; b=MbKNLW+vIGdBxSHGmz1TrUCs1NoYcSS4ThpwEbJT7H1WQYMrdmwQfJPvXlDFyY1yr4 0q4Rv0a7WKwFuxCi3eEeSvcQYNlOVYgrq2uRJzRwm6VnjNv7sq6PSzEch4HWtGQjlqGJ 3uiCjh1G3YUHtlJwWLnVEkL7UBBn1rQ3P7HD6/yjmB4JRp1WCJXR539c7u3QRz+ALGP4 IFWZIKlFAug9t+w6gxMbhIa5SX/dMLIWbg53P+0vkGSi5Td+mPRjvhT5rVikOJGTAYk/ bfvcUzBVfd28GD3vTXyg4Og5XjpXCAXr2kselJJ74IwQ/ZoEFoHNV5msouKqvaZ4a+uf xENw== X-Gm-Message-State: ANhLgQ3BLBkQ8PgWBIScmB1zWv2lGcEyw874P596eV6uPy2mT85ZiYOo XY/KFh7l9wKn+xhfuBGh6dsVOw== X-Google-Smtp-Source: ADFU+vv0sKAwmtjvye94x9YIogv5zw/a9rZvkYsOZEBQWXGc8PUw1JEbwM9HPDBC/WwH/q9h2REFKg== X-Received: by 2002:a5d:4b82:: with SMTP id b2mr4303504wrt.102.1583330920810; Wed, 04 Mar 2020 06:08:40 -0800 (PST) Received: from myrica ([2001:171b:c9a8:fbc0:116c:c27a:3e7f:5eaf]) by smtp.gmail.com with ESMTPSA id p17sm36750450wre.89.2020.03.04.06.08.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Mar 2020 06:08:39 -0800 (PST) Date: Wed, 4 Mar 2020 15:08:33 +0100 From: Jean-Philippe Brucker To: Jonathan Cameron Cc: iommu@lists.linux-foundation.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-mm@kvack.org, joro@8bytes.org, robh+dt@kernel.org, mark.rutland@arm.com, catalin.marinas@arm.com, will@kernel.org, robin.murphy@arm.com, kevin.tian@intel.com, baolu.lu@linux.intel.com, jacob.jun.pan@linux.intel.com, christian.koenig@amd.com, yi.l.liu@intel.com, zhangfei.gao@linaro.org, Jean-Philippe Brucker Subject: Re: [PATCH v4 23/26] iommu/arm-smmu-v3: Add stall support for platform devices Message-ID: <20200304140833.GB646000@myrica> References: <20200224182401.353359-1-jean-philippe@linaro.org> <20200224182401.353359-24-jean-philippe@linaro.org> <20200227181726.00007c9a@Huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200227181726.00007c9a@Huawei.com> X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Feb 27, 2020 at 06:17:26PM +0000, Jonathan Cameron wrote: > On Mon, 24 Feb 2020 19:23:58 +0100 > Jean-Philippe Brucker wrote: > > > From: Jean-Philippe Brucker > > > > The SMMU provides a Stall model for handling page faults in platform > > devices. It is similar to PCI PRI, but doesn't require devices to have > > their own translation cache. Instead, faulting transactions are parked and > > the OS is given a chance to fix the page tables and retry the transaction. > > > > Enable stall for devices that support it (opt-in by firmware). When an > > event corresponds to a translation error, call the IOMMU fault handler. If > > the fault is recoverable, it will call us back to terminate or continue > > the stall. > > > > Signed-off-by: Jean-Philippe Brucker > One question inline. > > Thanks, > > > --- > > drivers/iommu/arm-smmu-v3.c | 271 ++++++++++++++++++++++++++++++++++-- > > drivers/iommu/of_iommu.c | 5 +- > > include/linux/iommu.h | 2 + > > 3 files changed, 269 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > > index 6a5987cce03f..da5dda5ba26a 100644 > > --- a/drivers/iommu/arm-smmu-v3.c > > +++ b/drivers/iommu/arm-smmu-v3.c > > @@ -374,6 +374,13 @@ > > > > +/* > > + * arm_smmu_flush_evtq - wait until all events currently in the queue have been > > + * consumed. > > + * > > + * Wait until the evtq thread finished a batch, or until the queue is empty. > > + * Note that we don't handle overflows on q->batch. If it occurs, just wait for > > + * the queue to be empty. > > + */ > > +static int arm_smmu_flush_evtq(void *cookie, struct device *dev, int pasid) > > +{ > > + int ret; > > + u64 batch; > > + struct arm_smmu_device *smmu = cookie; > > + struct arm_smmu_queue *q = &smmu->evtq.q; > > + > > + spin_lock(&q->wq.lock); > > + if (queue_sync_prod_in(q) == -EOVERFLOW) > > + dev_err(smmu->dev, "evtq overflow detected -- requests lost\n"); > > + > > + batch = q->batch; > > So this is trying to be sure we have advanced the queue 2 spots? So we call arm_smmu_flush_evtq() before decommissioning a PASID, to make sure that there aren't any pending event for this PASID languishing in the fault queues. The main test is queue_empty(). If that succeeds then we know that there aren't any pending event (and the PASID is safe to reuse). But if new events are constantly added to the queue then we wait for the evtq thread to handle a full batch, where one batch corresponds to the queue size. For that we take the batch number when entering flush(), and wait for the evtq thread to increment it twice. > Is there a potential race here? q->batch could have updated before we take > a local copy. Yes we're just checking on the progress of the evtq thread. All accesses to batch are made while holding the wq lock. Flush is a rare event so the lock isn't contended, but the wake_up() that this patch introduces in arm_smmu_evtq_thread() does add some overhead (0.85% of arm_smmu_evtq_thread(), according to perf). It would be nice to get rid of it but I haven't found anything clever yet. Thanks, Jean > > > + ret = wait_event_interruptible_locked(q->wq, queue_empty(&q->llq) || > > + q->batch >= batch + 2); > > + spin_unlock(&q->wq.lock); > > + > > + return ret; > > +} > > + > ... >