| Message ID | 20180425101051.15349-2-thierry.reding@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [v2,1/5] drm/nouveau: tegra: Detach from ARM DMA/IOMMU mapping | expand |
On Wed, Apr 25, 2018 at 12:10:48PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > The dma_iommu_detach_device() API can be used by drivers to forcibly > detach a device from an IOMMU that architecture code might have attached > to. This is useful for drivers that need explicit control over the IOMMU > using the IOMMU API directly. Given that no one else implements it making it a generic API seems rather confusing. For now I'd rename it to arm_dma_iommu_detach_device() and only implement it in arm. Once I've got the dma mapping implementations consolidated to a small enough number we could think about something like a device quirk that tells the architecture to simply never even attach the iommu dma ops to start with. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 25, 2018 at 08:19:34AM -0700, Christoph Hellwig wrote: > On Wed, Apr 25, 2018 at 12:10:48PM +0200, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > The dma_iommu_detach_device() API can be used by drivers to forcibly > > detach a device from an IOMMU that architecture code might have attached > > to. This is useful for drivers that need explicit control over the IOMMU > > using the IOMMU API directly. > > Given that no one else implements it making it a generic API seems > rather confusing. For now I'd rename it to > arm_dma_iommu_detach_device() and only implement it in arm. That'd be suboptimal because this code is used on both 32-bit and 64-bit ARM. If we make the function 32-bit ARM specific then the driver code would need to use an #ifdef to make sure compilation doesn't break on 64-bit ARM. Thierry
On Thu, Apr 26, 2018 at 02:11:36PM +0200, Thierry Reding wrote: > On Wed, Apr 25, 2018 at 08:19:34AM -0700, Christoph Hellwig wrote: > > On Wed, Apr 25, 2018 at 12:10:48PM +0200, Thierry Reding wrote: > > > From: Thierry Reding <treding@nvidia.com> > > > > > > The dma_iommu_detach_device() API can be used by drivers to forcibly > > > detach a device from an IOMMU that architecture code might have attached > > > to. This is useful for drivers that need explicit control over the IOMMU > > > using the IOMMU API directly. > > > > Given that no one else implements it making it a generic API seems > > rather confusing. For now I'd rename it to > > arm_dma_iommu_detach_device() and only implement it in arm. > > That'd be suboptimal because this code is used on both 32-bit and 64-bit > ARM. If we make the function 32-bit ARM specific then the driver code > would need to use an #ifdef to make sure compilation doesn't break on > 64-bit ARM. Do you still want me to make this ARM specific? While I haven't encountered this issue on 64-bit ARM yet, I think it would happen there as well, under the right circumstances. I could take a shot at implementing the equivalent there (which means essentially implementing it for drivers/iommu/dma-iommu.c and calling that from 64-bit ARM code). Thierry
On 30/04/18 12:02, Thierry Reding wrote: > On Thu, Apr 26, 2018 at 02:11:36PM +0200, Thierry Reding wrote: >> On Wed, Apr 25, 2018 at 08:19:34AM -0700, Christoph Hellwig wrote: >>> On Wed, Apr 25, 2018 at 12:10:48PM +0200, Thierry Reding wrote: >>>> From: Thierry Reding <treding@nvidia.com> >>>> >>>> The dma_iommu_detach_device() API can be used by drivers to forcibly >>>> detach a device from an IOMMU that architecture code might have attached >>>> to. This is useful for drivers that need explicit control over the IOMMU >>>> using the IOMMU API directly. >>> >>> Given that no one else implements it making it a generic API seems >>> rather confusing. For now I'd rename it to >>> arm_dma_iommu_detach_device() and only implement it in arm. >> >> That'd be suboptimal because this code is used on both 32-bit and 64-bit >> ARM. If we make the function 32-bit ARM specific then the driver code >> would need to use an #ifdef to make sure compilation doesn't break on >> 64-bit ARM. > > Do you still want me to make this ARM specific? While I haven't > encountered this issue on 64-bit ARM yet, I think it would happen there > as well, under the right circumstances. I could take a shot at > implementing the equivalent there (which means essentially implementing > it for drivers/iommu/dma-iommu.c and calling that from 64-bit ARM code). It sounds like things are getting a bit backwards here: iommu-dma should have nothing to do with this, since if you've explicitly attached the device to your own IOMMU domain then you're already bypassing everything it knows about and has control over. Arch code calling into iommu-dma to do something which makes arch code not use iommu-dma makes very little sense. AFAICS the only aspect of arm_iommu_detach_device() which actually matters in this case is the set_dma_ops() bit, so what we're really after is a generic way for drivers to say "Hey, I actually have my own MMU (or want to control the one you already know about) so please give me direct DMA ops", which the arch code handles as appropriate (maybe it's also allowed to fail in cases like swiotlb=force or when there is RAM beyond the device's DMA mask). Robin. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 30, 2018 at 12:41:52PM +0100, Robin Murphy wrote: > On 30/04/18 12:02, Thierry Reding wrote: > > On Thu, Apr 26, 2018 at 02:11:36PM +0200, Thierry Reding wrote: > > > On Wed, Apr 25, 2018 at 08:19:34AM -0700, Christoph Hellwig wrote: > > > > On Wed, Apr 25, 2018 at 12:10:48PM +0200, Thierry Reding wrote: > > > > > From: Thierry Reding <treding@nvidia.com> > > > > > > > > > > The dma_iommu_detach_device() API can be used by drivers to forcibly > > > > > detach a device from an IOMMU that architecture code might have attached > > > > > to. This is useful for drivers that need explicit control over the IOMMU > > > > > using the IOMMU API directly. > > > > > > > > Given that no one else implements it making it a generic API seems > > > > rather confusing. For now I'd rename it to > > > > arm_dma_iommu_detach_device() and only implement it in arm. > > > > > > That'd be suboptimal because this code is used on both 32-bit and 64-bit > > > ARM. If we make the function 32-bit ARM specific then the driver code > > > would need to use an #ifdef to make sure compilation doesn't break on > > > 64-bit ARM. > > > > Do you still want me to make this ARM specific? While I haven't > > encountered this issue on 64-bit ARM yet, I think it would happen there > > as well, under the right circumstances. I could take a shot at > > implementing the equivalent there (which means essentially implementing > > it for drivers/iommu/dma-iommu.c and calling that from 64-bit ARM code). > > It sounds like things are getting a bit backwards here: iommu-dma should > have nothing to do with this, since if you've explicitly attached the device > to your own IOMMU domain then you're already bypassing everything it knows > about and has control over. Arch code calling into iommu-dma to do something > which makes arch code not use iommu-dma makes very little sense. My understanding is that iommu-dma will set up an IOMMU domain at device probe time anyway. So even if attaching to an own IOMMU domain will end up bypassing iommu-dma, we'd still want to clear up the IOMMU domain and any associated resources, right? Thierry
On 30/04/18 13:12, Thierry Reding wrote: > On Mon, Apr 30, 2018 at 12:41:52PM +0100, Robin Murphy wrote: >> On 30/04/18 12:02, Thierry Reding wrote: >>> On Thu, Apr 26, 2018 at 02:11:36PM +0200, Thierry Reding wrote: >>>> On Wed, Apr 25, 2018 at 08:19:34AM -0700, Christoph Hellwig wrote: >>>>> On Wed, Apr 25, 2018 at 12:10:48PM +0200, Thierry Reding wrote: >>>>>> From: Thierry Reding <treding@nvidia.com> >>>>>> >>>>>> The dma_iommu_detach_device() API can be used by drivers to forcibly >>>>>> detach a device from an IOMMU that architecture code might have attached >>>>>> to. This is useful for drivers that need explicit control over the IOMMU >>>>>> using the IOMMU API directly. >>>>> >>>>> Given that no one else implements it making it a generic API seems >>>>> rather confusing. For now I'd rename it to >>>>> arm_dma_iommu_detach_device() and only implement it in arm. >>>> >>>> That'd be suboptimal because this code is used on both 32-bit and 64-bit >>>> ARM. If we make the function 32-bit ARM specific then the driver code >>>> would need to use an #ifdef to make sure compilation doesn't break on >>>> 64-bit ARM. >>> >>> Do you still want me to make this ARM specific? While I haven't >>> encountered this issue on 64-bit ARM yet, I think it would happen there >>> as well, under the right circumstances. I could take a shot at >>> implementing the equivalent there (which means essentially implementing >>> it for drivers/iommu/dma-iommu.c and calling that from 64-bit ARM code). >> >> It sounds like things are getting a bit backwards here: iommu-dma should >> have nothing to do with this, since if you've explicitly attached the device >> to your own IOMMU domain then you're already bypassing everything it knows >> about and has control over. Arch code calling into iommu-dma to do something >> which makes arch code not use iommu-dma makes very little sense. > > My understanding is that iommu-dma will set up an IOMMU domain at device > probe time anyway. So even if attaching to an own IOMMU domain will end > up bypassing iommu-dma, we'd still want to clear up the IOMMU domain and > any associated resources, right? The lifetime of a "proper" IOMMU API default domain is that of the iommu_group, so more or less between device_add() and device_del() from the perspective of a device in that group. IOW at least one level below what that device's driver should be messing with. The domain itself is just a little bit of memory and shouldn't have to occupy any hardware resources while it's not active (and yes, I know the ARM SMMU driver is currently a bit crap in that regard). Robin. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c index d82566d6e237..18ddf32b10c9 100644 --- a/drivers/base/dma-mapping.c +++ b/drivers/base/dma-mapping.c @@ -366,3 +366,11 @@ void dma_deconfigure(struct device *dev) of_dma_deconfigure(dev); acpi_dma_deconfigure(dev); } + +void dma_iommu_detach_device(struct device *dev) +{ +#ifdef arch_iommu_detach_device + arch_iommu_detach_device(dev); +#endif +} +EXPORT_SYMBOL(dma_iommu_detach_device); diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index f8ab1c0f589e..732191a2c64e 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -671,6 +671,8 @@ static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base, static inline void arch_teardown_dma_ops(struct device *dev) { } #endif +extern void dma_iommu_detach_device(struct device *dev); + static inline unsigned int dma_get_max_seg_size(struct device *dev) { if (dev->dma_parms && dev->dma_parms->max_segment_size)