[Qemu-devel] [PATCH 2/3] device_isolation: Support isolation on POWER p5ioc2 bridges

[Qemu-devel] [PATCH 2/3] device_isolation: Support isolation on POWER p5ioc2 bridges

[David Gibson]

This patch adds code to the code for the powernv platform to create
and populate isolation groups on hardware using the p5ioc2 PCI host
bridge used on some IBM POWER systems.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 arch/powerpc/platforms/powernv/pci-p5ioc2.c |   14 +++++++++++++-
 arch/powerpc/platforms/powernv/pci.h        |    3 +++
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-p5ioc2.c b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
index 4c80f7c..24847a9 100644
--- a/arch/powerpc/platforms/powernv/pci-p5ioc2.c
+++ b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
@@ -20,6 +20,7 @@
 #include <linux/irq.h>
 #include <linux/io.h>
 #include <linux/msi.h>
+#include <linux/device_isolation.h>
 
 #include <asm/sections.h>
 #include <asm/io.h>
@@ -88,10 +89,21 @@ static void pnv_pci_init_p5ioc2_msis(struct pnv_phb *phb) { }
 static void __devinit pnv_pci_p5ioc2_dma_dev_setup(struct pnv_phb *phb,
 						   struct pci_dev *pdev)
 {
-	if (phb->p5ioc2.iommu_table.it_map == NULL)
+	if (phb->p5ioc2.iommu_table.it_map == NULL) {
 		iommu_init_table(&phb->p5ioc2.iommu_table, phb->hose->node);
+#ifdef CONFIG_DEVICE_ISOLATION
+		phb->p5ioc2.di_group = kzalloc(sizeof(*(phb->p5ioc2.di_group)),
+					       GFP_KERNEL);
+		BUG_ON(!phb->p5ioc2.di_group ||
+		       (device_isolation_group_init(phb->p5ioc2.di_group,
+						    "p5ioc2:%llx", phb->opal_id) < 0));
+#endif
+	}
 
 	set_iommu_table_base(&pdev->dev, &phb->p5ioc2.iommu_table);
+#ifdef CONFIG_DEVICE_ISOLATION
+	device_isolation_dev_add(phb->p5ioc2.di_group, &pdev->dev);
+#endif
 }
 
 static void __init pnv_pci_init_p5ioc2_phb(struct device_node *np,
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 28ae4ca..a2dc071 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -77,6 +77,9 @@ struct pnv_phb {
 	union {
 		struct {
 			struct iommu_table iommu_table;
+#ifdef CONFIG_DEVICE_ISOLATION
+			struct device_isolation_group *di_group;
+#endif
 		} p5ioc2;
 
 		struct {
-- 
1.7.7.3

[Qemu-devel] [PATCH 3/3] device_isolation: Support isolation on POWER p7ioc (IODA) bridges

[David Gibson]

This patch adds code to the code for the powernv platform to create
and populate isolation groups on hardware using the p7ioc (aka IODA) PCI host
bridge used on some IBM POWER systems.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |   18 ++++++++++++++++--
 arch/powerpc/platforms/powernv/pci.h      |    6 ++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 0cdc8302..6df632e 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -20,6 +20,7 @@
 #include <linux/irq.h>
 #include <linux/io.h>
 #include <linux/msi.h>
+#include <linux/device_isolation.h>
 
 #include <asm/sections.h>
 #include <asm/io.h>
@@ -861,6 +862,9 @@ static void __devinit pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
 		set_iommu_table_base(&dev->dev, &pe->tce32_table);
 		if (dev->subordinate)
 			pnv_ioda_setup_bus_dma(pe, dev->subordinate);
+#ifdef CONFIG_DEVICE_ISOLATION
+		device_isolation_dev_add(&pe->di_group, &dev->dev);
+#endif
 	}
 }
 
@@ -941,11 +945,21 @@ static void __devinit pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
 	}
 	iommu_init_table(tbl, phb->hose->node);
 
-	if (pe->pdev)
+#ifdef CONFIG_DEVICE_ISOLATION
+	BUG_ON(device_isolation_group_init(&pe->di_group, "ioda:rid%x-pe%x",
+					   pe->rid, pe->pe_number) < 0);
+#endif
+
+	if (pe->pdev) {
 		set_iommu_table_base(&pe->pdev->dev, tbl);
-	else
+#ifdef CONFIG_DEVICE_ISOLATION
+		device_isolation_dev_add(&pe->di_group, &pe->pdev->dev);
+#endif
+	} else
 		pnv_ioda_setup_bus_dma(pe, pe->pbus);
 
+
+
 	return;
  fail:
 	/* XXX Failure: Try to fallback to 64-bit only ? */
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index a2dc071..d663a26 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -1,6 +1,8 @@
 #ifndef __POWERNV_PCI_H
 #define __POWERNV_PCI_H
 
+#include <linux/device_isolation.h>
+
 struct pci_dn;
 
 enum pnv_phb_type {
@@ -51,6 +53,10 @@ struct pnv_ioda_pe {
 
 	/* Link in list of PE#s */
 	struct list_head	link;
+
+#ifdef CONFIG_DEVICE_ISOLATION
+	struct device_isolation_group di_group;
+#endif
 };
 
 struct pnv_phb {
-- 
1.7.7.3

[Qemu-devel] [RFC] Device isolation infrastructure v2

[David Gibson]

Here's the second spin of my preferred approach to handling grouping
of devices for safe assignment to guests.

Changes since v1:
 * Many name changes and file moves for improved consistency
 * Bugfixes and cleanups
 * The interface to the next layer up is considerably fleshed out,
   although it still needs work.
 * Example initialization of groups for p5ioc2 and p7ioc.

TODO:
 * Need sample initialization of groups for intel and/or amd iommus
 * Use of sysfs attributes to control group permission is probably a
   mistake.  Although it seems a bit odd, registering a chardev for
   each group is probably better, because perms can be set from udev
   rules, just like everything else.
 * Need more details of what the binder structure will need to
   contain.
 * Handle complete removal of groups.
 * Clarify what will need to happen on the hot unplug path.

[Qemu-devel] [PATCH 3/3] device_isolation: Support isolation on POWER p7ioc (IODA) bridges

[David Gibson]

This patch adds code to the code for the powernv platform to create
and populate isolation groups on hardware using the p7ioc (aka IODA) PCI host
bridge used on some IBM POWER systems.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |   18 ++++++++++++++++--
 arch/powerpc/platforms/powernv/pci.h      |    6 ++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 0cdc8302..6df632e 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -20,6 +20,7 @@
 #include <linux/irq.h>
 #include <linux/io.h>
 #include <linux/msi.h>
+#include <linux/device_isolation.h>
 
 #include <asm/sections.h>
 #include <asm/io.h>
@@ -861,6 +862,9 @@ static void __devinit pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
 		set_iommu_table_base(&dev->dev, &pe->tce32_table);
 		if (dev->subordinate)
 			pnv_ioda_setup_bus_dma(pe, dev->subordinate);
+#ifdef CONFIG_DEVICE_ISOLATION
+		device_isolation_dev_add(&pe->di_group, &dev->dev);
+#endif
 	}
 }
 
@@ -941,11 +945,21 @@ static void __devinit pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
 	}
 	iommu_init_table(tbl, phb->hose->node);
 
