Message ID | 20230622092742.74819-3-angelogioacchino.delregno@collabora.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp4944231vqr; Thu, 22 Jun 2023 02:50:59 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ66VjxiCekiqZxq/pYDQ48xThpFq8ge1T/lYZ9Tr1com6NBYRCYhYIWja5E0KfQu6vSYBlS X-Received: by 2002:a92:d090:0:b0:338:d170:6e32 with SMTP id h16-20020a92d090000000b00338d1706e32mr18634034ilh.7.1687427459598; Thu, 22 Jun 2023 02:50:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687427459; cv=none; d=google.com; s=arc-20160816; b=tTwGPns1XSZWXnVsdq3806mZAoXbvMYTgnG+9/bVrnBDxXD4soeGjI6cEvnjv34OFv bRUjxQSKGnO/TTBaCbGrw6meM974x0G68+CtawxcuJVKk7UyvGYVs5DGErK1nIgwroJV EcpTN/D4/PPEWKc0IjrGv7HvOyOaGNEIfixc3WTrrP8cKK3YqEUrvDS27ebwwY4QwJ/L 4qbWeadIMxkRL6X8vKDvpDmFA93vqcsZnYJXyRNwJ9K/RFRs7NpQuFRnpcW3BGuR0zAp LfRbNy6fBr69ja/myfOWi3tb4ni88/Hl4bicc2vyTtH9DpdCtxT6HRUNPv0+NWia6WTD lLxA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=8i/CEMt+caQXI4CQ6N0u4G0S/6Yq2Gtzmk3eKpoXTjI=; b=FbBosNyH0TQj86aZIetHW6wPOc+ak2z9MQs+Jlvx5Ayby8EOKV4bXquQENB672jTRp rtNgsbFZICC9pj8y0jZ30wAJhRcS3xEmNm/bD+jjubQsSjhq5+LSoOpmD77WnhJIGDGU BS7iigBtf/SMk/QVriJDnD7sZOSM+UsKn02eKnWnToV+ExHheH31Trl0wQFJh3m9zcpi CLZZhvaK+6jY5v+MUD4QFAtbiO1pwS4sfuM4zXTuff3t/ju/MLvKdcu0wFojN+LTLLS+ NfcjX77cC0PqibYSeTHDhd51HjGFLlTiaIBzx5s+7X18BYwal+4Mxws3fwO11qxjmbDn gydg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=m37hezI9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id e65-20020a636944000000b00553c4551a08si5979621pgc.887.2023.06.22.02.50.47; Thu, 22 Jun 2023 02:50:59 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=m37hezI9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232408AbjFVJeh (ORCPT <rfc822;maxin.john@gmail.com> + 99 others); Thu, 22 Jun 2023 05:34:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39486 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232068AbjFVJdl (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 22 Jun 2023 05:33:41 -0400 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0B9A92697; Thu, 22 Jun 2023 02:28:00 -0700 (PDT) Received: from IcarusMOD.eternityproject.eu (unknown [IPv6:2001:b07:2ed:14ed:c5f8:7372:f042:90a2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by madras.collabora.co.uk (Postfix) with ESMTPSA id 8FDE06607075; Thu, 22 Jun 2023 10:27:57 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1687426078; bh=cjwGV4PBGa2G0fyxMUorKbKCregA7+FHFAZQsfL0PRY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=m37hezI9O+FIW53mDrj1fpvmgKyTxgIcCC1DpCNs+GjhhVfb9UFw6hqoRzc2LXMPM Qkbk2d6MFxwk+xySVz2XHuURvTzHhfepVbWdVnG1LKvnOgHxuZa2XVi04qbn3yFSCw ATIyD6sh7+zFrrdRKjGqRHKWQSZv5PRjFYf4V1r/yizDBMFJiuSwsL6YG8HfUsBrE1 f0QYSj1rl+PSL08dCY3htW3h2Z+LpQQVOzyMyZNUSUz5EZegHDwS7dHgaRr0NfCeGi T3bVFrUlnA19w8/Z7YyRUod2UZpQG3B5dIA4XyKMt1RD1F8Mc191qdn8UmeYSyKNkG wsQImsYA5NYmQ== From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> To: agross@kernel.org Cc: andersson@kernel.org, luca@z3ntu.xyz, konrad.dybcio@linaro.org, dmitry.baryshkov@linaro.org, joro@8bytes.org, will@kernel.org, robin.murphy@arm.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, robdclark@gmail.com, linux-arm-msm@vger.kernel.org, iommu@lists.linux.dev, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, kernel@collabora.com, Marijn Suijten <marijn.suijten@somainline.org> Subject: [PATCH v5 2/6] iommu/qcom: Use the asid read from device-tree if specified Date: Thu, 22 Jun 2023 11:27:38 +0200 Message-Id: <20230622092742.74819-3-angelogioacchino.delregno@collabora.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230622092742.74819-1-angelogioacchino.delregno@collabora.com> References: <20230622092742.74819-1-angelogioacchino.delregno@collabora.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769395935324351793?= X-GMAIL-MSGID: =?utf-8?q?1769395935324351793?= |
Series |
Add support for Qualcomm's legacy IOMMU v2
|
|
Commit Message
AngeloGioacchino Del Regno
June 22, 2023, 9:27 a.m. UTC
As specified in this driver, the context banks are 0x1000 apart but on some SoCs the context number does not necessarily match this logic, hence we end up using the wrong ASID: keeping in mind that this IOMMU implementation relies heavily on SCM (TZ) calls, it is mandatory that we communicate the right context number. Since this is all about how context banks are mapped in firmware, which may be board dependent (as a different firmware version may eventually change the expected context bank numbers), introduce a new property "qcom,ctx-asid": when found, the ASID will be forced as read from the devicetree. When "qcom,ctx-asid" is not found, this driver retains the previous behavior as to avoid breaking older devicetrees or systems that do not require forcing ASID numbers. Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> [Marijn: Rebased over next-20221111] Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
Comments
On 22.06.2023 11:27, AngeloGioacchino Del Regno wrote: > As specified in this driver, the context banks are 0x1000 apart but > on some SoCs the context number does not necessarily match this > logic, hence we end up using the wrong ASID: keeping in mind that > this IOMMU implementation relies heavily on SCM (TZ) calls, it is > mandatory that we communicate the right context number. > > Since this is all about how context banks are mapped in firmware, > which may be board dependent (as a different firmware version may > eventually change the expected context bank numbers), introduce a > new property "qcom,ctx-asid": when found, the ASID will be forced > as read from the devicetree. > > When "qcom,ctx-asid" is not found, this driver retains the previous > behavior as to avoid breaking older devicetrees or systems that do > not require forcing ASID numbers. > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > [Marijn: Rebased over next-20221111] > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c > index a503ed758ec3..8face57c4180 100644 > --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c > +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c > @@ -531,7 +531,8 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args) > * index into qcom_iommu->ctxs: > */ > if (WARN_ON(asid < 1) || > - WARN_ON(asid > qcom_iommu->num_ctxs)) { > + WARN_ON(asid > qcom_iommu->num_ctxs) || > + WARN_ON(qcom_iommu->ctxs[asid - 1] == NULL)) { > put_device(&iommu_pdev->dev); > return -EINVAL; > } > @@ -617,7 +618,8 @@ static int qcom_iommu_sec_ptbl_init(struct device *dev) > > static int get_asid(const struct device_node *np) > { > - u32 reg; > + u32 reg, val; > + int asid; > > /* read the "reg" property directly to get the relative address > * of the context bank, and calculate the asid from that: > @@ -625,7 +627,17 @@ static int get_asid(const struct device_node *np) > if (of_property_read_u32_index(np, "reg", 0, ®)) > return -ENODEV; > > - return reg / 0x1000; /* context banks are 0x1000 apart */ > + /* > + * Context banks are 0x1000 apart but, in some cases, the ASID > + * number doesn't match to this logic and needs to be passed > + * from the DT configuration explicitly. > + */ > + if (!of_property_read_u32(np, "qcom,ctx-asid", &val)) > + asid = val; > + else > + asid = reg / 0x1000; > + > + return asid; > } > > static int qcom_iommu_ctx_probe(struct platform_device *pdev)
On Thu, Jun 22, 2023 at 11:27:38AM +0200, AngeloGioacchino Del Regno wrote: > As specified in this driver, the context banks are 0x1000 apart but > on some SoCs the context number does not necessarily match this > logic, hence we end up using the wrong ASID: keeping in mind that > this IOMMU implementation relies heavily on SCM (TZ) calls, it is > mandatory that we communicate the right context number. > > Since this is all about how context banks are mapped in firmware, > which may be board dependent (as a different firmware version may > eventually change the expected context bank numbers), introduce a > new property "qcom,ctx-asid": when found, the ASID will be forced > as read from the devicetree. > > When "qcom,ctx-asid" is not found, this driver retains the previous > behavior as to avoid breaking older devicetrees or systems that do > not require forcing ASID numbers. > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > [Marijn: Rebased over next-20221111] > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c > index a503ed758ec3..8face57c4180 100644 > --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c > +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c > @@ -531,7 +531,8 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args) > * index into qcom_iommu->ctxs: > */ > if (WARN_ON(asid < 1) || > - WARN_ON(asid > qcom_iommu->num_ctxs)) { > + WARN_ON(asid > qcom_iommu->num_ctxs) || > + WARN_ON(qcom_iommu->ctxs[asid - 1] == NULL)) { > put_device(&iommu_pdev->dev); > return -EINVAL; > } > @@ -617,7 +618,8 @@ static int qcom_iommu_sec_ptbl_init(struct device *dev) > > static int get_asid(const struct device_node *np) > { > - u32 reg; > + u32 reg, val; > + int asid; > > /* read the "reg" property directly to get the relative address > * of the context bank, and calculate the asid from that: > @@ -625,7 +627,17 @@ static int get_asid(const struct device_node *np) > if (of_property_read_u32_index(np, "reg", 0, ®)) > return -ENODEV; > > - return reg / 0x1000; /* context banks are 0x1000 apart */ > + /* > + * Context banks are 0x1000 apart but, in some cases, the ASID > + * number doesn't match to this logic and needs to be passed > + * from the DT configuration explicitly. > + */ > + if (!of_property_read_u32(np, "qcom,ctx-asid", &val)) > + asid = val; > + else > + asid = reg / 0x1000; > + > + return asid; Shouldn't we at least have some error checking here? For example, ensuring that the ASIDs are within range, aren't duplicates etc? Also, can you elaborate a little more on what sort of ASID-to-Context mappings you actually see in practice? Will
Il 01/08/23 15:49, Will Deacon ha scritto: > On Thu, Jun 22, 2023 at 11:27:38AM +0200, AngeloGioacchino Del Regno wrote: >> As specified in this driver, the context banks are 0x1000 apart but >> on some SoCs the context number does not necessarily match this >> logic, hence we end up using the wrong ASID: keeping in mind that >> this IOMMU implementation relies heavily on SCM (TZ) calls, it is >> mandatory that we communicate the right context number. >> >> Since this is all about how context banks are mapped in firmware, >> which may be board dependent (as a different firmware version may >> eventually change the expected context bank numbers), introduce a >> new property "qcom,ctx-asid": when found, the ASID will be forced >> as read from the devicetree. >> >> When "qcom,ctx-asid" is not found, this driver retains the previous >> behavior as to avoid breaking older devicetrees or systems that do >> not require forcing ASID numbers. >> >> Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> >> [Marijn: Rebased over next-20221111] >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- >> drivers/iommu/arm/arm-smmu/qcom_iommu.c | 18 +++++++++++++++--- >> 1 file changed, 15 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c >> index a503ed758ec3..8face57c4180 100644 >> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c >> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c >> @@ -531,7 +531,8 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args) >> * index into qcom_iommu->ctxs: >> */ >> if (WARN_ON(asid < 1) || >> - WARN_ON(asid > qcom_iommu->num_ctxs)) { >> + WARN_ON(asid > qcom_iommu->num_ctxs) || >> + WARN_ON(qcom_iommu->ctxs[asid - 1] == NULL)) { >> put_device(&iommu_pdev->dev); >> return -EINVAL; >> } >> @@ -617,7 +618,8 @@ static int qcom_iommu_sec_ptbl_init(struct device *dev) >> >> static int get_asid(const struct device_node *np) >> { >> - u32 reg; >> + u32 reg, val; >> + int asid; >> >> /* read the "reg" property directly to get the relative address >> * of the context bank, and calculate the asid from that: >> @@ -625,7 +627,17 @@ static int get_asid(const struct device_node *np) >> if (of_property_read_u32_index(np, "reg", 0, ®)) >> return -ENODEV; >> >> - return reg / 0x1000; /* context banks are 0x1000 apart */ >> + /* >> + * Context banks are 0x1000 apart but, in some cases, the ASID >> + * number doesn't match to this logic and needs to be passed >> + * from the DT configuration explicitly. >> + */ >> + if (!of_property_read_u32(np, "qcom,ctx-asid", &val)) >> + asid = val; >> + else >> + asid = reg / 0x1000; >> + >> + return asid; > > Shouldn't we at least have some error checking here? For example, ensuring > that the ASIDs are within range, aren't duplicates etc? > The only check that we can perform here for ASID-in-range is if ((asid * 0x1000 > (mmio_start + mmio_size - 0x1000)) return -EINVAL; ...as for duplicates, a check can *probably* (surely) be done... but I'm not sure I have any more time to feed more code to this series from years ago... > Also, can you elaborate a little more on what sort of ASID-to-Context > mappings you actually see in practice? > I'm sorry, but not really. The first version of this (including the whole research that I had to perform to write those patches) is from year 2019, so 4 years ago... ...I don't really remember the full details anymore - if not that all of this was done because context banks are fixed (and setup by TZ), tz takes an asid number when trying to perform any operation on the context bank, and there's no way to reset mappings because everything is protected by the hypervisor (which will fault and reboot the AP instantly if you try). Cheers, Angelo
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index a503ed758ec3..8face57c4180 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -531,7 +531,8 @@ static int qcom_iommu_of_xlate(struct device *dev, struct of_phandle_args *args) * index into qcom_iommu->ctxs: */ if (WARN_ON(asid < 1) || - WARN_ON(asid > qcom_iommu->num_ctxs)) { + WARN_ON(asid > qcom_iommu->num_ctxs) || + WARN_ON(qcom_iommu->ctxs[asid - 1] == NULL)) { put_device(&iommu_pdev->dev); return -EINVAL; } @@ -617,7 +618,8 @@ static int qcom_iommu_sec_ptbl_init(struct device *dev) static int get_asid(const struct device_node *np) { - u32 reg; + u32 reg, val; + int asid; /* read the "reg" property directly to get the relative address * of the context bank, and calculate the asid from that: @@ -625,7 +627,17 @@ static int get_asid(const struct device_node *np) if (of_property_read_u32_index(np, "reg", 0, ®)) return -ENODEV; - return reg / 0x1000; /* context banks are 0x1000 apart */ + /* + * Context banks are 0x1000 apart but, in some cases, the ASID + * number doesn't match to this logic and needs to be passed + * from the DT configuration explicitly. + */ + if (!of_property_read_u32(np, "qcom,ctx-asid", &val)) + asid = val; + else + asid = reg / 0x1000; + + return asid; } static int qcom_iommu_ctx_probe(struct platform_device *pdev)