Messages in this thread | | | Subject | Re: [PATCH 1/2] iommu: Add iommu_group_get/set_domain() | From | Lu Baolu <> | Date | Tue, 30 Jun 2020 09:03:11 +0800 |
| |
Hi Robin,
On 6/29/20 7:56 PM, Robin Murphy wrote: > On 2020-06-27 04:15, Lu Baolu wrote: >> The hardware assistant vfio mediated device is a use case of iommu >> aux-domain. The interactions between vfio/mdev and iommu during mdev >> creation and passthr are: >> >> - Create a group for mdev with iommu_group_alloc(); >> - Add the device to the group with >> group = iommu_group_alloc(); >> if (IS_ERR(group)) >> return PTR_ERR(group); >> >> ret = iommu_group_add_device(group, &mdev->dev); >> if (!ret) >> dev_info(&mdev->dev, "MDEV: group_id = %d\n", >> iommu_group_id(group)); >> - Allocate an aux-domain >> iommu_domain_alloc() >> - Attach the aux-domain to the physical device from which the mdev is >> created. >> iommu_aux_attach_device() >> >> In the whole process, an iommu group was allocated for the mdev and an >> iommu domain was attached to the group, but the group->domain leaves >> NULL. As the result, iommu_get_domain_for_dev() doesn't work anymore. >> >> This adds iommu_group_get/set_domain() so that group->domain could be >> managed whenever a domain is attached or detached through the aux-domain >> api's. > > Letting external callers poke around directly in the internals of > iommu_group doesn't look right to me.
Unfortunately, it seems that the vifo iommu abstraction is deeply bound to the IOMMU subsystem. We can easily find other examples:
iommu_group_get/set_iommudata() iommu_group_get/set_name() ...
> > If a regular device is attached to one or more aux domains for PASID > use, iommu_get_domain_for_dev() is still going to return the primary > domain, so why should it be expected to behave differently for mediated
Unlike the normal device attach, we will encounter two devices when it comes to aux-domain.
- Parent physical device - this might be, for example, a PCIe device with PASID feature support, hence it is able to tag an unique PASID for DMA transfers originated from its subset. The device driver hence is able to wrapper this subset into an isolated:
- Mediated device - a fake device created by the device driver mentioned above.
Yes. All you mentioned are right for the parent device. But for mediated device, iommu_get_domain_for_dev() doesn't work even it has an valid iommu_group and iommu_domain.
iommu_get_domain_for_dev() is a necessary interface for device drivers which want to support aux-domain. For example,
struct iommu_domain *domain; struct device *dev = mdev_dev(mdev); unsigned long pasid;
domain = iommu_get_domain_for_dev(dev); if (!domain) return -ENODEV;
pasid = iommu_aux_get_pasid(domain, dev->parent); if (pasid == IOASID_INVALID) return -EINVAL;
/* Program the device context with the PASID value */ ....
Without this fix, iommu_get_domain_for_dev() always returns NULL and the device driver has no means to support aux-domain.
Best regards, baolu
> devices? AFAICS it's perfectly legitimate to have no primary domain if > traffic-without-PASID is invalid. > > Robin.
| |