Tian, Kevin
2022-Apr-07 07:18 UTC
[PATCH 1/5] iommu: Replace uses of IOMMU_CAP_CACHE_COHERENCY with dev_is_dma_coherent()
> From: Jason Gunthorpe <jgg at nvidia.com> > Sent: Thursday, April 7, 2022 1:17 AM > > On Wed, Apr 06, 2022 at 06:10:31PM +0200, Christoph Hellwig wrote: > > On Wed, Apr 06, 2022 at 01:06:23PM -0300, Jason Gunthorpe wrote: > > > On Wed, Apr 06, 2022 at 05:50:56PM +0200, Christoph Hellwig wrote: > > > > On Wed, Apr 06, 2022 at 12:18:23PM -0300, Jason Gunthorpe wrote: > > > > > > Oh, I didn't know about device_get_dma_attr().. > > > > > > > > Which is completely broken for any non-OF, non-ACPI plaform. > > > > > > I saw that, but I spent some time searching and could not find an > > > iommu driver that would load independently of OF or ACPI. ie no IOMMU > > > platform drivers are created by board files. Things like Intel/AMD > > > discover only from ACPI, etc.Intel discovers IOMMUs (and optionally ACPI namespace devices) from ACPI, but there is no ACPI description for PCI devices i.e. the current logic of device_get_dma_attr() cannot be used on PCI devices.> > > > s390? > > Ah, I missed looking in s390, hyperv and virtio.. > > hyperv is not creating iommu_domains, just IRQ remapping > > virtio is using OF > > And s390 indeed doesn't obviously have OF or ACPI parts.. > > This seems like it would be consistent with other things: > > enum dev_dma_attr device_get_dma_attr(struct device *dev) > { > const struct fwnode_handle *fwnode = dev_fwnode(dev); > struct acpi_device *adev = to_acpi_device_node(fwnode); > > if (is_of_node(fwnode)) { > if (of_dma_is_coherent(to_of_node(fwnode))) > return DEV_DMA_COHERENT; > return DEV_DMA_NON_COHERENT; > } else if (adev) { > return acpi_get_dma_attr(adev); > } > > /* Platform is always DMA coherent */ > if (!IS_ENABLED(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) && > !IS_ENABLED(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) && > !IS_ENABLED(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL) && > device_iommu_mapped(dev)) > return DEV_DMA_COHERENT; > return DEV_DMA_NOT_SUPPORTED; > } > EXPORT_SYMBOL_GPL(device_get_dma_attr); > > ie s390 has no of or acpi but the entire platform is known DMA > coherent at config time so allow it. Not sure we need the > device_iommu_mapped() or not.Probably not. If adding an iommu may change the coherency it would come from specific IOPTE attributes for a device. The fact whether the device is mapped by iommu doesn't tell that information.> > We could alternatively use existing device_get_dma_attr() as a default > with an iommu wrapper and push the exception down through the iommu > driver and s390 can override it. >if going this way probably device_get_dma_attr() should be renamed to device_fwnode_get_dma_attr() instead to make it clearer? Thanks Kevin