RE: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support

[Bharat.Bhushan@xxxxxxxxxxxxx]

> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx]
> Sent: Tuesday, January 21, 2014 7:06 PM
> To: Bhushan Bharat-R65777
> Cc: Sethi Varun-B16395; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support
> 
> On Tue, 2014-01-21 at 07:27 +0000, Bharat.Bhushan@xxxxxxxxxxxxx wrote:
> > > +	domain->domain = iommu_domain_alloc(domain->bus);
> > > +	if (!domain->domain) {
> > > +		ret = -EIO;
> > > +		goto out_free;
> > > +	}
> > > +
> > > +	ret = iommu_attach_group(domain->domain, iommu_group);
> > > +	if (ret)
> > > +		goto out_domain;
> > > +
> > > +	INIT_LIST_HEAD(&domain->group_list);
> > > +	list_add(&group->next, &domain->group_list);
> > > +
> > > +	if (!allow_unsafe_interrupts &&
> > > +	    !iommu_domain_has_cap(domain->domain, IOMMU_CAP_INTR_REMAP)) {
> > > +		pr_warn("%s: No interrupt remapping support.  Use the module
> > > +param
> > > \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this
> > > platform\n",
> > > +		       __func__);
> > > +		ret = -EPERM;
> > > +		goto out_detach;
> > > +	}
> > > +
> > > +	if (iommu_domain_has_cap(domain->domain, IOMMU_CAP_CACHE_COHERENCY))
> > > +		domain->prot |= IOMMU_CACHE;
> > > +
> > > +	/* Try to match an existing compatible domain. */
> > > +	list_for_each_entry(d, &iommu->domain_list, next) {
> > > +		if (d->bus == domain->bus && d->prot == domain->prot) {
> >
> > Are not we talking about allowing a domain to support different bus type if
> domain/iommu-group properties matches.
> 
> This is where I was suggesting to Varun that we could test iommu_ops instead of
> bus_type.
> 
> > > +			iommu_detach_group(domain->domain, iommu_group);
> > > +			if (!iommu_attach_group(d->domain, iommu_group)) {
> > > +				list_add(&group->next, &d->group_list);
> > > +				iommu_domain_free(domain->domain);
> > > +				kfree(domain);
> > > +				mutex_unlock(&iommu->lock);
> > > +				return 0;
> > > +			}
> > > +
> > > +			ret = iommu_attach_group(domain->domain, iommu_group);
> > > +			if (ret)
> > > +				goto out_domain;
> > > +		}
> > > +	}
> > > +
> > > +	/* replay mappings on new domains */
> > > +	ret = vfio_iommu_replay(iommu, domain);
> >
> > IIUC; We created a new domain because an already existing domain does
> > not have same attribute; say domain->prot; But in vfio_iommu_replay() we pick
> up any domain, first domain, and create mapping accordingly.
> > Should not we use attributes of this domain otherwise we may get "reserved bit
> faults"?
> 
> We use an existing domain to get the iova to physical mappings, should those not
> be consistent regardless of the domain we pick?  We're not using any of the low
> level attributes that could cause something like a reserved bit fault.  Thanks,

You are right, we use dma->addr etc from any domain and but uses "prot" from the domain passed to replay function().
So effectively the only difference (from dma mapping perspective) between domains in a container is "prot"

Thanks
-Bharat
> 
> Alex
> 
> 

èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðÃnÆàþY&—