Re: [PATCH 1/2] vfio powerpc: implemented IOMMU driver for VFIO

2012-11-26 Thread David Gibson
On Tue, Nov 27, 2012 at 03:58:14PM +1100, Alexey Kardashevskiy wrote:
> On 27/11/12 15:29, Alex Williamson wrote:
> >On Tue, 2012-11-27 at 15:06 +1100, Alexey Kardashevskiy wrote:
> >>On 27/11/12 05:20, Alex Williamson wrote:
> >>>On Fri, 2012-11-23 at 20:03 +1100, Alexey Kardashevskiy wrote:
> VFIO implements platform independent stuff such as
> a PCI driver, BAR access (via read/write on a file descriptor
> or direct mapping when possible) and IRQ signaling.
> 
> The platform dependent part includes IOMMU initialization
> and handling. This patch implements an IOMMU driver for VFIO
> which does mapping/unmapping pages for the guest IO and
> provides information about DMA window (required by a POWERPC
> guest).
> 
> The counterpart in QEMU is required to support this functionality.
> 
> Cc: David Gibson 
> Signed-off-by: Alexey Kardashevskiy 
> ---
>    drivers/vfio/Kconfig|6 +
>    drivers/vfio/Makefile   |1 +
>    drivers/vfio/vfio_iommu_spapr_tce.c |  247 
>  +++
>    include/linux/vfio.h|   20 +++
>    4 files changed, 274 insertions(+)
>    create mode 100644 drivers/vfio/vfio_iommu_spapr_tce.c
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 7cd5dec..b464687 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -3,10 +3,16 @@ config VFIO_IOMMU_TYPE1
>   depends on VFIO
>   default n
> 
> +config VFIO_IOMMU_SPAPR_TCE
> + tristate
> + depends on VFIO && SPAPR_TCE_IOMMU
> + default n
> +
>    menuconfig VFIO
>   tristate "VFIO Non-Privileged userspace driver framework"
>   depends on IOMMU_API
>   select VFIO_IOMMU_TYPE1 if X86
> + select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
>   help
> VFIO provides a framework for secure userspace device drivers.
> See Documentation/vfio.txt for more details.
> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> index 2398d4a..72bfabc 100644
> --- a/drivers/vfio/Makefile
> +++ b/drivers/vfio/Makefile
> @@ -1,3 +1,4 @@
>    obj-$(CONFIG_VFIO) += vfio.o
>    obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
> +obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
>    obj-$(CONFIG_VFIO_PCI) += pci/
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
> b/drivers/vfio/vfio_iommu_spapr_tce.c
> new file mode 100644
> index 000..46a6298
> --- /dev/null
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -0,0 +1,247 @@
> +/*
> + * VFIO: IOMMU DMA mapping support for TCE on POWER
> + *
> + * Copyright (C) 2012 IBM Corp.  All rights reserved.
> + * Author: Alexey Kardashevskiy 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Derived from original vfio_iommu_type1.c:
> + * Copyright (C) 2012 Red Hat, Inc.  All rights reserved.
> + * Author: Alex Williamson 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "a...@ozlabs.ru"
> +#define DRIVER_DESC "VFIO IOMMU SPAPR TCE"
> +
> +static void tce_iommu_detach_group(void *iommu_data,
> + struct iommu_group *iommu_group);
> +
> +/*
> + * VFIO IOMMU fd for SPAPR_TCE IOMMU implementation
> + */
> +
> +/*
> + * The container descriptor supports only a single group per container.
> + * Required by the API as the container is not supplied with the IOMMU 
> group
> + * at the moment of initialization.
> + */
> +struct tce_container {
> + struct mutex lock;
> + struct iommu_table *tbl;
> +};
> +
> +static void *tce_iommu_open(unsigned long arg)
> +{
> + struct tce_container *container;
> +
> + if (arg != VFIO_SPAPR_TCE_IOMMU) {
> + printk(KERN_ERR "tce_vfio: Wrong IOMMU type\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + container = kzalloc(sizeof(*container), GFP_KERNEL);
> + if (!container)
> + return ERR_PTR(-ENOMEM);
> +
> + mutex_init(&container->lock);
> +
> + return container;
> +}
> +
> +static void tce_iommu_release(void *iommu_data)
> +{
> + struct tce_container *container = iommu_data;
> +
> + WARN_ON(container->tbl && !container->tbl->it_group);
> >>>
> >>>I think your patch ordering is backwards here.  it_group isn't added
> >>>until 2/2.  I'd really like to see the arch/powerpc code approved and
> >

Re: [PATCH 1/2] vfio powerpc: implemented IOMMU driver for VFIO

2012-11-26 Thread Alex Williamson
On Tue, 2012-11-27 at 15:58 +1100, Alexey Kardashevskiy wrote:
> On 27/11/12 15:29, Alex Williamson wrote:
> > On Tue, 2012-11-27 at 15:06 +1100, Alexey Kardashevskiy wrote:
> >> On 27/11/12 05:20, Alex Williamson wrote:
> >>> On Fri, 2012-11-23 at 20:03 +1100, Alexey Kardashevskiy wrote:
>  VFIO implements platform independent stuff such as
>  a PCI driver, BAR access (via read/write on a file descriptor
>  or direct mapping when possible) and IRQ signaling.
> 
>  The platform dependent part includes IOMMU initialization
>  and handling. This patch implements an IOMMU driver for VFIO
>  which does mapping/unmapping pages for the guest IO and
>  provides information about DMA window (required by a POWERPC
>  guest).
> 
>  The counterpart in QEMU is required to support this functionality.
> 
>  Cc: David Gibson 
>  Signed-off-by: Alexey Kardashevskiy 
>  ---
> drivers/vfio/Kconfig|6 +
> drivers/vfio/Makefile   |1 +
> drivers/vfio/vfio_iommu_spapr_tce.c |  247 
>  +++
> include/linux/vfio.h|   20 +++
> 4 files changed, 274 insertions(+)
> create mode 100644 drivers/vfio/vfio_iommu_spapr_tce.c
> 
>  diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
>  index 7cd5dec..b464687 100644
>  --- a/drivers/vfio/Kconfig
>  +++ b/drivers/vfio/Kconfig
>  @@ -3,10 +3,16 @@ config VFIO_IOMMU_TYPE1
>   depends on VFIO
>   default n
> 
>  +config VFIO_IOMMU_SPAPR_TCE
>  +tristate
>  +depends on VFIO && SPAPR_TCE_IOMMU
>  +default n
>  +
> menuconfig VFIO
>   tristate "VFIO Non-Privileged userspace driver framework"
>   depends on IOMMU_API
>   select VFIO_IOMMU_TYPE1 if X86
>  +select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
>   help
> VFIO provides a framework for secure userspace device drivers.
> See Documentation/vfio.txt for more details.
>  diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
>  index 2398d4a..72bfabc 100644
>  --- a/drivers/vfio/Makefile
>  +++ b/drivers/vfio/Makefile
>  @@ -1,3 +1,4 @@
> obj-$(CONFIG_VFIO) += vfio.o
> obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
>  +obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
> obj-$(CONFIG_VFIO_PCI) += pci/
>  diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
>  b/drivers/vfio/vfio_iommu_spapr_tce.c
>  new file mode 100644
>  index 000..46a6298
>  --- /dev/null
>  +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
>  @@ -0,0 +1,247 @@
>  +/*
>  + * VFIO: IOMMU DMA mapping support for TCE on POWER
>  + *
>  + * Copyright (C) 2012 IBM Corp.  All rights reserved.
>  + * Author: Alexey Kardashevskiy 
>  + *
>  + * This program is free software; you can redistribute it and/or modify
>  + * it under the terms of the GNU General Public License version 2 as
>  + * published by the Free Software Foundation.
>  + *
>  + * Derived from original vfio_iommu_type1.c:
>  + * Copyright (C) 2012 Red Hat, Inc.  All rights reserved.
>  + * Author: Alex Williamson 
>  + */
>  +
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +
>  +#define DRIVER_VERSION  "0.1"
>  +#define DRIVER_AUTHOR   "a...@ozlabs.ru"
>  +#define DRIVER_DESC "VFIO IOMMU SPAPR TCE"
>  +
>  +static void tce_iommu_detach_group(void *iommu_data,
>  +struct iommu_group *iommu_group);
>  +
>  +/*
>  + * VFIO IOMMU fd for SPAPR_TCE IOMMU implementation
>  + */
>  +
>  +/*
>  + * The container descriptor supports only a single group per container.
>  + * Required by the API as the container is not supplied with the IOMMU 
>  group
>  + * at the moment of initialization.
>  + */
>  +struct tce_container {
>  +struct mutex lock;
>  +struct iommu_table *tbl;
>  +};
>  +
>  +static void *tce_iommu_open(unsigned long arg)
>  +{
>  +struct tce_container *container;
>  +
>  +if (arg != VFIO_SPAPR_TCE_IOMMU) {
>  +printk(KERN_ERR "tce_vfio: Wrong IOMMU type\n");
>  +return ERR_PTR(-EINVAL);
>  +}
>  +
>  +container = kzalloc(sizeof(*container), GFP_KERNEL);
>  +if (!container)
>  +return ERR_PTR(-ENOMEM);
>  +
>  +mutex_init(&container->lock);
>  +
>  +return container;
>  +}
>  +
>  +static void tce_iommu_release(void *iommu_data)
>  +{
>  +struct tce_container *container = iommu_

Re: [PATCH 1/2] vfio powerpc: implemented IOMMU driver for VFIO

2012-11-26 Thread Alex Williamson
On Tue, 2012-11-27 at 15:06 +1100, Alexey Kardashevskiy wrote:
> On 27/11/12 05:20, Alex Williamson wrote:
> > On Fri, 2012-11-23 at 20:03 +1100, Alexey Kardashevskiy wrote:
> >> VFIO implements platform independent stuff such as
> >> a PCI driver, BAR access (via read/write on a file descriptor
> >> or direct mapping when possible) and IRQ signaling.
> >>
> >> The platform dependent part includes IOMMU initialization
> >> and handling. This patch implements an IOMMU driver for VFIO
> >> which does mapping/unmapping pages for the guest IO and
> >> provides information about DMA window (required by a POWERPC
> >> guest).
> >>
> >> The counterpart in QEMU is required to support this functionality.
> >>
> >> Cc: David Gibson 
> >> Signed-off-by: Alexey Kardashevskiy 
> >> ---
> >>   drivers/vfio/Kconfig|6 +
> >>   drivers/vfio/Makefile   |1 +
> >>   drivers/vfio/vfio_iommu_spapr_tce.c |  247 
> >> +++
> >>   include/linux/vfio.h|   20 +++
> >>   4 files changed, 274 insertions(+)
> >>   create mode 100644 drivers/vfio/vfio_iommu_spapr_tce.c
> >>
> >> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> >> index 7cd5dec..b464687 100644
> >> --- a/drivers/vfio/Kconfig
> >> +++ b/drivers/vfio/Kconfig
> >> @@ -3,10 +3,16 @@ config VFIO_IOMMU_TYPE1
> >>depends on VFIO
> >>default n
> >>
> >> +config VFIO_IOMMU_SPAPR_TCE
> >> +  tristate
> >> +  depends on VFIO && SPAPR_TCE_IOMMU
> >> +  default n
> >> +
> >>   menuconfig VFIO
> >>tristate "VFIO Non-Privileged userspace driver framework"
> >>depends on IOMMU_API
> >>select VFIO_IOMMU_TYPE1 if X86
> >> +  select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
> >>help
> >>  VFIO provides a framework for secure userspace device drivers.
> >>  See Documentation/vfio.txt for more details.
> >> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> >> index 2398d4a..72bfabc 100644
> >> --- a/drivers/vfio/Makefile
> >> +++ b/drivers/vfio/Makefile
> >> @@ -1,3 +1,4 @@
> >>   obj-$(CONFIG_VFIO) += vfio.o
> >>   obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
> >> +obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
> >>   obj-$(CONFIG_VFIO_PCI) += pci/
> >> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
> >> b/drivers/vfio/vfio_iommu_spapr_tce.c
> >> new file mode 100644
> >> index 000..46a6298
> >> --- /dev/null
> >> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> >> @@ -0,0 +1,247 @@
> >> +/*
> >> + * VFIO: IOMMU DMA mapping support for TCE on POWER
> >> + *
> >> + * Copyright (C) 2012 IBM Corp.  All rights reserved.
> >> + * Author: Alexey Kardashevskiy 
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + *
> >> + * Derived from original vfio_iommu_type1.c:
> >> + * Copyright (C) 2012 Red Hat, Inc.  All rights reserved.
> >> + * Author: Alex Williamson 
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#define DRIVER_VERSION  "0.1"
> >> +#define DRIVER_AUTHOR   "a...@ozlabs.ru"
> >> +#define DRIVER_DESC "VFIO IOMMU SPAPR TCE"
> >> +
> >> +static void tce_iommu_detach_group(void *iommu_data,
> >> +  struct iommu_group *iommu_group);
> >> +
> >> +/*
> >> + * VFIO IOMMU fd for SPAPR_TCE IOMMU implementation
> >> + */
> >> +
> >> +/*
> >> + * The container descriptor supports only a single group per container.
> >> + * Required by the API as the container is not supplied with the IOMMU 
> >> group
> >> + * at the moment of initialization.
> >> + */
> >> +struct tce_container {
> >> +  struct mutex lock;
> >> +  struct iommu_table *tbl;
> >> +};
> >> +
> >> +static void *tce_iommu_open(unsigned long arg)
> >> +{
> >> +  struct tce_container *container;
> >> +
> >> +  if (arg != VFIO_SPAPR_TCE_IOMMU) {
> >> +  printk(KERN_ERR "tce_vfio: Wrong IOMMU type\n");
> >> +  return ERR_PTR(-EINVAL);
> >> +  }
> >> +
> >> +  container = kzalloc(sizeof(*container), GFP_KERNEL);
> >> +  if (!container)
> >> +  return ERR_PTR(-ENOMEM);
> >> +
> >> +  mutex_init(&container->lock);
> >> +
> >> +  return container;
> >> +}
> >> +
> >> +static void tce_iommu_release(void *iommu_data)
> >> +{
> >> +  struct tce_container *container = iommu_data;
> >> +
> >> +  WARN_ON(container->tbl && !container->tbl->it_group);
> >
> > I think your patch ordering is backwards here.  it_group isn't added
> > until 2/2.  I'd really like to see the arch/powerpc code approved and
> > merged by the powerpc maintainer before we add the code that makes use
> > of it into vfio.  Otherwise we just get lots of churn if interfaces
> > change or they disapprove of it altogether.
> 
> 
> Makes sense, thanks.
> 
> 
> >> +  if (container->tbl && container->tbl->it_group)
> >> +  tce_io

Re: [PATCH 1/2] vfio powerpc: implemented IOMMU driver for VFIO

2012-11-26 Thread Alexey Kardashevskiy

On 27/11/12 15:29, Alex Williamson wrote:

On Tue, 2012-11-27 at 15:06 +1100, Alexey Kardashevskiy wrote:

On 27/11/12 05:20, Alex Williamson wrote:

On Fri, 2012-11-23 at 20:03 +1100, Alexey Kardashevskiy wrote:

VFIO implements platform independent stuff such as
a PCI driver, BAR access (via read/write on a file descriptor
or direct mapping when possible) and IRQ signaling.

The platform dependent part includes IOMMU initialization
and handling. This patch implements an IOMMU driver for VFIO
which does mapping/unmapping pages for the guest IO and
provides information about DMA window (required by a POWERPC
guest).

The counterpart in QEMU is required to support this functionality.

Cc: David Gibson 
Signed-off-by: Alexey Kardashevskiy 
---
   drivers/vfio/Kconfig|6 +
   drivers/vfio/Makefile   |1 +
   drivers/vfio/vfio_iommu_spapr_tce.c |  247 
+++
   include/linux/vfio.h|   20 +++
   4 files changed, 274 insertions(+)
   create mode 100644 drivers/vfio/vfio_iommu_spapr_tce.c

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 7cd5dec..b464687 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -3,10 +3,16 @@ config VFIO_IOMMU_TYPE1
depends on VFIO
default n

+config VFIO_IOMMU_SPAPR_TCE
+   tristate
+   depends on VFIO && SPAPR_TCE_IOMMU
+   default n
+
   menuconfig VFIO
tristate "VFIO Non-Privileged userspace driver framework"
depends on IOMMU_API
select VFIO_IOMMU_TYPE1 if X86
+   select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
help
  VFIO provides a framework for secure userspace device drivers.
  See Documentation/vfio.txt for more details.
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 2398d4a..72bfabc 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,3 +1,4 @@
   obj-$(CONFIG_VFIO) += vfio.o
   obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
+obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
   obj-$(CONFIG_VFIO_PCI) += pci/
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
b/drivers/vfio/vfio_iommu_spapr_tce.c
new file mode 100644
index 000..46a6298
--- /dev/null
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -0,0 +1,247 @@
+/*
+ * VFIO: IOMMU DMA mapping support for TCE on POWER
+ *
+ * Copyright (C) 2012 IBM Corp.  All rights reserved.
+ * Author: Alexey Kardashevskiy 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Derived from original vfio_iommu_type1.c:
+ * Copyright (C) 2012 Red Hat, Inc.  All rights reserved.
+ * Author: Alex Williamson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRIVER_VERSION  "0.1"
+#define DRIVER_AUTHOR   "a...@ozlabs.ru"
+#define DRIVER_DESC "VFIO IOMMU SPAPR TCE"
+
+static void tce_iommu_detach_group(void *iommu_data,
+   struct iommu_group *iommu_group);
+
+/*
+ * VFIO IOMMU fd for SPAPR_TCE IOMMU implementation
+ */
+
+/*
+ * The container descriptor supports only a single group per container.
+ * Required by the API as the container is not supplied with the IOMMU group
+ * at the moment of initialization.
+ */
+struct tce_container {
+   struct mutex lock;
+   struct iommu_table *tbl;
+};
+
+static void *tce_iommu_open(unsigned long arg)
+{
+   struct tce_container *container;
+
+   if (arg != VFIO_SPAPR_TCE_IOMMU) {
+   printk(KERN_ERR "tce_vfio: Wrong IOMMU type\n");
+   return ERR_PTR(-EINVAL);
+   }
+
+   container = kzalloc(sizeof(*container), GFP_KERNEL);
+   if (!container)
+   return ERR_PTR(-ENOMEM);
+
+   mutex_init(&container->lock);
+
+   return container;
+}
+
+static void tce_iommu_release(void *iommu_data)
+{
+   struct tce_container *container = iommu_data;
+
+   WARN_ON(container->tbl && !container->tbl->it_group);


I think your patch ordering is backwards here.  it_group isn't added
until 2/2.  I'd really like to see the arch/powerpc code approved and
merged by the powerpc maintainer before we add the code that makes use
of it into vfio.  Otherwise we just get lots of churn if interfaces
change or they disapprove of it altogether.



Makes sense, thanks.



+   if (container->tbl && container->tbl->it_group)
+   tce_iommu_detach_group(iommu_data, container->tbl->it_group);
+
+   mutex_destroy(&container->lock);
+
+   kfree(container);
+}
+
+static long tce_iommu_ioctl(void *iommu_data,
+unsigned int cmd, unsigned long arg)
+{
+   struct tce_container *container = iommu_data;
+   unsigned long minsz;
+
+   switch (cmd) {
+   case VFIO_CHECK_EXTENSION: {
+   return (arg == VFIO_SPAPR_TCE_IOMMU) ? 1 : 0;
+   }
+   cas

Re: [PATCH 1/2] vfio powerpc: implemented IOMMU driver for VFIO

2012-11-26 Thread Alexey Kardashevskiy

On 27/11/12 05:20, Alex Williamson wrote:

On Fri, 2012-11-23 at 20:03 +1100, Alexey Kardashevskiy wrote:

VFIO implements platform independent stuff such as
a PCI driver, BAR access (via read/write on a file descriptor
or direct mapping when possible) and IRQ signaling.

The platform dependent part includes IOMMU initialization
and handling. This patch implements an IOMMU driver for VFIO
which does mapping/unmapping pages for the guest IO and
provides information about DMA window (required by a POWERPC
guest).

The counterpart in QEMU is required to support this functionality.

Cc: David Gibson 
Signed-off-by: Alexey Kardashevskiy 
---
  drivers/vfio/Kconfig|6 +
  drivers/vfio/Makefile   |1 +
  drivers/vfio/vfio_iommu_spapr_tce.c |  247 +++
  include/linux/vfio.h|   20 +++
  4 files changed, 274 insertions(+)
  create mode 100644 drivers/vfio/vfio_iommu_spapr_tce.c

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 7cd5dec..b464687 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -3,10 +3,16 @@ config VFIO_IOMMU_TYPE1
depends on VFIO
default n

+config VFIO_IOMMU_SPAPR_TCE
+   tristate
+   depends on VFIO && SPAPR_TCE_IOMMU
+   default n
+
  menuconfig VFIO
tristate "VFIO Non-Privileged userspace driver framework"
depends on IOMMU_API
select VFIO_IOMMU_TYPE1 if X86
+   select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
help
  VFIO provides a framework for secure userspace device drivers.
  See Documentation/vfio.txt for more details.
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 2398d4a..72bfabc 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,3 +1,4 @@
  obj-$(CONFIG_VFIO) += vfio.o
  obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
+obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
  obj-$(CONFIG_VFIO_PCI) += pci/
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
b/drivers/vfio/vfio_iommu_spapr_tce.c
new file mode 100644
index 000..46a6298
--- /dev/null
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -0,0 +1,247 @@
+/*
+ * VFIO: IOMMU DMA mapping support for TCE on POWER
+ *
+ * Copyright (C) 2012 IBM Corp.  All rights reserved.
+ * Author: Alexey Kardashevskiy 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Derived from original vfio_iommu_type1.c:
+ * Copyright (C) 2012 Red Hat, Inc.  All rights reserved.
+ * Author: Alex Williamson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRIVER_VERSION  "0.1"
+#define DRIVER_AUTHOR   "a...@ozlabs.ru"
+#define DRIVER_DESC "VFIO IOMMU SPAPR TCE"
+
+static void tce_iommu_detach_group(void *iommu_data,
+   struct iommu_group *iommu_group);
+
+/*
+ * VFIO IOMMU fd for SPAPR_TCE IOMMU implementation
+ */
+
+/*
+ * The container descriptor supports only a single group per container.
+ * Required by the API as the container is not supplied with the IOMMU group
+ * at the moment of initialization.
+ */
+struct tce_container {
+   struct mutex lock;
+   struct iommu_table *tbl;
+};
+
+static void *tce_iommu_open(unsigned long arg)
+{
+   struct tce_container *container;
+
+   if (arg != VFIO_SPAPR_TCE_IOMMU) {
+   printk(KERN_ERR "tce_vfio: Wrong IOMMU type\n");
+   return ERR_PTR(-EINVAL);
+   }
+
+   container = kzalloc(sizeof(*container), GFP_KERNEL);
+   if (!container)
+   return ERR_PTR(-ENOMEM);
+
+   mutex_init(&container->lock);
+
+   return container;
+}
+
+static void tce_iommu_release(void *iommu_data)
+{
+   struct tce_container *container = iommu_data;
+
+   WARN_ON(container->tbl && !container->tbl->it_group);


I think your patch ordering is backwards here.  it_group isn't added
until 2/2.  I'd really like to see the arch/powerpc code approved and
merged by the powerpc maintainer before we add the code that makes use
of it into vfio.  Otherwise we just get lots of churn if interfaces
change or they disapprove of it altogether.



Makes sense, thanks.



+   if (container->tbl && container->tbl->it_group)
+   tce_iommu_detach_group(iommu_data, container->tbl->it_group);
+
+   mutex_destroy(&container->lock);
+
+   kfree(container);
+}
+
+static long tce_iommu_ioctl(void *iommu_data,
+unsigned int cmd, unsigned long arg)
+{
+   struct tce_container *container = iommu_data;
+   unsigned long minsz;
+
+   switch (cmd) {
+   case VFIO_CHECK_EXTENSION: {
+   return (arg == VFIO_SPAPR_TCE_IOMMU) ? 1 : 0;
+   }
+   case VFIO_IOMMU_SPAPR_TCE_GET_INFO: {
+   struct vfio_iommu_spapr_tce_info info;
+   struct iommu

Re: [PATCH 1/2] vfio powerpc: implemented IOMMU driver for VFIO

2012-11-26 Thread Alex Williamson
On Fri, 2012-11-23 at 20:03 +1100, Alexey Kardashevskiy wrote:
> VFIO implements platform independent stuff such as
> a PCI driver, BAR access (via read/write on a file descriptor
> or direct mapping when possible) and IRQ signaling.
> 
> The platform dependent part includes IOMMU initialization
> and handling. This patch implements an IOMMU driver for VFIO
> which does mapping/unmapping pages for the guest IO and
> provides information about DMA window (required by a POWERPC
> guest).
> 
> The counterpart in QEMU is required to support this functionality.
> 
> Cc: David Gibson 
> Signed-off-by: Alexey Kardashevskiy 
> ---
>  drivers/vfio/Kconfig|6 +
>  drivers/vfio/Makefile   |1 +
>  drivers/vfio/vfio_iommu_spapr_tce.c |  247 
> +++
>  include/linux/vfio.h|   20 +++
>  4 files changed, 274 insertions(+)
>  create mode 100644 drivers/vfio/vfio_iommu_spapr_tce.c
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 7cd5dec..b464687 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -3,10 +3,16 @@ config VFIO_IOMMU_TYPE1
>   depends on VFIO
>   default n
>  
> +config VFIO_IOMMU_SPAPR_TCE
> + tristate
> + depends on VFIO && SPAPR_TCE_IOMMU
> + default n
> +
>  menuconfig VFIO
>   tristate "VFIO Non-Privileged userspace driver framework"
>   depends on IOMMU_API
>   select VFIO_IOMMU_TYPE1 if X86
> + select VFIO_IOMMU_SPAPR_TCE if PPC_POWERNV
>   help
> VFIO provides a framework for secure userspace device drivers.
> See Documentation/vfio.txt for more details.
> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> index 2398d4a..72bfabc 100644
> --- a/drivers/vfio/Makefile
> +++ b/drivers/vfio/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_VFIO) += vfio.o
>  obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
> +obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
>  obj-$(CONFIG_VFIO_PCI) += pci/
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
> b/drivers/vfio/vfio_iommu_spapr_tce.c
> new file mode 100644
> index 000..46a6298
> --- /dev/null
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -0,0 +1,247 @@
> +/*
> + * VFIO: IOMMU DMA mapping support for TCE on POWER
> + *
> + * Copyright (C) 2012 IBM Corp.  All rights reserved.
> + * Author: Alexey Kardashevskiy 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Derived from original vfio_iommu_type1.c:
> + * Copyright (C) 2012 Red Hat, Inc.  All rights reserved.
> + * Author: Alex Williamson 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "a...@ozlabs.ru"
> +#define DRIVER_DESC "VFIO IOMMU SPAPR TCE"
> +
> +static void tce_iommu_detach_group(void *iommu_data,
> + struct iommu_group *iommu_group);
> +
> +/*
> + * VFIO IOMMU fd for SPAPR_TCE IOMMU implementation
> + */
> +
> +/*
> + * The container descriptor supports only a single group per container.
> + * Required by the API as the container is not supplied with the IOMMU group
> + * at the moment of initialization.
> + */
> +struct tce_container {
> + struct mutex lock;
> + struct iommu_table *tbl;
> +};
> +
> +static void *tce_iommu_open(unsigned long arg)
> +{
> + struct tce_container *container;
> +
> + if (arg != VFIO_SPAPR_TCE_IOMMU) {
> + printk(KERN_ERR "tce_vfio: Wrong IOMMU type\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + container = kzalloc(sizeof(*container), GFP_KERNEL);
> + if (!container)
> + return ERR_PTR(-ENOMEM);
> +
> + mutex_init(&container->lock);
> +
> + return container;
> +}
> +
> +static void tce_iommu_release(void *iommu_data)
> +{
> + struct tce_container *container = iommu_data;
> +
> + WARN_ON(container->tbl && !container->tbl->it_group);

I think your patch ordering is backwards here.  it_group isn't added
until 2/2.  I'd really like to see the arch/powerpc code approved and
merged by the powerpc maintainer before we add the code that makes use
of it into vfio.  Otherwise we just get lots of churn if interfaces
change or they disapprove of it altogether.

> + if (container->tbl && container->tbl->it_group)
> + tce_iommu_detach_group(iommu_data, container->tbl->it_group);
> +
> + mutex_destroy(&container->lock);
> +
> + kfree(container);
> +}
> +
> +static long tce_iommu_ioctl(void *iommu_data,
> +  unsigned int cmd, unsigned long arg)
> +{
> + struct tce_container *container = iommu_data;
> + unsigned long minsz;
> +
> + switch (cmd) {
> + case VFIO_CHECK_EXTENSION: {
> + return (arg == VFIO_SPAPR_TCE_IOMMU) ? 1 : 0;
> +