-	if (pe->pdev)
+#ifdef CONFIG_DEVICE_ISOLATION
+	BUG_ON(device_isolation_group_init(&pe->di_group, "ioda:rid%x-pe%x",
+					   pe->rid, pe->pe_number) < 0);
+#endif
+
+	if (pe->pdev) {
 		set_iommu_table_base(&pe->pdev->dev, tbl);
-	else
+#ifdef CONFIG_DEVICE_ISOLATION
+		device_isolation_dev_add(&pe->di_group, &pe->pdev->dev);
+#endif
+	} else
 		pnv_ioda_setup_bus_dma(pe, pe->pbus);
 
+
+
 	return;
  fail:
 	/* XXX Failure: Try to fallback to 64-bit only ? */
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index a2dc071..d663a26 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -1,6 +1,8 @@
 #ifndef __POWERNV_PCI_H
 #define __POWERNV_PCI_H
 
+#include <linux/device_isolation.h>
+
 struct pci_dn;
 
 enum pnv_phb_type {
@@ -51,6 +53,10 @@ struct pnv_ioda_pe {
 
 	/* Link in list of PE#s */
 	struct list_head	link;
+
+#ifdef CONFIG_DEVICE_ISOLATION
+	struct device_isolation_group di_group;
+#endif
 };
 
 struct pnv_phb {
-- 
1.7.7.3

[Qemu-devel] RFC: Device isolation groups

[David Gibson]

This patch series introduces a new infrastructure to the driver core
for representing "device isolation groups".  That is, groups of
devices which can be "isolated" in such a way that the rest of the
system can be protected from them, even in the presence of userspace
or a guest OS directly driving the devices.

Isolation will typically be due to an IOMMU which can safely remap DMA
and interrupts coming from these devices.  We need to represent whole
groups, rather than individual devices, because there are a number of
cases where the group can be isolated as a whole, but devices within
it cannot be safely isolated from each other - this usually occurs
because the IOMMU cannot reliably distinguish which device in the
group initiated a transaction.  In other words, isolation groups
represent the minimum safe granularity for passthrough to guests or
userspace.

This series provides the core infraustrcture for tracking isolation
groups, and example implementations initializing the groups
appropriately for two PCI bridges (which include IOMMUs) found on IBM
POWER systems.

Actually using the group information is not included here, but David
Woodhouse has expressed an interest in using a structure like this to
represent operations in iommu_ops more correctly.

Some tracking of groups is a prerequisite for safe passthrough of
devices to guests or userspace, such as done by VFIO.  Current VFIO
patches use the iommu_ops->device_group mechanism for this.  However,
that mechanism is awkward, because without an in-kernel concrete
representation of groups, enumerating a group requires traversing
every device on a given bus type.  It also fails to cover some very
plausible IOMMU topologies, because its groups cannot span devices on
multiple bus types.

[Qemu-devel] [PATCH 3/3] device_isolation: Support isolation on POWER p7ioc (IODA) bridges

[David Gibson]

This patch adds code to the code for the powernv platform to create
and populate isolation groups on hardware using the p7ioc (aka IODA) PCI host
bridge used on some IBM POWER systems.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |   18 ++++++++++++++++--
 arch/powerpc/platforms/powernv/pci.h      |    6 ++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 5e155df..4648475 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -20,6 +20,7 @@
 #include <linux/irq.h>
 #include <linux/io.h>
 #include <linux/msi.h>
+#include <linux/device_isolation.h>
 
 #include <asm/sections.h>
 #include <asm/io.h>
@@ -877,6 +878,9 @@ static void __devinit pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
 		set_iommu_table_base(&dev->dev, &pe->tce32_table);
 		if (dev->subordinate)
 			pnv_ioda_setup_bus_dma(pe, dev->subordinate);
+#ifdef CONFIG_DEVICE_ISOLATION
+		device_isolation_dev_add(&pe->di_group, &dev->dev);
+#endif
 	}
 }
 
