On Wed, 3 Oct 2018 18:52:16 +0100
Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> wrote:
> On 26/09/2018 23:35, Jacob Pan wrote:
> > On Thu, 20 Sep 2018 18:00:39 +0100
> > Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> wrote:
> >
> >> +
> >> +static int io_mm_attach(struct iommu_domain *domain, struct device
> >> *dev,
> >> + struct io_mm *io_mm, void *drvdata)
> >> +{
> >> + int ret;
> >> + bool attach_domain = true;
> >> + int pasid = io_mm->pasid;
> >> + struct iommu_bond *bond, *tmp;
> >> + struct iommu_sva_param *param =
> >> dev->iommu_param->sva_param; +
> >> + if (!domain->ops->mm_attach || !domain->ops->mm_detach)
> >> + return -ENODEV;
> >> +
> >> + if (pasid > param->max_pasid || pasid < param->min_pasid)
> >> + return -ERANGE;
> >> +
> >> + bond = kzalloc(sizeof(*bond), GFP_KERNEL);
> >> + if (!bond)
> >> + return -ENOMEM;
> >> +
> >> + bond->domain = domain;
> >> + bond->io_mm = io_mm;
> >> + bond->dev = dev;
> >> + bond->drvdata = drvdata;
> >> +
> >> + spin_lock(&iommu_sva_lock);
> >> + /*
> >> + * Check if this io_mm is already bound to the domain. In
> >> which case the
> >> + * IOMMU driver doesn't have to install the PASID table
> >> entry.
> >> + */
> >> + list_for_each_entry(tmp, &domain->mm_list, domain_head) {
> >> + if (tmp->io_mm == io_mm) {
> >> + attach_domain = false;
> >> + break;
> >> + }
> >> + }
> >> +
> >> + ret = domain->ops->mm_attach(domain, dev, io_mm,
> >> attach_domain);
> >> + if (ret) {
> >> + kfree(bond);
> >> + goto out_unlock;
> >> + }
> >> +
> >> + list_add(&bond->mm_head, &io_mm->devices);
> >> + list_add(&bond->domain_head, &domain->mm_list);
> >> + list_add(&bond->dev_head, ¶m->mm_list);
> >> +
> >
> > I am trying to understand if mm_list is needed for both per device
> > and per domain. Do you always unbind and detach domain? Seems
> > device could use the domain->mm_list to track all mm's, true?
>
> We need to track bonds per devices, since the bind/unbind() user
> interface in on devices. Tracking per domain is just a helper, so
> IOMMU drivers that have a single PASID table per domain know when
> they need to install a new entry (the attach_domain parameter above)
> and remove it. I think my code is wrong here: if binding two devices
> that are in the same domain to the same process we shouldn't add the
> io_mm to domain->mm_list twice.
>
> I'm still not sure if I should remove domains handling here though,
> could you confirm if you're planning to support
> iommu_get_domain_for_dev for vt-d?
>
yes. i am working on getting vt-d onto the same behavior in terms of
default domain. I have a patch being tested, we need to respect RMRR (
reserved region) that is setup before iommu_get_domain_for_dev().
> Thanks,
> Jean
[Jacob Pan]
