[PATCH V2] 2.6.34: simple IOMMU API extension to check safe interrupt remapping

[PATCH V2] 2.6.34: simple IOMMU API extension to check safe interrupt remapping

[Tom Lyon]

This patch allows IOMMU users to determine whether the hardware and software
support safe, isolated interrupt remapping.  Not all Intel IOMMUs have the
hardware, and the software for AMD is not there yet.
	Signed-off-by: Tom Lyon <pugs@cisco.com>
---

Version 2: previous ifdefs not needed.

MST has convinced me that any user level driver for PCI master devices can't 
be safe unless there is an IOMMU protecting the APIC MSI/MSI-X interrupt 
addresses from device writes.  This interrupt remapping is not present in all
Intel IOMMUs and the code for the interrupt mapping in the AMD IOMMUs is not
implemented yet.

Needed by not-yet-accepted VFIO driver.

diff -uprN linux-2.6.34/drivers/pci/intel-iommu.c iommuapi-linux-2.6.34/drivers/pci/intel-iommu.c
--- linux-2.6.34/drivers/pci/intel-iommu.c	2010-05-16 14:17:36.000000000 -0700
+++ iommuapi-linux-2.6.34/drivers/pci/intel-iommu.c	2010-06-30 15:47:10.000000000 -0700
@@ -3705,6 +3705,10 @@ static int intel_iommu_domain_has_cap(st
 
 	if (cap == IOMMU_CAP_CACHE_COHERENCY)
 		return dmar_domain->iommu_snooping;
+	if (cap == IOMMU_CAP_SAFE_INTR_REMAP)
+		return intr_remapping_enabled;
 
 	return 0;
 }
diff -uprN linux-2.6.34/include/linux/iommu.h iommuapi-linux-2.6.34/include/linux/iommu.h
--- linux-2.6.34/include/linux/iommu.h	2010-05-16 14:17:36.000000000 -0700
+++ iommuapi-linux-2.6.34/include/linux/iommu.h	2010-06-30 15:47:34.000000000 -0700
@@ -30,6 +30,7 @@ struct iommu_domain {
 };
 
 #define IOMMU_CAP_CACHE_COHERENCY	0x1
+#define IOMMU_CAP_SAFE_INTR_REMAP	0x2	/* isolates device intrs */
 
 struct iommu_ops {
 	int (*domain_init)(struct iommu_domain *domain);

Re: [PATCH V2] 2.6.34: simple IOMMU API extension to check safe interrupt remapping

[Roedel, Joerg]

On Thu, Jul 01, 2010 at 05:24:32PM -0400, Tom Lyon wrote:
> This patch allows IOMMU users to determine whether the hardware and software
> support safe, isolated interrupt remapping.  Not all Intel IOMMUs have the
> hardware, and the software for AMD is not there yet.
> 	Signed-off-by: Tom Lyon <pugs@cisco.com>

It does not make a lot of sense to check for this feature in the
IOMMU-API currently because there is no support for intr-remapping in
there. But it will be there when intr-remapping support for AMD IOMMU is
implemented. So this change can be considered as a first step in that
direction. Please repost with the change requested below an I'll add
this one to my tree.

> Version 2: previous ifdefs not needed.
> 
> MST has convinced me that any user level driver for PCI master devices can't 
> be safe unless there is an IOMMU protecting the APIC MSI/MSI-X interrupt 
> addresses from device writes.  This interrupt remapping is not present in all
> Intel IOMMUs and the code for the interrupt mapping in the AMD IOMMUs is not
> implemented yet.
> 
> Needed by not-yet-accepted VFIO driver.
> 
> diff -uprN linux-2.6.34/drivers/pci/intel-iommu.c iommuapi-linux-2.6.34/drivers/pci/intel-iommu.c
> --- linux-2.6.34/drivers/pci/intel-iommu.c	2010-05-16 14:17:36.000000000 -0700
> +++ iommuapi-linux-2.6.34/drivers/pci/intel-iommu.c	2010-06-30 15:47:10.000000000 -0700
> @@ -3705,6 +3705,10 @@ static int intel_iommu_domain_has_cap(st
>  
>  	if (cap == IOMMU_CAP_CACHE_COHERENCY)
>  		return dmar_domain->iommu_snooping;
> +	if (cap == IOMMU_CAP_SAFE_INTR_REMAP)
> +		return intr_remapping_enabled;
>  
>  	return 0;
>  }
> diff -uprN linux-2.6.34/include/linux/iommu.h iommuapi-linux-2.6.34/include/linux/iommu.h
> --- linux-2.6.34/include/linux/iommu.h	2010-05-16 14:17:36.000000000 -0700
> +++ iommuapi-linux-2.6.34/include/linux/iommu.h	2010-06-30 15:47:34.000000000 -0700
> @@ -30,6 +30,7 @@ struct iommu_domain {
>  };
>  
>  #define IOMMU_CAP_CACHE_COHERENCY	0x1
> +#define IOMMU_CAP_SAFE_INTR_REMAP	0x2	/* isolates device intrs */

I think the SAFE_ is not necessary in the name. It is misleading because
it indicates that there is an unsafe variant of intr-remapping available
when this capability is not set. Just call it IOMMU_CAP_INTR_REMAPPING.


	Joerg

-- 
Joerg Roedel - AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH V2] 2.6.34: simple IOMMU API extension to check safe interrupt remapping

