Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 11/16] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)

2016-03-21 Thread Alexey Kardashevskiy 

On 03/21/2016 03:53 PM, David Gibson wrote:

On Thu, Mar 17, 2016 at 08:23:35PM +1100, Alexey Kardashevskiy wrote:

On 03/17/2016 05:10 PM, David Gibson wrote:

On Thu, Mar 17, 2016 at 04:04:29PM +1100, Alexey Kardashevskiy wrote:

On 03/15/2016 04:42 PM, David Gibson wrote:

On Tue, Mar 15, 2016 at 01:53:48PM +1100, Alexey Kardashevskiy wrote:

On 03/03/2016 05:30 PM, David Gibson wrote:

On Tue, Mar 01, 2016 at 08:10:36PM +1100, Alexey Kardashevskiy wrote:

This makes use of the new "memory registering" feature. The idea is
to provide the userspace ability to notify the host kernel about pages
which are going to be used for DMA. Having this information, the host
kernel can pin them all once per user process, do locked pages
accounting (once) and not spent time on doing that in real time with
possible failures which cannot be handled nicely in some cases.

This adds a prereg memory listener which listens on address_space_memory
and notifies a VFIO container about memory which needs to be
pinned/unpinned. VFIO MMIO regions (i.e. "skip dump" regions) are skipped.

As there is no per-IOMMU-type release() callback anymore, this stores
the IOMMU type in the container so vfio_listener_release() can device
if it needs to unregister @prereg_listener.

The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
not call it when v2 is detected and enabled.

This does not change the guest visible interface.

Signed-off-by: Alexey Kardashevskiy 
---
  hw/vfio/Makefile.objs |   1 +
  hw/vfio/common.c  |  39 +---
  hw/vfio/prereg.c  | 138 ++
  include/hw/vfio/vfio-common.h |   4 ++
  trace-events  |   2 +
  5 files changed, 175 insertions(+), 9 deletions(-)
  create mode 100644 hw/vfio/prereg.c

diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index ceddbb8..5800e0e 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -4,4 +4,5 @@ obj-$(CONFIG_PCI) += pci.o pci-quirks.o
  obj-$(CONFIG_SOFTMMU) += platform.o
  obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
  obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
+obj-$(CONFIG_SOFTMMU) += prereg.o
  endif
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 3aaa6b5..f2a03e0 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -531,6 +531,9 @@ static const MemoryListener vfio_iommu_listener = {
  static void vfio_listener_release(VFIOContainer *container)
  {
  memory_listener_unregister(>iommu_listener.listener);
+if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
+memory_listener_unregister(>prereg_listener.listener);
+}
  }

  int vfio_mmap_region(Object *obj, VFIORegion *region,
@@ -722,8 +725,8 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  goto free_container_exit;
  }

-ret = ioctl(fd, VFIO_SET_IOMMU,
-v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU);
+container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU;
+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
  if (ret) {
  error_report("vfio: failed to set iommu for container: %m");
  ret = -errno;
@@ -748,8 +751,10 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
  container->iova_pgsizes = info.iova_pgsizes;
  }
-} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
+} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
+   ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
  struct vfio_iommu_spapr_tce_info info;
+bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);

  ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
  if (ret) {
@@ -757,7 +762,9 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  ret = -errno;
  goto free_container_exit;
  }
-ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
+container->iommu_type =
+v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);


It'd be nice to consolidate the setting of container->iommu_type and
then the SET_IOMMU ioctl() rather than having more or less duplicated
logic for Type1 and SPAPR, but it's not a big deal.



May be but I cannot think of any nice way of doing this though.





  if (ret) {
  error_report("vfio: failed to set iommu for container: %m");
  ret = -errno;
@@ -769,11 +776,25 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
   * when container fd is closed so we do not call it explicitly
   * in this file.
   */
-ret = ioctl(fd,

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 11/16] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)

