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

Jason Gunthorpe
2024-07-29T18:24:57Z

On Sun, Jul 28, 2024 at 04:51:06PM -0700, Nicolin Chen wrote:
> IOMMU_RESV_SW_MSI is a unique region defined by an IOMMU driver. Though it
> is eventually used by a device for address translation to an MSI location
> (including nested cases), practically it is a universal region across all
> domains allocated for the IOMMU that defines it.
>
> Currently IOMMUFD core fetches and reserves the region during an attach to
> an hwpt_paging. It works with a hwpt_paging-only case, but might not work
> with a nested case where a device could directly attach to a hwpt_nested,
> bypassing the hwpt_paging attachment.

Well, it does this because the attach is the only place where we have
*all* the devices available.

Doing it doing allocation means you get only one device.

So, I'd imagine more like we allocate the MSI region during allocation
for the device specific during allocation

But continue to enforce that every attached device also has its MSI
region allocated.. Which probably just means checking that the
driver's reported MSI address is the same as the address during
allocation?

> @@ -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);
> if (rc)
> goto err_unlock;

And this seems kind of weird, shouldn't any change the ioas regions
happen when the domain is joined to the ioas with the MSI mapping, not
during attach?? Like we don't want any change of the IOAS to blow away
the MSI region.

This should probably get selftest coverage as well, it seems easy
enough to add to the mock iommu driver?

Jason