Re: [PATCH v2 2/3] iommu/dma: Support MSIs through nested domains

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, Aug 06, 2024 at 08:25:33AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@nvidia.com>
> > Sent: Saturday, August 3, 2024 8:32 AM
> >
> > From: Robin Murphy <robin.murphy@arm.com>
> >
> > Currently, iommu-dma is the only place outside of IOMMUFD and drivers
> > which might need to be aware of the stage 2 domain encapsulated within
> > a nested domain. This would be in the legacy-VFIO-style case where we're
> 
> why is it a legacy-VFIO-style? We only support nested in IOMMUFD.

I think it's describing the RMR solution that was decided in
Eric's VFIO solution prior to we having IOMMUFD at all.

So long as Robin won't mind (hopefully), I can rephrase it:

Currently, iommu-dma is the only place outside of IOMMUFD and drivers
which might need to be aware of the stage 2 domain encapsulated within
a nested domain. This would be still the RMR solution where we're using
host-managed MSIs with an identity mapping at stage 1, where it is
the underlying stage 2 domain which owns an MSI cookie and holds the
corresponding dynamic mappings. Hook up the new op to resolve what we
need from a nested domain.

> > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> > ---
> >  drivers/iommu/dma-iommu.c | 18 ++++++++++++++++--
> >  include/linux/iommu.h     |  4 ++++
> >  2 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> > index 7b1dfa0665df6..05e04934a5f81 100644
> > --- a/drivers/iommu/dma-iommu.c
> > +++ b/drivers/iommu/dma-iommu.c
> > @@ -1799,6 +1799,20 @@ static struct iommu_dma_msi_page
> > *iommu_dma_get_msi_page(struct device *dev,
> >       return NULL;
> >  }
> >
> > +/*
> > + * Nested domains may not have an MSI cookie or accept mappings, but
> > they may
> > + * be related to a domain which does, so we let them tell us what they need.
> > + */
> > +static struct iommu_domain
> > *iommu_dma_get_msi_mapping_domain(struct device *dev)
> > +{
> > +     struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> > +
> > +     if (domain && domain->type == IOMMU_DOMAIN_NESTED &&
> > +         domain->ops->get_msi_mapping_domain)
> 
> I'm not sure the core should restrict it to the NESTED type. Given
> there is a new domain ops any type restriction can be handled
> inside the callback. Anyway the driver should register the op
> for a domain only when there is a need.

I think we can do either way, given that the use case is very
particular for IOMMU_DOMAIN_NESTED. Otherwise, driver doesn't
need to be aware of the msi mapping domain at all that should
be just taken care of by dma-iommu. If the domain pointer had
a generic parent iommu pointer, the get_msi_mapping_domain op
could have been omitted too.

That being said, yea, likely we should check !!domain->ops at
least.

Thanks
Nicolin