2016-03-20 Thread David Gibson 
On Thu, Mar 17, 2016 at 08:23:35PM +1100, Alexey Kardashevskiy wrote:
> On 03/17/2016 05:10 PM, David Gibson wrote:
> >On Thu, Mar 17, 2016 at 04:04:29PM +1100, Alexey Kardashevskiy wrote:
> >>On 03/15/2016 04:42 PM, David Gibson wrote:
> >>>On Tue, Mar 15, 2016 at 01:53:48PM +1100, Alexey Kardashevskiy wrote:
> On 03/03/2016 05:30 PM, David Gibson wrote:
> >On Tue, Mar 01, 2016 at 08:10:36PM +1100, Alexey Kardashevskiy wrote:
> >>This makes use of the new "memory registering" feature. The idea is
> >>to provide the userspace ability to notify the host kernel about pages
> >>which are going to be used for DMA. Having this information, the host
> >>kernel can pin them all once per user process, do locked pages
> >>accounting (once) and not spent time on doing that in real time with
> >>possible failures which cannot be handled nicely in some cases.
> >>
> >>This adds a prereg memory listener which listens on address_space_memory
> >>and notifies a VFIO container about memory which needs to be
> >>pinned/unpinned. VFIO MMIO regions (i.e. "skip dump" regions) are 
> >>skipped.
> >>
> >>As there is no per-IOMMU-type release() callback anymore, this stores
> >>the IOMMU type in the container so vfio_listener_release() can device
> >>if it needs to unregister @prereg_listener.
> >>
> >>The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
> >>are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this 
> >>does
> >>not call it when v2 is detected and enabled.
> >>
> >>This does not change the guest visible interface.
> >>
> >>Signed-off-by: Alexey Kardashevskiy 
> >>---
> >>  hw/vfio/Makefile.objs |   1 +
> >>  hw/vfio/common.c  |  39 +---
> >>  hw/vfio/prereg.c  | 138 
> >> ++
> >>  include/hw/vfio/vfio-common.h |   4 ++
> >>  trace-events  |   2 +
> >>  5 files changed, 175 insertions(+), 9 deletions(-)
> >>  create mode 100644 hw/vfio/prereg.c
> >>
> >>diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> >>index ceddbb8..5800e0e 100644
> >>--- a/hw/vfio/Makefile.objs
> >>+++ b/hw/vfio/Makefile.objs
> >>@@ -4,4 +4,5 @@ obj-$(CONFIG_PCI) += pci.o pci-quirks.o
> >>  obj-$(CONFIG_SOFTMMU) += platform.o
> >>  obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
> >>  obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
> >>+obj-$(CONFIG_SOFTMMU) += prereg.o
> >>  endif
> >>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> >>index 3aaa6b5..f2a03e0 100644
> >>--- a/hw/vfio/common.c
> >>+++ b/hw/vfio/common.c
> >>@@ -531,6 +531,9 @@ static const MemoryListener vfio_iommu_listener = {
> >>  static void vfio_listener_release(VFIOContainer *container)
> >>  {
> >>  memory_listener_unregister(>iommu_listener.listener);
> >>+if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
> >>+
> >>memory_listener_unregister(>prereg_listener.listener);
> >>+}
> >>  }
> >>
> >>  int vfio_mmap_region(Object *obj, VFIORegion *region,
> >>@@ -722,8 +725,8 @@ static int vfio_connect_container(VFIOGroup *group, 
> >>AddressSpace *as)
> >>  goto free_container_exit;
> >>  }
> >>
> >>-ret = ioctl(fd, VFIO_SET_IOMMU,
> >>-v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU);
> >>+container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : 
> >>VFIO_TYPE1_IOMMU;
> >>+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
> >>  if (ret) {
> >>  error_report("vfio: failed to set iommu for container: 
> >> %m");
> >>  ret = -errno;
> >>@@ -748,8 +751,10 @@ static int vfio_connect_container(VFIOGroup 
> >>*group, AddressSpace *as)
> >>  if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
> >>  container->iova_pgsizes = info.iova_pgsizes;
> >>  }
> >>-} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
> >>+} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
> >>+   ioctl(fd, VFIO_CHECK_EXTENSION, 
> >>VFIO_SPAPR_TCE_v2_IOMMU)) {
> >>  struct vfio_iommu_spapr_tce_info info;
> >>+bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, 
> >>VFIO_SPAPR_TCE_v2_IOMMU);
> >>
> >>  ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
> >>  if (ret) {
> >>@@ -757,7 +762,9 @@ static int vfio_connect_container(VFIOGroup *group, 
> >>AddressSpace *as)
> >>  ret = -errno;
> >>  goto free_container_exit;
> >>  }
> >>-ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
> >>+container->iommu_type =
> >>+

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 11/16] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)

2016-03-19 Thread Alexey Kardashevskiy 

On 03/15/2016 04:42 PM, David Gibson wrote:

On Tue, Mar 15, 2016 at 01:53:48PM +1100, Alexey Kardashevskiy wrote:

On 03/03/2016 05:30 PM, David Gibson wrote:

On Tue, Mar 01, 2016 at 08:10:36PM +1100, Alexey Kardashevskiy wrote:

This makes use of the new "memory registering" feature. The idea is
to provide the userspace ability to notify the host kernel about pages
which are going to be used for DMA. Having this information, the host
kernel can pin them all once per user process, do locked pages
accounting (once) and not spent time on doing that in real time with
possible failures which cannot be handled nicely in some cases.

This adds a prereg memory listener which listens on address_space_memory
and notifies a VFIO container about memory which needs to be
pinned/unpinned. VFIO MMIO regions (i.e. "skip dump" regions) are skipped.

As there is no per-IOMMU-type release() callback anymore, this stores
the IOMMU type in the container so vfio_listener_release() can device
if it needs to unregister @prereg_listener.

The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
not call it when v2 is detected and enabled.

This does not change the guest visible interface.

Signed-off-by: Alexey Kardashevskiy 
---
  hw/vfio/Makefile.objs |   1 +
  hw/vfio/common.c  |  39 +---
  hw/vfio/prereg.c  | 138 ++
  include/hw/vfio/vfio-common.h |   4 ++
  trace-events  |   2 +
  5 files changed, 175 insertions(+), 9 deletions(-)
  create mode 100644 hw/vfio/prereg.c

diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index ceddbb8..5800e0e 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -4,4 +4,5 @@ obj-$(CONFIG_PCI) += pci.o pci-quirks.o
  obj-$(CONFIG_SOFTMMU) += platform.o
  obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
  obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
+obj-$(CONFIG_SOFTMMU) += prereg.o
  endif
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 3aaa6b5..f2a03e0 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -531,6 +531,9 @@ static const MemoryListener vfio_iommu_listener = {
  static void vfio_listener_release(VFIOContainer *container)
  {
  memory_listener_unregister(>iommu_listener.listener);
+if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
+memory_listener_unregister(>prereg_listener.listener);
+}
  }

  int vfio_mmap_region(Object *obj, VFIORegion *region,
@@ -722,8 +725,8 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  goto free_container_exit;
  }

-ret = ioctl(fd, VFIO_SET_IOMMU,
-v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU);
+container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU;
+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
  if (ret) {
  error_report("vfio: failed to set iommu for container: %m");
  ret = -errno;
@@ -748,8 +751,10 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
  container->iova_pgsizes = info.iova_pgsizes;
  }
-} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
+} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
+   ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
  struct vfio_iommu_spapr_tce_info info;
+bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);

  ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
  if (ret) {
@@ -757,7 +762,9 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  ret = -errno;
  goto free_container_exit;
  }
-ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
+container->iommu_type =
+v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);


It'd be nice to consolidate the setting of container->iommu_type and
then the SET_IOMMU ioctl() rather than having more or less duplicated
logic for Type1 and SPAPR, but it's not a big deal.



May be but I cannot think of any nice way of doing this though.





  if (ret) {
  error_report("vfio: failed to set iommu for container: %m");
  ret = -errno;
@@ -769,11 +776,25 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
   * when container fd is closed so we do not call it explicitly
   * in this file.
   */
-ret = ioctl(fd, VFIO_IOMMU_ENABLE);
-if (ret) {
-error_report("vfio: failed to enable container: %m");
-ret = -errno;
-goto free_container_exit;
+if (!v2) {
+ret = ioctl(fd,

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 11/16] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)

2016-03-19 Thread David Gibson 
On Thu, Mar 17, 2016 at 04:04:29PM +1100, Alexey Kardashevskiy wrote:
> On 03/15/2016 04:42 PM, David Gibson wrote:
> >On Tue, Mar 15, 2016 at 01:53:48PM +1100, Alexey Kardashevskiy wrote:
> >>On 03/03/2016 05:30 PM, David Gibson wrote:
> >>>On Tue, Mar 01, 2016 at 08:10:36PM +1100, Alexey Kardashevskiy wrote:
> This makes use of the new "memory registering" feature. The idea is
> to provide the userspace ability to notify the host kernel about pages
> which are going to be used for DMA. Having this information, the host
> kernel can pin them all once per user process, do locked pages
> accounting (once) and not spent time on doing that in real time with
> possible failures which cannot be handled nicely in some cases.
> 
> This adds a prereg memory listener which listens on address_space_memory
> and notifies a VFIO container about memory which needs to be
> pinned/unpinned. VFIO MMIO regions (i.e. "skip dump" regions) are skipped.
> 
> As there is no per-IOMMU-type release() callback anymore, this stores
> the IOMMU type in the container so vfio_listener_release() can device
> if it needs to unregister @prereg_listener.
> 
> The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
> are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
> not call it when v2 is detected and enabled.
> 
> This does not change the guest visible interface.
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
>   hw/vfio/Makefile.objs |   1 +
>   hw/vfio/common.c  |  39 +---
>   hw/vfio/prereg.c  | 138 
>  ++
>   include/hw/vfio/vfio-common.h |   4 ++
>   trace-events  |   2 +
>   5 files changed, 175 insertions(+), 9 deletions(-)
>   create mode 100644 hw/vfio/prereg.c
> 
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> index ceddbb8..5800e0e 100644
> --- a/hw/vfio/Makefile.objs
> +++ b/hw/vfio/Makefile.objs
> @@ -4,4 +4,5 @@ obj-$(CONFIG_PCI) += pci.o pci-quirks.o
>   obj-$(CONFIG_SOFTMMU) += platform.o
>   obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
>   obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
> +obj-$(CONFIG_SOFTMMU) += prereg.o
>   endif
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 3aaa6b5..f2a03e0 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -531,6 +531,9 @@ static const MemoryListener vfio_iommu_listener = {
>   static void vfio_listener_release(VFIOContainer *container)
>   {
>   memory_listener_unregister(>iommu_listener.listener);
> +if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
> +memory_listener_unregister(>prereg_listener.listener);
> +}
>   }
> 
>   int vfio_mmap_region(Object *obj, VFIORegion *region,
> @@ -722,8 +725,8 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as)
>   goto free_container_exit;
>   }
> 
> -ret = ioctl(fd, VFIO_SET_IOMMU,
> -v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU);
> +container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : 
> VFIO_TYPE1_IOMMU;
> +ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>   if (ret) {
>   error_report("vfio: failed to set iommu for container: %m");
>   ret = -errno;
> @@ -748,8 +751,10 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as)
>   if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
>   container->iova_pgsizes = info.iova_pgsizes;
>   }
> -} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
> +} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
> +   ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) 
> {
>   struct vfio_iommu_spapr_tce_info info;
> +bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, 
> VFIO_SPAPR_TCE_v2_IOMMU);
> 
>   ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
>   if (ret) {
> @@ -757,7 +762,9 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as)
>   ret = -errno;
>   goto free_container_exit;
>   }
> -ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
> +container->iommu_type =
> +v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
> +ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
> >>>
> >>>It'd be nice to consolidate the setting of container->iommu_type and
> >>>then the SET_IOMMU ioctl() rather than having more or less duplicated
> >>>logic for Type1 and SPAPR, but it's not a big deal.
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 11/16] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)

2016-03-19 Thread Alexey Kardashevskiy 

On 03/17/2016 05:10 PM, David Gibson wrote:

On Thu, Mar 17, 2016 at 04:04:29PM +1100, Alexey Kardashevskiy wrote:

On 03/15/2016 04:42 PM, David Gibson wrote:

On Tue, Mar 15, 2016 at 01:53:48PM +1100, Alexey Kardashevskiy wrote:

On 03/03/2016 05:30 PM, David Gibson wrote:

On Tue, Mar 01, 2016 at 08:10:36PM +1100, Alexey Kardashevskiy wrote:

This makes use of the new "memory registering" feature. The idea is
to provide the userspace ability to notify the host kernel about pages
which are going to be used for DMA. Having this information, the host
kernel can pin them all once per user process, do locked pages
accounting (once) and not spent time on doing that in real time with
possible failures which cannot be handled nicely in some cases.

This adds a prereg memory listener which listens on address_space_memory
and notifies a VFIO container about memory which needs to be
pinned/unpinned. VFIO MMIO regions (i.e. "skip dump" regions) are skipped.

As there is no per-IOMMU-type release() callback anymore, this stores
the IOMMU type in the container so vfio_listener_release() can device
if it needs to unregister @prereg_listener.

The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
not call it when v2 is detected and enabled.

This does not change the guest visible interface.

Signed-off-by: Alexey Kardashevskiy 
---
  hw/vfio/Makefile.objs |   1 +
  hw/vfio/common.c  |  39 +---
  hw/vfio/prereg.c  | 138 ++
  include/hw/vfio/vfio-common.h |   4 ++
  trace-events  |   2 +
  5 files changed, 175 insertions(+), 9 deletions(-)
  create mode 100644 hw/vfio/prereg.c

diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index ceddbb8..5800e0e 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -4,4 +4,5 @@ obj-$(CONFIG_PCI) += pci.o pci-quirks.o
  obj-$(CONFIG_SOFTMMU) += platform.o
  obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
  obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
+obj-$(CONFIG_SOFTMMU) += prereg.o
  endif
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 3aaa6b5..f2a03e0 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -531,6 +531,9 @@ static const MemoryListener vfio_iommu_listener = {
  static void vfio_listener_release(VFIOContainer *container)
  {
  memory_listener_unregister(>iommu_listener.listener);
+if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
+memory_listener_unregister(>prereg_listener.listener);
+}
  }

  int vfio_mmap_region(Object *obj, VFIORegion *region,
@@ -722,8 +725,8 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  goto free_container_exit;
  }

-ret = ioctl(fd, VFIO_SET_IOMMU,
-v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU);
+container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU;
+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
  if (ret) {
  error_report("vfio: failed to set iommu for container: %m");
  ret = -errno;
@@ -748,8 +751,10 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
  container->iova_pgsizes = info.iova_pgsizes;
  }
-} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
+} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
+   ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
  struct vfio_iommu_spapr_tce_info info;
+bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);

  ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
  if (ret) {
@@ -757,7 +762,9 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  ret = -errno;
  goto free_container_exit;
  }
-ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
+container->iommu_type =
+v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);


It'd be nice to consolidate the setting of container->iommu_type and
then the SET_IOMMU ioctl() rather than having more or less duplicated
logic for Type1 and SPAPR, but it's not a big deal.



May be but I cannot think of any nice way of doing this though.





  if (ret) {
  error_report("vfio: failed to set iommu for container: %m");
  ret = -errno;
@@ -769,11 +776,25 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
   * when container fd is closed so we do not call it explicitly
   * in this file.
   */
-ret = ioctl(fd, VFIO_IOMMU_ENABLE);
-if (ret) {
-error_report("vfio: failed to enable container: %m");
-

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 11/16] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)

2016-03-15 Thread David Gibson 
On Tue, Mar 15, 2016 at 01:53:48PM +1100, Alexey Kardashevskiy wrote:
> On 03/03/2016 05:30 PM, David Gibson wrote:
> >On Tue, Mar 01, 2016 at 08:10:36PM +1100, Alexey Kardashevskiy wrote:
> >>This makes use of the new "memory registering" feature. The idea is
> >>to provide the userspace ability to notify the host kernel about pages
> >>which are going to be used for DMA. Having this information, the host
> >>kernel can pin them all once per user process, do locked pages
> >>accounting (once) and not spent time on doing that in real time with
> >>possible failures which cannot be handled nicely in some cases.
> >>
> >>This adds a prereg memory listener which listens on address_space_memory
> >>and notifies a VFIO container about memory which needs to be
> >>pinned/unpinned. VFIO MMIO regions (i.e. "skip dump" regions) are skipped.
> >>
> >>As there is no per-IOMMU-type release() callback anymore, this stores
> >>the IOMMU type in the container so vfio_listener_release() can device
> >>if it needs to unregister @prereg_listener.
> >>
> >>The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
> >>are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
> >>not call it when v2 is detected and enabled.
> >>
> >>This does not change the guest visible interface.
> >>
> >>Signed-off-by: Alexey Kardashevskiy 
> >>---
> >>  hw/vfio/Makefile.objs |   1 +
> >>  hw/vfio/common.c  |  39 +---
> >>  hw/vfio/prereg.c  | 138 
> >> ++
> >>  include/hw/vfio/vfio-common.h |   4 ++
> >>  trace-events  |   2 +
> >>  5 files changed, 175 insertions(+), 9 deletions(-)
> >>  create mode 100644 hw/vfio/prereg.c
> >>
> >>diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> >>index ceddbb8..5800e0e 100644
> >>--- a/hw/vfio/Makefile.objs
> >>+++ b/hw/vfio/Makefile.objs
> >>@@ -4,4 +4,5 @@ obj-$(CONFIG_PCI) += pci.o pci-quirks.o
> >>  obj-$(CONFIG_SOFTMMU) += platform.o
> >>  obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
> >>  obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
> >>+obj-$(CONFIG_SOFTMMU) += prereg.o
> >>  endif
> >>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> >>index 3aaa6b5..f2a03e0 100644
> >>--- a/hw/vfio/common.c
> >>+++ b/hw/vfio/common.c
> >>@@ -531,6 +531,9 @@ static const MemoryListener vfio_iommu_listener = {
> >>  static void vfio_listener_release(VFIOContainer *container)
> >>  {
> >>  memory_listener_unregister(>iommu_listener.listener);
> >>+if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
> >>+memory_listener_unregister(>prereg_listener.listener);
> >>+}
> >>  }
> >>
> >>  int vfio_mmap_region(Object *obj, VFIORegion *region,
> >>@@ -722,8 +725,8 @@ static int vfio_connect_container(VFIOGroup *group, 
> >>AddressSpace *as)
> >>  goto free_container_exit;
> >>  }
> >>
> >>-ret = ioctl(fd, VFIO_SET_IOMMU,
> >>-v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU);
> >>+container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU;
> >>+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
> >>  if (ret) {
> >>  error_report("vfio: failed to set iommu for container: %m");
> >>  ret = -errno;
> >>@@ -748,8 +751,10 @@ static int vfio_connect_container(VFIOGroup *group, 
> >>AddressSpace *as)
> >>  if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
> >>  container->iova_pgsizes = info.iova_pgsizes;
> >>  }
> >>-} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
> >>+} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
> >>+   ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
> >>  struct vfio_iommu_spapr_tce_info info;
> >>+bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, 
> >>VFIO_SPAPR_TCE_v2_IOMMU);
> >>
> >>  ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
> >>  if (ret) {
> >>@@ -757,7 +762,9 @@ static int vfio_connect_container(VFIOGroup *group, 
> >>AddressSpace *as)
> >>  ret = -errno;
> >>  goto free_container_exit;
> >>  }
> >>-ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
> >>+container->iommu_type =
> >>+v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
> >>+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
> >
> >It'd be nice to consolidate the setting of container->iommu_type and
> >then the SET_IOMMU ioctl() rather than having more or less duplicated
> >logic for Type1 and SPAPR, but it's not a big deal.
> 
> 
> May be but I cannot think of any nice way of doing this though.
> 
> 
> >
> >>  if (ret) {
> >>  error_report("vfio: failed to set iommu for container: %m");
> >>  ret = -errno;
> >>@@ -769,11 +776,25 @@ static int vfio_connect_container(VFIOGroup *group, 
> >>AddressSpace *as)
> >>

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 11/16] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)

2016-03-14 Thread Alexey Kardashevskiy 

On 03/03/2016 05:30 PM, David Gibson wrote:

On Tue, Mar 01, 2016 at 08:10:36PM +1100, Alexey Kardashevskiy wrote:

This makes use of the new "memory registering" feature. The idea is
to provide the userspace ability to notify the host kernel about pages
which are going to be used for DMA. Having this information, the host
kernel can pin them all once per user process, do locked pages
accounting (once) and not spent time on doing that in real time with
possible failures which cannot be handled nicely in some cases.

This adds a prereg memory listener which listens on address_space_memory
and notifies a VFIO container about memory which needs to be
pinned/unpinned. VFIO MMIO regions (i.e. "skip dump" regions) are skipped.

As there is no per-IOMMU-type release() callback anymore, this stores
the IOMMU type in the container so vfio_listener_release() can device
if it needs to unregister @prereg_listener.

The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
not call it when v2 is detected and enabled.

This does not change the guest visible interface.

Signed-off-by: Alexey Kardashevskiy 
---
  hw/vfio/Makefile.objs |   1 +
  hw/vfio/common.c  |  39 +---
  hw/vfio/prereg.c  | 138 ++
  include/hw/vfio/vfio-common.h |   4 ++
  trace-events  |   2 +
  5 files changed, 175 insertions(+), 9 deletions(-)
  create mode 100644 hw/vfio/prereg.c

diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index ceddbb8..5800e0e 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -4,4 +4,5 @@ obj-$(CONFIG_PCI) += pci.o pci-quirks.o
  obj-$(CONFIG_SOFTMMU) += platform.o
  obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
  obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
+obj-$(CONFIG_SOFTMMU) += prereg.o
  endif
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 3aaa6b5..f2a03e0 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -531,6 +531,9 @@ static const MemoryListener vfio_iommu_listener = {
  static void vfio_listener_release(VFIOContainer *container)
  {
  memory_listener_unregister(>iommu_listener.listener);
+if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
+memory_listener_unregister(>prereg_listener.listener);
+}
  }

  int vfio_mmap_region(Object *obj, VFIORegion *region,
@@ -722,8 +725,8 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  goto free_container_exit;
  }

-ret = ioctl(fd, VFIO_SET_IOMMU,
-v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU);
+container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU;
+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
  if (ret) {
  error_report("vfio: failed to set iommu for container: %m");
  ret = -errno;
@@ -748,8 +751,10 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
  container->iova_pgsizes = info.iova_pgsizes;
  }
-} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
+} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
+   ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
  struct vfio_iommu_spapr_tce_info info;
+bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);

  ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
  if (ret) {
@@ -757,7 +762,9 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
  ret = -errno;
  goto free_container_exit;
  }
-ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
+container->iommu_type =
+v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
+ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);


It'd be nice to consolidate the setting of container->iommu_type and
then the SET_IOMMU ioctl() rather than having more or less duplicated
logic for Type1 and SPAPR, but it's not a big deal.



