Re: [PATCH 1/9] iommu/pages: Add support for a incoherent IOMMU page walker

[Baolu Lu <baolu.lu@linux.intel.com>]

On 7/30/25 06:32, Jason Gunthorpe wrote:
> On Mon, Jul 21, 2025 at 04:41:03PM +0800, Baolu Lu wrote:
>> On 7/17/2025 3:57 AM, Jason Gunthorpe wrote:
>>> @@ -98,4 +101,36 @@ static inline void *iommu_alloc_pages_sz(gfp_t gfp, size_t size)
>>>    	return iommu_alloc_pages_node_sz(NUMA_NO_NODE, gfp, size);
>>>    }
>>> -#endif	/* __IOMMU_PAGES_H */
>>> +int iommu_pages_start_incoherent(void *virt, struct device *dma_dev);
>>> +int iommu_pages_start_incoherent_list(struct iommu_pages_list *list,
>>> +                                      struct device *dma_dev);
>>> +
>>> +#ifdef CONFIG_X86
>>> +#include <asm/cacheflush.h>
>>> +
>>> +static inline void iommu_pages_flush_incoherent(struct device *dma_dev,
>>> +						void *virt, size_t offset,
>>> +						size_t len)
>>> +{
>>> +	clflush_cache_range(virt + offset, len);
>>> +}
>>> +static inline void
>>> +iommu_pages_stop_incoherent_list(struct iommu_pages_list *list,
>>> +				 struct device *dma_dev)
>>> +{
>> Do we need to clear iopt->incoherent for X86, given that
>> iopt->incoherent is set in the start path?
> No.. When I wrote this I felt we may as well keep x86 fast since it
> doesn't use the DMA API and did it like this:
> 
> static void __iommu_free_desc(struct ioptdesc *iopt)
> {
> [..]
> 	if (!IS_ENABLED(CONFIG_X86))
> 		WARN_ON_ONCE(iopt->incoherent);
> 
> To largely ignore the incoherent flag.
> 
> So:
> 
> 	/*
> 	 * For performance leave the incoherent flag alone which turns this into
> 	 * a NOP. For X86 the rest of the stop/free flow ignores the flag.
> 	 */

Okay.

I was just thinking about that: this field overlays struct page, so it
makes more sense to clear it before repurposing it. It's fine as long as
there's no functionality issue.

Thanks,
baolu