On Tue, Aug 06, 2024 at 08:25:33AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > Sent: Saturday, August 3, 2024 8:32 AM
> >
> > From: Robin Murphy <robin.murphy@xxxxxxx>
> >
> > 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@xxxxxxx>
> > Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> > ---
> > 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