May be but I cannot think of any nice way of doing this though.





  if (ret) {
  error_report("vfio: failed to set iommu for container: %m");
  ret = -errno;
@@ -769,11 +776,25 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
   * when container fd is closed so we do not call it explicitly
   * in this file.
   */
-ret = ioctl(fd, VFIO_IOMMU_ENABLE);
-if (ret) {
-error_report("vfio: failed to enable container: %m");
-ret = -errno;
-goto free_container_exit;
+if (!v2) {
+ret = ioctl(fd, VFIO_IOMMU_ENABLE);
+if (ret) {
+error_report("vfio: failed to enable container: %m");
+

Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu v13 11/16] vfio: spapr: Add SPAPR IOMMU v2 support (DMA memory preregistering)

2016-03-03 Thread David Gibson 
On Tue, Mar 01, 2016 at 08:10:36PM +1100, Alexey Kardashevskiy wrote:
> This makes use of the new "memory registering" feature. The idea is
> to provide the userspace ability to notify the host kernel about pages
> which are going to be used for DMA. Having this information, the host
> kernel can pin them all once per user process, do locked pages
> accounting (once) and not spent time on doing that in real time with
> possible failures which cannot be handled nicely in some cases.
> 
> This adds a prereg memory listener which listens on address_space_memory
> and notifies a VFIO container about memory which needs to be
> pinned/unpinned. VFIO MMIO regions (i.e. "skip dump" regions) are skipped.
> 
> As there is no per-IOMMU-type release() callback anymore, this stores
> the IOMMU type in the container so vfio_listener_release() can device
> if it needs to unregister @prereg_listener.
> 
> The feature is only enabled for SPAPR IOMMU v2. The host kernel changes
> are required. Since v2 does not need/support VFIO_IOMMU_ENABLE, this does
> not call it when v2 is detected and enabled.
> 
> This does not change the guest visible interface.
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
>  hw/vfio/Makefile.objs |   1 +
>  hw/vfio/common.c  |  39 +---
>  hw/vfio/prereg.c  | 138 
> ++
>  include/hw/vfio/vfio-common.h |   4 ++
>  trace-events  |   2 +
>  5 files changed, 175 insertions(+), 9 deletions(-)
>  create mode 100644 hw/vfio/prereg.c
> 
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> index ceddbb8..5800e0e 100644
> --- a/hw/vfio/Makefile.objs
> +++ b/hw/vfio/Makefile.objs
> @@ -4,4 +4,5 @@ obj-$(CONFIG_PCI) += pci.o pci-quirks.o
>  obj-$(CONFIG_SOFTMMU) += platform.o
>  obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
>  obj-$(CONFIG_SOFTMMU) += amd-xgbe.o
> +obj-$(CONFIG_SOFTMMU) += prereg.o
>  endif
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 3aaa6b5..f2a03e0 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -531,6 +531,9 @@ static const MemoryListener vfio_iommu_listener = {
>  static void vfio_listener_release(VFIOContainer *container)
>  {
>  memory_listener_unregister(>iommu_listener.listener);
> +if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
> +memory_listener_unregister(>prereg_listener.listener);
> +}
>  }
>  
>  int vfio_mmap_region(Object *obj, VFIORegion *region,
> @@ -722,8 +725,8 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as)
>  goto free_container_exit;
>  }
>  
> -ret = ioctl(fd, VFIO_SET_IOMMU,
> -v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU);
> +container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU;
> +ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>  if (ret) {
>  error_report("vfio: failed to set iommu for container: %m");
>  ret = -errno;
> @@ -748,8 +751,10 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as)
>  if ((ret == 0) && (info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
>  container->iova_pgsizes = info.iova_pgsizes;
>  }
> -} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
> +} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
> +   ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) {
>  struct vfio_iommu_spapr_tce_info info;
> +bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU);
>  
>  ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
>  if (ret) {
> @@ -757,7 +762,9 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as)
>  ret = -errno;
>  goto free_container_exit;
>  }
> -ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
> +container->iommu_type =
> +v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
> +ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);

It'd be nice to consolidate the setting of container->iommu_type and
then the SET_IOMMU ioctl() rather than having more or less duplicated
logic for Type1 and SPAPR, but it's not a big deal.

>  if (ret) {
>  error_report("vfio: failed to set iommu for container: %m");
>  ret = -errno;
> @@ -769,11 +776,25 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as)
>   * when container fd is closed so we do not call it explicitly
>   * in this file.
>   */
> -ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> -if (ret) {
> -error_report("vfio: failed to enable container: %m");
> -ret = -errno;
> -goto free_container_exit;
> +if (!v2) {
> +ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> +if (ret) {
> +