Re: [PATCH v1 10/13] iommu/amd: Consolidate domain flush logic

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Nov 07, 2023 at 10:14:17AM +0530, Vasant Hegde wrote:

> > I can say that in SMMUv3 structuring it so that the attach op at the
> > top of the call chain knows what to do, and called a bunch of specific
> > helpers was reasonable. We know at the op level if we are going to do
> > a V1 or V2 invalidation because the OP itself already knows if it is
> > working on a V1/V2 iommu domain due to how it has to load the domain
> > into the DTE/GCR3.
> 
> AMD IOMMU invalidation is based on domain and we have single command which takes
> PASID as well. Current code does have separate functions but that made code
> duplicate. So I try to consolidate all low level functions.

It is really not different from SMMU, invalidation commands for S1 or
S2 tables are constructed differently.

I would say the driver should have a way to issue the commands and a
way to construct the commands. Pass the actual command in instead of
passing in a mess of arguments to try to guess what command the caller
wanted.

> > As working functions then the ops should be writable to know exactly
> > which of those call based on the op.
> > 
> > For instance, if you have a v1 page table then you should have a v1
> > specific flush op that does:
> > 
> >    invalidate_iotlb_range/all_v1(domain)
> >    if (domain->ats in use)
> >      invalidate_atc_range/all(domain)
> 
> That's exactly domain_flush_page() does (flush IOMMU TLB and then flush device
> IOTLB). Just that I need to fix upper layer function and pass few extra params
> to low level functions.

Yes, but that is not always helpful to bury it like this because the
ATC invalidation needs to be done in a few places seperated from the
IOTLB invalidation, and sometimes done without a domain.

Jason