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

[Nicolin Chen]

On Wed, Jul 31, 2024 at 07:50:53AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > Sent: Tuesday, July 30, 2024 4:06 AM
> >
> > On Mon, Jul 29, 2024 at 03:24:46PM -0300, Jason Gunthorpe wrote:
> > > 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?
> >
> > The idea here is to treat IOMMU_RESV_SW_MSI as a per-IOMMU thing
> > v.s. a per-device thing, because it's defined by SW for an IOMMU
> > that doesn't have a HW MSI window. In another word, devices don't
> > really matter, so long as we know which IOMMU that can be decided
> > by any "one device".
> >
> 
> The problem is that the entire reserved region interface in IOMMU is
> per-device, leaving the room that though IOMMU_RESV_SW_MSI
> is per-IOMMU on existing platforms there might be a new
> implementation future with per-device difference.
>
> Then we'll have trouble in the callers which assume a specific type
> is per-IOMMU.
 
Looking at the existing cases, we are fine. But yes, I agree it
would potentially break if an IOMMU defines an IOMMU_RESV_SW_MSI
for one device not for another, i.e. one device has MSI behind
the IOMMU while another one doesn't.

> If we really want to go this route I wonder whether a per-IOMMU
> get_resv_regions() API should first exist then there is a clear definition
> what types are per-IOMMU and what are not.

Probably it'd be necessary.

Thanks
Nic