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=-10.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 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 866FBC433B4 for ; Wed, 7 Apr 2021 09:59:16 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F0DA86135D for ; Wed, 7 Apr 2021 09:59:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F0DA86135D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=icLY62p29YbWvlapd5wW04AYVXcNDenQHBgK682nvWk=; b=AdzOAFOA4uSX8rVFjnPGnzJ63 uAMl5XtKKn6K3aQoXh029qTQevJUT7EsYoaeq/VT0hhUnseguke4IcRJ7IerENC3Jv8AIyZBVfZmh q0nRseyUei1tBo5YsA1eTdYFtoUsY9MLZDeRIfB5Yznxi5BRT5oSs+uuesvddPMLGtQg9YK16FMbC 3C12kslj+xMhK2hTrPcXz4xHylXKpYGMTPmxcALx/e8Q/5pCq22v7QNl1arTSGA/Iu5i/gc87n0l6 jQpofYHBSI+CSZyOQ166nWfg/szvexbq3v5PeEgw5c/LMlShRza/kdKEUOhhe8P+Fa2HLZWjasUlp Cftkffcew==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lU4wC-004fGl-W8; Wed, 07 Apr 2021 09:57:31 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lU4vy-004fDi-4J for linux-arm-kernel@lists.infradead.org; Wed, 07 Apr 2021 09:57:24 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 5DB6461382; Wed, 7 Apr 2021 09:57:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1617789432; bh=fhKlUlM+Gs719SnNxTdgOJWeVJK859Ec/KnqmXBj+hg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=DV/gbYWoKUl9kJouL2WkPQEvxW7khfD7FeSZhy0EZG69PsyXUZhjHJAl9waorjQY6 Dqh46XGM8ZBuIK1htL3uB5Y9454Hb5R+pDFi0Hx4kcj8bSR3SeQu4sDer2lZVynXtk 5nPECX8HDQtuihBnxn0GhJwgONtLYfd6ktspPTNSnVSfjfT991WsmX546go+p9iNgn SaRl7QrwajJAC7gX3XNQNmCgR6fbvBHNLHbw+Rz4xDTFgRTOXsRAtyVL4J2B8yWcpk uDE+zH3eNLe+pvg2u4Q4/6VY2TX03FdRQVdr6gHHLLVTsHAZb/c/Xe+YGYVxOMz58W OA6pu9VIQL4/g== Date: Wed, 7 Apr 2021 10:57:06 +0100 From: Will Deacon To: isaacm@codeaurora.org Cc: iommu@lists.linux-foundation.org, linux-arm-kernel@lists.infradead.org, robin.murphy@arm.com, pratikp@codeaurora.org, baolu.lu@linux.intel.com Subject: Re: [RFC PATCH v3 09/12] iommu/io-pgtable-arm: Implement arm_lpae_unmap_pages() Message-ID: <20210407095706.GB15057@willie-the-truck> References: <20210405191112.28192-1-isaacm@codeaurora.org> <20210405191112.28192-10-isaacm@codeaurora.org> <20210406121552.GF13747@willie-the-truck> <8f78f5d051c9d40981fc6868c9245cd3@codeaurora.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <8f78f5d051c9d40981fc6868c9245cd3@codeaurora.org> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210407_105719_744837_2C1F7C21 X-CRM114-Status: GOOD ( 36.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Apr 06, 2021 at 02:02:26PM -0700, isaacm@codeaurora.org wrote: > On 2021-04-06 05:15, Will Deacon wrote: > > On Mon, Apr 05, 2021 at 12:11:09PM -0700, Isaac J. Manjarres wrote: > > > Implement the unmap_pages() callback for the ARM LPAE io-pgtable > > > format. > > > > > > Signed-off-by: Isaac J. Manjarres > > > Suggested-by: Will Deacon > > > --- > > > drivers/iommu/io-pgtable-arm.c | 124 > > > +++++++++++++++++++++++++++------ > > > 1 file changed, 104 insertions(+), 20 deletions(-) > > > > [...] > > > > > +static size_t arm_lpae_unmap_pages(struct io_pgtable_ops *ops, > > > unsigned long iova, > > > + size_t pgsize, size_t pgcount, > > > + struct iommu_iotlb_gather *gather) > > > +{ > > > + struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops); > > > + struct io_pgtable_cfg *cfg = &data->iop.cfg; > > > + arm_lpae_iopte *ptep = data->pgd; > > > + long iaext = (s64)iova >> cfg->ias; > > > + size_t unmapped = 0, unmapped_page; > > > + int last_lvl; > > > + size_t table_size, pages, tbl_offset, max_entries; > > > + > > > + if (WARN_ON(!pgsize || (pgsize & cfg->pgsize_bitmap) != pgsize || > > > !pgcount)) > > > + return 0; > > > + > > > + if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1) > > > + iaext = ~iaext; > > > + if (WARN_ON(iaext)) > > > + return 0; > > > + > > > + /* > > > + * Calculating the page table size here helps avoid situations where > > > + * a page range that is being unmapped may be mapped at the same > > > level > > > + * but not mapped by the same tables. Allowing such a scenario to > > > + * occur can complicate the logic in arm_lpae_split_blk_unmap(). > > > + */ > > > + last_lvl = ARM_LPAE_BLOCK_SIZE_LVL(pgsize, data); > > > + > > > + if (last_lvl == data->start_level) > > > + table_size = ARM_LPAE_PGD_SIZE(data); > > > + else > > > + table_size = ARM_LPAE_GRANULE(data); > > > + > > > + max_entries = table_size / sizeof(*ptep); > > > + > > > + while (pgcount) { > > > + tbl_offset = ARM_LPAE_LVL_IDX(iova, last_lvl, data); > > > + pages = min_t(size_t, pgcount, max_entries - tbl_offset); > > > + unmapped_page = __arm_lpae_unmap(data, gather, iova, pgsize, > > > + pages, data->start_level, > > > + ptep); > > > + if (!unmapped_page) > > > + break; > > > + > > > + unmapped += unmapped_page; > > > + iova += unmapped_page; > > > + pgcount -= pages; > > > + } > > > > Robin has some comments on the first version of this patch, and I > > don't think you > > addressed them: > > > > https://lore.kernel.org/r/b93fa0b1-e2a4-1aad-8b88-4d0dfecdfef7@arm.com > > > > I'm inclined to agree that iterating here doesn't make a lot of sense -- > > we've already come back out of the page-table walk, so I think we should > > just return to the caller (who is well prepared to handle a partial > > unmap). > > Same for the map side of things. > > > > If we get numbers showing that this is causing a performance issue, then > > we should rework the page-table code to handle this at the lower level > > (because I doubt the loop that you have is really worse than returning > > to > > the caller anyway). > > > Sorry, I seem to have overlooked those comments. > > I will go ahead and address them. I think it might be ideal to try to do > as much work as possible in the io-pgtable level, so as to minimize the > number > of indirect calls incurred by jumping back and forth between iommu fwk, > iommu > driver, and io-pgtable code. > > Perhaps we can try something like how the linear mappings are created on > arm64 i.e. > on the previous level, we can determine how many pages can be unmapped in > one page table > in one iteration, and on the subsequent iterations, we can tackle another > page table at > the lower level. Looking at the code, it doesn't seem too difficult to add > this in. Thoughts? I don't object to getting there eventually, but as an initial step I think returning back to the caller is perfectly reasonable and will probably make the patches easier to review. In other words, implement the simple (correct) thing first, and then have subsequent patches to improve it. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel