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=-14.1 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 B1B7CC47082 for ; Thu, 3 Jun 2021 11:57:12 +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 7B368613E3 for ; Thu, 3 Jun 2021 11:57:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7B368613E3 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=34jWLDvm6XXr8bLT/M+PTuUbjvkcH2rNgM4XyXwYbeE=; b=T/JJJgTI7YPJ7u PcaVqTY8RAiI7SAS3kd/IywAzpm0nVxLwZb3iwvVUmMMdnpoKGazGutwDilVvvVfUnXqoLdCzTXF9 Yh0QpL8w628yd2f0lkUSHl51OlUx+KNl6gsI3ubGdCUmweLFharhu0GUeSs639axPcZyJsoexLqTK R/cMz9PeqapqA1yJHZUfYaXMta8e0d9KOHDYv0yGIbNLw7M4WSHl63HhLwONj5vJQyHsmac8Ns1rz 8t7L1RH3Zo+jS2q/9UXj93IqhWZrPXdG3YI/G8XzXUTfOZYWNygvnyakU36Fh1WQvRHs5oqULlNLH S5otkm7dgMFXz0LmgOkw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lolw6-008T6y-Bk; Thu, 03 Jun 2021 11:54:54 +0000 Received: from mail-ed1-f48.google.com ([209.85.208.48]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1loluE-008Rx0-Hc for linux-arm-kernel@lists.infradead.org; Thu, 03 Jun 2021 11:53:00 +0000 Received: by mail-ed1-f48.google.com with SMTP id o5so6772629edc.5 for ; Thu, 03 Jun 2021 04:52:58 -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=HFp854olGgFJmXGnnWOBlcDacyWdiAmNy6/uBhlQEwk=; b=Q6I36TVtig+AD1IxJypcOMC/R5GDbjL5rYHfQVI8voZFQQHheQ4fHGzCKFBi6rx+gP KSDv3H3vFKgsFVp97V0P312PEjXZem0GsixIc2VZ63rcV64kT53ylxZcZj8unJIX0bmV VUzXJUbn40RgN/lsT4QO9HcdDmycHjo8DLADYvyEcbmXVath15Bt5NDoMCwAiQde4dPf FFLh2nvJ7ud0SIb2ZSpbTsNm8pGXLIyZfvXWMTjaMEGJr9e0dJgVE5SLo5daSOKF7jvS au0scHdqkW/nteXgCpXxpGnXxsFHlTNccE1hGdx6a6olMN1WWFv9CA+vUePTEz0vGxcX BSSA== 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=HFp854olGgFJmXGnnWOBlcDacyWdiAmNy6/uBhlQEwk=; b=H6NlHUbs7bHj85R/ZaYGxkpKTS/yQvpPRYCv9Kj8VXPANX0jKYfDslEi9iKdHk2FC1 6SxjxNXLRDJfQ1XOPYuVwKV7gHm2CIH06pkUNFCHyCgM5X2CYTTbRhM/y8ziyVBCVZfk Y3JsYTuNvIRgaNv3VCHmV6MDDdv9I3Tau7bLgpSnYwNDVQcjHZ4WIhl2ZvQXt+T+Ba9K eL7vvF9dTB/+pauQGtXc0ao2uzsZ0Tpwoym+cWAwODdFucaosXB9wSCoknb+dwPR3EHj uZtnNXCt3g0UhuZ1JJ7SbJgBfYIcY0qJ4Hpn6AyZgEc0aSvGU2HV5B2LSMeJ7T97MGMK OnzA== X-Gm-Message-State: AOAM532bSz77IZJ1eTCCH0Dsm/B2DxJnfQsGLWToYrVTvE9oHiYoneyi jNMXfvPf9/cGRzfmkZ0uz3+OaaWoMv193pYnEu+8Cw== X-Google-Smtp-Source: ABdhPJwltdaiHIpT1D7VO6Zwu05x53ILY/BZkyb0KSc7UMu9ZmCBVI+CsB3LJQQfU7Vz0N4C/z2LGya+ya/1yRX0+sU= X-Received: by 2002:aa7:db49:: with SMTP id n9mr24789379edt.361.1622721116921; Thu, 03 Jun 2021 04:51:56 -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: <32dc72fa-534a-7eb4-dfcc-9bc244845a28@arm.com> From: Jon Nettleton Date: Thu, 3 Jun 2021 13:51:19 +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-20210603_045258_632978_E5E362C7 X-CRM114-Status: GOOD ( 46.58 ) 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: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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel