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=-15.9 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 96D8DC4708F for ; Tue, 1 Jun 2021 15:59:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6C298613AE for ; Tue, 1 Jun 2021 15:59:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234346AbhFAQBS (ORCPT ); Tue, 1 Jun 2021 12:01:18 -0400 Received: from foss.arm.com ([217.140.110.172]:53250 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232490AbhFAQBR (ORCPT ); Tue, 1 Jun 2021 12:01:17 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 53C1C101E; Tue, 1 Jun 2021 08:59:35 -0700 (PDT) Received: from [10.57.73.64] (unknown [10.57.73.64]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 356943F719; Tue, 1 Jun 2021 08:59:34 -0700 (PDT) Subject: Re: [PATCH 3/4] iommu/amd: Do not sync on page size changes To: Nadav Amit , Joerg Roedel , Will Deacon Cc: iommu@lists.linux-foundation.org, Nadav Amit , Jiajun Cao , linux-kernel@vger.kernel.org References: <20210502070001.1559127-1-namit@vmware.com> <20210502070001.1559127-5-namit@vmware.com> From: Robin Murphy Message-ID: Date: Tue, 1 Jun 2021 16:59:29 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <20210502070001.1559127-5-namit@vmware.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2021-05-02 07:59, Nadav Amit wrote: > From: Nadav Amit > > Some IOMMU architectures perform invalidations regardless of the page > size. In such architectures there is no need to sync when the page size > changes or to regard pgsize when making interim flush in > iommu_iotlb_gather_add_page(). > > Add a "ignore_gather_pgsize" property for each IOMMU-ops to decide > whether gather's pgsize is tracked and triggers a flush. I've objected before[1], and I'll readily object again ;) I still think it's very silly to add a bunch of indirection all over the place to make a helper function not do the main thing it's intended to help with. If you only need trivial address gathering then it's far simpler to just implement trivial address gathering. I suppose if you really want to you could factor out another helper to share the 5 lines of code which ended up in mtk-iommu (see commit f21ae3b10084). Robin. [1] https://lore.kernel.org/linux-iommu/49bae447-d662-e6cf-7500-ab78e3b75dc4@arm.com/ > Cc: Joerg Roedel > Cc: Will Deacon > Cc: Jiajun Cao > Cc: iommu@lists.linux-foundation.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Nadav Amit > --- > drivers/iommu/amd/iommu.c | 1 + > include/linux/iommu.h | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c > index b8cabbbeed71..1849b53f2149 100644 > --- a/drivers/iommu/amd/iommu.c > +++ b/drivers/iommu/amd/iommu.c > @@ -2215,6 +2215,7 @@ const struct iommu_ops amd_iommu_ops = { > .put_resv_regions = generic_iommu_put_resv_regions, > .is_attach_deferred = amd_iommu_is_attach_deferred, > .pgsize_bitmap = AMD_IOMMU_PGSIZES, > + .ignore_gather_pgsize = true, > .flush_iotlb_all = amd_iommu_flush_iotlb_all, > .iotlb_sync = amd_iommu_iotlb_sync, > .def_domain_type = amd_iommu_def_domain_type, > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index 32d448050bf7..1fb2695418e9 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -284,6 +284,7 @@ struct iommu_ops { > int (*def_domain_type)(struct device *dev); > > unsigned long pgsize_bitmap; > + bool ignore_gather_pgsize; > struct module *owner; > }; > > @@ -508,7 +509,7 @@ static inline void iommu_iotlb_gather_add_page(struct iommu_domain *domain, > * a different granularity, then sync the TLB so that the gather > * structure can be rewritten. > */ > - if (gather->pgsize != size || > + if ((gather->pgsize != size && !domain->ops->ignore_gather_pgsize) || > end + 1 < gather->start || start > gather->end + 1) { > if (gather->pgsize) > iommu_iotlb_sync(domain, gather); >