@@ -957,11 +961,21 @@ static void __devinit pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
 	}
 	iommu_init_table(tbl, phb->hose->node);
 
-	if (pe->pdev)
+#ifdef CONFIG_DEVICE_ISOLATION
+	BUG_ON(device_isolation_group_init(&pe->di_group, "ioda:rid%x-pe%x",
+					   pe->rid, pe->pe_number) < 0);
+#endif
+
+	if (pe->pdev) {
 		set_iommu_table_base(&pe->pdev->dev, tbl);
-	else
+#ifdef CONFIG_DEVICE_ISOLATION
+		device_isolation_dev_add(&pe->di_group, &pe->pdev->dev);
+#endif
+	} else
 		pnv_ioda_setup_bus_dma(pe, pe->pbus);
 
+
+
 	return;
  fail:
 	/* XXX Failure: Try to fallback to 64-bit only ? */
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 64ede1e..3e282b7 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -1,6 +1,8 @@
 #ifndef __POWERNV_PCI_H
 #define __POWERNV_PCI_H
 
+#include <linux/device_isolation.h>
+
 struct pci_dn;
 
 enum pnv_phb_type {
@@ -60,6 +62,10 @@ struct pnv_ioda_pe {
 
 	/* Link in list of PE#s */
 	struct list_head	link;
+
+#ifdef CONFIG_DEVICE_ISOLATION
+	struct device_isolation_group di_group;
+#endif
 };
 
 struct pnv_phb {
-- 
1.7.8.3

Re: [Qemu-devel] [PATCH 3/3] device_isolation: Support isolation on POWER p7ioc (IODA) bridges

[Alex Williamson]

On Wed, 2012-02-01 at 15:46 +1100, David Gibson wrote:
> This patch adds code to the code for the powernv platform to create
> and populate isolation groups on hardware using the p7ioc (aka IODA) PCI host
> bridge used on some IBM POWER systems.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  arch/powerpc/platforms/powernv/pci-ioda.c |   18 ++++++++++++++++--
>  arch/powerpc/platforms/powernv/pci.h      |    6 ++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 5e155df..4648475 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -20,6 +20,7 @@
>  #include <linux/irq.h>
>  #include <linux/io.h>
>  #include <linux/msi.h>
> +#include <linux/device_isolation.h>
>  
>  #include <asm/sections.h>
>  #include <asm/io.h>
> @@ -877,6 +878,9 @@ static void __devinit pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
>  		set_iommu_table_base(&dev->dev, &pe->tce32_table);
>  		if (dev->subordinate)
>  			pnv_ioda_setup_bus_dma(pe, dev->subordinate);
> +#ifdef CONFIG_DEVICE_ISOLATION
> +		device_isolation_dev_add(&pe->di_group, &dev->dev);
> +#endif
>  	}
>  }
>  
> @@ -957,11 +961,21 @@ static void __devinit pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
>  	}
>  	iommu_init_table(tbl, phb->hose->node);
>  
> -	if (pe->pdev)
> +#ifdef CONFIG_DEVICE_ISOLATION
> +	BUG_ON(device_isolation_group_init(&pe->di_group, "ioda:rid%x-pe%x",
> +					   pe->rid, pe->pe_number) < 0);
> +#endif
> +
> +	if (pe->pdev) {
>  		set_iommu_table_base(&pe->pdev->dev, tbl);
> -	else
> +#ifdef CONFIG_DEVICE_ISOLATION
> +		device_isolation_dev_add(&pe->di_group, &pe->pdev->dev);
> +#endif
> +	} else
>  		pnv_ioda_setup_bus_dma(pe, pe->pbus);
>  

