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 C30DDC11F66 for ; Tue, 29 Jun 2021 13:23:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A10A361DC2 for ; Tue, 29 Jun 2021 13:23:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232487AbhF2NZ2 (ORCPT ); Tue, 29 Jun 2021 09:25:28 -0400 Received: from foss.arm.com ([217.140.110.172]:51046 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233625AbhF2NZ0 (ORCPT ); Tue, 29 Jun 2021 09:25:26 -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 43A0D106F; Tue, 29 Jun 2021 06:22:58 -0700 (PDT) Received: from [10.57.46.146] (unknown [10.57.46.146]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 906FD3F718; Tue, 29 Jun 2021 06:22:56 -0700 (PDT) Subject: Re: [PATCH v5 7/8] iommu/arm-smmu: Get associated RMR info and install bypass SMR To: Jon Nettleton Cc: Shameer Kolothum , linux-arm-kernel , ACPI Devel Maling List , iommu@lists.linux-foundation.org, Linuxarm , Steven Price , Hanjun Guo , yangyicong , Sami.Mujawar@arm.com, wanghuiqiang References: <20210524110222.2212-1-shameerali.kolothum.thodi@huawei.com> <20210524110222.2212-8-shameerali.kolothum.thodi@huawei.com> <2bc3ae21-f2af-ee2c-5e9d-d47633e0439e@arm.com> From: Robin Murphy Message-ID: Date: Tue, 29 Jun 2021 14:22:52 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: 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-acpi@vger.kernel.org On 2021-06-29 08:03, Jon Nettleton wrote: > On Mon, Jun 14, 2021 at 12:06 PM Robin Murphy wrote: >> >> On 2021-05-24 12:02, Shameer Kolothum wrote: >>> From: Jon Nettleton >>> >>> Check if there is any RMR info associated with the devices behind >>> the SMMU and if any, install bypass SMRs for them. This is to >>> keep any ongoing traffic associated with these devices alive >>> when we enable/reset SMMU during probe(). >>> >>> Signed-off-by: Jon Nettleton >>> Signed-off-by: Steven Price >>> Signed-off-by: Shameer Kolothum >>> --- >>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 65 +++++++++++++++++++++++++++ >>> 1 file changed, 65 insertions(+) >>> >>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> index 6f72c4d208ca..56db3d3238fc 100644 >>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> @@ -2042,6 +2042,67 @@ err_reset_platform_ops: __maybe_unused; >>> return err; >>> } >>> >>> +static void arm_smmu_rmr_install_bypass_smr(struct arm_smmu_device *smmu) >>> +{ >>> + struct list_head rmr_list; >>> + struct iommu_resv_region *e; >>> + int i, cnt = 0; >>> + u32 smr; >>> + u32 reg; >>> + >>> + INIT_LIST_HEAD(&rmr_list); >>> + if (iommu_dma_get_rmrs(dev_fwnode(smmu->dev), &rmr_list)) >>> + return; >>> + >>> + reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0); >>> + >>> + if ((reg & ARM_SMMU_sCR0_USFCFG) && !(reg & ARM_SMMU_sCR0_CLIENTPD)) { >>> + /* >>> + * SMMU is already enabled and disallowing bypass, so preserve >>> + * the existing SMRs >>> + */ >>> + for (i = 0; i < smmu->num_mapping_groups; i++) { >>> + smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(i)); >> >> To reiterate, just because a bootloader/crashed kernel/whatever may have >> left some configuration behind doesn't mean that it matters (e.g. what >> if these SMRs are pointing at translation contexts?). All we should be >> doing here is setting the relevant RMR accommodations in our "clean >> slate" software state before the reset routine applies it to the >> hardware, just like patch #5 does for SMMUv3. >> >> Trying to safely reset an SMMU when we discover it with CLIENTPD=0 is >> really another issue entirely, and I'd say is beyond the scope of this >> series >> >>> + if (!FIELD_GET(ARM_SMMU_SMR_VALID, smr)) >>> + continue; >> >> Note that that's not even necessarily correct (thanks to EXIDS). >> >>> + smmu->smrs[i].id = FIELD_GET(ARM_SMMU_SMR_ID, smr); >>> + smmu->smrs[i].mask = FIELD_GET(ARM_SMMU_SMR_MASK, smr); >>> + smmu->smrs[i].valid = true; >>> + } >>> + } >>> + >>> + list_for_each_entry(e, &rmr_list, list) { >>> + u32 sid = e->fw_data.rmr.sid; >>> + >>> + i = arm_smmu_find_sme(smmu, sid, ~0); >>> + if (i < 0) >>> + continue; >>> + if (smmu->s2crs[i].count == 0) { >>> + smmu->smrs[i].id = sid; >>> + smmu->smrs[i].mask = ~0; >> >> This is very wrong (as has now already been pointed out). >> >>> + smmu->smrs[i].valid = true; >>> + } >>> + smmu->s2crs[i].count++; >>> + smmu->s2crs[i].type = S2CR_TYPE_BYPASS; >>> + smmu->s2crs[i].privcfg = S2CR_PRIVCFG_DEFAULT; >>> + smmu->s2crs[i].cbndx = 0xff; >> >> Nit: cbndx is left uninitialised for bypass/fault entries elsewhere, so >> there's little point touching it here. >> >>> + >>> + cnt++; >>> + } >>> + >>> + if ((reg & ARM_SMMU_sCR0_USFCFG) && !(reg & ARM_SMMU_sCR0_CLIENTPD)) { >>> + /* Remove the valid bit for unused SMRs */ >>> + for (i = 0; i < smmu->num_mapping_groups; i++) { >>> + if (smmu->s2crs[i].count == 0) >>> + smmu->smrs[i].valid = false; >>> + } >> >> If this dance is only about avoiding stream match conflicts when trying >> to reprogram live SMRs, simply turning the SMMU off beforehand would be >> a lot simpler. > > Robin, > > I am not sure what you mean here, and maybe Steve wants to jump in and > help clarify. > > My understanding is that "dance" is required for regions that need to > continue to work > throughout the boot process. I think the example I have seen the most > is for GOP drivers that > use system memory rather than dedicated VRAM. All that is required is to install bypass SMEs for the relevant streams before enabling the SMMU. That much is achieved by the list_for_each_entry(e, &rmr_list, list) loop setting up initial state followed by arm_smmu_device_reset(). The "dance" I'm referring to is the additional reading out of (possibly nonsense) SMR state beforehand to pre-bias the SMR allocator, then trying to clean up the remnants afterwards. If we're going to pretend to be robust against finding the SMMU already enabled *with* live traffic ongoing, there's frankly an awful lot more care we'd need to take beyond here (and for some aspects there's still no right answer). If on the other hand we're implicitly assuming that any boot-time enabled SMRs exactly match the RMR configuration anyway, then simply disabling the SMMU until we enable it again in the reset routine still preserves the necessary bypass behaviour for RMR streams while sidestepping any issues related to reprogramming live SMMU state. Robin. >>> + } >>> + >>> + dev_notice(smmu->dev, "\tpreserved %d boot mapping%s\n", cnt, >>> + cnt == 1 ? "" : "s"); >>> + iommu_dma_put_rmrs(dev_fwnode(smmu->dev), &rmr_list); >>> +} >>> + >>> static int arm_smmu_device_probe(struct platform_device *pdev) >>> { >>> struct resource *res; >>> @@ -2168,6 +2229,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev) >>> } >>> >>> platform_set_drvdata(pdev, smmu); >>> + >>> + /* Check for RMRs and install bypass SMRs if any */ >>> + arm_smmu_rmr_install_bypass_smr(smmu); >>> + >>> arm_smmu_device_reset(smmu); >>> arm_smmu_test_smr_masks(smmu); >>> >>> 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 BB89CC11F66 for ; Tue, 29 Jun 2021 13:23:09 +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 6A4EF61DC2 for ; Tue, 29 Jun 2021 13:23:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6A4EF61DC2 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 smtp4.osuosl.org (Postfix) with ESMTP id 194CD402B8; Tue, 29 Jun 2021 13:23:09 +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 WVi23vMsB5Oy; Tue, 29 Jun 2021 13:23:07 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 35D6A4046E; Tue, 29 Jun 2021 13:23:07 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E3B05C001B; Tue, 29 Jun 2021 13:23:06 +0000 (UTC) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id D6EF4C000E for ; Tue, 29 Jun 2021 13:23:05 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id B442F4045E for ; Tue, 29 Jun 2021 13:23:05 +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 5SOcMbgmy5Z9 for ; Tue, 29 Jun 2021 13:23:04 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp4.osuosl.org (Postfix) with ESMTP id 444BE402B8 for ; Tue, 29 Jun 2021 13:23:04 +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 43A0D106F; Tue, 29 Jun 2021 06:22:58 -0700 (PDT) Received: from [10.57.46.146] (unknown [10.57.46.146]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 906FD3F718; Tue, 29 Jun 2021 06:22:56 -0700 (PDT) Subject: Re: [PATCH v5 7/8] iommu/arm-smmu: Get associated RMR info and install bypass SMR To: Jon Nettleton References: <20210524110222.2212-1-shameerali.kolothum.thodi@huawei.com> <20210524110222.2212-8-shameerali.kolothum.thodi@huawei.com> <2bc3ae21-f2af-ee2c-5e9d-d47633e0439e@arm.com> From: Robin Murphy Message-ID: Date: Tue, 29 Jun 2021 14:22:52 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB Cc: Linuxarm , Steven Price , ACPI Devel Maling List , iommu@lists.linux-foundation.org, wanghuiqiang , Hanjun Guo , yangyicong , Sami.Mujawar@arm.com, linux-arm-kernel 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-06-29 08:03, Jon Nettleton wrote: > On Mon, Jun 14, 2021 at 12:06 PM Robin Murphy wrote: >> >> On 2021-05-24 12:02, Shameer Kolothum wrote: >>> From: Jon Nettleton >>> >>> Check if there is any RMR info associated with the devices behind >>> the SMMU and if any, install bypass SMRs for them. This is to >>> keep any ongoing traffic associated with these devices alive >>> when we enable/reset SMMU during probe(). >>> >>> Signed-off-by: Jon Nettleton >>> Signed-off-by: Steven Price >>> Signed-off-by: Shameer Kolothum >>> --- >>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 65 +++++++++++++++++++++++++++ >>> 1 file changed, 65 insertions(+) >>> >>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> index 6f72c4d208ca..56db3d3238fc 100644 >>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> @@ -2042,6 +2042,67 @@ err_reset_platform_ops: __maybe_unused; >>> return err; >>> } >>> >>> +static void arm_smmu_rmr_install_bypass_smr(struct arm_smmu_device *smmu) >>> +{ >>> + struct list_head rmr_list; >>> + struct iommu_resv_region *e; >>> + int i, cnt = 0; >>> + u32 smr; >>> + u32 reg; >>> + >>> + INIT_LIST_HEAD(&rmr_list); >>> + if (iommu_dma_get_rmrs(dev_fwnode(smmu->dev), &rmr_list)) >>> + return; >>> + >>> + reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0); >>> + >>> + if ((reg & ARM_SMMU_sCR0_USFCFG) && !(reg & ARM_SMMU_sCR0_CLIENTPD)) { >>> + /* >>> + * SMMU is already enabled and disallowing bypass, so preserve >>> + * the existing SMRs >>> + */ >>> + for (i = 0; i < smmu->num_mapping_groups; i++) { >>> + smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(i)); >> >> To reiterate, just because a bootloader/crashed kernel/whatever may have >> left some configuration behind doesn't mean that it matters (e.g. what >> if these SMRs are pointing at translation contexts?). All we should be >> doing here is setting the relevant RMR accommodations in our "clean >> slate" software state before the reset routine applies it to the >> hardware, just like patch #5 does for SMMUv3. >> >> Trying to safely reset an SMMU when we discover it with CLIENTPD=0 is >> really another issue entirely, and I'd say is beyond the scope of this >> series >> >>> + if (!FIELD_GET(ARM_SMMU_SMR_VALID, smr)) >>> + continue; >> >> Note that that's not even necessarily correct (thanks to EXIDS). >> >>> + smmu->smrs[i].id = FIELD_GET(ARM_SMMU_SMR_ID, smr); >>> + smmu->smrs[i].mask = FIELD_GET(ARM_SMMU_SMR_MASK, smr); >>> + smmu->smrs[i].valid = true; >>> + } >>> + } >>> + >>> + list_for_each_entry(e, &rmr_list, list) { >>> + u32 sid = e->fw_data.rmr.sid; >>> + >>> + i = arm_smmu_find_sme(smmu, sid, ~0); >>> + if (i < 0) >>> + continue; >>> + if (smmu->s2crs[i].count == 0) { >>> + smmu->smrs[i].id = sid; >>> + smmu->smrs[i].mask = ~0; >> >> This is very wrong (as has now already been pointed out). >> >>> + smmu->smrs[i].valid = true; >>> + } >>> + smmu->s2crs[i].count++; >>> + smmu->s2crs[i].type = S2CR_TYPE_BYPASS; >>> + smmu->s2crs[i].privcfg = S2CR_PRIVCFG_DEFAULT; >>> + smmu->s2crs[i].cbndx = 0xff; >> >> Nit: cbndx is left uninitialised for bypass/fault entries elsewhere, so >> there's little point touching it here. >> >>> + >>> + cnt++; >>> + } >>> + >>> + if ((reg & ARM_SMMU_sCR0_USFCFG) && !(reg & ARM_SMMU_sCR0_CLIENTPD)) { >>> + /* Remove the valid bit for unused SMRs */ >>> + for (i = 0; i < smmu->num_mapping_groups; i++) { >>> + if (smmu->s2crs[i].count == 0) >>> + smmu->smrs[i].valid = false; >>> + } >> >> If this dance is only about avoiding stream match conflicts when trying >> to reprogram live SMRs, simply turning the SMMU off beforehand would be >> a lot simpler. > > Robin, > > I am not sure what you mean here, and maybe Steve wants to jump in and > help clarify. > > My understanding is that "dance" is required for regions that need to > continue to work > throughout the boot process. I think the example I have seen the most > is for GOP drivers that > use system memory rather than dedicated VRAM. All that is required is to install bypass SMEs for the relevant streams before enabling the SMMU. That much is achieved by the list_for_each_entry(e, &rmr_list, list) loop setting up initial state followed by arm_smmu_device_reset(). The "dance" I'm referring to is the additional reading out of (possibly nonsense) SMR state beforehand to pre-bias the SMR allocator, then trying to clean up the remnants afterwards. If we're going to pretend to be robust against finding the SMMU already enabled *with* live traffic ongoing, there's frankly an awful lot more care we'd need to take beyond here (and for some aspects there's still no right answer). If on the other hand we're implicitly assuming that any boot-time enabled SMRs exactly match the RMR configuration anyway, then simply disabling the SMMU until we enable it again in the reset routine still preserves the necessary bypass behaviour for RMR streams while sidestepping any issues related to reprogramming live SMMU state. Robin. >>> + } >>> + >>> + dev_notice(smmu->dev, "\tpreserved %d boot mapping%s\n", cnt, >>> + cnt == 1 ? "" : "s"); >>> + iommu_dma_put_rmrs(dev_fwnode(smmu->dev), &rmr_list); >>> +} >>> + >>> static int arm_smmu_device_probe(struct platform_device *pdev) >>> { >>> struct resource *res; >>> @@ -2168,6 +2229,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev) >>> } >>> >>> platform_set_drvdata(pdev, smmu); >>> + >>> + /* Check for RMRs and install bypass SMRs if any */ >>> + arm_smmu_rmr_install_bypass_smr(smmu); >>> + >>> arm_smmu_device_reset(smmu); >>> arm_smmu_test_smr_masks(smmu); >>> >>> _______________________________________________ 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.7 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 A36C4C11F66 for ; Tue, 29 Jun 2021 13:41:09 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7070061D90 for ; Tue, 29 Jun 2021 13:41:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7070061D90 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=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:Cc:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Mcux1nM5/Z3A17p3+RumvSZwNHXAfhCyaEaSad6hDJo=; b=DfaMvV+G3KYOrMPsBhvjTRrVhz ucgVM3pHLIXvFZvcGA3rDTXyvlaFuscWZwvD+h2kfpGueuS86D4n+6zMfnwRRUt15h1vCmssuFVat 7+M25kNIpwcXaWqhB750Pzjb4OtK3YAQSauiFK3NLozPTr0AZcwmRcfwko0FpwX256O2nLQYCaj6A 0xJPCKtBpaw8Kb1zcWfxIYdueBTrVn+HgTIoo8mr3UWc9SGh8GB+SGbaaxusYRT9cMUeOfLOZAJ2X yYmAnyveEd1nWfwLo8BEYHnSS0/DTlutNQAI/wMVv0ul5ubUfEMpJfEPA4Qt7+oOmRwvnQmVHX0lp ipWr4sBw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lyDxF-00B6sl-Eo; Tue, 29 Jun 2021 13:39:10 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lyDhb-00B0Al-Sm for linux-arm-kernel@lists.infradead.org; Tue, 29 Jun 2021 13:23:01 +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 43A0D106F; Tue, 29 Jun 2021 06:22:58 -0700 (PDT) Received: from [10.57.46.146] (unknown [10.57.46.146]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 906FD3F718; Tue, 29 Jun 2021 06:22:56 -0700 (PDT) Subject: Re: [PATCH v5 7/8] iommu/arm-smmu: Get associated RMR info and install bypass SMR To: Jon Nettleton Cc: Shameer Kolothum , linux-arm-kernel , ACPI Devel Maling List , iommu@lists.linux-foundation.org, Linuxarm , Steven Price , Hanjun Guo , yangyicong , Sami.Mujawar@arm.com, wanghuiqiang References: <20210524110222.2212-1-shameerali.kolothum.thodi@huawei.com> <20210524110222.2212-8-shameerali.kolothum.thodi@huawei.com> <2bc3ae21-f2af-ee2c-5e9d-d47633e0439e@arm.com> From: Robin Murphy Message-ID: Date: Tue, 29 Jun 2021 14:22:52 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210629_062300_126304_55B9251F X-CRM114-Status: GOOD ( 36.23 ) 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-06-29 08:03, Jon Nettleton wrote: > On Mon, Jun 14, 2021 at 12:06 PM Robin Murphy wrote: >> >> On 2021-05-24 12:02, Shameer Kolothum wrote: >>> From: Jon Nettleton >>> >>> Check if there is any RMR info associated with the devices behind >>> the SMMU and if any, install bypass SMRs for them. This is to >>> keep any ongoing traffic associated with these devices alive >>> when we enable/reset SMMU during probe(). >>> >>> Signed-off-by: Jon Nettleton >>> Signed-off-by: Steven Price >>> Signed-off-by: Shameer Kolothum >>> --- >>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 65 +++++++++++++++++++++++++++ >>> 1 file changed, 65 insertions(+) >>> >>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> index 6f72c4d208ca..56db3d3238fc 100644 >>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> @@ -2042,6 +2042,67 @@ err_reset_platform_ops: __maybe_unused; >>> return err; >>> } >>> >>> +static void arm_smmu_rmr_install_bypass_smr(struct arm_smmu_device *smmu) >>> +{ >>> + struct list_head rmr_list; >>> + struct iommu_resv_region *e; >>> + int i, cnt = 0; >>> + u32 smr; >>> + u32 reg; >>> + >>> + INIT_LIST_HEAD(&rmr_list); >>> + if (iommu_dma_get_rmrs(dev_fwnode(smmu->dev), &rmr_list)) >>> + return; >>> + >>> + reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0); >>> + >>> + if ((reg & ARM_SMMU_sCR0_USFCFG) && !(reg & ARM_SMMU_sCR0_CLIENTPD)) { >>> + /* >>> + * SMMU is already enabled and disallowing bypass, so preserve >>> + * the existing SMRs >>> + */ >>> + for (i = 0; i < smmu->num_mapping_groups; i++) { >>> + smr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_SMR(i)); >> >> To reiterate, just because a bootloader/crashed kernel/whatever may have >> left some configuration behind doesn't mean that it matters (e.g. what >> if these SMRs are pointing at translation contexts?). All we should be >> doing here is setting the relevant RMR accommodations in our "clean >> slate" software state before the reset routine applies it to the >> hardware, just like patch #5 does for SMMUv3. >> >> Trying to safely reset an SMMU when we discover it with CLIENTPD=0 is >> really another issue entirely, and I'd say is beyond the scope of this >> series >> >>> + if (!FIELD_GET(ARM_SMMU_SMR_VALID, smr)) >>> + continue; >> >> Note that that's not even necessarily correct (thanks to EXIDS). >> >>> + smmu->smrs[i].id = FIELD_GET(ARM_SMMU_SMR_ID, smr); >>> + smmu->smrs[i].mask = FIELD_GET(ARM_SMMU_SMR_MASK, smr); >>> + smmu->smrs[i].valid = true; >>> + } >>> + } >>> + >>> + list_for_each_entry(e, &rmr_list, list) { >>> + u32 sid = e->fw_data.rmr.sid; >>> + >>> + i = arm_smmu_find_sme(smmu, sid, ~0); >>> + if (i < 0) >>> + continue; >>> + if (smmu->s2crs[i].count == 0) { >>> + smmu->smrs[i].id = sid; >>> + smmu->smrs[i].mask = ~0; >> >> This is very wrong (as has now already been pointed out). >> >>> + smmu->smrs[i].valid = true; >>> + } >>> + smmu->s2crs[i].count++; >>> + smmu->s2crs[i].type = S2CR_TYPE_BYPASS; >>> + smmu->s2crs[i].privcfg = S2CR_PRIVCFG_DEFAULT; >>> + smmu->s2crs[i].cbndx = 0xff; >> >> Nit: cbndx is left uninitialised for bypass/fault entries elsewhere, so >> there's little point touching it here. >> >>> + >>> + cnt++; >>> + } >>> + >>> + if ((reg & ARM_SMMU_sCR0_USFCFG) && !(reg & ARM_SMMU_sCR0_CLIENTPD)) { >>> + /* Remove the valid bit for unused SMRs */ >>> + for (i = 0; i < smmu->num_mapping_groups; i++) { >>> + if (smmu->s2crs[i].count == 0) >>> + smmu->smrs[i].valid = false; >>> + } >> >> If this dance is only about avoiding stream match conflicts when trying >> to reprogram live SMRs, simply turning the SMMU off beforehand would be >> a lot simpler. > > Robin, > > I am not sure what you mean here, and maybe Steve wants to jump in and > help clarify. > > My understanding is that "dance" is required for regions that need to > continue to work > throughout the boot process. I think the example I have seen the most > is for GOP drivers that > use system memory rather than dedicated VRAM. All that is required is to install bypass SMEs for the relevant streams before enabling the SMMU. That much is achieved by the list_for_each_entry(e, &rmr_list, list) loop setting up initial state followed by arm_smmu_device_reset(). The "dance" I'm referring to is the additional reading out of (possibly nonsense) SMR state beforehand to pre-bias the SMR allocator, then trying to clean up the remnants afterwards. If we're going to pretend to be robust against finding the SMMU already enabled *with* live traffic ongoing, there's frankly an awful lot more care we'd need to take beyond here (and for some aspects there's still no right answer). If on the other hand we're implicitly assuming that any boot-time enabled SMRs exactly match the RMR configuration anyway, then simply disabling the SMMU until we enable it again in the reset routine still preserves the necessary bypass behaviour for RMR streams while sidestepping any issues related to reprogramming live SMMU state. Robin. >>> + } >>> + >>> + dev_notice(smmu->dev, "\tpreserved %d boot mapping%s\n", cnt, >>> + cnt == 1 ? "" : "s"); >>> + iommu_dma_put_rmrs(dev_fwnode(smmu->dev), &rmr_list); >>> +} >>> + >>> static int arm_smmu_device_probe(struct platform_device *pdev) >>> { >>> struct resource *res; >>> @@ -2168,6 +2229,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev) >>> } >>> >>> platform_set_drvdata(pdev, smmu); >>> + >>> + /* Check for RMRs and install bypass SMRs if any */ >>> + arm_smmu_rmr_install_bypass_smr(smmu); >>> + >>> arm_smmu_device_reset(smmu); >>> arm_smmu_test_smr_masks(smmu); >>> >>> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel