Re: [PATCH] iommufd: Enforce IOMMU_RESV_SW_MSI upon hwpt_paging allocation

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Jul 31, 2024 at 02:21:40PM -0700, Nicolin Chen wrote:
> On Wed, Jul 31, 2024 at 11:13:11AM -0700, Nicolin Chen wrote:
> > On Wed, Jul 31, 2024 at 07:45:46AM +0000, Tian, Kevin wrote:
> > > > From: Nicolin Chen <nicolinc@nvidia.com>
> > > > Sent: Monday, July 29, 2024 7:51 AM
> > > > @@ -364,7 +305,8 @@ int iommufd_hw_pagetable_attach(struct
> > > > iommufd_hw_pagetable *hwpt,
> > > >       }
> > > >
> > > >       if (hwpt_is_paging(hwpt)) {
> > > > -             rc = iommufd_hwpt_paging_attach(to_hwpt_paging(hwpt),
> > > > idev);
> > > > +             rc = iopt_table_enforce_dev_resv_regions(
> > > > +                             &to_hwpt_paging(hwpt)->ioas->iopt, idev-
> > > > >dev);
> > > 
> > > Is it simpler to extend the original operation to the parent S2 when
> > > it's hwpt_nested?
> > 
> > Likely. I recall that was what one of our WIP versions did.
> > 
> > > The name iommufd_hwpt_paging_attach() is a bit misleading. The
> > > actual work there is all about reservations. It doesn't change any
> > > tracking structure about attachment between device and hwpt.
> > 
> > How about iommufd_hwpt_enforce/remove_rr() taking hwpt v.s.
> > hwpt_paging.
>  
> > > With that I think continuing this per-device reservation scheme is
> > > easier than adding specific reservation for SW_MSI at hwpt creation
> > > time and then further requiring check at attach time to verify
> > > the attached device is allocated with the same address as the one
> > > during allocation.
> > 
> > Jason, do you agree?
> 
> I came up with something plus a bit of naming alignment:
>     iommufd_device_attach_reserved_iova()
>     iommufd_group_remove_reserved_iova()
>     iommufd_group_do_replace_reserved_iova()
> 
> If it looks good to both of you, I will send a formal patch.

This seems like a more consistent direction, let's try to make

Jason