From: Tomasz Figa <tfiga@chromium.org> To: Arnd Bergmann <arnd@arndb.de> Cc: Christoph Hellwig <hch@lst.de>, "open list:IOMMU DRIVERS" <iommu@lists.linux-foundation.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Marek Szyprowski <m.szyprowski@samsung.com>, Robin Murphy <robin.murphy@arm.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Joerg Roedel <joro@8bytes.org>, Will Deacon <will.deacon@arm.com>, Vineet Gupta <vgupta@synopsys.com>, Hans-Christian Noren Egtvedt <egtvedt@samfundet.no>, Mitchel Humpherys <mitchelh@codeaurora.org>, Krzysztof Kozlowski <krzk@kernel.org>, Hans Verkuil <hverkuil@xs4all.nl>, Sakari Ailus <sakari.ailus@iki.fi>, Pawel Osciak <posciak@chromium.org>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Linux Media Mailing List <linux-media@vger.kernel.org> Subject: Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Date: Thu, 6 Jul 2017 17:34:25 +0900 [thread overview] Message-ID: <CAAFQd5BMorNa8CD+cEap2=boD2-=jr+DFF9cXTNXzSSfe7FnLg@mail.gmail.com> (raw) In-Reply-To: <CAK8P3a2htZ7q=npfwJVW7Lr90O78Ey+OR5e0ivaR7GwV4YBs=A@mail.gmail.com> On Thu, Jul 6, 2017 at 5:26 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Thu, Jul 6, 2017 at 3:44 AM, Tomasz Figa <tfiga@chromium.org> wrote: >> On Thu, Jul 6, 2017 at 2:20 AM, Christoph Hellwig <hch@lst.de> wrote: >>> On Thu, Jul 06, 2017 at 12:22:35AM +0900, Tomasz Figa wrote: > >>> In general I think moving dma >>> ops and iommu implementations into modules is a bad idea >> >> Could you elaborate on this? I'd be interested in seeing the reasoning >> behind this. >> >>> but I >>> don't want to reject the idea before seeing the code. Or maybe >>> by looking at the user we can come up with an even better idea >>> to solve the original issue you're trying to solve, so please also >>> explain your rationale. > > I had pretty much the same thoughts here. > >> Basically we have an x86 platform with a camera subsystem that is a >> PCI device, has its own MMU and needs cache maintenance. Moreover, the >> V4L2 subsystem, which is the right place for camera drivers, heavily >> relies on DMA mapping as a way to abstract memory allocation, mapping >> and cache maintenance. So it feels natural to me to hide the hardware >> details (additional cache maintenance, mapping into the built-in >> IOMMU) in the DMA mapping ops for this camera subsystem and simply >> make V4L2 just work without knowing those details. > > I can understand your reasoning here, but I'm also not convinced > that this is the best approach. There may be a middle ground somewhere > though. > > Generally speaking I don't want to have to deal with the horrors of > deciding whether an IOMMU is going to be there eventually or not > at probe() time. At some point, we had decided that IOMMUs need to > be initialized (almost) as early as irqchips and clocksources so we can > rely on them to be there at device discovery time. That got pushed > back already, and now we may have to deal with -EPROBE_DEFER > when an IOMMU has not been fully initialized at device probe time, > but at least we can reliably see if one is there or not. Making IOMMUs > modular will add further uncertainty here. Obviously we cannot attach > an IOMMU to a device once we have started using DMA mapping > calls on it. The hardware can only work with IOMMU and so the main module is highly tied with the IOMMU module and it initialized it directly. There is no separate struct driver or device associated with the IOMMU, as it's a part of the one and only one PCI device (as visible from the system PCI bus point of view) and technically handled by one pci_driver. > > For your particular use case, I would instead leave the knowledge > about the IOMMU in the driver itself, like we do for the IOMMUs > that are integrated in desktop GPUs, and have the code use the > DMA mapping API with the system-provided dma_map_ops to > get dma_addr_t tokens which you then program into the device > IOMMU. > > An open question however would be whether to use the IOMMU > API without the DMA mapping API here, or whether to completely > leave the knowledge of the IOMMU inside of the driver itself. > I don't have a strong opinion on that part, and I guess this mostly > depends on what the hardware looks like. + linux-media and some media folks I'd say that this is something that has been consistently tried to be avoided by V4L2 and that's why it's so tightly integrated with DMA mapping. IMHO re-implementing the code that's already there in videobuf2 again in the driver, only because, for no good reason mentioned as for now, having a loadable module providing DMA ops was disliked. Similarly with IOMMU API. It provides a lot of help in managing the mappings and re-implementing this would be IMHO backwards. Best regards, Tomasz
WARNING: multiple messages have this Message-ID (diff)
From: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> To: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> Cc: Linux Media Mailing List <linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>, "open list:IOMMU DRIVERS" <iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>, Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>, Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>, Vineet Gupta <vgupta-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>, Hans-Christian Noren Egtvedt <egtvedt-BrfabpQBY5qlHtIdYg32fQ@public.gmane.org>, Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>, Pawel Osciak <posciak-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> Subject: Re: [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Date: Thu, 6 Jul 2017 17:34:25 +0900 [thread overview] Message-ID: <CAAFQd5BMorNa8CD+cEap2=boD2-=jr+DFF9cXTNXzSSfe7FnLg@mail.gmail.com> (raw) In-Reply-To: <CAK8P3a2htZ7q=npfwJVW7Lr90O78Ey+OR5e0ivaR7GwV4YBs=A@mail.gmail.com> On Thu, Jul 6, 2017 at 5:26 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote: > On Thu, Jul 6, 2017 at 3:44 AM, Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: >> On Thu, Jul 6, 2017 at 2:20 AM, Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> wrote: >>> On Thu, Jul 06, 2017 at 12:22:35AM +0900, Tomasz Figa wrote: > >>> In general I think moving dma >>> ops and iommu implementations into modules is a bad idea >> >> Could you elaborate on this? I'd be interested in seeing the reasoning >> behind this. >> >>> but I >>> don't want to reject the idea before seeing the code. Or maybe >>> by looking at the user we can come up with an even better idea >>> to solve the original issue you're trying to solve, so please also >>> explain your rationale. > > I had pretty much the same thoughts here. > >> Basically we have an x86 platform with a camera subsystem that is a >> PCI device, has its own MMU and needs cache maintenance. Moreover, the >> V4L2 subsystem, which is the right place for camera drivers, heavily >> relies on DMA mapping as a way to abstract memory allocation, mapping >> and cache maintenance. So it feels natural to me to hide the hardware >> details (additional cache maintenance, mapping into the built-in >> IOMMU) in the DMA mapping ops for this camera subsystem and simply >> make V4L2 just work without knowing those details. > > I can understand your reasoning here, but I'm also not convinced > that this is the best approach. There may be a middle ground somewhere > though. > > Generally speaking I don't want to have to deal with the horrors of > deciding whether an IOMMU is going to be there eventually or not > at probe() time. At some point, we had decided that IOMMUs need to > be initialized (almost) as early as irqchips and clocksources so we can > rely on them to be there at device discovery time. That got pushed > back already, and now we may have to deal with -EPROBE_DEFER > when an IOMMU has not been fully initialized at device probe time, > but at least we can reliably see if one is there or not. Making IOMMUs > modular will add further uncertainty here. Obviously we cannot attach > an IOMMU to a device once we have started using DMA mapping > calls on it. The hardware can only work with IOMMU and so the main module is highly tied with the IOMMU module and it initialized it directly. There is no separate struct driver or device associated with the IOMMU, as it's a part of the one and only one PCI device (as visible from the system PCI bus point of view) and technically handled by one pci_driver. > > For your particular use case, I would instead leave the knowledge > about the IOMMU in the driver itself, like we do for the IOMMUs > that are integrated in desktop GPUs, and have the code use the > DMA mapping API with the system-provided dma_map_ops to > get dma_addr_t tokens which you then program into the device > IOMMU. > > An open question however would be whether to use the IOMMU > API without the DMA mapping API here, or whether to completely > leave the knowledge of the IOMMU inside of the driver itself. > I don't have a strong opinion on that part, and I guess this mostly > depends on what the hardware looks like. + linux-media and some media folks I'd say that this is something that has been consistently tried to be avoided by V4L2 and that's why it's so tightly integrated with DMA mapping. IMHO re-implementing the code that's already there in videobuf2 again in the driver, only because, for no good reason mentioned as for now, having a loadable module providing DMA ops was disliked. Similarly with IOMMU API. It provides a lot of help in managing the mappings and re-implementing this would be IMHO backwards. Best regards, Tomasz
next prev parent reply other threads:[~2017-07-06 8:34 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-07-05 7:12 [RFC PATCH 0/5] Fixes for loadable modules implementing DMA/IOMMU APIs Tomasz Figa 2017-07-05 7:12 ` [RFC PATCH 1/5] base: dma-mapping: Export commonly used symbols Tomasz Figa 2017-07-05 7:12 ` Tomasz Figa 2017-07-05 15:17 ` Christoph Hellwig 2017-07-05 15:17 ` Christoph Hellwig 2017-07-05 15:22 ` Tomasz Figa 2017-07-05 15:22 ` Tomasz Figa 2017-07-05 17:20 ` Christoph Hellwig 2017-07-05 17:20 ` Christoph Hellwig 2017-07-06 1:44 ` Tomasz Figa 2017-07-06 1:44 ` Tomasz Figa 2017-07-06 8:26 ` Arnd Bergmann 2017-07-06 8:26 ` Arnd Bergmann 2017-07-06 8:34 ` Tomasz Figa [this message] 2017-07-06 8:34 ` Tomasz Figa 2017-07-06 8:36 ` Tomasz Figa 2017-07-06 12:23 ` Arnd Bergmann 2017-07-06 12:23 ` Arnd Bergmann 2017-07-06 13:31 ` Tomasz Figa 2017-07-06 13:31 ` Tomasz Figa 2017-07-06 13:49 ` Tomasz Figa 2017-07-06 14:02 ` Arnd Bergmann 2017-07-06 14:02 ` Arnd Bergmann 2017-07-06 14:06 ` Tomasz Figa 2017-07-06 14:27 ` Arnd Bergmann 2017-07-06 14:27 ` Arnd Bergmann 2017-07-06 14:35 ` Tomasz Figa 2017-07-06 13:56 ` Arnd Bergmann 2017-07-06 13:56 ` Arnd Bergmann 2017-07-05 7:12 ` [RFC PATCH 2/5] base: dma-mapping: Provide a function to look up remapped pages Tomasz Figa 2017-07-05 7:12 ` Tomasz Figa 2017-07-05 7:12 ` [RFC PATCH 3/5] iommu: Export non-static functions to use in modules Tomasz Figa 2017-07-05 7:12 ` Tomasz Figa 2017-07-05 7:12 ` [RFC PATCH 4/5] iommu/dma: " Tomasz Figa 2017-07-05 7:12 ` Tomasz Figa 2017-07-05 16:22 ` Robin Murphy 2017-07-05 16:22 ` Robin Murphy 2017-07-06 2:25 ` Tomasz Figa 2017-07-06 2:25 ` Tomasz Figa 2017-07-06 11:09 ` Robin Murphy 2017-07-06 11:09 ` Robin Murphy 2017-07-06 14:10 ` Christoph Hellwig 2017-07-06 14:10 ` Christoph Hellwig 2017-07-06 14:17 ` Tomasz Figa 2017-07-06 14:24 ` Tomasz Figa 2017-07-06 14:24 ` Tomasz Figa 2017-07-06 14:35 ` Arnd Bergmann 2017-07-06 14:41 ` Tomasz Figa 2017-07-05 7:12 ` [RFC PATCH 5/5] iommu/dma: Add iommu_dma_cleanup() Tomasz Figa 2017-07-05 7:12 ` Tomasz Figa
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='CAAFQd5BMorNa8CD+cEap2=boD2-=jr+DFF9cXTNXzSSfe7FnLg@mail.gmail.com' \ --to=tfiga@chromium.org \ --cc=arnd@arndb.de \ --cc=egtvedt@samfundet.no \ --cc=gregkh@linuxfoundation.org \ --cc=hch@lst.de \ --cc=hverkuil@xs4all.nl \ --cc=iommu@lists.linux-foundation.org \ --cc=joro@8bytes.org \ --cc=krzk@kernel.org \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=mitchelh@codeaurora.org \ --cc=posciak@chromium.org \ --cc=robin.murphy@arm.com \ --cc=sakari.ailus@iki.fi \ --cc=vgupta@synopsys.com \ --cc=will.deacon@arm.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.