iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
From: Sven Peter via iommu <iommu@lists.linux-foundation.org>
To: "Mark Kettenis" <mark.kettenis@xs4all.nl>
Cc: arnd@kernel.org, devicetree@vger.kernel.org, will@kernel.org,
	marcan@marcan.st, linux-kernel@vger.kernel.org,
	iommu@lists.linux-foundation.org, robh+dt@kernel.org,
	maz@kernel.org, mohamed.mediouni@caramail.com,
	robin.murphy@arm.com, linux-arm-kernel@lists.infradead.org,
	stan@corellium.com
Subject: Re: [PATCH 0/3] Apple M1 DART IOMMU driver
Date: Mon, 22 Mar 2021 23:17:31 +0100	[thread overview]
Message-ID: <d280843b-77e3-4fa8-9452-5a2f8a45052e@www.fastmail.com> (raw)
In-Reply-To: <c1bcc0be8ae6e500@bloch.sibelius.xs4all.nl>


Hi Mark,

On Sun, Mar 21, 2021, at 19:35, Mark Kettenis wrote:
>
> Guess we do need to understand a little bit better how the USB DART
> actually works.  My hypothesis (based on our discussion on #asahi) is
> that the XHCI host controller and the peripheral controller of the
> DWC3 block use different DMA "streams" that are handled by the
> different sub-DARTs.

I've done some more experiments and the situation is unfortunately more
complicated: Most DMA transfers are translated with the first DART.
But sometimes (and I have not been able to figure out the exact conditions)
transfers instead have to go through the second DART. 
This happens e.g. with one of my USB keyboards after a stop EP command
is issued: Suddenly the xhci_ep_ctx struct must be translated through the
second DART.

What this likely means is that we'll need to point both DARTs
to the same pagetables and just issue the TLB maintenance operations
as a group.

> 
> The Corellium folks use a DART + sub-DART model in their driver and a
> single node in the device tree that represents both.  That might sense
> since the error registers and interrupts are shared.  Maybe it would
> make sense to select the appropriate sub-DART based on the DMA stream
> ID?

dwc3 specifically seems to require stream id #1 from the DART
at <0x5 0x02f00000> and stream id #0 from the DART at <0x5 0x02f80000>.
Both of these only share a IRQ line but are otherwise completely independent.
Each has their own error registers, etc. and we need some way to
specify these two DARTs + the appropriate stream ID.

Essentially we have three options to represent this now:

1) Add both DARTs as separate regs, use #iommu-cells = <2> and have the
   first cell select the DART and the second one the stream ID.
   We could allow #iommu-cells = <1> in case only one reg is specified
   for the PCIe DART:

   usb_dart1@502f00000 {
     compatible = "apple,t8103-dart";
     reg = <0x5 0x02f00000 0x0 0x4000>, <0x5 0x02f80000 0x0 0x4000>;
     #iommu-cells = <2>;
     ...
   };

   usb1 {
     iommus = <&usb_dart1 0 1>, <&usb_dart1 1 0>;
     ...
   };

   I prefer this option because we fully describe the DART in a single
   device node here. It also feels natural to group them like this because
   they need to share some properties (like dma-window and the interrupt)
   anyway. 

2) Create two DART nodes which share the same IRQ line and attach them
   both to the master node:

   usb_dart1a@502f00000 {
     compatible = "apple,t8103-dart";
     reg = <0x5 0x02f00000 0x0 0x4000>;
     #iommu-cells = <1>;
     ...
   };
   usb_dart1b@502f80000 {
     compatible = "apple,t8103-dart";
     reg = <0x5 0x02f80000 0x0 0x4000>;
     #iommu-cells = <1>;
     ...
   };

   usb1 {
     iommus = <&usb_dart1a 1>, <&usb_dart1b 0>;
     ...
   };

   I dislike this one because attaching those two DARTs to a single device
   seems rather unusual. We'd also have to duplicate the dma-window setting,
   make sure it's the same for both DARTs and there are probably even more
   complications I can't think of right now. It seems like this would also
   make the device tree very verbose and the implementation itself more
   complicated.