Blech, #ifdefs.

> +
> +
>  	return;
>   fail:
>  	/* XXX Failure: Try to fallback to 64-bit only ? */
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index 64ede1e..3e282b7 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -1,6 +1,8 @@
>  #ifndef __POWERNV_PCI_H
>  #define __POWERNV_PCI_H
>  
> +#include <linux/device_isolation.h>
> +
>  struct pci_dn;
>  
>  enum pnv_phb_type {
> @@ -60,6 +62,10 @@ struct pnv_ioda_pe {
>  
>  	/* Link in list of PE#s */
>  	struct list_head	link;
> +
> +#ifdef CONFIG_DEVICE_ISOLATION
> +	struct device_isolation_group di_group;
> +#endif

Embedding the struct means we need to know the size, which means we
can't get rid of the #ifdef.  Probably better to use a pointer if we
don't mind adding a few bytes in the #ifndef case.  Thanks,

Alex

>  };
>  
>  struct pnv_phb {

Re: [Qemu-devel] [PATCH 3/3] device_isolation: Support isolation on POWER p7ioc (IODA) bridges

[David Gibson]

On Wed, Feb 01, 2012 at 12:17:05PM -0700, Alex Williamson wrote:
> On Wed, 2012-02-01 at 15:46 +1100, David Gibson wrote:
> > This patch adds code to the code for the powernv platform to create
> > and populate isolation groups on hardware using the p7ioc (aka IODA) PCI host
> > bridge used on some IBM POWER systems.
> > 
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  arch/powerpc/platforms/powernv/pci-ioda.c |   18 ++++++++++++++++--
> >  arch/powerpc/platforms/powernv/pci.h      |    6 ++++++
> >  2 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> > index 5e155df..4648475 100644
> > --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> > @@ -20,6 +20,7 @@
> >  #include <linux/irq.h>
> >  #include <linux/io.h>
> >  #include <linux/msi.h>
> > +#include <linux/device_isolation.h>
> >  
> >  #include <asm/sections.h>
> >  #include <asm/io.h>
> > @@ -877,6 +878,9 @@ static void __devinit pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
> >  		set_iommu_table_base(&dev->dev, &pe->tce32_table);
> >  		if (dev->subordinate)
> >  			pnv_ioda_setup_bus_dma(pe, dev->subordinate);
> > +#ifdef CONFIG_DEVICE_ISOLATION
> > +		device_isolation_dev_add(&pe->di_group, &dev->dev);
> > +#endif
> >  	}
> >  }
> >  
> > @@ -957,11 +961,21 @@ static void __devinit pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
> >  	}
> >  	iommu_init_table(tbl, phb->hose->node);
> >  
> > -	if (pe->pdev)
> > +#ifdef CONFIG_DEVICE_ISOLATION
> > +	BUG_ON(device_isolation_group_init(&pe->di_group, "ioda:rid%x-pe%x",
> > +					   pe->rid, pe->pe_number) < 0);
> > +#endif
> > +
> > +	if (pe->pdev) {
> >  		set_iommu_table_base(&pe->pdev->dev, tbl);
> > -	else
> > +#ifdef CONFIG_DEVICE_ISOLATION
> > +		device_isolation_dev_add(&pe->di_group, &pe->pdev->dev);
> > +#endif
> > +	} else
> >  		pnv_ioda_setup_bus_dma(pe, pe->pbus);
> 
> Blech, #ifdefs.

