On Wed, Apr 06, 2022 at 11:24:32AM -0300, Jason Gunthorpe wrote:
> On Wed, Apr 06, 2022 at 02:56:56PM +0100, Robin Murphy wrote:
> > On 2022-04-05 17:16, Jason Gunthorpe wrote:
> > > vdpa and usnic are trying to test if IOMMU_CACHE is supported. The correct
> > > way to do this is via dev_is_dma_coherent()
> >
> > Not necessarily...
> >
> > Disregarding the complete disaster of PCIe No Snoop on Arm-Based systems,
> > there's the more interesting effectively-opposite scenario where an SMMU
> > bridges non-coherent devices to a coherent interconnect. It's not something
> > we take advantage of yet in Linux, and it can only be properly described in
> > ACPI, but there do exist situations where IOMMU_CACHE is capable of making
> > the device's traffic snoop, but dev_is_dma_coherent() - and
> > device_get_dma_attr() for external users - would still say non-coherent
> > because they can't assume that the SMMU is enabled and programmed in just
> > the right way.
>
> Oh, I didn't know about device_get_dma_attr()..
>
> Considering your future issue, maybe this:
>
> /*
> * true if the given domain supports IOMMU_CACHE and when dev is attached to
> * that domain it will have coherent DMA and require no cache
> * maintenance when IOMMU_CACHE is used.
> */
> bool iommu_domain_supports_coherent_dma(struct iommu_domain *domain, struct device *dev)
> {
> return device_get_dma_attr(dev) == DEV_DMA_COHERENT;
> }
>
> ? In future it could become a domain op and the SMMU driver could
> figure out the situation you described?
I also spent some time looking at something like this:
struct iommu_domain *iommu_domain_alloc_coherent(struct device *device)
{
if (device_get_dma_attr(device) == DEV_DMA_COHERENT)
return NULL;
return __iommu_domain_alloc(device->bus, IOMMU_DOMAIN_UNMANAGED);
}
EXPORT_SYMBOL_GPL(iommu_domain_alloc_coherent);
Which could evolve into to passing the flag down to the iommu driver
and then it could ensure SMMU is "programmed in just the right way"
or fail?
Could also go like this:
#define IOMMU_DOMAIN_FLAG_COHERENT 1
struct iommu_domain *iommu_device_alloc_domain(struct device *device,
unsigned int flags)
A new alloc option is not so easy to fit VFIO into right now though.
Advices?
Thanks,
Jason
