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=-13.9 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,SPF_HELO_NONE,SPF_PASS 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 B516BC48BCF for ; Sun, 13 Jun 2021 07:43:40 +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 6F79F611C0 for ; Sun, 13 Jun 2021 07:43:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6F79F611C0 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=solid-run.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-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=aVAhggYn3Yg1UbHecBsW0A0tCBnzNKN2Z9sU0OyIXuc=; b=2CbbwLET75zgzs bUEcGAVqXsfjoqShUuiIhwiZFUeOT5h4IB57gSb1ZjYDQd2ZeJ0qems5f53G8LrDoo8ts6V0kOX+5 6V7PamHmmiQfVHCpwow08Ce9UX6wEKAnak5hFiRyX7I9jYF68i1llhWurX9MEk95/Jig9UtubR9dE kN5ugXOm4/RSqZ86mzuwitxAw2Hlakup9mHFZJbR44zJGMsE1mxPuq7dHJ3wvMaFdLzQiKV5TMhgg 1PzOOdepRmwVn/KrY9wYfA+cmi2q+q0d5YVGFNMIoz0UQbiV2uKFv145LLBt0yJ1PWr9NvSS3Em4C jVvXPIv8DsyfgTJFMenA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lsKkR-009sDf-Ae; Sun, 13 Jun 2021 07:41:35 +0000 Received: from mail-ej1-x632.google.com ([2a00:1450:4864:20::632]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lsKkM-009sCi-CK for linux-arm-kernel@lists.infradead.org; Sun, 13 Jun 2021 07:41:33 +0000 Received: by mail-ej1-x632.google.com with SMTP id my49so11152877ejc.7 for ; Sun, 13 Jun 2021 00:41:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=solid-run-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=uHghi3ehEDHpR81l5YKPvLBlsk0T7HfCV7TacfpzbeI=; b=o+cUnx2G2KEoTlVhnWeDA1LHpQNoOSamaIrl9Xany7HtsGTSTpChd1v+RUJt9V9YbP mK9QP64NseXN2M246A50Obv9CIKrbw0kS6kXCAxBVRpnmJtqboZRpJAbX3W4j8IWBtci 5hwgm6eQDUXHFZTgW/lrp91ZW7MPfbSU4UraEtP4V7d0IvrTicr8JdcvIYzsY4xMnkYn v40CzXIknHTjwMITH18IlHB5jbnb+K4emfDXAy5pHQMPYbgdzKC56pQO2sqUJ7T2yro6 d5CGE48TepJQSYlyaVXyaBajEEpEe2HqJheQPKrFAICmty6QIsSzBiVOHqv2yGMBEPxb 5HcQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uHghi3ehEDHpR81l5YKPvLBlsk0T7HfCV7TacfpzbeI=; b=DC4+v5m9uxJJq1gAqGpitjfBz7okcbr8XtAn18X4AVie8x/XncdYotTqvovEIWzPGO q8ZKZbozTL9hUzNhZTTLEPqPSEcylXu0afHjrjdUjGO9Wq3ckR7jeddbuWLp/mUf3f+r 4Qm2IEtUVH5WHsnv6F929e67XxmJptztn1K6obTY2WvzPVCDtP4b5pVAbigI3v3j6pWL ikHJ/yc4am2WQo7gwqdudpxBaPgPncgZYcGS5O3siBfqTH4Df5Bbh5JJiLobiYSC+yTv pOn3MVUhmSqIurGOniJbKEv63UUCN2e2HbpEmK7japvUU/PIZFLJcKZe88EAIMi7n9sF NKQw== X-Gm-Message-State: AOAM532c6tYeP+YVy3u4ZlVpKgAULWzIcSTMiBxerR6m1r06QmnBeYEl ctEUGt5g6gBdISnpB7sH+kUrJQanqCMex5TdIykN1w== X-Google-Smtp-Source: ABdhPJyyHOxaif3zCXoDKw9sxiVm1/jwSz+9YN4CehDyz/mM0SNU/fqrCv/2O3iI+Xk59n2q0qBOS0emsDSI12cnDfM= X-Received: by 2002:a17:906:3ed0:: with SMTP id d16mr10267396ejj.16.1623570086865; Sun, 13 Jun 2021 00:41:26 -0700 (PDT) MIME-Version: 1.0 References: <20210524110222.2212-1-shameerali.kolothum.thodi@huawei.com> <20210524110222.2212-8-shameerali.kolothum.thodi@huawei.com> <32dc72fa-534a-7eb4-dfcc-9bc244845a28@arm.com> In-Reply-To: From: Jon Nettleton Date: Sun, 13 Jun 2021 09:40:50 +0200 Message-ID: Subject: Re: [PATCH v5 7/8] iommu/arm-smmu: Get associated RMR info and install bypass SMR To: Steven Price Cc: Shameer Kolothum , linux-arm-kernel , ACPI Devel Maling List , iommu@lists.linux-foundation.org, Linuxarm , Lorenzo Pieralisi , joro@8bytes.org, Robin Murphy , wanghuiqiang , Hanjun Guo , Sami.Mujawar@arm.com, eric.auger@redhat.com, yangyicong X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210613_004131_324042_84EE596E X-CRM114-Status: GOOD ( 49.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Jun 3, 2021 at 1:51 PM Jon Nettleton wrote: > > On Thu, Jun 3, 2021 at 1:27 PM Steven Price wrote: > > > > On 03/06/2021 09:52, Jon Nettleton wrote: > > > On Mon, May 24, 2021 at 1:04 PM 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)); > > >> + if (!FIELD_GET(ARM_SMMU_SMR_VALID, smr)) > > >> + continue; > > >> + 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; > > > > Looking at this code again, that mask looks wrong! According to the SMMU > > spec MASK[i]==1 means ID[i] is ignored. I.e. this means completely > > ignore the ID... > > > > I'm not at all sure why they designed the hardware that way round. > > > > >> + 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; > > >> + > > >> + 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; > > >> + } > > >> + } > > >> + > > >> + 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); > > >> > > >> -- > > >> 2.17.1 > > >> > > > > > > Shameer and Robin > > > > > > I finally got around to updating edk2 and the HoneyComb IORT tables to > > > reflect the new standards. > > > Out of the box the new patchset was generating errors immediatly after > > > the smmu bringup. > > > > > > arm-smmu arm-smmu.0.auto: Unhandled context fault: fsr=0x402, iova=0x2080000140, > > > fsynr=0x1d0040, cbfrsynra=0x4000, cb=0 > > > > > > These errors were generated even with disable_bypass=0 > > > > > > I tracked down the issue to > > > > > > This code is skipped as Robin said would be correct > > > > If you're skipping the first bit of code, then that (hopefully) means > > that the SMMU is disabled... > > > > >> + 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)); > > >> + if (!FIELD_GET(ARM_SMMU_SMR_VALID, smr)) > > >> + continue; > > >> + 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; > > >> + }[ 2.707729] arm-smmu: setting up rmr list on 0x4000 > > > [ 2.712598] arm-smmu: s2crs count is 0 smrs index 0x0 > > > [ 2.717638] arm-smmu: s2crs count is 0 smrs id is 0x4000 > > > [ 2.722939] arm-smmu: s2crs count is 0 smrs mask is 0x8000 > > > [ 2.728417] arm-smmu arm-smmu.0.auto: preserved 1 boot mapping > > > > > >> + } > > > > > > Then this code block was hit which is correct > > > > > >> + if (smmu->s2crs[i].count == 0) { > > >> + smmu->smrs[i].id = sid; > > >> + smmu->smrs[i].mask = ~0; > > >> + smmu->smrs[i].valid = true; > > >> + } > > > > > > The mask was causing the issue. If I think ammended that segment to read > > > the mask as setup by the hardware everything was back to functioning both > > > with and without disable_bypass set. > > > > ...so reading a mask from it doesn't sounds quite right. > > > > Can you have a go with a corrected mask of '0' rather than all-1s and > > see if that helps? My guess is that the mask of ~0 was causing multiple > > matches in the SMRs which is 'UNPREDICTABLE'. > > > > Sadly in my test setup there's only the one device behind the SMMU so > > I didn't spot this (below patch works for me, but that's not saying > > much). > > > > Steve > > > > --->8--- > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c > > index 56db3d3238fc..44831d0c78e4 100644 > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c > > @@ -2079,7 +2079,7 @@ static void arm_smmu_rmr_install_bypass_smr(struct arm_smmu_device *smmu) > > continue; > > if (smmu->s2crs[i].count == 0) { > > smmu->smrs[i].id = sid; > > - smmu->smrs[i].mask = ~0; > > + smmu->smrs[i].mask = 0; > > smmu->smrs[i].valid = true; > > } > > smmu->s2crs[i].count++; > > Yes this works fine. Thanks Shameer, Can you pick up this change into your next patch set? Also are there any objections to adding this to the SMMUv2 code from the maintainers? -Jon _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel