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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Jan 2014 16:46:22 +0000
From:	"Bharat.Bhushan@...escale.com" <Bharat.Bhushan@...escale.com>
To:	Alex Williamson <alex.williamson@...hat.com>
CC:	Varun Sethi <Varun.Sethi@...escale.com>,
	"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support



> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@...hat.com]
> Sent: Tuesday, January 21, 2014 7:06 PM
> To: Bhushan Bharat-R65777
> Cc: Sethi Varun-B16395; iommu@...ts.linux-foundation.org; linux-
> kernel@...r.kernel.org
> Subject: Re: [RFC PATCH] vfio/iommu_type1: Multi-IOMMU domain support
> 
> On Tue, 2014-01-21 at 07:27 +0000, Bharat.Bhushan@...escale.com 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
> 
>