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.3 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=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 AE18AC433ED for ; Wed, 31 Mar 2021 13:10:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 854176196C for ; Wed, 31 Mar 2021 13:10:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235314AbhCaNJy (ORCPT ); Wed, 31 Mar 2021 09:09:54 -0400 Received: from foss.arm.com ([217.140.110.172]:41848 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235733AbhCaNJp (ORCPT ); Wed, 31 Mar 2021 09:09:45 -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 55658D6E; Wed, 31 Mar 2021 06:09:44 -0700 (PDT) Received: from [10.57.24.208] (unknown [10.57.24.208]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3D9523F694; Wed, 31 Mar 2021 06:09:42 -0700 (PDT) Subject: Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE To: Will Deacon Cc: freedreno@lists.freedesktop.org, kvm@vger.kernel.org, Michael Ellerman , linuxppc-dev@lists.ozlabs.org, dri-devel@lists.freedesktop.org, Li Yang , iommu@lists.linux-foundation.org, netdev@vger.kernel.org, linux-arm-msm@vger.kernel.org, virtualization@lists.linux-foundation.org, David Woodhouse , Christoph Hellwig , linux-arm-kernel@lists.infradead.org References: <20210316153825.135976-1-hch@lst.de> <20210316153825.135976-17-hch@lst.de> <20210330131149.GP5908@willie-the-truck> <20210330135801.GA6187@willie-the-truck> <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> <20210331114947.GA7626@willie-the-truck> From: Robin Murphy Message-ID: Date: Wed, 31 Mar 2021 14:09:37 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <20210331114947.GA7626@willie-the-truck> 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-arm-msm@vger.kernel.org On 2021-03-31 12:49, Will Deacon wrote: > On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: >> On 2021-03-30 14:58, Will Deacon wrote: >>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: >>>> On 2021-03-30 14:11, Will Deacon wrote: >>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >>>>>> From: Robin Murphy >>>>>> >>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >>>>>> exporting helpers to get and set it and use those directly in the drivers. >>>>>> >>>>>> This make sure that the iommu.strict parameter also works for the AMD and >>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new >>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >>>>>> represent the default if not overriden by an explicit parameter. >>>>>> >>>>>> Signed-off-by: Robin Murphy . >>>>>> [ported on top of the other iommu_attr changes and added a few small >>>>>> missing bits] >>>>>> Signed-off-by: Christoph Hellwig >>>>>> --- >>>>>> drivers/iommu/amd/iommu.c | 23 +------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >>>>>> drivers/iommu/dma-iommu.c | 9 +-- >>>>>> drivers/iommu/intel/iommu.c | 64 ++++----------------- >>>>>> drivers/iommu/iommu.c | 27 ++++++--- >>>>>> include/linux/iommu.h | 4 +- >>>>>> 8 files changed, 40 insertions(+), 165 deletions(-) >>>>> >>>>> I really like this cleanup, but I can't help wonder if it's going in the >>>>> wrong direction. With SoCs often having multiple IOMMU instances and a >>>>> distinction between "trusted" and "untrusted" devices, then having the >>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound >>>>> unreasonable to me, but this change makes it a global property. >>>> >>>> The intent here was just to streamline the existing behaviour of stuffing a >>>> global property into a domain attribute then pulling it out again in the >>>> illusion that it was in any way per-domain. We're still checking >>>> dev_is_untrusted() before making an actual decision, and it's not like we >>>> can't add more factors at that point if we want to. >>> >>> Like I say, the cleanup is great. I'm just wondering whether there's a >>> better way to express the complicated logic to decide whether or not to use >>> the flush queue than what we end up with: >>> >>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && >>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) >>> >>> which is mixing up globals, device properties and domain properties. The >>> result is that the driver code ends up just using the global to determine >>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, >>> which is a departure from the current way of doing things. >> >> But previously, SMMU only ever saw the global policy piped through the >> domain attribute by iommu_group_alloc_default_domain(), so there's no >> functional change there. > > For DMA domains sure, but I don't think that's the case for unmanaged > domains such as those used by VFIO. Eh? This is only relevant to DMA domains anyway. Flush queues are part of the IOVA allocator that VFIO doesn't even use. It's always been the case that unmanaged domains only use strict invalidation. >> Obviously some of the above checks could be factored out into some kind of >> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need >> to keep in sync. Or maybe we just allow iommu-dma to set >> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're >> treating that as a generic thing now. > > I think a helper that takes a domain would be a good starting point. You mean device, right? The one condition we currently have is at the device level, and there's really nothing inherent to the domain itself that matters (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this). Another idea that's just come to mind is now that IOMMU_DOMAIN_DMA has a standard meaning, maybe we could split out a separate IOMMU_DOMAIN_DMA_STRICT type such that it can all propagate from iommu_get_def_domain_type()? That feels like it might be quite promising, but I'd still do it as an improvement on top of this patch, since it's beyond just cleaning up the abuse of domain attributes to pass a command-line option around. Robin. 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.3 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=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 87EE4C433B4 for ; Wed, 31 Mar 2021 13:10:13 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 D3C3B61968 for ; Wed, 31 Mar 2021 13:10:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D3C3B61968 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4F9RW740JXz3c5b for ; Thu, 1 Apr 2021 00:10:11 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=arm.com (client-ip=217.140.110.172; helo=foss.arm.com; envelope-from=robin.murphy@arm.com; receiver=) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lists.ozlabs.org (Postfix) with ESMTP id 4F9RVj2KH3z301q for ; Thu, 1 Apr 2021 00:09:47 +1100 (AEDT) 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 55658D6E; Wed, 31 Mar 2021 06:09:44 -0700 (PDT) Received: from [10.57.24.208] (unknown [10.57.24.208]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3D9523F694; Wed, 31 Mar 2021 06:09:42 -0700 (PDT) Subject: Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE To: Will Deacon References: <20210316153825.135976-1-hch@lst.de> <20210316153825.135976-17-hch@lst.de> <20210330131149.GP5908@willie-the-truck> <20210330135801.GA6187@willie-the-truck> <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> <20210331114947.GA7626@willie-the-truck> From: Robin Murphy Message-ID: Date: Wed, 31 Mar 2021 14:09:37 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <20210331114947.GA7626@willie-the-truck> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kvm@vger.kernel.org, linux-arm-msm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, dri-devel@lists.freedesktop.org, Li Yang , iommu@lists.linux-foundation.org, Christoph Hellwig , netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, freedreno@lists.freedesktop.org, David Woodhouse , linux-arm-kernel@lists.infradead.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On 2021-03-31 12:49, Will Deacon wrote: > On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: >> On 2021-03-30 14:58, Will Deacon wrote: >>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: >>>> On 2021-03-30 14:11, Will Deacon wrote: >>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >>>>>> From: Robin Murphy >>>>>> >>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >>>>>> exporting helpers to get and set it and use those directly in the drivers. >>>>>> >>>>>> This make sure that the iommu.strict parameter also works for the AMD and >>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new >>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >>>>>> represent the default if not overriden by an explicit parameter. >>>>>> >>>>>> Signed-off-by: Robin Murphy . >>>>>> [ported on top of the other iommu_attr changes and added a few small >>>>>> missing bits] >>>>>> Signed-off-by: Christoph Hellwig >>>>>> --- >>>>>> drivers/iommu/amd/iommu.c | 23 +------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >>>>>> drivers/iommu/dma-iommu.c | 9 +-- >>>>>> drivers/iommu/intel/iommu.c | 64 ++++----------------- >>>>>> drivers/iommu/iommu.c | 27 ++++++--- >>>>>> include/linux/iommu.h | 4 +- >>>>>> 8 files changed, 40 insertions(+), 165 deletions(-) >>>>> >>>>> I really like this cleanup, but I can't help wonder if it's going in the >>>>> wrong direction. With SoCs often having multiple IOMMU instances and a >>>>> distinction between "trusted" and "untrusted" devices, then having the >>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound >>>>> unreasonable to me, but this change makes it a global property. >>>> >>>> The intent here was just to streamline the existing behaviour of stuffing a >>>> global property into a domain attribute then pulling it out again in the >>>> illusion that it was in any way per-domain. We're still checking >>>> dev_is_untrusted() before making an actual decision, and it's not like we >>>> can't add more factors at that point if we want to. >>> >>> Like I say, the cleanup is great. I'm just wondering whether there's a >>> better way to express the complicated logic to decide whether or not to use >>> the flush queue than what we end up with: >>> >>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && >>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) >>> >>> which is mixing up globals, device properties and domain properties. The >>> result is that the driver code ends up just using the global to determine >>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, >>> which is a departure from the current way of doing things. >> >> But previously, SMMU only ever saw the global policy piped through the >> domain attribute by iommu_group_alloc_default_domain(), so there's no >> functional change there. > > For DMA domains sure, but I don't think that's the case for unmanaged > domains such as those used by VFIO. Eh? This is only relevant to DMA domains anyway. Flush queues are part of the IOVA allocator that VFIO doesn't even use. It's always been the case that unmanaged domains only use strict invalidation. >> Obviously some of the above checks could be factored out into some kind of >> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need >> to keep in sync. Or maybe we just allow iommu-dma to set >> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're >> treating that as a generic thing now. > > I think a helper that takes a domain would be a good starting point. You mean device, right? The one condition we currently have is at the device level, and there's really nothing inherent to the domain itself that matters (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this). Another idea that's just come to mind is now that IOMMU_DOMAIN_DMA has a standard meaning, maybe we could split out a separate IOMMU_DOMAIN_DMA_STRICT type such that it can all propagate from iommu_get_def_domain_type()? That feels like it might be quite promising, but I'd still do it as an improvement on top of this patch, since it's beyond just cleaning up the abuse of domain attributes to pass a command-line option around. Robin. 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.3 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=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 E25CDC433B4 for ; Wed, 31 Mar 2021 13:09:50 +0000 (UTC) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 9250C6190A for ; Wed, 31 Mar 2021 13:09:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9250C6190A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 5A23084874; Wed, 31 Mar 2021 13:09:50 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 550OMTP9AKiz; Wed, 31 Mar 2021 13:09:49 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTP id 34A088486E; Wed, 31 Mar 2021 13:09:49 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8FAC9C0013; Wed, 31 Mar 2021 13:09:48 +0000 (UTC) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4F47EC000A; Wed, 31 Mar 2021 13:09:46 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 30C6184871; Wed, 31 Mar 2021 13:09:46 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id k21W6_YsKs6n; Wed, 31 Mar 2021 13:09:45 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp1.osuosl.org (Postfix) with ESMTP id 1ED548486E; Wed, 31 Mar 2021 13:09:44 +0000 (UTC) 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 55658D6E; Wed, 31 Mar 2021 06:09:44 -0700 (PDT) Received: from [10.57.24.208] (unknown [10.57.24.208]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3D9523F694; Wed, 31 Mar 2021 06:09:42 -0700 (PDT) Subject: Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE To: Will Deacon References: <20210316153825.135976-1-hch@lst.de> <20210316153825.135976-17-hch@lst.de> <20210330131149.GP5908@willie-the-truck> <20210330135801.GA6187@willie-the-truck> <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> <20210331114947.GA7626@willie-the-truck> From: Robin Murphy Message-ID: Date: Wed, 31 Mar 2021 14:09:37 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <20210331114947.GA7626@willie-the-truck> Content-Language: en-GB Cc: kvm@vger.kernel.org, Michael Ellerman , linux-arm-msm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, dri-devel@lists.freedesktop.org, Li Yang , iommu@lists.linux-foundation.org, Christoph Hellwig , netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, freedreno@lists.freedesktop.org, David Woodhouse , linux-arm-kernel@lists.infradead.org X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" On 2021-03-31 12:49, Will Deacon wrote: > On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: >> On 2021-03-30 14:58, Will Deacon wrote: >>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: >>>> On 2021-03-30 14:11, Will Deacon wrote: >>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >>>>>> From: Robin Murphy >>>>>> >>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >>>>>> exporting helpers to get and set it and use those directly in the drivers. >>>>>> >>>>>> This make sure that the iommu.strict parameter also works for the AMD and >>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new >>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >>>>>> represent the default if not overriden by an explicit parameter. >>>>>> >>>>>> Signed-off-by: Robin Murphy . >>>>>> [ported on top of the other iommu_attr changes and added a few small >>>>>> missing bits] >>>>>> Signed-off-by: Christoph Hellwig >>>>>> --- >>>>>> drivers/iommu/amd/iommu.c | 23 +------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >>>>>> drivers/iommu/dma-iommu.c | 9 +-- >>>>>> drivers/iommu/intel/iommu.c | 64 ++++----------------- >>>>>> drivers/iommu/iommu.c | 27 ++++++--- >>>>>> include/linux/iommu.h | 4 +- >>>>>> 8 files changed, 40 insertions(+), 165 deletions(-) >>>>> >>>>> I really like this cleanup, but I can't help wonder if it's going in the >>>>> wrong direction. With SoCs often having multiple IOMMU instances and a >>>>> distinction between "trusted" and "untrusted" devices, then having the >>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound >>>>> unreasonable to me, but this change makes it a global property. >>>> >>>> The intent here was just to streamline the existing behaviour of stuffing a >>>> global property into a domain attribute then pulling it out again in the >>>> illusion that it was in any way per-domain. We're still checking >>>> dev_is_untrusted() before making an actual decision, and it's not like we >>>> can't add more factors at that point if we want to. >>> >>> Like I say, the cleanup is great. I'm just wondering whether there's a >>> better way to express the complicated logic to decide whether or not to use >>> the flush queue than what we end up with: >>> >>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && >>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) >>> >>> which is mixing up globals, device properties and domain properties. The >>> result is that the driver code ends up just using the global to determine >>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, >>> which is a departure from the current way of doing things. >> >> But previously, SMMU only ever saw the global policy piped through the >> domain attribute by iommu_group_alloc_default_domain(), so there's no >> functional change there. > > For DMA domains sure, but I don't think that's the case for unmanaged > domains such as those used by VFIO. Eh? This is only relevant to DMA domains anyway. Flush queues are part of the IOVA allocator that VFIO doesn't even use. It's always been the case that unmanaged domains only use strict invalidation. >> Obviously some of the above checks could be factored out into some kind of >> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need >> to keep in sync. Or maybe we just allow iommu-dma to set >> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're >> treating that as a generic thing now. > > I think a helper that takes a domain would be a good starting point. You mean device, right? The one condition we currently have is at the device level, and there's really nothing inherent to the domain itself that matters (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this). Another idea that's just come to mind is now that IOMMU_DOMAIN_DMA has a standard meaning, maybe we could split out a separate IOMMU_DOMAIN_DMA_STRICT type such that it can all propagate from iommu_get_def_domain_type()? That feels like it might be quite promising, but I'd still do it as an improvement on top of this patch, since it's beyond just cleaning up the abuse of domain attributes to pass a command-line option around. Robin. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu 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.3 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=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 AF1EEC433ED for ; Wed, 31 Mar 2021 13:09:50 +0000 (UTC) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (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 40E6061963 for ; Wed, 31 Mar 2021 13:09:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 40E6061963 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=virtualization-bounces@lists.linux-foundation.org Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 08FDC60ACC; Wed, 31 Mar 2021 13:09:50 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dvb_hIw23gw0; Wed, 31 Mar 2021 13:09:49 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id 748DB60AA3; Wed, 31 Mar 2021 13:09:48 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4F939C000C; Wed, 31 Mar 2021 13:09:48 +0000 (UTC) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4F47EC000A; Wed, 31 Mar 2021 13:09:46 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 30C6184871; Wed, 31 Mar 2021 13:09:46 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id k21W6_YsKs6n; Wed, 31 Mar 2021 13:09:45 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp1.osuosl.org (Postfix) with ESMTP id 1ED548486E; Wed, 31 Mar 2021 13:09:44 +0000 (UTC) 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 55658D6E; Wed, 31 Mar 2021 06:09:44 -0700 (PDT) Received: from [10.57.24.208] (unknown [10.57.24.208]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3D9523F694; Wed, 31 Mar 2021 06:09:42 -0700 (PDT) Subject: Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE To: Will Deacon References: <20210316153825.135976-1-hch@lst.de> <20210316153825.135976-17-hch@lst.de> <20210330131149.GP5908@willie-the-truck> <20210330135801.GA6187@willie-the-truck> <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> <20210331114947.GA7626@willie-the-truck> From: Robin Murphy Message-ID: Date: Wed, 31 Mar 2021 14:09:37 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <20210331114947.GA7626@willie-the-truck> Content-Language: en-GB Cc: kvm@vger.kernel.org, Michael Ellerman , linux-arm-msm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, dri-devel@lists.freedesktop.org, Li Yang , iommu@lists.linux-foundation.org, Christoph Hellwig , netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, freedreno@lists.freedesktop.org, David Woodhouse , linux-arm-kernel@lists.infradead.org X-BeenThere: virtualization@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Linux virtualization List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: virtualization-bounces@lists.linux-foundation.org Sender: "Virtualization" On 2021-03-31 12:49, Will Deacon wrote: > On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: >> On 2021-03-30 14:58, Will Deacon wrote: >>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: >>>> On 2021-03-30 14:11, Will Deacon wrote: >>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >>>>>> From: Robin Murphy >>>>>> >>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >>>>>> exporting helpers to get and set it and use those directly in the drivers. >>>>>> >>>>>> This make sure that the iommu.strict parameter also works for the AMD and >>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new >>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >>>>>> represent the default if not overriden by an explicit parameter. >>>>>> >>>>>> Signed-off-by: Robin Murphy . >>>>>> [ported on top of the other iommu_attr changes and added a few small >>>>>> missing bits] >>>>>> Signed-off-by: Christoph Hellwig >>>>>> --- >>>>>> drivers/iommu/amd/iommu.c | 23 +------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >>>>>> drivers/iommu/dma-iommu.c | 9 +-- >>>>>> drivers/iommu/intel/iommu.c | 64 ++++----------------- >>>>>> drivers/iommu/iommu.c | 27 ++++++--- >>>>>> include/linux/iommu.h | 4 +- >>>>>> 8 files changed, 40 insertions(+), 165 deletions(-) >>>>> >>>>> I really like this cleanup, but I can't help wonder if it's going in the >>>>> wrong direction. With SoCs often having multiple IOMMU instances and a >>>>> distinction between "trusted" and "untrusted" devices, then having the >>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound >>>>> unreasonable to me, but this change makes it a global property. >>>> >>>> The intent here was just to streamline the existing behaviour of stuffing a >>>> global property into a domain attribute then pulling it out again in the >>>> illusion that it was in any way per-domain. We're still checking >>>> dev_is_untrusted() before making an actual decision, and it's not like we >>>> can't add more factors at that point if we want to. >>> >>> Like I say, the cleanup is great. I'm just wondering whether there's a >>> better way to express the complicated logic to decide whether or not to use >>> the flush queue than what we end up with: >>> >>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && >>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) >>> >>> which is mixing up globals, device properties and domain properties. The >>> result is that the driver code ends up just using the global to determine >>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, >>> which is a departure from the current way of doing things. >> >> But previously, SMMU only ever saw the global policy piped through the >> domain attribute by iommu_group_alloc_default_domain(), so there's no >> functional change there. > > For DMA domains sure, but I don't think that's the case for unmanaged > domains such as those used by VFIO. Eh? This is only relevant to DMA domains anyway. Flush queues are part of the IOVA allocator that VFIO doesn't even use. It's always been the case that unmanaged domains only use strict invalidation. >> Obviously some of the above checks could be factored out into some kind of >> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need >> to keep in sync. Or maybe we just allow iommu-dma to set >> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're >> treating that as a generic thing now. > > I think a helper that takes a domain would be a good starting point. You mean device, right? The one condition we currently have is at the device level, and there's really nothing inherent to the domain itself that matters (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this). Another idea that's just come to mind is now that IOMMU_DOMAIN_DMA has a standard meaning, maybe we could split out a separate IOMMU_DOMAIN_DMA_STRICT type such that it can all propagate from iommu_get_def_domain_type()? That feels like it might be quite promising, but I'd still do it as an improvement on top of this patch, since it's beyond just cleaning up the abuse of domain attributes to pass a command-line option around. Robin. _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization 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.3 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, 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 4CA75C433B4 for ; Wed, 31 Mar 2021 13:12:12 +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 C824361968 for ; Wed, 31 Mar 2021 13:12:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C824361968 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com 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-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=f5MrIweb0niF67b6umwla36FOz6tsaUSvdv+ucXUotM=; b=XVIQvvH+7QHvCZ5D2smgyTCC2 PUfByeZF0zby/qicoOUDSDb3evv9nM6vj5LzYR7EGJpi8Pkyci9J3Vi9n7VKZHJoB/vx2P+60gr0j r59Ife2cifx8rqTr6uYrAYQ6RpBucgFsgEJu8iwprUgo4SQ4e1wCkz81jI47Ip7kQCxyLYv0paLO8 0RE1zfasJOIWO5X2Ss00rzAvreEVMfOYUPU3SOeLgzJHf2IpDLkrEC/hQFrCiNWT0xpZyj+6/OPZE VN8VseR+S+ZKbKBFr4nJ1nsrSVeAcN0BpfjVgsFNncCHZhkfHpmEmuQaa8Wr0U3NRQXF+XrJpiA1k 2wSLMO1Zg==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lRabd-006cI8-GH; Wed, 31 Mar 2021 13:09:58 +0000 Received: from foss.arm.com ([217.140.110.172]) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lRabS-006cGK-Vy for linux-arm-kernel@lists.infradead.org; Wed, 31 Mar 2021 13:09:51 +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 55658D6E; Wed, 31 Mar 2021 06:09:44 -0700 (PDT) Received: from [10.57.24.208] (unknown [10.57.24.208]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3D9523F694; Wed, 31 Mar 2021 06:09:42 -0700 (PDT) Subject: Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE To: Will Deacon Cc: freedreno@lists.freedesktop.org, kvm@vger.kernel.org, Michael Ellerman , linuxppc-dev@lists.ozlabs.org, dri-devel@lists.freedesktop.org, Li Yang , iommu@lists.linux-foundation.org, netdev@vger.kernel.org, linux-arm-msm@vger.kernel.org, virtualization@lists.linux-foundation.org, David Woodhouse , Christoph Hellwig , linux-arm-kernel@lists.infradead.org References: <20210316153825.135976-1-hch@lst.de> <20210316153825.135976-17-hch@lst.de> <20210330131149.GP5908@willie-the-truck> <20210330135801.GA6187@willie-the-truck> <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> <20210331114947.GA7626@willie-the-truck> From: Robin Murphy Message-ID: Date: Wed, 31 Mar 2021 14:09:37 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <20210331114947.GA7626@willie-the-truck> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210331_140947_643957_E0966321 X-CRM114-Status: GOOD ( 29.35 ) 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-03-31 12:49, Will Deacon wrote: > On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: >> On 2021-03-30 14:58, Will Deacon wrote: >>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: >>>> On 2021-03-30 14:11, Will Deacon wrote: >>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >>>>>> From: Robin Murphy >>>>>> >>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >>>>>> exporting helpers to get and set it and use those directly in the drivers. >>>>>> >>>>>> This make sure that the iommu.strict parameter also works for the AMD and >>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new >>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >>>>>> represent the default if not overriden by an explicit parameter. >>>>>> >>>>>> Signed-off-by: Robin Murphy . >>>>>> [ported on top of the other iommu_attr changes and added a few small >>>>>> missing bits] >>>>>> Signed-off-by: Christoph Hellwig >>>>>> --- >>>>>> drivers/iommu/amd/iommu.c | 23 +------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >>>>>> drivers/iommu/dma-iommu.c | 9 +-- >>>>>> drivers/iommu/intel/iommu.c | 64 ++++----------------- >>>>>> drivers/iommu/iommu.c | 27 ++++++--- >>>>>> include/linux/iommu.h | 4 +- >>>>>> 8 files changed, 40 insertions(+), 165 deletions(-) >>>>> >>>>> I really like this cleanup, but I can't help wonder if it's going in the >>>>> wrong direction. With SoCs often having multiple IOMMU instances and a >>>>> distinction between "trusted" and "untrusted" devices, then having the >>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound >>>>> unreasonable to me, but this change makes it a global property. >>>> >>>> The intent here was just to streamline the existing behaviour of stuffing a >>>> global property into a domain attribute then pulling it out again in the >>>> illusion that it was in any way per-domain. We're still checking >>>> dev_is_untrusted() before making an actual decision, and it's not like we >>>> can't add more factors at that point if we want to. >>> >>> Like I say, the cleanup is great. I'm just wondering whether there's a >>> better way to express the complicated logic to decide whether or not to use >>> the flush queue than what we end up with: >>> >>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && >>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) >>> >>> which is mixing up globals, device properties and domain properties. The >>> result is that the driver code ends up just using the global to determine >>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, >>> which is a departure from the current way of doing things. >> >> But previously, SMMU only ever saw the global policy piped through the >> domain attribute by iommu_group_alloc_default_domain(), so there's no >> functional change there. > > For DMA domains sure, but I don't think that's the case for unmanaged > domains such as those used by VFIO. Eh? This is only relevant to DMA domains anyway. Flush queues are part of the IOVA allocator that VFIO doesn't even use. It's always been the case that unmanaged domains only use strict invalidation. >> Obviously some of the above checks could be factored out into some kind of >> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need >> to keep in sync. Or maybe we just allow iommu-dma to set >> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're >> treating that as a generic thing now. > > I think a helper that takes a domain would be a good starting point. You mean device, right? The one condition we currently have is at the device level, and there's really nothing inherent to the domain itself that matters (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this). Another idea that's just come to mind is now that IOMMU_DOMAIN_DMA has a standard meaning, maybe we could split out a separate IOMMU_DOMAIN_DMA_STRICT type such that it can all propagate from iommu_get_def_domain_type()? That feels like it might be quite promising, but I'd still do it as an improvement on top of this patch, since it's beyond just cleaning up the abuse of domain attributes to pass a command-line option around. Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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.3 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 0C006C433B4 for ; Wed, 31 Mar 2021 13:09:47 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 97D886190A for ; Wed, 31 Mar 2021 13:09:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 97D886190A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EFFE36E1E6; Wed, 31 Mar 2021 13:09:45 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 1C50C6E1E6; Wed, 31 Mar 2021 13:09:44 +0000 (UTC) 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 55658D6E; Wed, 31 Mar 2021 06:09:44 -0700 (PDT) Received: from [10.57.24.208] (unknown [10.57.24.208]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3D9523F694; Wed, 31 Mar 2021 06:09:42 -0700 (PDT) Subject: Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE To: Will Deacon References: <20210316153825.135976-1-hch@lst.de> <20210316153825.135976-17-hch@lst.de> <20210330131149.GP5908@willie-the-truck> <20210330135801.GA6187@willie-the-truck> <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> <20210331114947.GA7626@willie-the-truck> From: Robin Murphy Message-ID: Date: Wed, 31 Mar 2021 14:09:37 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <20210331114947.GA7626@willie-the-truck> Content-Language: en-GB X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kvm@vger.kernel.org, Michael Ellerman , linux-arm-msm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, dri-devel@lists.freedesktop.org, Li Yang , iommu@lists.linux-foundation.org, Christoph Hellwig , netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, freedreno@lists.freedesktop.org, David Woodhouse , linux-arm-kernel@lists.infradead.org Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 2021-03-31 12:49, Will Deacon wrote: > On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: >> On 2021-03-30 14:58, Will Deacon wrote: >>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: >>>> On 2021-03-30 14:11, Will Deacon wrote: >>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >>>>>> From: Robin Murphy >>>>>> >>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >>>>>> exporting helpers to get and set it and use those directly in the drivers. >>>>>> >>>>>> This make sure that the iommu.strict parameter also works for the AMD and >>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new >>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >>>>>> represent the default if not overriden by an explicit parameter. >>>>>> >>>>>> Signed-off-by: Robin Murphy . >>>>>> [ported on top of the other iommu_attr changes and added a few small >>>>>> missing bits] >>>>>> Signed-off-by: Christoph Hellwig >>>>>> --- >>>>>> drivers/iommu/amd/iommu.c | 23 +------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >>>>>> drivers/iommu/dma-iommu.c | 9 +-- >>>>>> drivers/iommu/intel/iommu.c | 64 ++++----------------- >>>>>> drivers/iommu/iommu.c | 27 ++++++--- >>>>>> include/linux/iommu.h | 4 +- >>>>>> 8 files changed, 40 insertions(+), 165 deletions(-) >>>>> >>>>> I really like this cleanup, but I can't help wonder if it's going in the >>>>> wrong direction. With SoCs often having multiple IOMMU instances and a >>>>> distinction between "trusted" and "untrusted" devices, then having the >>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound >>>>> unreasonable to me, but this change makes it a global property. >>>> >>>> The intent here was just to streamline the existing behaviour of stuffing a >>>> global property into a domain attribute then pulling it out again in the >>>> illusion that it was in any way per-domain. We're still checking >>>> dev_is_untrusted() before making an actual decision, and it's not like we >>>> can't add more factors at that point if we want to. >>> >>> Like I say, the cleanup is great. I'm just wondering whether there's a >>> better way to express the complicated logic to decide whether or not to use >>> the flush queue than what we end up with: >>> >>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && >>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) >>> >>> which is mixing up globals, device properties and domain properties. The >>> result is that the driver code ends up just using the global to determine >>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, >>> which is a departure from the current way of doing things. >> >> But previously, SMMU only ever saw the global policy piped through the >> domain attribute by iommu_group_alloc_default_domain(), so there's no >> functional change there. > > For DMA domains sure, but I don't think that's the case for unmanaged > domains such as those used by VFIO. Eh? This is only relevant to DMA domains anyway. Flush queues are part of the IOVA allocator that VFIO doesn't even use. It's always been the case that unmanaged domains only use strict invalidation. >> Obviously some of the above checks could be factored out into some kind of >> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need >> to keep in sync. Or maybe we just allow iommu-dma to set >> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're >> treating that as a generic thing now. > > I think a helper that takes a domain would be a good starting point. You mean device, right? The one condition we currently have is at the device level, and there's really nothing inherent to the domain itself that matters (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this). Another idea that's just come to mind is now that IOMMU_DOMAIN_DMA has a standard meaning, maybe we could split out a separate IOMMU_DOMAIN_DMA_STRICT type such that it can all propagate from iommu_get_def_domain_type()? That feels like it might be quite promising, but I'd still do it as an improvement on top of this patch, since it's beyond just cleaning up the abuse of domain attributes to pass a command-line option around. Robin. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel