Re: [PATCH v3] iommufd/device: Enforce reserved IOVA also when attached to hwpt_nested

Nicolin Chen
2024-08-09T20:23:31Z

On Fri, Aug 09, 2024 at 02:00:10AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@nvidia.com>
> > Sent: Wednesday, August 7, 2024 8:35 AM
> >
> > Currently, device reserved regions are only enforced when the device is
> > attached to an hwpt_paging. In other words, if the device gets attached
> > to an hwpt_nested directly, the parent hwpt_paging of the hwpt_nested's
> > would not enforce those reserved IOVAs. This works for most of reserved
> > region types, but not for IOMMU_RESV_SW_MSI, which is a unique software
> > defined window, required by a nesting case too to setup an MSI doorbell
> > on the parent stage-2 hwpt/domain.
> >
> > Kevin pointed out that:
> > 1) there is no usage using up closely the entire IOVA space yet,
> > 2) guest may change the viommu mode to switch between nested
> > and paging then VMM has to take all devices' reserved regions
> > into consideration anyway, when composing the GPA space.
> > Link:
> > https://lore.kernel.org/all/BN9PR11MB5276497781C96415272E6FED8CB12@
> > BN9PR11MB5276.namprd11.prod.outlook.com/
> >
> > So it would be actually convenient for us to also enforce reserved IOVA
> > onto the parent hwpt_paging, when attaching a device to an hwpt_nested.
> >
> > Repurpose the existing attach/replace_paging helpers to attach device's
> > reserved IOVAs exclusively.
> >
> > Add a new find_hwpt_paging helper, which is only used by these reserved
> > IOVA functions, to allow an IOMMUFD_OBJ_HWPT_NESTED hwpt to redirect
> > to
> > its parent hwpt_paging. Return a NULL in these two helpers for any new
> > HWPT type in the future.
> >
> > Suggested-by: Tian, Kevin <kevin.tian@intel.com>
> > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
>
> I'm not sure the name find_hwpt_paging() is good enough but can't
> find a better alternative. So,
>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Thanks for the review. I couldn't figure out a better name but
that :)

With the ongoing discussion with Robin at the other thread, we
we might be able to get rid of the msi_cookie. If so, this one
will be unnecessary. So, we might want to put this on hold?

That being said, a new solution will take some time. I'll need
to go through all cases and untangle the msi_cookie cleanly..

Thanks
Nicolin