Re: [PATCH v2 04/10] iommupt: Flush the CPU cache after any writes to the page table

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

On 9/22/25 22:44, Jason Gunthorpe wrote:
> On Mon, Sep 22, 2025 at 10:31:49AM +0800, Baolu Lu wrote:
>> On 8/27/25 01:26, Jason Gunthorpe wrote:
>>> @@ -585,6 +635,7 @@ static __always_inline int __do_map_single_page(struct pt_range *range,
>>>    			return -EADDRINUSE;
>>>    		pt_install_leaf_entry(&pts, map->oa, PAGE_SHIFT,
>>>    				      &map->attrs);
>>> +		/* No flush, not used when incoherent */
>>>    		map->oa += PAGE_SIZE;
>>>    		return 0;
>>>    	}
>>> @@ -811,7 +862,8 @@ int DOMAIN_NS(map_pages)(struct iommu_domain *domain, unsigned long iova,
>>>    	PT_WARN_ON(map.leaf_level > range.top_level);
>>>    	do {
>>> -		if (single_page) {
>>> +		if (single_page &&
>>> +		    !pt_feature(common, PT_FEAT_DMA_INCOHERENT)) {
>>>    			ret = pt_walk_range(&range, __map_single_page, &map);
>>>    			if (ret != -EAGAIN)
>>>    				break;
>> I don't follow the single_page logic here. Why is single_page exclusive
>> with PT_FEAT_DMA_INCOHERENT? To my understanding, PT_FEAT_DMA_INCOHERENT
>> has no relationship with how the page table is organized. Could you
>> elaborate a bit?
> It is this comment above:
> 
>                  /* No flush, not used when incoherent */
> 
> __do_map_single_page() doesn't implement the coherency logic. As an
> agressive inline I didn't want to bloat it.

But, is that functionally correct, though? In the incoherent case, even
if a leaf page entry is changed, the cache should be flushed so that the
hardware can see the change. Basically, I don't understand why
__do_map_single_page() is "not used when incoherent". I must have
overlooked something. :-)

Thanks,
baolu