Message ID | 20231019021923.13939-2-quic_c_gdjako@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2010:b0:403:3b70:6f57 with SMTP id fe16csp118450vqb; Wed, 18 Oct 2023 19:20:26 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHkagnTvHrVuNCPoiGNe/lWsho2F3eXTpaJtcOfU9VLUSpy2+GJ6+Ms3pNGvyCbPD80xjuC X-Received: by 2002:a05:6870:6c02:b0:1e9:de37:a75d with SMTP id na2-20020a0568706c0200b001e9de37a75dmr1395227oab.17.1697682026569; Wed, 18 Oct 2023 19:20:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697682026; cv=none; d=google.com; s=arc-20160816; b=XCAgxx21v8H+9Mu2I77E3z7DD07P9vKEauWKkVXdTF+a5Y7LlKGVcjsR4tNk+zEMdU 6sQ3AgXReqU8A8bPqR+0nb5saKjsMD7JsiHsmKb89rEAZxhYWb76852QTzWz98r+aiKX DBc79joEkSDg1sdKipU4jEnlmqujJ0ccl4Pf9JgcyLVub+4aK+IlHh6Qpfck4s6ZmmKA 3DmP29Omo1s4EzR+cAgPcKDOVJYQTBspL4JZk9VcPU6e+ITOqWG2DNs6LfxEnn1x3NTl wKFiEPGbHKlbXgU9FZO8DRcA4Gh+PLehU5PfpVuXwxzWSA0SDNqEOM787MtOeNyA3m8p UssQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:references:in-reply-to:message-id :date:subject:cc:to:from:dkim-signature; bh=693Rhk+lQnzIGwgGutNXFeZ2mkIq6DSTUDmC7J7Oplg=; fh=ajqwJHB27W/J8EhLqkTAR2+yy30uzELfRT2VzGpH0WI=; b=lC4ZFQluwcRJ9pSUcu8/mk2F8c6jmMx3BCl89KYcGpgPsMxJ6ptLJ3fhiF0kZI77mp 9cKMe8FmckXYIrSfkVqZ+UwR7pGg3ZU+u2E37lygunBqltoIc+oWiqUOxQlZ3fDVhYE2 Zv3SMvr3avF0rJJr+Fy98HqZMHkBMkxyJ3+uNAI7tS8b5ffJR/mP/7FfHOVMen5mLZvs rJrxSrDdgxxQSl3UTXuuujCq6vd6UQ3M3wBD5w3jZqWBAz5p2qdTtZxXILC40aa3Pxm4 kTmFLaeuV/OuXyKiZzDGSYGS2DaTMib0/9/YrAJOmJkRR4f/4ocX8VNwPCZ/b5pPHk4A HryA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=ECe0jJ7w; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id a22-20020a637f16000000b005adba954597si3361927pgd.504.2023.10.18.19.20.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Oct 2023 19:20:26 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=ECe0jJ7w; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 49ECF82164FB; Wed, 18 Oct 2023 19:20:25 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232466AbjJSCUU (ORCPT <rfc822;zwp10758@gmail.com> + 24 others); Wed, 18 Oct 2023 22:20:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37720 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232490AbjJSCUQ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 18 Oct 2023 22:20:16 -0400 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D78F13E; Wed, 18 Oct 2023 19:20:11 -0700 (PDT) Received: from pps.filterd (m0279871.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 39J2Ia91001191; Thu, 19 Oct 2023 02:19:42 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-type; s=qcppdkim1; bh=693Rhk+lQnzIGwgGutNXFeZ2mkIq6DSTUDmC7J7Oplg=; b=ECe0jJ7wS+YXszLn+IvVPCcMFoxuSx2USOnJJ94rG6lLPAyUK64AYNlIAOACMXPDC2EF RAFkQZvck+tzVwwmW9VoiqVczvJiTkfuRq9ELcZdgp4cP5220P9Y8LUNA4GzhXVlk60X Cr5x3v9dcz1FjtWIbU4fdlOC0HmdUSPiZt3r9Yb2vSoPXc1HQRNkBUneiW0uqyjgOEb3 WF+Fcqz5G45jHCrKf5GfHLNGfYuwGv8NgYdhvflB9oL8uGLDQ5FXHpXTLhTHytnZ7haW gm4URHHUx4rIisEWw/5hN4Lrcl6xvZQnNbfdgGplSZQjz95IC/4rFAN9MbTW55gWc5SL Ig== Received: from nasanppmta04.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3tt5v82qy8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 19 Oct 2023 02:19:42 +0000 Received: from nasanex01a.na.qualcomm.com (nasanex01a.na.qualcomm.com [10.52.223.231]) by NASANPPMTA04.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 39J2Jep2027375 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 19 Oct 2023 02:19:40 GMT Received: from hu-c-gdjako-lv.qualcomm.com (10.49.16.6) by nasanex01a.na.qualcomm.com (10.52.223.231) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.39; Wed, 18 Oct 2023 19:19:40 -0700 From: Georgi Djakov <quic_c_gdjako@quicinc.com> To: <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>, <will@kernel.org>, <robin.murphy@arm.com>, <joro@8bytes.org> CC: <devicetree@vger.kernel.org>, <andersson@kernel.org>, <konrad.dybcio@linaro.org>, <linux-arm-kernel@lists.infradead.org>, <iommu@lists.linux.dev>, <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>, <quic_cgoldswo@quicinc.com>, <quic_sukadev@quicinc.com>, <quic_pdaly@quicinc.com>, <quic_sudaraja@quicinc.com>, <djakov@kernel.org> Subject: [PATCH 1/6] dt-bindings: iommu: Add Translation Buffer Unit bindings Date: Wed, 18 Oct 2023 19:19:18 -0700 Message-ID: <20231019021923.13939-2-quic_c_gdjako@quicinc.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20231019021923.13939-1-quic_c_gdjako@quicinc.com> References: <20231019021923.13939-1-quic_c_gdjako@quicinc.com> MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.49.16.6] X-ClientProxiedBy: nalasex01a.na.qualcomm.com (10.47.209.196) To nasanex01a.na.qualcomm.com (10.52.223.231) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: Ajg6GGsAZFcHJohQqndeTSociNDKdG33 X-Proofpoint-ORIG-GUID: Ajg6GGsAZFcHJohQqndeTSociNDKdG33 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-10-19_02,2023-10-18_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 mlxlogscore=999 lowpriorityscore=0 phishscore=0 impostorscore=0 suspectscore=0 mlxscore=0 clxscore=1015 bulkscore=0 priorityscore=1501 spamscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2309180000 definitions=main-2310190017 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 18 Oct 2023 19:20:25 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780148628694455085 X-GMAIL-MSGID: 1780148628694455085 |
Series |
Add support for Translation Buffer Units
|
|
Commit Message
Georgi Djakov
Oct. 19, 2023, 2:19 a.m. UTC
The "apps_smmu" on the Qualcomm sdm845 platform is an implementation
of the ARM SMMU-500, that consists of a single TCU (Translation Control
Unit) and multiple TBUs (Translation Buffer Units). The TCU is already
being described in the ARM SMMU DT schema. Add also bindings for the
TBUs so that we can describe their properties.
In this DT schema, the TBUs are modelled as a child devices of the TCU
and each of them is described with it's own resources such as clocks,
power domains, interconnects etc.
Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com>
---
.../devicetree/bindings/iommu/arm,smmu.yaml | 13 ++++
.../bindings/iommu/qcom,qsmmuv500-tbu.yaml | 67 +++++++++++++++++++
2 files changed, 80 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml
Comments
On 10/19/23 04:19, Georgi Djakov wrote: > The "apps_smmu" on the Qualcomm sdm845 platform is an implementation > of the ARM SMMU-500, that consists of a single TCU (Translation Control > Unit) and multiple TBUs (Translation Buffer Units). The TCU is already > being described in the ARM SMMU DT schema. Add also bindings for the > TBUs so that we can describe their properties. > > In this DT schema, the TBUs are modelled as a child devices of the TCU > and each of them is described with it's own resources such as clocks, > power domains, interconnects etc. > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > --- > .../devicetree/bindings/iommu/arm,smmu.yaml | 13 ++++ > .../bindings/iommu/qcom,qsmmuv500-tbu.yaml | 67 +++++++++++++++++++ > 2 files changed, 80 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > index cf29ab10501c..afc323b4bbc5 100644 > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > @@ -230,6 +230,19 @@ properties: > enabled for any given device. > $ref: /schemas/types.yaml#/definitions/phandle > > + '#address-cells': > + const: 2 > + > + '#size-cells': > + const: 2 > + > + ranges: true > + > +patternProperties: > + "^tbu@[0-9a-f]+$": > + $ref: qcom,qsmmuv500-tbu.yaml > + description: The SMMU may include Translation Buffer Units (TBU) as subnodes > + > required: > - compatible > - reg > diff --git a/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > new file mode 100644 > index 000000000000..4baba7397e90 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > @@ -0,0 +1,67 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iommu/qcom,qsmmuv500-tbu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm TBU (Translation Buffer Unit) > + > +maintainers: > + - Georgi Djakov <quic_c_gdjako@quicinc.com> > + > +description: > + TBU nodes represent Translation Buffer Units in an ARM SMMU. Each TBU node > + should be a child node of the SMMU in the device tree. description: refers to the hardware, so it should say what this IP is, what it does and things like that > + > +properties: > + compatible: > + enum: > + - qcom,qsmmuv500-tbu Should we expect this list to grow? > + > + reg: > + items: > + - description: Address and size of the TBU's register space. > + > + reg-names: > + items: > + - const: base > + > + clocks: > + maxItems: 1 > + > + interconnects: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + qcom,stream-id-range: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: Stream ID range (address and size) that is assigned by the TBU I believe you need to size-limit this. If it's only supposed to be a single tuple, perhaps it could be said explicitly. > + > +required: > + - compatible > + - reg > + - interconnects > + - qcom,stream-id-range > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/qcom,gcc-sdm845.h> > + #include <dt-bindings/interconnect/qcom,sdm845.h> > + #include <dt-bindings/power/qcom-rpmpd.h> > + > + 2 newlines seems excessive > + tbu@150e1000 { > + compatible = "qcom,qsmmuv500-tbu"; > + reg = <0x150e1000 0x1000>; > + reg-names = "base"; > + clocks = <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>; > + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_PCIE_TBU_GDSC>; > + interconnects = <&system_noc MASTER_GNOC_SNOC 0 &config_noc SLAVE_IMEM_CFG 0>; > + qcom,stream-id-range = <0x1c00 0x400>; > + }; I think it would be beneficial if this tbu was a child of some smmu node like it's intended to be. Konrad
On Wed, Oct 18, 2023 at 07:19:18PM -0700, Georgi Djakov wrote: > The "apps_smmu" on the Qualcomm sdm845 platform is an implementation > of the ARM SMMU-500, that consists of a single TCU (Translation Control > Unit) and multiple TBUs (Translation Buffer Units). The TCU is already > being described in the ARM SMMU DT schema. Add also bindings for the > TBUs so that we can describe their properties. Arm SMMU-500 is an implementation, too. Is QCom's a modified implementation or you are just the first to want to control TBU resources? You need to split this into what could be any SMMU-500 implementation and what is truly QCom specific (i.e. modified). Unlike some licensed IP that's a free-for-all on DT resources, Arm IP has public specs so we don't have to guess. > In this DT schema, the TBUs are modelled as a child devices of the TCU > and each of them is described with it's own resources such as clocks, > power domains, interconnects etc. > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > --- > .../devicetree/bindings/iommu/arm,smmu.yaml | 13 ++++ > .../bindings/iommu/qcom,qsmmuv500-tbu.yaml | 67 +++++++++++++++++++ > 2 files changed, 80 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > index cf29ab10501c..afc323b4bbc5 100644 > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml > @@ -230,6 +230,19 @@ properties: > enabled for any given device. > $ref: /schemas/types.yaml#/definitions/phandle > > + '#address-cells': > + const: 2 > + > + '#size-cells': > + const: 2 > + > + ranges: true > + > +patternProperties: > + "^tbu@[0-9a-f]+$": > + $ref: qcom,qsmmuv500-tbu.yaml Generic SMMU binding includes something QCom specific. That's not right. > + description: The SMMU may include Translation Buffer Units (TBU) as subnodes > + > required: > - compatible > - reg > diff --git a/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > new file mode 100644 > index 000000000000..4baba7397e90 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml > @@ -0,0 +1,67 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iommu/qcom,qsmmuv500-tbu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm TBU (Translation Buffer Unit) > + > +maintainers: > + - Georgi Djakov <quic_c_gdjako@quicinc.com> > + > +description: > + TBU nodes represent Translation Buffer Units in an ARM SMMU. Each TBU node > + should be a child node of the SMMU in the device tree. > + > +properties: > + compatible: > + enum: > + - qcom,qsmmuv500-tbu > + > + reg: > + items: > + - description: Address and size of the TBU's register space. > + > + reg-names: > + items: > + - const: base > + > + clocks: > + maxItems: 1 > + > + interconnects: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + qcom,stream-id-range: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: Stream ID range (address and size) that is assigned by the TBU > + > +required: > + - compatible > + - reg > + - interconnects > + - qcom,stream-id-range > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/qcom,gcc-sdm845.h> > + #include <dt-bindings/interconnect/qcom,sdm845.h> > + #include <dt-bindings/power/qcom-rpmpd.h> > + > + > + tbu@150e1000 { > + compatible = "qcom,qsmmuv500-tbu"; > + reg = <0x150e1000 0x1000>; > + reg-names = "base"; > + clocks = <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>; > + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_PCIE_TBU_GDSC>; > + interconnects = <&system_noc MASTER_GNOC_SNOC 0 &config_noc SLAVE_IMEM_CFG 0>; > + qcom,stream-id-range = <0x1c00 0x400>; > + }; > + > +...
On 2023-10-24 19:42, Rob Herring wrote: > On Wed, Oct 18, 2023 at 07:19:18PM -0700, Georgi Djakov wrote: >> The "apps_smmu" on the Qualcomm sdm845 platform is an implementation >> of the ARM SMMU-500, that consists of a single TCU (Translation Control >> Unit) and multiple TBUs (Translation Buffer Units). The TCU is already >> being described in the ARM SMMU DT schema. Add also bindings for the >> TBUs so that we can describe their properties. > > Arm SMMU-500 is an implementation, too. Is QCom's a modified > implementation or you are just the first to want to control TBU > resources? It's very very modified. The stock MMU-500 has very few microarchitectural registers[1], they all live within the regular SMMU address space, are all Secure-only by default, and don't do anything like the shenanigans here. That said, looking at patch #3, I don't really understand why we need any of this stuff upstream... AFAICS it's doing an insane amount of work to use complicated imp-def debug functionality to duplicate things that the main driver can already do far more efficiently. Sure, in general it seems like it could potentially be useful stuff for bringing up and debugging a new driver, but the Linux SMMUv2 driver is mature and frankly already closer to being obsolete than to being new... [ digression since I can't be bothered to split this discussion by replying separately to patch #3: ] I mean, just looking at qsmmuv500_iova_to_phys(), you do realise that that's going to be called potentially multiple times by iommu-dma for *every* dma_sync and dma_unmap call and really wants to be fast, right? This brings to mind all the work I did a couple of years back[2] because strict TLB invalidation on unmap was considered too slow for certain devices on QCom platforms by ChromeOS, yet what this achieves looks like it could easily be up to an order of magnitude slower again :( > You need to split this into what could be any SMMU-500 implementation > and what is truly QCom specific (i.e. modified). Unlike some licensed IP > that's a free-for-all on DT resources, Arm IP has public specs so we > don't have to guess. > >> In this DT schema, the TBUs are modelled as a child devices of the TCU >> and each of them is described with it's own resources such as clocks, >> power domains, interconnects etc. >> >> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> >> --- >> .../devicetree/bindings/iommu/arm,smmu.yaml | 13 ++++ >> .../bindings/iommu/qcom,qsmmuv500-tbu.yaml | 67 +++++++++++++++++++ >> 2 files changed, 80 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml >> >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> index cf29ab10501c..afc323b4bbc5 100644 >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml >> @@ -230,6 +230,19 @@ properties: >> enabled for any given device. >> $ref: /schemas/types.yaml#/definitions/phandle >> >> + '#address-cells': >> + const: 2 >> + >> + '#size-cells': >> + const: 2 >> + >> + ranges: true >> + >> +patternProperties: >> + "^tbu@[0-9a-f]+$": >> + $ref: qcom,qsmmuv500-tbu.yaml > > Generic SMMU binding includes something QCom specific. That's not right. > > >> + description: The SMMU may include Translation Buffer Units (TBU) as subnodes >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml >> new file mode 100644 >> index 000000000000..4baba7397e90 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml >> @@ -0,0 +1,67 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/iommu/qcom,qsmmuv500-tbu.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm TBU (Translation Buffer Unit) >> + >> +maintainers: >> + - Georgi Djakov <quic_c_gdjako@quicinc.com> >> + >> +description: >> + TBU nodes represent Translation Buffer Units in an ARM SMMU. Each TBU node >> + should be a child node of the SMMU in the device tree. >> + >> +properties: >> + compatible: >> + enum: >> + - qcom,qsmmuv500-tbu >> + >> + reg: >> + items: >> + - description: Address and size of the TBU's register space. >> + >> + reg-names: >> + items: >> + - const: base >> + >> + clocks: >> + maxItems: 1 >> + >> + interconnects: >> + maxItems: 1 What does this interconnect represent? MMU-500 TBUs don't access memory themselves[3], they only have an internal AXI Stream interface to the TCU to request translations. Thanks, Robin. [1] https://developer.arm.com/documentation/ddi0517/f/programmers-model/memory-model [2] https://lore.kernel.org/all/d652966348c78457c38bf18daf369272a4ebc2c9.1628682049.git.robin.murphy@arm.com/ [3] Yeah yeah, other than the special case of TBU0 issuing pagetable walks on behalf of the TCU when it's not configured with its own dedicated PTW interface, I know :P >> + >> + power-domains: >> + maxItems: 1 >> + >> + qcom,stream-id-range: >> + $ref: /schemas/types.yaml#/definitions/uint32-array >> + description: Stream ID range (address and size) that is assigned by the TBU >> + >> +required: >> + - compatible >> + - reg >> + - interconnects >> + - qcom,stream-id-range >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/qcom,gcc-sdm845.h> >> + #include <dt-bindings/interconnect/qcom,sdm845.h> >> + #include <dt-bindings/power/qcom-rpmpd.h> >> + >> + >> + tbu@150e1000 { >> + compatible = "qcom,qsmmuv500-tbu"; >> + reg = <0x150e1000 0x1000>; >> + reg-names = "base"; >> + clocks = <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>; >> + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_PCIE_TBU_GDSC>; >> + interconnects = <&system_noc MASTER_GNOC_SNOC 0 &config_noc SLAVE_IMEM_CFG 0>; >> + qcom,stream-id-range = <0x1c00 0x400>; >> + }; >> + >> +...
Hi Robin, Thanks for taking a look at this! On 10/25/2023 1:26 AM, Robin Murphy wrote: > On 2023-10-24 19:42, Rob Herring wrote: >> On Wed, Oct 18, 2023 at 07:19:18PM -0700, Georgi Djakov wrote: >>> The "apps_smmu" on the Qualcomm sdm845 platform is an implementation >>> of the ARM SMMU-500, that consists of a single TCU (Translation Control >>> Unit) and multiple TBUs (Translation Buffer Units). The TCU is already >>> being described in the ARM SMMU DT schema. Add also bindings for the >>> TBUs so that we can describe their properties. >> >> Arm SMMU-500 is an implementation, too. Is QCom's a modified >> implementation or you are just the first to want to control TBU >> resources? > > It's very very modified. The stock MMU-500 has very few microarchitectural registers[1], they all live within the regular SMMU address space, are all Secure-only by default, and don't do anything like the shenanigans here. > > That said, looking at patch #3, I don't really understand why we need any of this stuff upstream... AFAICS it's doing an insane amount of work to use complicated imp-def debug functionality to duplicate things that the main driver can already do far more efficiently. Sure, in general it seems like it could potentially be useful stuff for bringing up and debugging a new driver, but the Linux SMMUv2 driver is mature and frankly already closer to being obsolete than to being new... Yes, the arm-smmu driver already does similar stuff with the ATS feature, but this unfortunately isn't available in Qualcomm's implementation. Instead of that, there is this eCATS thing for debugging various issues including hardware issues. It supports many features, but here we use it just for hardware page table walks. And in the majority of cases it's expected that the software and hardware page table walks give the same result, but if there is a difference, it's sign of a problem. For example, it helped in the past to trace some power management issues of the SMMU. This of course is a debug feature and can enabled when needed. > [ digression since I can't be bothered to split this discussion by replying separately to patch #3: ] > > I mean, just looking at qsmmuv500_iova_to_phys(), you do realise that that's going to be called potentially multiple times by iommu-dma for *every* dma_sync and dma_unmap call and really wants to be fast, right? This brings to mind all the work I did a couple of years back[2] because strict TLB invalidation on unmap was considered too slow for certain devices on QCom platforms by ChromeOS, yet what this achieves looks like it could easily be up to an order of magnitude slower again :( No, this is not going to be called for every dma_sync and dma_unmap. In patch 5 we register a custom context_fault handler that uses this code to get information from the TBUs. So all of this is executed only when a context fault occurs. Does this sound acceptable? [..]>>> +description: >>> + TBU nodes represent Translation Buffer Units in an ARM SMMU. Each TBU node >>> + should be a child node of the SMMU in the device tree. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - qcom,qsmmuv500-tbu >>> + >>> + reg: >>> + items: >>> + - description: Address and size of the TBU's register space. >>> + >>> + reg-names: >>> + items: >>> + - const: base >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> + interconnects: >>> + maxItems: 1 > > What does this interconnect represent? MMU-500 TBUs don't access memory themselves[3], they only have an internal AXI Stream interface to the TCU to request translations. It's to enable access from the CPU to the register space of the TBUs. Thanks, Georgi
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index cf29ab10501c..afc323b4bbc5 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -230,6 +230,19 @@ properties: enabled for any given device. $ref: /schemas/types.yaml#/definitions/phandle + '#address-cells': + const: 2 + + '#size-cells': + const: 2 + + ranges: true + +patternProperties: + "^tbu@[0-9a-f]+$": + $ref: qcom,qsmmuv500-tbu.yaml + description: The SMMU may include Translation Buffer Units (TBU) as subnodes + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml new file mode 100644 index 000000000000..4baba7397e90 --- /dev/null +++ b/Documentation/devicetree/bindings/iommu/qcom,qsmmuv500-tbu.yaml @@ -0,0 +1,67 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iommu/qcom,qsmmuv500-tbu.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm TBU (Translation Buffer Unit) + +maintainers: + - Georgi Djakov <quic_c_gdjako@quicinc.com> + +description: + TBU nodes represent Translation Buffer Units in an ARM SMMU. Each TBU node + should be a child node of the SMMU in the device tree. + +properties: + compatible: + enum: + - qcom,qsmmuv500-tbu + + reg: + items: + - description: Address and size of the TBU's register space. + + reg-names: + items: + - const: base + + clocks: + maxItems: 1 + + interconnects: + maxItems: 1 + + power-domains: + maxItems: 1 + + qcom,stream-id-range: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: Stream ID range (address and size) that is assigned by the TBU + +required: + - compatible + - reg + - interconnects + - qcom,stream-id-range + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/qcom,gcc-sdm845.h> + #include <dt-bindings/interconnect/qcom,sdm845.h> + #include <dt-bindings/power/qcom-rpmpd.h> + + + tbu@150e1000 { + compatible = "qcom,qsmmuv500-tbu"; + reg = <0x150e1000 0x1000>; + reg-names = "base"; + clocks = <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>; + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_PCIE_TBU_GDSC>; + interconnects = <&system_noc MASTER_GNOC_SNOC 0 &config_noc SLAVE_IMEM_CFG 0>; + qcom,stream-id-range = <0x1c00 0x400>; + }; + +...