From: Tomasz Jeznach <tjeznach@rivosinc.com>
To: Rob Herring <robh+dt@kernel.org>
Cc: Conor Dooley <conor@kernel.org>, Joerg Roedel <joro@8bytes.org>,
Will Deacon <will@kernel.org>,
Robin Murphy <robin.murphy@arm.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Anup Patel <apatel@ventanamicro.com>,
Albert Ou <aou@eecs.berkeley.edu>,
linux@rivosinc.com, linux-kernel@vger.kernel.org,
Sebastien Boeuf <seb@rivosinc.com>,
iommu@lists.linux.dev, Palmer Dabbelt <palmer@dabbelt.com>,
Nick Kossifidis <mick@ics.forth.gr>,
linux-riscv@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 03/11] dt-bindings: Add RISC-V IOMMU bindings
Date: Wed, 19 Jul 2023 16:04:41 -0700 [thread overview]
Message-ID: <CAH2o1u5VdOx+MgQFyjEr1__DZmsxLGGqf8v1pDvTHoPJ4OGfGA@mail.gmail.com> (raw)
In-Reply-To: <CAL_JsqLsb801Z4H7+ViboBSGwd+XidcpYcJMHgw+6fofsXB=9Q@mail.gmail.com>
On Wed, Jul 19, 2023 at 2:37 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Wed, Jul 19, 2023 at 2:19 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > Hey Tomasz,
> >
> > On Wed, Jul 19, 2023 at 12:33:47PM -0700, Tomasz Jeznach wrote:
> > > From: Anup Patel <apatel@ventanamicro.com>
> > >
> > > We add DT bindings document for RISC-V IOMMU platform and PCI devices
> > > defined by the RISC-V IOMMU specification.
> > >
> > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> >
> > Your signoff is missing from here.
> >
> > Secondly, as get_maintainer.pl would have told you, dt-bindings patches
> > need to be sent to the dt-binding maintainers and list.
> > +CC maintainers & list.
> >
> > Thirdly, dt-binding patches should come before their users.
> >
> > > ---
> > > .../bindings/iommu/riscv,iommu.yaml | 146 ++++++++++++++++++
> > > 1 file changed, 146 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/iommu/riscv,iommu.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/iommu/riscv,iommu.yaml b/Documentation/devicetree/bindings/iommu/riscv,iommu.yaml
> > > new file mode 100644
> > > index 000000000000..8a9aedb61768
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iommu/riscv,iommu.yaml
> > > @@ -0,0 +1,146 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iommu/riscv,iommu.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: RISC-V IOMMU Implementation
> > > +
> > > +maintainers:
> > > + - Tomasz Jeznach <tjeznach@rivosinc.com>
> >
> > What about Anup, who seems to have written this?
> > Or your co-authors of the drivers?
> >
> > > +
> > > +description:
> > > + The RISC-V IOMMU specificaiton defines an IOMMU for RISC-V platforms
>
> typo
>
ack
> > > + which can be a regular platform device or a PCI device connected to
> > > + the host root port.
> > > +
> > > + The RISC-V IOMMU provides two stage translation, device directory table,
> > > + command queue and fault reporting as wired interrupt or MSIx event for
> > > + both PCI and platform devices.
>
> TBC, you want a PCI device that's an IOMMU and the IOMMU serves
> (provides translation for) PCI devices?
>
Yes, IOMMU as a PCIe device providing address translation services for
connect PCIe root complex.
> > > +
> > > + Visit https://github.com/riscv-non-isa/riscv-iommu for more details.
> > > +
> > > +properties:
> > > + compatible:
> > > + oneOf:
> > > + - description: RISC-V IOMMU as a platform device
>
> "platform device" is a Linux term. Don't use Linux terms in bindings.
>
ack.
> > > + items:
> > > + - enum:
> > > + - vendor,chip-iommu
> >
> > These dummy compatibles are not valid, as was pointed out to Anup on
> > the AIA series. Please go look at what was done there instead:
> > https://lore.kernel.org/all/20230719113542.2293295-7-apatel@ventanamicro.com/
> >
> > > + - const: riscv,iommu
> > > +
> > > + - description: RISC-V IOMMU as a PCI device connected to root port
> > > + items:
> > > + - enum:
> > > + - vendor,chip-pci-iommu
> > > + - const: riscv,pci-iommu
> >
> > I'm not really au fait with the arm smmu stuff, but do any of its
> > versions support being connected to a root port?
>
> PCI devices have a defined format for the compatible string based on
> VID/PID. For PCI, also usually don't need to be described in DT
> because they are discoverable. The exception is when there's parts
> which aren't. Which parts aren't?
>
We've put 'riscv,pci-iommu' node here to describe relationship between PCIe
devices and IOMMU(s), needed for the pcie root complex description (iommu-map).
If there is a better way to reference PCI-IOMMU without adding
pci-iommu definition
that would solve the problem. Every other property of pci-iommu should
be discoverable.
> > > + reg:
> > > + maxItems: 1
> > > + description:
> > > + For RISC-V IOMMU as a platform device, this represents the MMIO base
> > > + address of registers.
> > > +
> > > + For RISC-V IOMMU as a PCI device, this represents the PCI-PCI bridge
>
> Your IOMMU is also a PCI-PCI bridge? Is that a normal PCI thing?
>
It's allowed to be integrated with root complex / IO bridge, but it is
as a separate PCIe device.
I'll clarify the description.
>
> > > + details as described in Documentation/devicetree/bindings/pci/pci.txt
>
> Don't refer to pci.txt. It is going to be removed.
>
ack.
> > > +
> > > + '#iommu-cells':
> > > + const: 2
> > > + description: |
> >
> > |s are only needed where formatting needs to be preserved.
> >
> > > + Each IOMMU specifier represents the base device ID and number of
> > > + device IDs.
>
> Doesn't that assume device IDs are contiguous? Generally not a safe assumption.
>
ack.
> > > +
> > > + interrupts:
> > > + minItems: 1
> > > + maxItems: 16
> >
> > What are any of these interrupts?
> >
> > > + description:
> > > + The presence of this property implies that given RISC-V IOMMU uses
> > > + wired interrupts to notify the RISC-V HARTS (or CPUs).
> > > +
> > > + msi-parent:
> > > + description:
> > > + The presence of this property implies that given RISC-V IOMMU uses
> > > + MSIx to notify the RISC-V HARTs (or CPUs). This property should be
> > > + considered only when the interrupts property is absent.
>
> This doesn't make sense for a PCI device. PCI defines its own way to
> describe MSI support.
>
Agree, this is for IOMMU as a non-PCI device, capable of sending MSI.
Follows 'MSI clients' notes from
devicetree/bindings/interrupt-controller/msi.txt
Is this a proper way to describe this relationship?
> > > +
> > > + dma-coherent:
> >
> > RISC-V is dma-coherent by default, should this not be dma-noncoherent
> > instead?
> >
> > > + description:
> > > + Present if page table walks and DMA accessed made by the RISC-V IOMMU
> > > + are cache coherent with the CPU.
> > > +
> > > + power-domains:
> > > + maxItems: 1
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - '#iommu-cells'
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + /* Example 1 (IOMMU platform device with wired interrupts) */
> > > + immu1: iommu@1bccd000 {
> >
> > Why is this "immu"? typo or intentional?
> >
> > > + compatible = "vendor,chip-iommu", "riscv,iommu";
> > > + reg = <0x1bccd000 0x1000>;
> > > + interrupt-parent = <&aplic_smode>;
> > > + interrupts = <32 4>, <33 4>, <34 4>, <35 4>;
> > > + #iommu-cells = <2>;
> > > + };
> > > +
> > > + /* Device with two IOMMU device IDs, 0 and 7 */
> > > + master1 {
> > > + iommus = <&immu1 0 1>, <&immu1 7 1>;
> > > + };
> > > +
> > > + - |
> > > + /* Example 2 (IOMMU platform device with MSIs) */
> > > + immu2: iommu@1bcdd000 {
> > > + compatible = "vendor,chip-iommu", "riscv,iommu";
> > > + reg = <0x1bccd000 0x1000>;
> > > + msi-parent = <&imsics_smode>;
> > > + #iommu-cells = <2>;
> > > + };
> > > +
> > > + bus {
> > > + #address-cells = <2>;
> > > + #size-cells = <2>;
> > > +
> > > + /* Device with IOMMU device IDs ranging from 32 to 64 */
> > > + master1 {
> > > + iommus = <&immu2 32 32>;
> > > + };
> > > +
> > > + pcie@40000000 {
> > > + compatible = "pci-host-cam-generic";
> > > + device_type = "pci";
> > > + #address-cells = <3>;
> > > + #size-cells = <2>;
> > > + bus-range = <0x0 0x1>;
> > > +
> > > + /* CPU_PHYSICAL(2) SIZE(2) */
>
> I'm guessing there was more after this, but I don't have it...
Complete patch 3 is at:
https://lore.kernel.org/linux-iommu/cover.1689792825.git.tjeznach@rivosinc.com/T/#mbf8dc4098fb09b87b2618c5c545ae882f11b114b
>
> Guessing, immu2 is a PCI device, but it translates for master1 which
> is not a PCI device? Weird. Why would anyone build such a thing?
>
In this example immu2 is a non-PCI device. Agree, otherwise would be weird.
>
> Rob
Thank you,
- Tomasz
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2023-07-19 23:05 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-19 19:33 [PATCH 00/13] Linux RISC-V IOMMU Support Tomasz Jeznach
2023-07-19 19:33 ` [PATCH 01/11] RISC-V: drivers/iommu: Add RISC-V IOMMU - Ziommu support Tomasz Jeznach
2023-07-19 20:49 ` Conor Dooley
2023-07-19 21:43 ` Tomasz Jeznach
2023-07-20 19:27 ` Conor Dooley
2023-07-21 9:44 ` Conor Dooley
2023-07-20 10:38 ` Baolu Lu
2023-07-20 12:31 ` Baolu Lu
2023-07-20 17:30 ` Tomasz Jeznach
2023-07-28 2:42 ` Zong Li
2023-08-02 20:15 ` Tomasz Jeznach
2023-08-02 20:25 ` Conor Dooley
2023-08-03 3:37 ` Zong Li
2023-08-03 0:18 ` Jason Gunthorpe
2023-08-03 8:27 ` Zong Li
2023-08-16 18:05 ` Robin Murphy
2024-04-13 10:15 ` Xingyou Chen
2023-07-19 19:33 ` [PATCH 02/11] RISC-V: arch/riscv/config: enable RISC-V IOMMU support Tomasz Jeznach
2023-07-19 20:22 ` Conor Dooley
2023-07-19 21:07 ` Tomasz Jeznach
2023-07-20 6:37 ` Krzysztof Kozlowski
2023-07-19 19:33 ` [PATCH 03/11] dt-bindings: Add RISC-V IOMMU bindings Tomasz Jeznach
2023-07-19 20:19 ` Conor Dooley
[not found] ` <CAH2o1u6CZSb7pXcaXmh7dJQmNZYh3uORk4x7vJPrb+uCwFdU5g@mail.gmail.com>
2023-07-19 20:57 ` Conor Dooley
2023-07-19 21:37 ` Rob Herring
2023-07-19 23:04 ` Tomasz Jeznach [this message]
2023-07-24 8:03 ` Zong Li
2023-07-24 10:02 ` Anup Patel
2023-07-24 11:31 ` Zong Li
2023-07-24 12:10 ` Anup Patel
2023-07-24 13:23 ` Zong Li
2023-07-26 3:21 ` Baolu Lu
2023-07-26 4:26 ` Zong Li
2023-07-26 12:17 ` Jason Gunthorpe
2023-07-27 2:42 ` Zong Li
2023-08-09 14:57 ` Jason Gunthorpe
2023-08-15 1:28 ` Zong Li
2023-08-15 18:38 ` Jason Gunthorpe
2023-08-16 2:16 ` Zong Li
2023-08-16 4:10 ` Baolu Lu
2023-07-19 19:33 ` [PATCH 04/11] MAINTAINERS: Add myself for RISC-V IOMMU driver Tomasz Jeznach
2023-07-20 12:42 ` Baolu Lu
2023-07-20 17:32 ` Tomasz Jeznach
2023-07-19 19:33 ` [PATCH 05/11] RISC-V: drivers/iommu/riscv: Add sysfs interface Tomasz Jeznach
2023-07-20 6:38 ` Krzysztof Kozlowski
2023-07-20 18:30 ` Tomasz Jeznach
2023-07-20 21:37 ` Krzysztof Kozlowski
2023-07-20 22:08 ` Conor Dooley
2023-07-21 3:49 ` Tomasz Jeznach
2023-07-20 12:50 ` Baolu Lu
2023-07-20 17:47 ` Tomasz Jeznach
2023-07-19 19:33 ` [PATCH 06/11] RISC-V: drivers/iommu/riscv: Add command, fault, page-req queues Tomasz Jeznach
2023-07-20 3:11 ` Nick Kossifidis
2023-07-20 18:00 ` Tomasz Jeznach
2023-07-20 18:43 ` Conor Dooley
2023-07-24 9:47 ` Zong Li
2023-07-28 5:18 ` Tomasz Jeznach
2023-07-28 8:48 ` Zong Li
2023-07-20 13:08 ` Baolu Lu
2023-07-20 17:49 ` Tomasz Jeznach
2023-07-29 12:58 ` Zong Li
2023-07-31 9:32 ` Nick Kossifidis
2023-07-31 13:15 ` Zong Li
2023-07-31 23:35 ` Nick Kossifidis
2023-08-01 0:37 ` Zong Li
2023-08-02 20:28 ` Tomasz Jeznach
2023-08-02 20:50 ` Tomasz Jeznach
2023-08-03 8:24 ` Zong Li
2023-08-16 18:49 ` Robin Murphy
2023-07-19 19:33 ` [PATCH 07/11] RISC-V: drivers/iommu/riscv: Add device context support Tomasz Jeznach
2023-08-16 19:08 ` Robin Murphy
2023-07-19 19:33 ` [PATCH 08/11] RISC-V: drivers/iommu/riscv: Add page table support Tomasz Jeznach
2023-07-25 13:13 ` Zong Li
2023-07-31 7:19 ` Zong Li
2023-08-16 21:04 ` Robin Murphy
2023-07-19 19:33 ` [PATCH 09/11] RISC-V: drivers/iommu/riscv: Add SVA with PASID/ATS/PRI support Tomasz Jeznach
2023-07-31 9:04 ` Zong Li
2023-07-19 19:33 ` [PATCH 10/11] RISC-V: drivers/iommu/riscv: Add MSI identity remapping Tomasz Jeznach
2023-07-31 8:02 ` Zong Li
2023-08-16 21:43 ` Robin Murphy
2023-07-19 19:33 ` [PATCH 11/11] RISC-V: drivers/iommu/riscv: Add G-Stage translation support Tomasz Jeznach
2023-07-31 8:12 ` Zong Li
2023-08-16 21:13 ` Robin Murphy
[not found] ` <CAHCEehJKYu3-GSX2L6L4_VVvYt1MagRgPJvYTbqekrjPw3ZSkA@mail.gmail.com>
2024-02-23 14:04 ` [PATCH 00/13] Linux RISC-V IOMMU Support Zong Li
2024-04-04 17:37 ` Tomasz Jeznach
2024-04-10 5:38 ` Zong Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAH2o1u5VdOx+MgQFyjEr1__DZmsxLGGqf8v1pDvTHoPJ4OGfGA@mail.gmail.com \
--to=tjeznach@rivosinc.com \
--cc=aou@eecs.berkeley.edu \
--cc=apatel@ventanamicro.com \
--cc=conor@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=iommu@lists.linux.dev \
--cc=joro@8bytes.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux@rivosinc.com \
--cc=mick@ics.forth.gr \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=robh+dt@kernel.org \
--cc=robin.murphy@arm.com \
--cc=seb@rivosinc.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).