[RFC PATCH] vfio/type1: Do not support IOMMUs that allow bypass

[RFC PATCH] vfio/type1: Do not support IOMMUs that allow bypass

[Alex Williamson]

We can only provide isolation if DMA is forced through the IOMMU
aperture.  Don't allow type1 to be used if this is not the case.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

Eric, I see a number of IOMMU drivers enable this, do the ones you
care about for ARM set geometry.force_aperture?  Thanks,

Alex

 drivers/vfio/vfio_iommu_type1.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 57d8c37..6afa9d4 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -728,6 +728,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 	struct vfio_group *group, *g;
 	struct vfio_domain *domain, *d;
 	struct bus_type *bus = NULL;
+	struct iommu_domain_geometry geometry;
 	int ret;
 
 	mutex_lock(&iommu->lock);
@@ -762,6 +763,17 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
 		goto out_free;
 	}
 
+	/*
+	 * If a domain does not force DMA within the aperture, devices are not
+	 * isolated and type1 is not an appropriate IOMMU model.
+	 */
+	ret = iommu_domain_get_attr(domain->domain,
+				    DOMAIN_ATTR_GEOMETRY, &geometry);
+	if (ret || !geometry.force_aperture) {
+		ret = -EPERM;
+		goto out_domain;
+	}
+
 	if (iommu->nesting) {
 		int attr = 1;
 

Re: [RFC PATCH] vfio/type1: Do not support IOMMUs that allow bypass

[Eric Auger]

Hi Alex,
On 10/15/2015 10:52 PM, Alex Williamson wrote:
> We can only provide isolation if DMA is forced through the IOMMU
> aperture.  Don't allow type1 to be used if this is not the case.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
> Eric, I see a number of IOMMU drivers enable this, do the ones you
> care about for ARM set geometry.force_aperture?  Thanks,
I am currently using arm-smmu.c. I don't see force_aperture being set.

Best Regards

Eric
> 
> Alex
> 
>  drivers/vfio/vfio_iommu_type1.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 57d8c37..6afa9d4 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -728,6 +728,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>  	struct vfio_group *group, *g;
>  	struct vfio_domain *domain, *d;
>  	struct bus_type *bus = NULL;
> +	struct iommu_domain_geometry geometry;
>  	int ret;
>  
>  	mutex_lock(&iommu->lock);
> @@ -762,6 +763,17 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>  		goto out_free;
>  	}
>  
> +	/*
> +	 * If a domain does not force DMA within the aperture, devices are not
> +	 * isolated and type1 is not an appropriate IOMMU model.
> +	 */
> +	ret = iommu_domain_get_attr(domain->domain,
> +				    DOMAIN_ATTR_GEOMETRY, &geometry);
> +	if (ret || !geometry.force_aperture) {
> +		ret = -EPERM;
> +		goto out_domain;
> +	}
> +
>  	if (iommu->nesting) {
>  		int attr = 1;
>  
> 

Re: [RFC PATCH] vfio/type1: Do not support IOMMUs that allow bypass

[Alex Williamson]

On Fri, 2015-10-16 at 16:03 +0200, Eric Auger wrote:
> Hi Alex,
> On 10/15/2015 10:52 PM, Alex Williamson wrote:
> > We can only provide isolation if DMA is forced through the IOMMU
> > aperture.  Don't allow type1 to be used if this is not the case.
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> > 
> > Eric, I see a number of IOMMU drivers enable this, do the ones you
> > care about for ARM set geometry.force_aperture?  Thanks,
> I am currently using arm-smmu.c. I don't see force_aperture being set.

Hi Will,

Would it be possible to add iommu_domain_geometry support to arm-smmu.c?
In addition to this test to verify that DMA cannot bypass the IOMMU, I'd
eventually like to pass the aperture information out through the VFIO
API.  Thanks,

Alex
 
> >  drivers/vfio/vfio_iommu_type1.c |   12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> > index 57d8c37..6afa9d4 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -728,6 +728,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> >  	struct vfio_group *group, *g;
> >  	struct vfio_domain *domain, *d;
> >  	struct bus_type *bus = NULL;
> > +	struct iommu_domain_geometry geometry;
> >  	int ret;
> >  
> >  	mutex_lock(&iommu->lock);
> > @@ -762,6 +763,17 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> >  		goto out_free;
> >  	}
> >  
> > +	/*
> > +	 * If a domain does not force DMA within the aperture, devices are not
> > +	 * isolated and type1 is not an appropriate IOMMU model.
> > +	 */
> > +	ret = iommu_domain_get_attr(domain->domain,
> > +				    DOMAIN_ATTR_GEOMETRY, &geometry);
> > +	if (ret || !geometry.force_aperture) {
> > +		ret = -EPERM;
> > +		goto out_domain;
> > +	}
> > +
> >  	if (iommu->nesting) {
> >  		int attr = 1;
> >  
> > 
> 

Re: [RFC PATCH] vfio/type1: Do not support IOMMUs that allow bypass

[Will Deacon]

On Fri, Oct 16, 2015 at 09:51:22AM -0600, Alex Williamson wrote:
> On Fri, 2015-10-16 at 16:03 +0200, Eric Auger wrote:
> > Hi Alex,
> > On 10/15/2015 10:52 PM, Alex Williamson wrote:
> > > We can only provide isolation if DMA is forced through the IOMMU
> > > aperture.  Don't allow type1 to be used if this is not the case.
> > > 
> > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > ---
> > > 
> > > Eric, I see a number of IOMMU drivers enable this, do the ones you
> > > care about for ARM set geometry.force_aperture?  Thanks,
> > I am currently using arm-smmu.c. I don't see force_aperture being set.
> 
> Hi Will,

Hi Alex,

> Would it be possible to add iommu_domain_geometry support to arm-smmu.c?
> In addition to this test to verify that DMA cannot bypass the IOMMU, I'd
> eventually like to pass the aperture information out through the VFIO
> API.  Thanks,

The slight snag here is that we don't know which SMMU in the system the
domain is attached to at the point when the geometry is queried, so I
can't give you an upper bound on the aperture. For example, if there is
an SMMU with a 32-bit input address and another with a 48-bit input
address.

We could play the same horrible games that we do with the pgsize bitmap,
and truncate some global aperture everytime we probe an SMMU device, but
I'd really like to have fewer hacks like that if possible. The root of
the problem is still that domains are allocated for a bus, rather than
an IOMMU instance.

Will

Re: [RFC PATCH] vfio/type1: Do not support IOMMUs that allow bypass

[Alex Williamson]

[cc +iommu]

On Tue, 2015-10-27 at 15:40 +0000, Will Deacon wrote:
> On Fri, Oct 16, 2015 at 09:51:22AM -0600, Alex Williamson wrote:
> > On Fri, 2015-10-16 at 16:03 +0200, Eric Auger wrote:
> > > Hi Alex,
> > > On 10/15/2015 10:52 PM, Alex Williamson wrote:
> > > > We can only provide isolation if DMA is forced through the IOMMU
> > > > aperture.  Don't allow type1 to be used if this is not the case.
> > > > 
> > > > Signed-off-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > > ---
> > > > 
> > > > Eric, I see a number of IOMMU drivers enable this, do the ones you
> > > > care about for ARM set geometry.force_aperture?  Thanks,
> > > I am currently using arm-smmu.c. I don't see force_aperture being set.
> > 
> > Hi Will,
> 
> Hi Alex,
> 
> > Would it be possible to add iommu_domain_geometry support to arm-smmu.c?
> > In addition to this test to verify that DMA cannot bypass the IOMMU, I'd
> > eventually like to pass the aperture information out through the VFIO
> > API.  Thanks,
> 
> The slight snag here is that we don't know which SMMU in the system the
> domain is attached to at the point when the geometry is queried, so I
> can't give you an upper bound on the aperture. For example, if there is
> an SMMU with a 32-bit input address and another with a 48-bit input
> address.
> 
> We could play the same horrible games that we do with the pgsize bitmap,
> and truncate some global aperture everytime we probe an SMMU device, but
> I'd really like to have fewer hacks like that if possible. The root of
> the problem is still that domains are allocated for a bus, rather than
> an IOMMU instance.

Hi Will,

Yes, Intel VT-d has this issue as well.  In theory we can have
heterogeneous IOMMU hardware units (DRHDs) in a system and the upper
bound of the geometry could be diminished if we add a less capable DRHD
into the domain.  I suspect this is more a theoretical problem than a
practical one though as we're typically mixing similar DRHDs and I think
we're still capable of 39-bit addressing in the least capable version
per the spec.

In any case, I really want to start testing geometry.force_aperture,
even if we're not yet comfortable to expose the IOMMU limits to the
user.  The vfio type1 shouldn't be enabled at all for underlying
hardware that allows DMA bypass.  Thanks,

Alex

Re: [RFC PATCH] vfio/type1: Do not support IOMMUs that allow bypass

[Will Deacon]

On Tue, Oct 27, 2015 at 10:00:11AM -0600, Alex Williamson wrote:
> On Tue, 2015-10-27 at 15:40 +0000, Will Deacon wrote:
> > On Fri, Oct 16, 2015 at 09:51:22AM -0600, Alex Williamson wrote:
> > > Would it be possible to add iommu_domain_geometry support to arm-smmu.c?
> > > In addition to this test to verify that DMA cannot bypass the IOMMU, I'd
> > > eventually like to pass the aperture information out through the VFIO
> > > API.  Thanks,
> > 
> > The slight snag here is that we don't know which SMMU in the system the
> > domain is attached to at the point when the geometry is queried, so I
> > can't give you an upper bound on the aperture. For example, if there is
> > an SMMU with a 32-bit input address and another with a 48-bit input
> > address.
> > 
> > We could play the same horrible games that we do with the pgsize bitmap,
> > and truncate some global aperture everytime we probe an SMMU device, but
> > I'd really like to have fewer hacks like that if possible. The root of
> > the problem is still that domains are allocated for a bus, rather than
> > an IOMMU instance.
> 
> Yes, Intel VT-d has this issue as well.  In theory we can have
> heterogeneous IOMMU hardware units (DRHDs) in a system and the upper
> bound of the geometry could be diminished if we add a less capable DRHD
> into the domain.  I suspect this is more a theoretical problem than a
> practical one though as we're typically mixing similar DRHDs and I think
> we're still capable of 39-bit addressing in the least capable version
> per the spec.
> 
> In any case, I really want to start testing geometry.force_aperture,
> even if we're not yet comfortable to expose the IOMMU limits to the
> user.  The vfio type1 shouldn't be enabled at all for underlying
> hardware that allows DMA bypass.  Thanks,

Ok, I'll put it on my list of things to look at under the assumption that
the actual aperture limits don't need to be accurate as long as DMA to
an arbitrary unmapped address always faults.

Will

Re: [RFC PATCH] vfio/type1: Do not support IOMMUs that allow bypass

[Robin Murphy]

On 29/10/15 18:28, Will Deacon wrote:
> On Tue, Oct 27, 2015 at 10:00:11AM -0600, Alex Williamson wrote:
>> On Tue, 2015-10-27 at 15:40 +0000, Will Deacon wrote:
>>> On Fri, Oct 16, 2015 at 09:51:22AM -0600, Alex Williamson wrote:
>>>> Would it be possible to add iommu_domain_geometry support to arm-smmu.c?
>>>> In addition to this test to verify that DMA cannot bypass the IOMMU, I'd
>>>> eventually like to pass the aperture information out through the VFIO
>>>> API.  Thanks,
>>>
>>> The slight snag here is that we don't know which SMMU in the system the
>>> domain is attached to at the point when the geometry is queried, so I
>>> can't give you an upper bound on the aperture. For example, if there is
>>> an SMMU with a 32-bit input address and another with a 48-bit input
>>> address.
>>>
>>> We could play the same horrible games that we do with the pgsize bitmap,
>>> and truncate some global aperture everytime we probe an SMMU device, but
>>> I'd really like to have fewer hacks like that if possible. The root of
>>> the problem is still that domains are allocated for a bus, rather than
>>> an IOMMU instance.
>>
>> Yes, Intel VT-d has this issue as well.  In theory we can have
>> heterogeneous IOMMU hardware units (DRHDs) in a system and the upper
>> bound of the geometry could be diminished if we add a less capable DRHD
>> into the domain.  I suspect this is more a theoretical problem than a
>> practical one though as we're typically mixing similar DRHDs and I think
>> we're still capable of 39-bit addressing in the least capable version
>> per the spec.
>>
>> In any case, I really want to start testing geometry.force_aperture,
>> even if we're not yet comfortable to expose the IOMMU limits to the
>> user.  The vfio type1 shouldn't be enabled at all for underlying
>> hardware that allows DMA bypass.  Thanks,
>
> Ok, I'll put it on my list of things to look at under the assumption that
> the actual aperture limits don't need to be accurate as long as DMA to
> an arbitrary unmapped address always faults.

I'm pretty sure we'd only ever set the aperture to the full input 
address range anyway (since we're not a GART-type thing), in which case 
we should only need to worry about unmatched streams that don't hit in a 
domain at all. Doesn't the disable_bypass option already cover that? 
(FWIW I hacked it up for v2 a while back, too[0]).

Robin.

[0]:http://www.linux-arm.org/git?p=linux-rm.git;a=commitdiff;h=23a251189fa3330b799a837bd8eb1023aa2dcea4

Re: [RFC PATCH] vfio/type1: Do not support IOMMUs that allow bypass

[Will Deacon]

On Thu, Oct 29, 2015 at 06:42:10PM +0000, Robin Murphy wrote:
> On 29/10/15 18:28, Will Deacon wrote:
> >On Tue, Oct 27, 2015 at 10:00:11AM -0600, Alex Williamson wrote:
> >>On Tue, 2015-10-27 at 15:40 +0000, Will Deacon wrote:
> >>>On Fri, Oct 16, 2015 at 09:51:22AM -0600, Alex Williamson wrote:
> >>>>Would it be possible to add iommu_domain_geometry support to arm-smmu.c?
> >>>>In addition to this test to verify that DMA cannot bypass the IOMMU, I'd
> >>>>eventually like to pass the aperture information out through the VFIO
> >>>>API.  Thanks,
> >>>
> >>>The slight snag here is that we don't know which SMMU in the system the
> >>>domain is attached to at the point when the geometry is queried, so I
> >>>can't give you an upper bound on the aperture. For example, if there is
> >>>an SMMU with a 32-bit input address and another with a 48-bit input
> >>>address.
> >>>
> >>>We could play the same horrible games that we do with the pgsize bitmap,
> >>>and truncate some global aperture everytime we probe an SMMU device, but
> >>>I'd really like to have fewer hacks like that if possible. The root of
> >>>the problem is still that domains are allocated for a bus, rather than
> >>>an IOMMU instance.
> >>
> >>Yes, Intel VT-d has this issue as well.  In theory we can have
> >>heterogeneous IOMMU hardware units (DRHDs) in a system and the upper
> >>bound of the geometry could be diminished if we add a less capable DRHD
> >>into the domain.  I suspect this is more a theoretical problem than a
> >>practical one though as we're typically mixing similar DRHDs and I think
> >>we're still capable of 39-bit addressing in the least capable version
> >>per the spec.
> >>
> >>In any case, I really want to start testing geometry.force_aperture,
> >>even if we're not yet comfortable to expose the IOMMU limits to the
> >>user.  The vfio type1 shouldn't be enabled at all for underlying
> >>hardware that allows DMA bypass.  Thanks,
> >
> >Ok, I'll put it on my list of things to look at under the assumption that
> >the actual aperture limits don't need to be accurate as long as DMA to
> >an arbitrary unmapped address always faults.
> 
> I'm pretty sure we'd only ever set the aperture to the full input address
> range anyway (since we're not a GART-type thing), in which case we should
> only need to worry about unmatched streams that don't hit in a domain at
> all. Doesn't the disable_bypass option already cover that? (FWIW I hacked it
> up for v2 a while back, too[0]).

Well, the "full input address range" is tricky when you have multiple SMMU
instances with different input address sizes. I can do something similar
to the pgsize_bitmap.

I also don't think the disable_bypass option is what we're after -- this
is about devices attached to a VFIO domain that can still mysteriously
bypass the SMMU for some ranges AFAIU (and shouldn't be an issue for ARM).

Will