Re: [PATCH v1 09/13] iommu/amd: Refactor domain flush global function

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Nov 07, 2023 at 10:23:16AM +0530, Vasant Hegde wrote:
> >>>> @@ -1859,7 +1867,7 @@ static void do_detach(struct iommu_dev_data *dev_data)
> >>>>  	device_flush_dte(dev_data);
> >>>>  
> >>>>  	/* Flush IOTLB and wait for the flushes to finish */
> >>>> -	amd_iommu_domain_flush_tlb_pde(domain);
> >>>> +	amd_iommu_domain_flush_all(domain);
> >>>
> >>> This is weird.. The cache tag for a domain shouldn't necessarily be
> >>> flushed from the iotlb when a domain is removed from a dte, should it?
> >>
> >> In this path we need to flush IOMMU cache along with device IOTLB.
> > 
> > The iommu cache only needs to be flushed if the IOPTEs associated with
> > the cache tag change, and that doesn't happen during "detach".
> > 
> > Some of this is confusing because we've largely removed the word
> > 'detach' from the iommu language, we now just have attach, eg replace.
> 
> But we still need detach() function inside our driver so that we can safely
> remove it from current domain and attach it to new domain.

No, you don't. It is like that today, but it is a functional bug that
it works like this. The driver can't meet the core API expectations
with this design. Review the smmuv3 series I sent to undertand this
fully.

Please try to organize things so we are moving toward a direction of
correctness and not gaining more problems.

Jason