3) Introduce another property and let the DART driver take care of
   mirroring the pagetables. I believe this would be similar to
   the sid-remap property:

   usb_dart1@502f00000 {
     compatible = "apple,t8103-dart";
     reg = <0x5 0x02f00000 0x0 0x4000>, <0x5 0x02f80000 0x0 0x4000>;
     #iommu-cells = <1>;
     sid-remap = <0 1>;
   };
   usb1 {
     iommus = <&usb_dart1 0>;
   };

   I slightly dislike this one because we now specify which stream id 
   to use in two places: Once in the device node and another time in the
   new property in the DART node. I also don't think the binding is much
   simpler than the first one.


> > where #dma-address-cells and #dma-size-cells default to
> > #address-cells and #size-cells respectively if I understand
> > the code correctly. That way we could also just always use
> > a 64bit address and size in the DT, e.g.
> > 
> >   pcie_dart {
> >       [ ... ]
> >       dma-window = <0 0x100000 0 0x3fe00000>;
> >       [ ... ]
> >   };
> 
> That sounds like a serious contender to me!  Hopefully one of the
> Linux kernel developers can give this some sort of blessing.
> 
> I think it would make sense for us to just rely on the #address-cells
> and #size-cells defaults for the M1 device tree.
>

Agreed.


Best,

Sven
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2021-03-22 22:18 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-20 15:19 Sven Peter via iommu
2021-03-20 15:19 ` [PATCH 1/3] iommu: io-pgtable: add DART pagetable format Sven Peter via iommu
2021-03-24 16:37   ` Robin Murphy
2021-03-25 20:47     ` Sven Peter via iommu
2021-03-20 15:20 ` [PATCH 2/3] dt-bindings: iommu: add DART iommu bindings Sven Peter via iommu
2021-03-22  0:15   ` Rob Herring
2021-03-22 18:16     ` Sven Peter via iommu
2021-03-21 16:00 ` [PATCH 0/3] Apple M1 DART IOMMU driver Mark Kettenis
2021-03-21 17:22   ` Sven Peter via iommu
2021-03-21 18:35     ` Mark Kettenis
2021-03-22 22:17       ` Sven Peter via iommu [this message]
2021-03-23 20:00         ` Mark Kettenis
2021-03-23 21:03           ` Sven Peter via iommu
2021-03-21 17:28   ` Sven Peter via iommu
2021-03-23 20:53   ` Rob Herring
2021-03-23 22:33     ` Mark Kettenis
2021-03-25  7:53     ` Sven Peter via iommu
2021-03-25 11:50       ` Robin Murphy
2021-03-25 20:49         ` Sven Peter via iommu
2021-03-27 15:33         ` Sven Peter via iommu
2021-03-25 21:41       ` Arnd Bergmann
2021-03-26 15:59         ` Mark Kettenis
2021-03-26 16:09           ` Arnd Bergmann
2021-03-26 16:10           ` Sven Peter via iommu
2021-03-26 16:38             ` Arnd Bergmann
2021-03-26 17:06               ` Sven Peter via iommu
2021-03-26 17:26               ` Mark Kettenis
2021-03-26 17:34                 ` Robin Murphy
2021-03-26 17:51                   ` Sven Peter via iommu
2021-03-26 19:59                     ` Arnd Bergmann
2021-03-26 21:16                       ` Mark Kettenis
2021-03-27 15:30                       ` Sven Peter via iommu
2021-03-26 20:03                 ` Arnd Bergmann
2021-03-26 21:13                   ` Mark Kettenis
2021-03-24 15:29 ` Robin Murphy
2021-03-25  7:58   ` Sven Peter via iommu

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=d280843b-77e3-4fa8-9452-5a2f8a45052e@www.fastmail.com \
    --to=iommu@lists.linux-foundation.org \
    --cc=arnd@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcan@marcan.st \
    --cc=mark.kettenis@xs4all.nl \
    --cc=maz@kernel.org \
    --cc=mohamed.mediouni@caramail.com \
    --cc=robh+dt@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=stan@corellium.com \
    --cc=sven@svenpeter.dev \
    --cc=will@kernel.org \
    --subject='Re: [PATCH 0/3] Apple M1 DART IOMMU driver' \
    /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

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).