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,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 5F51DC433E1 for ; Tue, 30 Mar 2021 16:29:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0EA59619CF for ; Tue, 30 Mar 2021 16:29:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232001AbhC3Q2x (ORCPT ); Tue, 30 Mar 2021 12:28:53 -0400 Received: from foss.arm.com ([217.140.110.172]:40940 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232250AbhC3Q21 (ORCPT ); Tue, 30 Mar 2021 12:28:27 -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 79B92D6E; Tue, 30 Mar 2021 09:28:26 -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 C45EC3F719; Tue, 30 Mar 2021 09:28:22 -0700 (PDT) Subject: Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE To: Will Deacon Cc: Christoph Hellwig , Joerg Roedel , Li Yang , Michael Ellerman , David Woodhouse , Lu Baolu , linuxppc-dev@lists.ozlabs.org, linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, iommu@lists.linux-foundation.org, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org References: <20210316153825.135976-1-hch@lst.de> <20210316153825.135976-17-hch@lst.de> <20210330131149.GP5908@willie-the-truck> <20210330135801.GA6187@willie-the-truck> From: Robin Murphy Message-ID: <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> Date: Tue, 30 Mar 2021 17:28:19 +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: <20210330135801.GA6187@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-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. 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. >>> For example, see the recent patch from Lu Baolu: >>> >>> https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com >> >> Erm, this patch is based on that one, it's right there in the context :/ > > Ah, sorry, I didn't spot that! I was just trying to illustrate that this > is per-device. Sure, I understand - and I'm just trying to bang home that despite appearances it's never actually been treated as such for SMMU, so anything that's wrong after this change was already wrong before. 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,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 B3935C433C1 for ; Tue, 30 Mar 2021 16:28:55 +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 BA9E86190A for ; Tue, 30 Mar 2021 16:28:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BA9E86190A 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 4F8vyr4tcgz3c6R for ; Wed, 31 Mar 2021 03:28:52 +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 4F8vyS6DsVz3bpj for ; Wed, 31 Mar 2021 03:28:30 +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 79B92D6E; Tue, 30 Mar 2021 09:28:26 -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 C45EC3F719; Tue, 30 Mar 2021 09:28:22 -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> From: Robin Murphy Message-ID: <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> Date: Tue, 30 Mar 2021 17:28:19 +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: <20210330135801.GA6187@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: freedreno@lists.freedesktop.org, kvm@vger.kernel.org, Joerg Roedel , 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, Lu Baolu Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" 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. 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. >>> For example, see the recent patch from Lu Baolu: >>> >>> https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com >> >> Erm, this patch is based on that one, it's right there in the context :/ > > Ah, sorry, I didn't spot that! I was just trying to illustrate that this > is per-device. Sure, I understand - and I'm just trying to bang home that despite appearances it's never actually been treated as such for SMMU, so anything that's wrong after this change was already wrong before. 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,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 17697C433DB for ; Tue, 30 Mar 2021 16:28:33 +0000 (UTC) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.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 6FC63619C8 for ; Tue, 30 Mar 2021 16:28:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6FC63619C8 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 smtp2.osuosl.org (Postfix) with ESMTP id 07F21401D1; Tue, 30 Mar 2021 16:28:32 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JRwjUIjiogpo; Tue, 30 Mar 2021 16:28:31 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTP id E7DDB401BE; Tue, 30 Mar 2021 16:28:30 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id B4153C000B; Tue, 30 Mar 2021 16:28:30 +0000 (UTC) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 27238C000A; Tue, 30 Mar 2021 16:28:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 12227606FD; Tue, 30 Mar 2021 16:28:29 +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 ReP8siV3L_vY; Tue, 30 Mar 2021 16:28:27 +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 smtp3.osuosl.org (Postfix) with ESMTP id 9C22C600B9; Tue, 30 Mar 2021 16:28:27 +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 79B92D6E; Tue, 30 Mar 2021 09:28:26 -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 C45EC3F719; Tue, 30 Mar 2021 09:28:22 -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> From: Robin Murphy Message-ID: <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> Date: Tue, 30 Mar 2021 17:28:19 +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: <20210330135801.GA6187@willie-the-truck> Content-Language: en-GB 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 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-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. 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. >>> For example, see the recent patch from Lu Baolu: >>> >>> https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com >> >> Erm, this patch is based on that one, it's right there in the context :/ > > Ah, sorry, I didn't spot that! I was just trying to illustrate that this > is per-device. Sure, I understand - and I'm just trying to bang home that despite appearances it's never actually been treated as such for SMMU, so anything that's wrong after this change was already wrong before. 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,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 37F54C433E0 for ; Tue, 30 Mar 2021 16:28:34 +0000 (UTC) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (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 86E2361994 for ; Tue, 30 Mar 2021 16:28:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 86E2361994 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 smtp4.osuosl.org (Postfix) with ESMTP id 15B3B40480; Tue, 30 Mar 2021 16:28:33 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id DRjntPhiLBwG; Tue, 30 Mar 2021 16:28:31 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTP id 5D2DC4047B; Tue, 30 Mar 2021 16:28:31 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 105AFC0013; Tue, 30 Mar 2021 16:28:31 +0000 (UTC) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 27238C000A; Tue, 30 Mar 2021 16:28:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 12227606FD; Tue, 30 Mar 2021 16:28:29 +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 ReP8siV3L_vY; Tue, 30 Mar 2021 16:28:27 +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 smtp3.osuosl.org (Postfix) with ESMTP id 9C22C600B9; Tue, 30 Mar 2021 16:28:27 +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 79B92D6E; Tue, 30 Mar 2021 09:28:26 -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 C45EC3F719; Tue, 30 Mar 2021 09:28:22 -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> From: Robin Murphy Message-ID: <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> Date: Tue, 30 Mar 2021 17:28:19 +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: <20210330135801.GA6187@willie-the-truck> Content-Language: en-GB Cc: freedreno@lists.freedesktop.org, kvm@vger.kernel.org, Michael Ellerman , Joerg Roedel , 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, Lu Baolu 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-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. 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. >>> For example, see the recent patch from Lu Baolu: >>> >>> https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com >> >> Erm, this patch is based on that one, it's right there in the context :/ > > Ah, sorry, I didn't spot that! I was just trying to illustrate that this > is per-device. Sure, I understand - and I'm just trying to bang home that despite appearances it's never actually been treated as such for SMMU, so anything that's wrong after this change was already wrong before. 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, 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 E66EFC433DB for ; Tue, 30 Mar 2021 16:30:17 +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 4D3FF61994 for ; Tue, 30 Mar 2021 16:30:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4D3FF61994 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=eF9cbaXrQ/t4y+i9f01cCVia0vJZNeR3mhNguxCn1e0=; b=kgTTjAqKm8bXnDbUyo4drLQyY wQyD3Li1cSHrFKX8qySEOnhh/X3zfRZtBXLFBOAf52WUgWLHCNBvEvKdXqECNxDT5QEGGXnslKZ5G Bpdrqz5+e2xIJTJOQkJ/S7XWAuB1b9t3dujKBsBMPZMTgSn7ZRpq3LvOi19mwSG7tbCw53hg6oRGV HxZmHoWQRdnPR8eQC1sXvsClDxr98f2iskWOOV3LjMPDzC2R2katAOHOK2hy4WTsiqx2bm7TrbbWX QZrekEMlzQxQb/l6Z2QUhlQz0SAq4HsKx4CnGNrHLZxtod7i7UR8N33I+fVQ3PVyNsQXeUl0Fouim AwFtT0cag==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lRHEK-004Mpu-QF; Tue, 30 Mar 2021 16:28:37 +0000 Received: from foss.arm.com ([217.140.110.172]) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lRHED-004MoU-8b for linux-arm-kernel@lists.infradead.org; Tue, 30 Mar 2021 16:28:32 +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 79B92D6E; Tue, 30 Mar 2021 09:28:26 -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 C45EC3F719; Tue, 30 Mar 2021 09:28:22 -0700 (PDT) Subject: Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE To: Will Deacon Cc: Christoph Hellwig , Joerg Roedel , Li Yang , Michael Ellerman , David Woodhouse , Lu Baolu , linuxppc-dev@lists.ozlabs.org, linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, iommu@lists.linux-foundation.org, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org References: <20210316153825.135976-1-hch@lst.de> <20210316153825.135976-17-hch@lst.de> <20210330131149.GP5908@willie-the-truck> <20210330135801.GA6187@willie-the-truck> From: Robin Murphy Message-ID: <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> Date: Tue, 30 Mar 2021 17:28:19 +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: <20210330135801.GA6187@willie-the-truck> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210330_172830_084949_D2CBC25D X-CRM114-Status: GOOD ( 28.94 ) 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-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. 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. >>> For example, see the recent patch from Lu Baolu: >>> >>> https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com >> >> Erm, this patch is based on that one, it's right there in the context :/ > > Ah, sorry, I didn't spot that! I was just trying to illustrate that this > is per-device. Sure, I understand - and I'm just trying to bang home that despite appearances it's never actually been treated as such for SMMU, so anything that's wrong after this change was already wrong before. 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,URIBL_BLOCKED, 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 C652DC433C1 for ; Tue, 30 Mar 2021 16:28:30 +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 5A726617C9 for ; Tue, 30 Mar 2021 16:28:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5A726617C9 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 AF5736E93E; Tue, 30 Mar 2021 16:28:29 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 749676E93D; Tue, 30 Mar 2021 16:28:27 +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 79B92D6E; Tue, 30 Mar 2021 09:28:26 -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 C45EC3F719; Tue, 30 Mar 2021 09:28:22 -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> From: Robin Murphy Message-ID: <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com> Date: Tue, 30 Mar 2021 17:28:19 +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: <20210330135801.GA6187@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: freedreno@lists.freedesktop.org, kvm@vger.kernel.org, Michael Ellerman , Joerg Roedel , 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, Lu Baolu 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-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. 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. >>> For example, see the recent patch from Lu Baolu: >>> >>> https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com >> >> Erm, this patch is based on that one, it's right there in the context :/ > > Ah, sorry, I didn't spot that! I was just trying to illustrate that this > is per-device. Sure, I understand - and I'm just trying to bang home that despite appearances it's never actually been treated as such for SMMU, so anything that's wrong after this change was already wrong before. Robin. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel