> From: Tian, Kevin > Sent: Friday, July 31, 2020 8:26 AM > > > From: Alex Williamson <alex.william...@redhat.com> > > Sent: Friday, July 31, 2020 4:17 AM > > > > On Wed, 29 Jul 2020 23:49:20 +0000 > > "Tian, Kevin" <kevin.t...@intel.com> wrote: > > > > > > From: Alex Williamson <alex.william...@redhat.com> > > > > Sent: Thursday, July 30, 2020 4:25 AM > > > > > > > > On Tue, 14 Jul 2020 13:57:02 +0800 > > > > Lu Baolu <baolu...@linux.intel.com> wrote: > > > > > > > > > The device driver needs an API to get its aux-domain. A typical usage > > > > > scenario is: > > > > > > > > > > unsigned long pasid; > > > > > struct iommu_domain *domain; > > > > > struct device *dev = mdev_dev(mdev); > > > > > struct device *iommu_device = > vfio_mdev_get_iommu_device(dev); > > > > > > > > > > domain = iommu_aux_get_domain_for_dev(dev); > > > > > if (!domain) > > > > > return -ENODEV; > > > > > > > > > > pasid = iommu_aux_get_pasid(domain, iommu_device); > > > > > if (pasid <= 0) > > > > > return -EINVAL; > > > > > > > > > > /* Program the device context */ > > > > > .... > > > > > > > > > > This adds an API for such use case. > > > > > > > > > > Suggested-by: Alex Williamson <alex.william...@redhat.com> > > > > > Signed-off-by: Lu Baolu <baolu...@linux.intel.com> > > > > > --- > > > > > drivers/iommu/iommu.c | 18 ++++++++++++++++++ > > > > > include/linux/iommu.h | 7 +++++++ > > > > > 2 files changed, 25 insertions(+) > > > > > > > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > > > > index cad5a19ebf22..434bf42b6b9b 100644 > > > > > --- a/drivers/iommu/iommu.c > > > > > +++ b/drivers/iommu/iommu.c > > > > > @@ -2817,6 +2817,24 @@ void iommu_aux_detach_group(struct > > > > iommu_domain *domain, > > > > > } > > > > > EXPORT_SYMBOL_GPL(iommu_aux_detach_group); > > > > > > > > > > +struct iommu_domain *iommu_aux_get_domain_for_dev(struct > > device > > > > *dev) > > > > > +{ > > > > > + struct iommu_domain *domain = NULL; > > > > > + struct iommu_group *group; > > > > > + > > > > > + group = iommu_group_get(dev); > > > > > + if (!group) > > > > > + return NULL; > > > > > + > > > > > + if (group->aux_domain_attached) > > > > > + domain = group->domain; > > > > > > > > Why wouldn't the aux domain flag be on the domain itself rather than > > > > the group? Then if we wanted sanity checking in patch 1/ we'd only > > > > need to test the flag on the object we're provided. > > > > > > > > If we had such a flag, we could create an iommu_domain_is_aux() > > > > function and then simply use iommu_get_domain_for_dev() and test > that > > > > it's an aux domain in the example use case. It seems like that would > > > > > > IOMMU layer manages domains per parent device. Here given a > > > > Is this the IOMMU layer or the VT-d driver? I don't see any notion of > > managing domains relative to a parent in the IOMMU layer. Please point > > to something more specific if I'm wrong here. > > it's maintained in VT-d driver (include/linux/intel-iommu.h) > > struct device_domain_info { > struct list_head link; /* link to domain siblings */ > struct list_head global; /* link to global list */ > struct list_head table; /* link to pasid table */ > struct list_head auxiliary_domains; /* auxiliary domains > * attached to this device > */ > ... > > > > > > dev (of mdev), we need a way to find its associated domain under its > > > parent device. And we cannot simply use iommu_get_domain_for_dev > > > on the parent device of the mdev, as it will give us the primary domain > > > of parent device. > > > > Not the parent device of the mdev, but the mdev_dev(mdev) device. > > Isn't that what this series is enabling, being able to return the > > domain from the group that contains the mdev_dev? We shouldn't need > to > > leave breadcrumbs on the group to know about the domain, the domain > > itself should be the source of knowledge, or provide a mechanism/ops to > > learn that knowledge. Thanks, > > > > Alex > > It's the tradeoff between leaving breadcrumb in domain or in group. > Today the domain has no knowledge of mdev. It just includes a list > of physical devices which are attached to the domain (either due to > the device is assigned in a whole or as the parent device of an assigned > mdev). Then we have two choices. One is to save the mdev_dev info > in device_domain_info and maintain a mapping between mdev_dev > and related aux domain. The other is to record the domain info directly > in group. Earlier we choose the latter one as it looks simpler. If you > prefer to the former one, we can think more and have a try. >
Please skip this comment. I have a wrong understanding of the problem and is discussing with Baolu. He will reply with our conclusion later. Thanks Kevin