lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 19 Dec 2023 15:25:19 +0000
From: Robin Murphy <robin.murphy@....com>
To: David Heidelberg <david@...t.cz>, Joerg Roedel <joro@...tes.org>,
 Will Deacon <will@...nel.org>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Andy Gross <agross@...nel.org>,
 Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konrad.dybcio@...aro.org>
Cc: iommu@...ts.linux.dev, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v5] dt-bindings: iommu: Convert msm,iommu-v0 to yaml

On 2023-12-16 12:22 am, David Heidelberg wrote:
> Convert Qualcomm IOMMU v0 implementation to yaml format.
> 
> Signed-off-by: David Heidelberg <david@...t.cz>
> ---
> v5:
>   - updated example (thx @Konrad)
>   - ordering of requirements + dropped > and | and reformatted (thx @Konrad)
> v4:
>   - renamed to qcom,apq8064-iommu as Rob requested
>   - changed title to Qualcomm APQ8064 IOMMU
>   - dropped quotes around URLs
>   - dropped mdp node
>   - dropped unused mdp_port0 label
> 
> v3:
>   - I kept the name as -v0, since we have other binding -v1 and it look
>     good, I can change thou in v4 if requested.
>   - dropped non-existent smmu_clk part (and adjusted example, which was
>     using it)
>   - dropped iommu description
>   - moved iommu-cells description to the property #iommu-cells
> 
> v2:
>   - fix wrong path in binding $id
>   - comment qcom,mdp4 node example (we don't want to validate it yet)
> 
>   .../bindings/iommu/msm,iommu-v0.txt           | 64 ---------------
>   .../bindings/iommu/qcom,apq8064-iommu.yaml    | 82 +++++++++++++++++++
>   2 files changed, 82 insertions(+), 64 deletions(-)
>   delete mode 100644 Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
>   create mode 100644 Documentation/devicetree/bindings/iommu/qcom,apq8064-iommu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt b/Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
> deleted file mode 100644
> index 20236385f26e..000000000000
> --- a/Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
> +++ /dev/null
> @@ -1,64 +0,0 @@
> -* QCOM IOMMU
> -
> -The MSM IOMMU is an implementation compatible with the ARM VMSA short
> -descriptor page tables. It provides address translation for bus masters outside
> -of the CPU, each connected to the IOMMU through a port called micro-TLB.
> -
> -Required Properties:
> -
> -  - compatible: Must contain "qcom,apq8064-iommu".
> -  - reg: Base address and size of the IOMMU registers.
> -  - interrupts: Specifiers for the MMU fault interrupts. For instances that
> -    support secure mode two interrupts must be specified, for non-secure and
> -    secure mode, in that order. For instances that don't support secure mode a
> -    single interrupt must be specified.
> -  - #iommu-cells: The number of cells needed to specify the stream id. This
> -		  is always 1.
> -  - qcom,ncb:	  The total number of context banks in the IOMMU.
> -  - clocks	: List of clocks to be used during SMMU register access. See
> -		  Documentation/devicetree/bindings/clock/clock-bindings.txt
> -		  for information about the format. For each clock specified
> -		  here, there must be a corresponding entry in clock-names
> -		  (see below).
> -
> -  - clock-names	: List of clock names corresponding to the clocks specified in
> -		  the "clocks" property (above).
> -		  Should be "smmu_pclk" for specifying the interface clock
> -		  required for iommu's register accesses.
> -		  Should be "smmu_clk" for specifying the functional clock
> -		  required by iommu for bus accesses.
> -
> -Each bus master connected to an IOMMU must reference the IOMMU in its device
> -node with the following property:
> -
> -  - iommus: A reference to the IOMMU in multiple cells. The first cell is a
> -	    phandle to the IOMMU and the second cell is the stream id.
> -	    A single master device can be connected to more than one iommu
> -	    and multiple contexts in each of the iommu. So multiple entries
> -	    are required to list all the iommus and the stream ids that the
> -	    master is connected to.
> -
> -Example: mdp iommu and its bus master
> -
> -                mdp_port0: iommu@...0000 {
> -			compatible = "qcom,apq8064-iommu";
> -			#iommu-cells = <1>;
> -			clock-names =
> -			    "smmu_pclk",
> -			    "smmu_clk";
> -			clocks =
> -			    <&mmcc SMMU_AHB_CLK>,
> -			    <&mmcc MDP_AXI_CLK>;
> -			reg = <0x07500000 0x100000>;
> -			interrupts =
> -			    <GIC_SPI 63 0>,
> -			    <GIC_SPI 64 0>;
> -			qcom,ncb = <2>;
> -		};
> -
> -		mdp: qcom,mdp@...0000 {
> -			compatible = "qcom,mdp";
> -			...
> -			iommus = <&mdp_port0 0
> -				  &mdp_port0 2>;
> -		};
> diff --git a/Documentation/devicetree/bindings/iommu/qcom,apq8064-iommu.yaml b/Documentation/devicetree/bindings/iommu/qcom,apq8064-iommu.yaml
> new file mode 100644
> index 000000000000..5af59305d277
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/qcom,apq8064-iommu.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/iommu/qcom,apq8064-iommu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm APQ8064 IOMMU
> +
> +maintainers:
> +  - Will Deacon <will@...nel.org>
> +
> +description:
> +  The MSM IOMMU is an implementation compatible with the ARM VMSA short
> +  descriptor page tables. It provides address translation for bus masters
> +  outside of the CPU, each connected to the IOMMU through a port called micro-TLB.
> +
> +properties:
> +  compatible:
> +    const: qcom,apq8064-iommu
> +
> +  clocks:
> +    items:
> +      - description: interface clock for register accesses
> +      - description: functional clock for bus accesses
> +
> +  clock-names:
> +    items:
> +      - const: smmu_pclk
> +      - const: iommu_clk
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description: Specifiers for the MMU fault interrupts.
> +    minItems: 1
> +    items:
> +      - description: non-secure mode interrupt
> +      - description: secure mode interrupt (for instances which supports it)
> +
> +  "#iommu-cells":
> +    const: 1
> +    description:
> +      The first cell is a phandle to the IOMMU and the second cell
> +      is the stream id.
> +      A single master device can be connected to more than one iommu
> +      and multiple contexts in each of the iommu.
> +      So multiple entries are required to list all the iommus and
> +      the stream ids that the master is connected to.

This seems pretty confusing since it's the description for some other 
property, not this one. FWIW I'd just put this as something like "Each 
IOMMU specifier describes a single Stream ID.", and if you think it's 
really worth calling out that clients can have multiple IOMMU 
specifiers, I'd stick that in the overall description.

Thanks,
Robin.

> +
> +  qcom,ncb:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: The total number of context banks in the IOMMU.
> +
> +required:
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - qcom,ncb
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,mmcc-msm8960.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    iommu@...0000 {
> +            compatible = "qcom,apq8064-iommu";
> +            reg = <0x07500000 0x100000>;
> +            interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&clk SMMU_AHB_CLK>,
> +                     <&clk MDP_AXI_CLK>;
> +            clock-names = "smmu_pclk",
> +                          "iommu_clk";
> +            #iommu-cells = <1>;
> +            qcom,ncb = <2>;
> +    };

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