Re: [PATCH v7 03/19] iommufd: Replace the hwpt->devices list with iommufd_group

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, May 18, 2023 at 03:05:23PM +0800, Baolu Lu wrote:

> > It doesn't make any sense to store a struct like that in dev_iommu.
> > 
> > The fault handler should come from the domain and we should be able to
> > have a unique 'void *data' cookie linked to the (dev,PASID) to go
> > along with the fault handler.
> 
> If I get your point correctly, the iommu core should provide some places
> for the iommufd to put a cookie for each pair of {device, pasid}, and
> provide interfaces to manage it. For example,

I'd say when you attach a PRI capable domain (eg to a PASID) then provide
a 'void * data' during the attach.

> If so, perhaps we need some special treatment for ARM as a user hwpt
> actually presents the PASID table of the device and the guest setting
> pasid table entry will not be propagated to host. Then, the @pasid in
> above interfaces is meaningless.

As above, when attaching to a RID you'd still pass in the *data

> 1) Move iommu faults uapi from uapi/linux/iommu.h to uapi/linux
>   /iommufd.h and remove the former.

Please no, delete all the dead code from here and move whatever is
still in use into include/linux/

Then we can talk about what parts of it become uAPI and how best to
manage that on a patch by patch basis.

> 2) Add a device id in the iommu_fault structure.
>  struct iommu_fault {
>         __u32   type;
> -       __u32   padding;
> +       __u32   dev_id;

Why? This is iommufd internal, the void * above should cover it.

> 3) Add the device pointer to the parameters of domain fault handler.

That makes some sense
 
> 4) Decouple I/O page fault handling from IOMMU_SVA in the iommu core and
>    the drivers.

Definately, this SVA stuff need to be scrubbed out.

SVA is only a special domain type that takes the page table top from a
mmu_stuct and a shared fault handler in the core code to do handle_mm_fault()

It should not be in drivers any more deeply than that. We still have a
ways to go.

Jason