[Tom Lyon]

On Friday 02 July 2010 02:26:46 am Roedel, Joerg wrote:
> On Thu, Jul 01, 2010 at 05:24:32PM -0400, Tom Lyon wrote:
> > This patch allows IOMMU users to determine whether the hardware and software
> > support safe, isolated interrupt remapping.  Not all Intel IOMMUs have the
> > hardware, and the software for AMD is not there yet.
> > 	Signed-off-by: Tom Lyon <pugs@cisco.com>
> 
> It does not make a lot of sense to check for this feature in the
> IOMMU-API currently because there is no support for intr-remapping in
> there. But it will be there when intr-remapping support for AMD IOMMU is
> implemented. So this change can be considered as a first step in that
> direction. Please repost with the change requested below an I'll add
> this one to my tree.
OK, but I'm not sure what you mean by the first sentence.  The Intel stuff
today does intr remapping as a side effect of X2APIC support.

> 
> > Version 2: previous ifdefs not needed.
> > 
> > MST has convinced me that any user level driver for PCI master devices can't 
> > be safe unless there is an IOMMU protecting the APIC MSI/MSI-X interrupt 
> > addresses from device writes.  This interrupt remapping is not present in all
> > Intel IOMMUs and the code for the interrupt mapping in the AMD IOMMUs is not
> > implemented yet.
> > 
> > Needed by not-yet-accepted VFIO driver.
> > 
> > diff -uprN linux-2.6.34/drivers/pci/intel-iommu.c iommuapi-linux-2.6.34/drivers/pci/intel-iommu.c
> > --- linux-2.6.34/drivers/pci/intel-iommu.c	2010-05-16 14:17:36.000000000 -0700
> > +++ iommuapi-linux-2.6.34/drivers/pci/intel-iommu.c	2010-06-30 15:47:10.000000000 -0700
> > @@ -3705,6 +3705,10 @@ static int intel_iommu_domain_has_cap(st
> >  
> >  	if (cap == IOMMU_CAP_CACHE_COHERENCY)
> >  		return dmar_domain->iommu_snooping;
> > +	if (cap == IOMMU_CAP_SAFE_INTR_REMAP)
> > +		return intr_remapping_enabled;
> >  
> >  	return 0;
> >  }
> > diff -uprN linux-2.6.34/include/linux/iommu.h iommuapi-linux-2.6.34/include/linux/iommu.h
> > --- linux-2.6.34/include/linux/iommu.h	2010-05-16 14:17:36.000000000 -0700
> > +++ iommuapi-linux-2.6.34/include/linux/iommu.h	2010-06-30 15:47:34.000000000 -0700
> > @@ -30,6 +30,7 @@ struct iommu_domain {
> >  };
> >  
> >  #define IOMMU_CAP_CACHE_COHERENCY	0x1
> > +#define IOMMU_CAP_SAFE_INTR_REMAP	0x2	/* isolates device intrs */
> 
> I think the SAFE_ is not necessary in the name. It is misleading because
> it indicates that there is an unsafe variant of intr-remapping available
> when this capability is not set. Just call it IOMMU_CAP_INTR_REMAPPING.
> 
> 
> 	Joerg
>