Hm, yeah.  The problem is the di_group member not even existing when
!DEVICE_ISOLATION.  Might be able to avoid that with an empty
structure in that case.

> > +
> > +
> >  	return;
> >   fail:
> >  	/* XXX Failure: Try to fallback to 64-bit only ? */
> > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> > index 64ede1e..3e282b7 100644
> > --- a/arch/powerpc/platforms/powernv/pci.h
> > +++ b/arch/powerpc/platforms/powernv/pci.h
> > @@ -1,6 +1,8 @@
> >  #ifndef __POWERNV_PCI_H
> >  #define __POWERNV_PCI_H
> >  
> > +#include <linux/device_isolation.h>
> > +
> >  struct pci_dn;
> >  
> >  enum pnv_phb_type {
> > @@ -60,6 +62,10 @@ struct pnv_ioda_pe {
> >  
> >  	/* Link in list of PE#s */
> >  	struct list_head	link;
> > +
> > +#ifdef CONFIG_DEVICE_ISOLATION
> > +	struct device_isolation_group di_group;
> > +#endif
> 
> Embedding the struct means we need to know the size, which means we
> can't get rid of the #ifdef.  Probably better to use a pointer if we
> don't mind adding a few bytes in the #ifndef case.  Thanks,

I've been back and forth a few types on this, and I've convinced
myself that allowing the group structure to be embedded is a better
idea.  It's a particular help when you need to construct one from
platform or bridge init code that runs before mem_init_done.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson