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=-16.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,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 2FD41C432BE for ; Mon, 2 Aug 2021 14:17:27 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 EE93E60F36 for ; Mon, 2 Aug 2021 14:17:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org EE93E60F36 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=lLng0BSAkn+dICY8gBhiKV9pa856CDrYbJc3jeivG5Q=; b=m24DhadNWCkjZqXS421XGO1bpf OcGJvk2Caf6h1VlzVzrNtCmX+FdwOjnKOX5Mr1FjdWq9kbeKBVLBuff3Gr9iSHUqySIQchjhMexZr u0lAGxM7mzkWPGgg54mi1qZ7LPGlct81atodt6vpuE9Feh+V/5z4LMCQ4Jlf45+tn6g3GRrc0OEuX RTcMMatWSPjGwJ6TuM5kz3CvNFTap75f/F0akzOgTBbHr/yb+TTgEE2oRzWE+n2TPwgl7G0qP/522 lYWuLJHu1FvN1Shnsn9tQJhrseRDluMfJLWemtsVqxq7Tsv7/RXaCuqha7qs2A7FX+5X/8ufrdZeW wMwjskOw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAYje-00Gb31-OA; Mon, 02 Aug 2021 14:16:06 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAYja-00Gb1h-Ri for linux-arm-kernel@lists.infradead.org; Mon, 02 Aug 2021 14:16:04 +0000 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 947C411D4; Mon, 2 Aug 2021 07:15:58 -0700 (PDT) Received: from [10.57.36.146] (unknown [10.57.36.146]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1911B3F66F; Mon, 2 Aug 2021 07:15:56 -0700 (PDT) Subject: Re: [PATCH v2 23/24] iommu/arm-smmu: Allow non-strict in pgtable_quirks interface To: Will Deacon Cc: joro@8bytes.org, iommu@lists.linux-foundation.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, suravee.suthikulpanit@amd.com, baolu.lu@linux.intel.com, john.garry@huawei.com, dianders@chromium.org References: <0fcd1f5225a6b435cbc697551a6247cd435c04db.1627468310.git.robin.murphy@arm.com> <20210802130438.GA28547@willie-the-truck> From: Robin Murphy Message-ID: <3dd7cdae-7111-6ff2-6350-a0e19fe4ab66@arm.com> Date: Mon, 2 Aug 2021 15:15:50 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20210802130438.GA28547@willie-the-truck> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210802_071603_049351_E3732DD2 X-CRM114-Status: GOOD ( 24.87 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2021-08-02 14:04, Will Deacon wrote: > On Wed, Jul 28, 2021 at 04:58:44PM +0100, Robin Murphy wrote: >> To make io-pgtable aware of a flush queue being dynamically enabled, >> allow IO_PGTABLE_QUIRK_NON_STRICT to be set even after a domain has been >> attached to, and hook up the final piece of the puzzle in iommu-dma. >> >> Signed-off-by: Robin Murphy >> --- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++ >> drivers/iommu/arm/arm-smmu/arm-smmu.c | 11 +++++++++++ >> drivers/iommu/dma-iommu.c | 3 +++ >> 3 files changed, 29 insertions(+) >> >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> index 19400826eba7..40fa9cb382c3 100644 >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> @@ -2711,6 +2711,20 @@ static int arm_smmu_enable_nesting(struct iommu_domain *domain) >> return ret; >> } >> >> +static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain, >> + unsigned long quirks) >> +{ >> + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); >> + >> + if (quirks == IO_PGTABLE_QUIRK_NON_STRICT && smmu_domain->pgtbl_ops) { >> + struct io_pgtable *iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops); >> + >> + iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT; >> + return 0; >> + } >> + return -EINVAL; >> +} > > I don't see anything serialising this against a concurrent iommu_unmap(), so > the ordering and atomicity looks quite suspicious to me here. I don't think > it's just the page-table quirks either, but also setting cookie->fq_domain. Heh, I confess to very much taking the cheeky "let's say nothing and see what Will thinks about concurrency" approach here :) The beauty of only allowing relaxation in the strict->non-strict direction is that it shouldn't need serialising as such - it doesn't matter if the update to cookie->fq_domain is observed between iommu_unmap() and iommu_dma_free_iova(), since there's no correctness impact to queueing IOVAs which may already have been invalidated and may or may not have been synced. AFAICS the only condition which matters is that the setting of the io-pgtable quirk must observe fq_domain being set. It feels like there must be enough dependencies on the read side, but we might need an smp_wmb() between the two in iommu_dma_init_fq()? I've also flip-flopped a bit on whether fq_domain needs to be accessed with READ_ONCE/WRITE_ONCE - by the time of posting I'd convinced myself that it was probably OK, but looking again now I suppose this wacky reordering is theoretically possible: iommu_dma_unmap() { bool free_fq = cookie->fq_domain; // == false iommu_unmap(); if (!cookie->fq_domain) // observes new non-NULL value iommu_tlb_sync(); // skipped iommu_dma_free_iova { // inlined if (free_fq) // false queue_iova(); else free_iova_fast(); // Uh-oh! } } so although I still can't see atomicity being a problem I guess we do need it for the sake of reordering at least. Cheers, Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel