RE: [RFC 10/20] iommu/iommufd: Add IOMMU_DEVICE_GET_INFO

[Tian, Kevin]

> From: Jason Gunthorpe
> Sent: Wednesday, September 29, 2021 8:37 PM
> 
> On Wed, Sep 29, 2021 at 08:48:28AM +0000, Tian, Kevin wrote:
> 
> > ARM:
> >     - set to snoop format if IOMMU_CACHE
> >     - set to nonsnoop format if !IOMMU_CACHE
> > (in both cases TLP snoop bit is ignored?)
> 
> Where do you see this? I couldn't even find this functionality in the
> ARM HW manual??

Honestly speaking I'm getting confused by the complex attribute
transformation control (default, replace, combine, input, output, etc.)
in SMMU manual. Above was my impression after last check, but now
I cannot find necessary info to build the same picture (except below 
code). :/

> 
> What I saw is ARM linking the IOMMU_CACHE to a IO PTE bit that causes
> the cache coherence to be disabled, which is not ignoring no snoop.

My impression was that snoop is one way of implementing cache
coherency and now since the PTE can explicitly specify cache coherency 
like below:

                else if (prot & IOMMU_CACHE)
                        pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
                else
                        pte |= ARM_LPAE_PTE_MEMATTR_NC;

This setting in concept overrides the snoop attribute from the device thus
make it sort of ignored?

But I did see the manual says that:
--
Note: To achieve this 'pull-down' behavior, the No_snoop flag might 
be carried through the SMMU and used to transform the SMMU output 
downstream.
--

So again, just got confused here...

> 
> > I didn't identify the exact commit for above meaning change.
> >
> > Robin, could you help share some thoughts here?
> 
> It is this:
> 
> static int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
> 		     unsigned long attrs)
> {
> 	int prot = coherent ? IOMMU_CACHE : 0;
> 
> Which sets IOMMU_CACHE based on:
> 
> static void *iommu_dma_alloc(struct device *dev, size_t size,
> 		dma_addr_t *handle, gfp_t gfp, unsigned long attrs)
> {
> 	bool coherent = dev_is_dma_coherent(dev);
> 	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
> 
> Driving IOMMU_CACHE from dev_is_dma_coherent() has *NOTHING* to do
> with no-snoop TLPs and everything to do with the arch cache
> maintenance API

Maybe I'll get a clearer picture on this after understanding the difference 
between cache coherency and snoop on ARM. They are sort of inter-
changeable on Intel (or possibly on x86 since I just found that AMD 
completely ignores IOMMU_CACHE).

Thanks
Kevin