Re: [PATCH v2 13/13] vfio: powerpc/spapr: Enable Dynamic DMA windows

2014-10-10 Thread Alexey Kardashevskiy
On 09/23/2014 11:56 PM, Alex Williamson wrote:
 On Tue, 2014-09-23 at 13:01 +1000, Alexey Kardashevskiy wrote:
 This defines and implements VFIO IOMMU API which lets the userspace
 create and remove DMA windows.

 This updates VFIO_IOMMU_SPAPR_TCE_GET_INFO to return the number of
 available windows and page mask.

 This adds VFIO_IOMMU_SPAPR_TCE_CREATE and VFIO_IOMMU_SPAPR_TCE_REMOVE
 to allow the user space to create and remove window(s).

 The VFIO IOMMU driver does basic sanity checks and calls corresponding
 SPAPR TCE functions. At the moment only IODA2 (POWER8 PCI host bridge)
 implements them.

 This advertises VFIO_IOMMU_SPAPR_TCE_FLAG_DDW capability via
 VFIO_IOMMU_SPAPR_TCE_GET_INFO.

 This calls platform DDW reset() callback when IOMMU is being disabled
 to reset the DMA configuration to its original state.

 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
  drivers/vfio/vfio_iommu_spapr_tce.c | 135 
 ++--
  include/uapi/linux/vfio.h   |  25 ++-
  2 files changed, 153 insertions(+), 7 deletions(-)

 diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
 b/drivers/vfio/vfio_iommu_spapr_tce.c
 index 0dccbc4..b518891 100644
 --- a/drivers/vfio/vfio_iommu_spapr_tce.c
 +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
 @@ -190,18 +190,25 @@ static void tce_iommu_disable(struct tce_container 
 *container)
  
  container-enabled = false;
  
 -if (!container-grp || !current-mm)
 +if (!container-grp)
  return;
  
  data = iommu_group_get_iommudata(container-grp);
  if (!data || !data-iommu_owner || !data-ops-get_table)
  return;
  
 -tbl = data-ops-get_table(data, 0);
 -if (!tbl)
 -return;
 +if (current-mm) {
 +tbl = data-ops-get_table(data, 0);
 +if (tbl)
 +decrement_locked_vm(tbl);
  
 -decrement_locked_vm(tbl);
 +tbl = data-ops-get_table(data, 1);
 +if (tbl)
 +decrement_locked_vm(tbl);
 +}
 +
 +if (data-ops-reset)
 +data-ops-reset(data);
  }
  
  static void *tce_iommu_open(unsigned long arg)
 @@ -243,7 +250,7 @@ static long tce_iommu_ioctl(void *iommu_data,
   unsigned int cmd, unsigned long arg)
  {
  struct tce_container *container = iommu_data;
 -unsigned long minsz;
 +unsigned long minsz, ddwsz;
  long ret;
  
  switch (cmd) {
 @@ -288,6 +295,28 @@ static long tce_iommu_ioctl(void *iommu_data,
  info.dma32_window_size = tbl-it_size  tbl-it_page_shift;
  info.flags = 0;
  
 +ddwsz = offsetofend(struct vfio_iommu_spapr_tce_info,
 +page_size_mask);
 +
 +if (info.argsz == ddwsz) {
 
 =
 
 +if (data-ops-query  data-ops-create 
 +data-ops-remove) {
 +info.flags |= VFIO_IOMMU_SPAPR_TCE_FLAG_DDW;
 
 I think you want to set this flag regardless of whether the user has
 provided space for it.  A valid use model is to call with the minimum
 size and look at the flags to determine if it needs to be called again
 with a larger size.
 
 +
 +ret = data-ops-query(data,
 +info.current_windows,
 +info.windows_available,
 +info.page_size_mask);
 +if (ret)
 +return ret;
 +} else {
 +info.current_windows = 0;
 +info.windows_available = 0;
 +info.page_size_mask = 0;
 +}
 +minsz = ddwsz;
 
 It's not really any longer the min size, is it?
 
 +}
 +
  if (copy_to_user((void __user *)arg, info, minsz))
  return -EFAULT;
  
 @@ -412,12 +441,106 @@ static long tce_iommu_ioctl(void *iommu_data,
  tce_iommu_disable(container);
  mutex_unlock(container-lock);
  return 0;
 +
  case VFIO_EEH_PE_OP:
  if (!container-grp)
  return -ENODEV;
  
  return vfio_spapr_iommu_eeh_ioctl(container-grp,
cmd, arg);
 +
 +case VFIO_IOMMU_SPAPR_TCE_CREATE: {
 +struct vfio_iommu_spapr_tce_create create;
 +struct spapr_tce_iommu_group *data;
 +struct iommu_table *tbl;
 +
 +if (WARN_ON(!container-grp))
 
 redux previous comment on this warning
 
 +return -ENXIO;
 +
 +data = iommu_group_get_iommudata(container-grp);
 +
 +minsz = offsetofend(struct vfio_iommu_spapr_tce_create,
 +start_addr);
 +
 +if (copy_from_user(create, (void __user *)arg, minsz))
 + 

Re: [PATCH v2 13/13] vfio: powerpc/spapr: Enable Dynamic DMA windows

2014-09-23 Thread Alex Williamson
On Tue, 2014-09-23 at 13:01 +1000, Alexey Kardashevskiy wrote:
 This defines and implements VFIO IOMMU API which lets the userspace
 create and remove DMA windows.
 
 This updates VFIO_IOMMU_SPAPR_TCE_GET_INFO to return the number of
 available windows and page mask.
 
 This adds VFIO_IOMMU_SPAPR_TCE_CREATE and VFIO_IOMMU_SPAPR_TCE_REMOVE
 to allow the user space to create and remove window(s).
 
 The VFIO IOMMU driver does basic sanity checks and calls corresponding
 SPAPR TCE functions. At the moment only IODA2 (POWER8 PCI host bridge)
 implements them.
 
 This advertises VFIO_IOMMU_SPAPR_TCE_FLAG_DDW capability via
 VFIO_IOMMU_SPAPR_TCE_GET_INFO.
 
 This calls platform DDW reset() callback when IOMMU is being disabled
 to reset the DMA configuration to its original state.
 
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
  drivers/vfio/vfio_iommu_spapr_tce.c | 135 
 ++--
  include/uapi/linux/vfio.h   |  25 ++-
  2 files changed, 153 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
 b/drivers/vfio/vfio_iommu_spapr_tce.c
 index 0dccbc4..b518891 100644
 --- a/drivers/vfio/vfio_iommu_spapr_tce.c
 +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
 @@ -190,18 +190,25 @@ static void tce_iommu_disable(struct tce_container 
 *container)
  
   container-enabled = false;
  
 - if (!container-grp || !current-mm)
 + if (!container-grp)
   return;
  
   data = iommu_group_get_iommudata(container-grp);
   if (!data || !data-iommu_owner || !data-ops-get_table)
   return;
  
 - tbl = data-ops-get_table(data, 0);
 - if (!tbl)
 - return;
 + if (current-mm) {
 + tbl = data-ops-get_table(data, 0);
 + if (tbl)
 + decrement_locked_vm(tbl);
  
 - decrement_locked_vm(tbl);
 + tbl = data-ops-get_table(data, 1);
 + if (tbl)
 + decrement_locked_vm(tbl);
 + }
 +
 + if (data-ops-reset)
 + data-ops-reset(data);
  }
  
  static void *tce_iommu_open(unsigned long arg)
 @@ -243,7 +250,7 @@ static long tce_iommu_ioctl(void *iommu_data,
unsigned int cmd, unsigned long arg)
  {
   struct tce_container *container = iommu_data;
 - unsigned long minsz;
 + unsigned long minsz, ddwsz;
   long ret;
  
   switch (cmd) {
 @@ -288,6 +295,28 @@ static long tce_iommu_ioctl(void *iommu_data,
   info.dma32_window_size = tbl-it_size  tbl-it_page_shift;
   info.flags = 0;
  
 + ddwsz = offsetofend(struct vfio_iommu_spapr_tce_info,
 + page_size_mask);
 +
 + if (info.argsz == ddwsz) {

=

 + if (data-ops-query  data-ops-create 
 + data-ops-remove) {
 + info.flags |= VFIO_IOMMU_SPAPR_TCE_FLAG_DDW;

I think you want to set this flag regardless of whether the user has
provided space for it.  A valid use model is to call with the minimum
size and look at the flags to determine if it needs to be called again
with a larger size.

 +
 + ret = data-ops-query(data,
 + info.current_windows,
 + info.windows_available,
 + info.page_size_mask);
 + if (ret)
 + return ret;
 + } else {
 + info.current_windows = 0;
 + info.windows_available = 0;
 + info.page_size_mask = 0;
 + }
 + minsz = ddwsz;

It's not really any longer the min size, is it?

 + }
 +
   if (copy_to_user((void __user *)arg, info, minsz))
   return -EFAULT;
  
 @@ -412,12 +441,106 @@ static long tce_iommu_ioctl(void *iommu_data,
   tce_iommu_disable(container);
   mutex_unlock(container-lock);
   return 0;
 +
   case VFIO_EEH_PE_OP:
   if (!container-grp)
   return -ENODEV;
  
   return vfio_spapr_iommu_eeh_ioctl(container-grp,
 cmd, arg);
 +
 + case VFIO_IOMMU_SPAPR_TCE_CREATE: {
 + struct vfio_iommu_spapr_tce_create create;
 + struct spapr_tce_iommu_group *data;
 + struct iommu_table *tbl;
 +
 + if (WARN_ON(!container-grp))

redux previous comment on this warning

 + return -ENXIO;
 +
 + data = iommu_group_get_iommudata(container-grp);
 +
 + minsz = offsetofend(struct vfio_iommu_spapr_tce_create,
 + start_addr);
 +
 + if (copy_from_user(create, (void __user *)arg, minsz))
 + 

[PATCH v2 13/13] vfio: powerpc/spapr: Enable Dynamic DMA windows

2014-09-22 Thread Alexey Kardashevskiy
This defines and implements VFIO IOMMU API which lets the userspace
create and remove DMA windows.

This updates VFIO_IOMMU_SPAPR_TCE_GET_INFO to return the number of
available windows and page mask.

This adds VFIO_IOMMU_SPAPR_TCE_CREATE and VFIO_IOMMU_SPAPR_TCE_REMOVE
to allow the user space to create and remove window(s).

The VFIO IOMMU driver does basic sanity checks and calls corresponding
SPAPR TCE functions. At the moment only IODA2 (POWER8 PCI host bridge)
implements them.

This advertises VFIO_IOMMU_SPAPR_TCE_FLAG_DDW capability via
VFIO_IOMMU_SPAPR_TCE_GET_INFO.

This calls platform DDW reset() callback when IOMMU is being disabled
to reset the DMA configuration to its original state.

Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
 drivers/vfio/vfio_iommu_spapr_tce.c | 135 ++--
 include/uapi/linux/vfio.h   |  25 ++-
 2 files changed, 153 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c 
b/drivers/vfio/vfio_iommu_spapr_tce.c
index 0dccbc4..b518891 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -190,18 +190,25 @@ static void tce_iommu_disable(struct tce_container 
*container)
 
container-enabled = false;
 
-   if (!container-grp || !current-mm)
+   if (!container-grp)
return;
 
data = iommu_group_get_iommudata(container-grp);
if (!data || !data-iommu_owner || !data-ops-get_table)
return;
 
-   tbl = data-ops-get_table(data, 0);
-   if (!tbl)
-   return;
+   if (current-mm) {
+   tbl = data-ops-get_table(data, 0);
+   if (tbl)
+   decrement_locked_vm(tbl);
 
-   decrement_locked_vm(tbl);
+   tbl = data-ops-get_table(data, 1);
+   if (tbl)
+   decrement_locked_vm(tbl);
+   }
+
+   if (data-ops-reset)
+   data-ops-reset(data);
 }
 
 static void *tce_iommu_open(unsigned long arg)
@@ -243,7 +250,7 @@ static long tce_iommu_ioctl(void *iommu_data,
 unsigned int cmd, unsigned long arg)
 {
struct tce_container *container = iommu_data;
-   unsigned long minsz;
+   unsigned long minsz, ddwsz;
long ret;
 
switch (cmd) {
@@ -288,6 +295,28 @@ static long tce_iommu_ioctl(void *iommu_data,
info.dma32_window_size = tbl-it_size  tbl-it_page_shift;
info.flags = 0;
 
+   ddwsz = offsetofend(struct vfio_iommu_spapr_tce_info,
+   page_size_mask);
+
+   if (info.argsz == ddwsz) {
+   if (data-ops-query  data-ops-create 
+   data-ops-remove) {
+   info.flags |= VFIO_IOMMU_SPAPR_TCE_FLAG_DDW;
+
+   ret = data-ops-query(data,
+   info.current_windows,
+   info.windows_available,
+   info.page_size_mask);
+   if (ret)
+   return ret;
+   } else {
+   info.current_windows = 0;
+   info.windows_available = 0;
+   info.page_size_mask = 0;
+   }
+   minsz = ddwsz;
+   }
+
if (copy_to_user((void __user *)arg, info, minsz))
return -EFAULT;
 
@@ -412,12 +441,106 @@ static long tce_iommu_ioctl(void *iommu_data,
tce_iommu_disable(container);
mutex_unlock(container-lock);
return 0;
+
case VFIO_EEH_PE_OP:
if (!container-grp)
return -ENODEV;
 
return vfio_spapr_iommu_eeh_ioctl(container-grp,
  cmd, arg);
+
+   case VFIO_IOMMU_SPAPR_TCE_CREATE: {
+   struct vfio_iommu_spapr_tce_create create;
+   struct spapr_tce_iommu_group *data;
+   struct iommu_table *tbl;
+
+   if (WARN_ON(!container-grp))
+   return -ENXIO;
+
+   data = iommu_group_get_iommudata(container-grp);
+
+   minsz = offsetofend(struct vfio_iommu_spapr_tce_create,
+   start_addr);
+
+   if (copy_from_user(create, (void __user *)arg, minsz))
+   return -EFAULT;
+
+   if (create.argsz  minsz)
+   return -EINVAL;
+
+   if (create.flags)
+   return -EINVAL;
+
+   if (!data-ops-create || !data-iommu_owner)
+   return -ENOSYS;
+
+   BUG_ON(!data || !data-ops || !data-ops-remove);
+
+