Re: [PATCH 3/9] ARM: dma-mapping: Always pass proper prot flags to iommu_map()

2013-09-30 Thread Marek Szyprowski

Hello,

On 2013-09-27 00:36, Andreas Herrmann wrote:

... otherwise it is impossible for the low level iommu driver to
figure out which pte flags should be used.

In __map_sg_chunk we can derive the flags from dma_data_direction.

In __iommu_create_mapping we should treat the memory like
DMA_BIDIRECTIONAL and pass both IOMMU_READ and IOMMU_WRITE to
iommu_map.
__iommu_create_mapping is used during dma_alloc_coherent (via
arm_iommu_alloc_attrs).  AFAIK dma_alloc_coherent is responsible for
allocation _and_ mapping.  I think this implies that access to the
mapped pages should be allowed.

Cc: Marek Szyprowski m.szyprow...@samsung.com
Signed-off-by: Andreas Herrmann andreas.herrm...@calxeda.com


Thanks pointing the issue and preparing the patch. I will push it to the 
dma-mapping fixes branch.



---
  arch/arm/mm/dma-mapping.c |   43 ---
  1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index f5e1a84..1272ed2 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1232,7 +1232,8 @@ __iommu_create_mapping(struct device *dev, struct page 
**pages, size_t size)
break;
  
  		len = (j - i)  PAGE_SHIFT;

-   ret = iommu_map(mapping-domain, iova, phys, len, 0);
+   ret = iommu_map(mapping-domain, iova, phys, len,
+   IOMMU_READ|IOMMU_WRITE);
if (ret  0)
goto fail;
iova += len;
@@ -1431,6 +1432,27 @@ static int arm_iommu_get_sgtable(struct device *dev, 
struct sg_table *sgt,
 GFP_KERNEL);
  }
  
+static int __dma_direction_to_prot(enum dma_data_direction dir)

+{
+   int prot;
+
+   switch (dir) {
+   case DMA_BIDIRECTIONAL:
+   prot = IOMMU_READ | IOMMU_WRITE;
+   break;
+   case DMA_TO_DEVICE:
+   prot = IOMMU_READ;
+   break;
+   case DMA_FROM_DEVICE:
+   prot = IOMMU_WRITE;
+   break;
+   default:
+   prot = 0;
+   }
+
+   return prot;
+}
+
  /*
   * Map a part of the scatter-gather list into contiguous io address space
   */
@@ -1444,6 +1466,7 @@ static int __map_sg_chunk(struct device *dev, struct 
scatterlist *sg,
int ret = 0;
unsigned int count;
struct scatterlist *s;
+   int prot;
  
  	size = PAGE_ALIGN(size);

*handle = DMA_ERROR_CODE;
@@ -1460,7 +1483,9 @@ static int __map_sg_chunk(struct device *dev, struct 
scatterlist *sg,
!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
__dma_page_cpu_to_dev(sg_page(s), s-offset, s-length, 
dir);
  
-		ret = iommu_map(mapping-domain, iova, phys, len, 0);

+   prot = __dma_direction_to_prot(dir);
+
+   ret = iommu_map(mapping-domain, iova, phys, len, prot);
if (ret  0)
goto fail;
count += len  PAGE_SHIFT;
@@ -1665,19 +1690,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct 
device *dev, struct page *p
if (dma_addr == DMA_ERROR_CODE)
return dma_addr;
  
-	switch (dir) {

-   case DMA_BIDIRECTIONAL:
-   prot = IOMMU_READ | IOMMU_WRITE;
-   break;
-   case DMA_TO_DEVICE:
-   prot = IOMMU_READ;
-   break;
-   case DMA_FROM_DEVICE:
-   prot = IOMMU_WRITE;
-   break;
-   default:
-   prot = 0;
-   }
+   prot = __dma_direction_to_prot(dir);
  
  	ret = iommu_map(mapping-domain, dma_addr, page_to_phys(page), len, prot);

if (ret  0)


Best regards
--
Marek Szyprowski
Samsung RD Institute Poland

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

2013-09-30 Thread Antonios Motakis
This is a workaround to make the VFIO_IOMMU_TYPE1 driver usable with
platform devices instead of PCI. A future permanent fix should support
both. This is required in order to use the Exynos SMMU, or ARM SMMU
driver with VFIO.

Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com
---
 drivers/vfio/Kconfig|  2 +-
 drivers/vfio/vfio_iommu_type1.c | 22 ++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 7cd5dec..1f84eda 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -6,7 +6,7 @@ config VFIO_IOMMU_TYPE1
 menuconfig VFIO
tristate VFIO Non-Privileged userspace driver framework
depends on IOMMU_API
-   select VFIO_IOMMU_TYPE1 if X86
+   select VFIO_IOMMU_TYPE1 if X86 || ARM
help
  VFIO provides a framework for secure userspace device drivers.
  See Documentation/vfio.txt for more details.
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 6f3fbc4..d7e6a12 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -30,7 +30,8 @@
 #include linux/iommu.h
 #include linux/module.h
 #include linux/mm.h
-#include linux/pci.h /* pci_bus_type */
+#include linux/pci.h /* pci_bus_type */
+#include linux/platform_device.h /* platform_bus_type */
 #include linux/sched.h
 #include linux/slab.h
 #include linux/uaccess.h
@@ -46,6 +47,8 @@ module_param_named(allow_unsafe_interrupts,
   allow_unsafe_interrupts, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(allow_unsafe_interrupts,
 Enable VFIO IOMMU support for on platforms without interrupt 
remapping support.);
+static struct bus_type *iommu_bus_type = NULL;
+static bool require_cap_intr_remap = false;
 
 struct vfio_iommu {
struct iommu_domain *domain;
@@ -612,7 +615,8 @@ static void *vfio_iommu_type1_open(unsigned long arg)
/*
 * Wish we didn't have to know about bus_type here.
 */
-   iommu-domain = iommu_domain_alloc(pci_bus_type);
+   iommu-domain = iommu_domain_alloc(iommu_bus_type);
+
if (!iommu-domain) {
kfree(iommu);
return ERR_PTR(-EIO);
@@ -624,7 +628,7 @@ static void *vfio_iommu_type1_open(unsigned long arg)
 * the way.  Fortunately we know interrupt remapping is global for
 * our iommus.
 */
-   if (!allow_unsafe_interrupts 
+   if (require_cap_intr_remap  !allow_unsafe_interrupts 
!iommu_domain_has_cap(iommu-domain, IOMMU_CAP_INTR_REMAP)) {
pr_warn(%s: No interrupt remapping support.  Use the module 
param \allow_unsafe_interrupts\ to enable VFIO IOMMU support on this 
platform\n,
   __func__);
@@ -733,7 +737,17 @@ static const struct vfio_iommu_driver_ops 
vfio_iommu_driver_ops_type1 = {
 
 static int __init vfio_iommu_type1_init(void)
 {
-   if (!iommu_present(pci_bus_type))
+#ifdef CONFIG_PCI
+   if (iommu_present(pci_bus_type)) {
+   iommu_bus_type = pci_bus_type;
+   /* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
+   require_cap_intr_remap = true;
+   }
+#endif
+   if (!iommu_bus_type  iommu_present(platform_bus_type))
+   iommu_bus_type = platform_bus_type;
+
+   if(!iommu_bus_type)
return -ENODEV;
 
return vfio_register_iommu_driver(vfio_iommu_driver_ops_type1);
-- 
1.8.1.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 2/7] Initial skeleton of VFIO support for Device Tree based devices

2013-09-30 Thread Antonios Motakis
Platform devices in the Linux kernel are usually managed by the DT
interface. This patch forms the base to support these kind of devices
with VFIO.

Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com
---
 drivers/vfio/Kconfig |  11 +++
 drivers/vfio/Makefile|   1 +
 drivers/vfio/vfio_platform.c | 187 +++
 include/uapi/linux/vfio.h|   1 +
 4 files changed, 200 insertions(+)
 create mode 100644 drivers/vfio/vfio_platform.c

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 1f84eda..35254b7 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -13,4 +13,15 @@ menuconfig VFIO
 
  If you don't know what to do here, say N.
 
+config VFIO_PLATFORM
+   tristate VFIO support for device tree based platform devices
+   depends on VFIO  EVENTFD  OF
+   help
+ Support for platform devices with VFIO. This is required to make
+ use of platform devices present on device tree nodes using the VFIO
+ framework. Devices that are not described in the device tree cannot
+ be used by this driver.
+
+ If you don't know what to do here, say N.
+
 source drivers/vfio/pci/Kconfig
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 2398d4a..575c8dd 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_PCI) += pci/
+obj-$(CONFIG_VFIO_PLATFORM) += vfio_platform.o
diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
new file mode 100644
index 000..b9686b0
--- /dev/null
+++ b/drivers/vfio/vfio_platform.c
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ * Author: Antonios Motakis a.mota...@virtualopensystems.com
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#include linux/device.h
+#include linux/eventfd.h
+#include linux/interrupt.h
+#include linux/iommu.h
+#include linux/module.h
+#include linux/mutex.h
+#include linux/notifier.h
+#include linux/pm_runtime.h
+#include linux/slab.h
+#include linux/types.h
+#include linux/uaccess.h
+#include linux/vfio.h
+
+#define DRIVER_VERSION  0.1
+#define DRIVER_AUTHOR   Antonios Motakis a.mota...@virtualopensystems.com
+#define DRIVER_DESC VFIO Device Tree devices - User Level meta-driver
+
+struct vfio_platform_device {
+   struct platform_device  *pdev;
+};
+
+static void vfio_platform_release(void *device_data)
+{
+   module_put(THIS_MODULE);
+}
+
+static int vfio_platform_open(void *device_data)
+{
+   if (!try_module_get(THIS_MODULE))
+   return -ENODEV;
+
+   return 0;
+}
+
+static long vfio_platform_ioctl(void *device_data,
+  unsigned int cmd, unsigned long arg)
+{
+   struct vfio_platform_device *vdev = device_data;
+   unsigned long minsz;
+
+   if (cmd == VFIO_DEVICE_GET_INFO) {
+   struct vfio_device_info info;
+
+   minsz = offsetofend(struct vfio_device_info, num_irqs);
+
+   if (copy_from_user(info, (void __user *)arg, minsz))
+   return -EFAULT;
+
+   if (info.argsz  minsz)
+   return -EINVAL;
+
+   info.flags = VFIO_DEVICE_FLAGS_PLATFORM;
+   info.num_regions = 0;
+   info.num_irqs = 0;
+
+   return copy_to_user((void __user *)arg, info, minsz);
+
+   } else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
+   return -EINVAL;
+
+   else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
+   return -EINVAL;
+
+   else if (cmd == VFIO_DEVICE_SET_IRQS)
+   return -EINVAL;
+
+   else if (cmd == VFIO_DEVICE_RESET)
+   return -EINVAL;
+
+   return -ENOTTY;
+}
+
+static ssize_t vfio_platform_read(void *device_data, char __user *buf,
+size_t count, loff_t *ppos)
+{
+   return 0;
+}
+
+static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+   return 0;
+}
+
+static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
+{
+   return -EINVAL;
+}
+
+static const struct vfio_device_ops vfio_platform_ops = {
+   .name   = vfio-platform,
+   

[PATCH 7/7] VFIO: VFIO_PLATFORM: Update documentation for platform specific devices

2013-09-30 Thread Antonios Motakis
Update Documentation/vfio.txt with information regarding the device tree
based platform devices support.

What is still missing in this RFC series, is information on how to bind
the driver to platform devices, as there is currently for PCI. This will
be added when proper VFIO driver binding is implemented.
---
 Documentation/vfio.txt | 17 +
 1 file changed, 17 insertions(+)

diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index b4eafa6..e8daa2b 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -237,6 +237,23 @@ group and can access them as follows:
/* Gratuitous device reset and go... */
ioctl(device, VFIO_DEVICE_RESET);
 
+For platform devices, if a device has multiple regions and multiple IRQs the
+index that will be assigned by VFIO to those resources will correspond to the
+order within the associated reg and interrupt properties in the device tree
+representation of the target device.
+
+For example, if a device is represented like this in the device tree:
+
+reg = 0x101e2000 0x1000 0x101e4000 0x1000;
+interrupts = 24 25 26 27;
+
+Region #0 is 0x101e2000, region #1 is 0x101e4000
+Interrupt #0 is 24, and so on.
+
+Additionally for a platform device, unlike PCI devices, an offset referring to
+a region within a VFIO device file descriptor will match the physical address
+of that region as defined in the device tree.
+
 VFIO User API
 ---
 
-- 
1.8.1.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 4/7] VFIO: DT: Support MMAP of MMIO regions

2013-09-30 Thread Antonios Motakis
Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com
---
 drivers/vfio/vfio_platform.c | 44 
 1 file changed, 44 insertions(+)

diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
index a0abcfa..6364316 100644
--- a/drivers/vfio/vfio_platform.c
+++ b/drivers/vfio/vfio_platform.c
@@ -103,6 +103,10 @@ static long vfio_platform_ioctl(void *device_data,
info.offset = res.start;/* map phys addr with offset */
info.size = resource_size(res);
info.flags = 0;
+   /* Only regions addressed with PAGE granularity can be MMAPed
+* securely. */
+   if (!(info.offset  ~PAGE_MASK)  !(info.size  ~PAGE_MASK))
+   info.flags |= VFIO_REGION_INFO_FLAG_MMAP;
 
return copy_to_user((void __user *)arg, info, minsz);
 
@@ -134,6 +138,18 @@ static long vfio_platform_ioctl(void *device_data,
return -ENOTTY;
 }
 
+static bool is_in_resource(struct resource res, u64 addr, u64 size)
+{
+   if (addr  res.start)
+   return false;
+   if (addr  res.end)
+   return false;
+   if (addr + size - 1  res.end)
+   return false;
+
+   return true;
+}
+
 static ssize_t vfio_platform_read(void *device_data, char __user *buf,
 size_t count, loff_t *ppos)
 {
@@ -148,6 +164,34 @@ static ssize_t vfio_platform_write(void *device_data, 
const char __user *buf,
 
 static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
 {
+   struct vfio_platform_device *vdev = device_data;
+   struct device_node *of_node = vdev-pdev-dev.of_node;
+   u64 req_len = vma-vm_end - vma-vm_start;
+   u64 addr = vma-vm_pgoff  PAGE_SHIFT;
+   struct resource res;
+   int i;
+
+   if (vma-vm_end  vma-vm_start)
+   return -EINVAL;
+   if (vma-vm_start  ~PAGE_MASK)
+   return -EINVAL;
+   if (vma-vm_end  ~PAGE_MASK)
+   return -EINVAL;
+   if ((vma-vm_flags  VM_SHARED) == 0)
+   return -EINVAL;
+
+   for (i = 0; !of_address_to_resource(of_node, i, res); i++) {
+   if (!is_in_resource(res, addr, req_len))
+   continue;
+
+   vma-vm_private_data = vdev;
+   vma-vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
+   vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
+
+   return remap_pfn_range(vma, vma-vm_start, addr,
+  req_len, vma-vm_page_prot);
+   }
+
return -EINVAL;
 }
 
-- 
1.8.1.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 6/7] VFIO: Update documentation for VFIO_IOMMU_TYPE1 driver

2013-09-30 Thread Antonios Motakis
The VFIO documentation is slightly out of date. This minor correction
replaces references to VFIO_IOMMU_X86 with the correct reference to
VFIO_IOMMU_TYPE1

Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com
---
 Documentation/vfio.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index 8eda363..b4eafa6 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -167,8 +167,8 @@ group and can access them as follows:
int container, group, device, i;
struct vfio_group_status group_status =
{ .argsz = sizeof(group_status) };
-   struct vfio_iommu_x86_info iommu_info = { .argsz = sizeof(iommu_info) };
-   struct vfio_iommu_x86_dma_map dma_map = { .argsz = sizeof(dma_map) };
+   struct vfio_iommu_type1_info iommu_info = { .argsz = sizeof(iommu_info) 
};
+   struct vfio_iommu_type1_dma_map dma_map = { .argsz = sizeof(dma_map) };
struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
 
/* Create a new container */
@@ -177,7 +177,7 @@ group and can access them as follows:
if (ioctl(container, VFIO_GET_API_VERSION) != VFIO_API_VERSION)
/* Unknown API version */
 
-   if (!ioctl(container, VFIO_CHECK_EXTENSION, VFIO_X86_IOMMU))
+   if (!ioctl(container, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU))
/* Doesn't support the IOMMU driver we want. */
 
/* Open the group */
@@ -193,7 +193,7 @@ group and can access them as follows:
ioctl(group, VFIO_GROUP_SET_CONTAINER, container);
 
/* Enable the IOMMU model we want */
-   ioctl(container, VFIO_SET_IOMMU, VFIO_X86_IOMMU)
+   ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_IOMMU)
 
/* Get addition IOMMU info */
ioctl(container, VFIO_IOMMU_GET_INFO, iommu_info);
-- 
1.8.1.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 5/7] VFIO: DT: Read and write support for the device fd

2013-09-30 Thread Antonios Motakis
VFIO returns a file descriptor which we can use to manipulate the memory
regions of the device. Since some memory regions we cannot mmap due to
security concerns, we also allow to read and write to this file descriptor
directly.

Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com
---
 drivers/vfio/vfio_platform.c | 37 ++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
index 6364316..b92d7bb 100644
--- a/drivers/vfio/vfio_platform.c
+++ b/drivers/vfio/vfio_platform.c
@@ -102,7 +102,8 @@ static long vfio_platform_ioctl(void *device_data,
 
info.offset = res.start;/* map phys addr with offset */
info.size = resource_size(res);
-   info.flags = 0;
+   info.flags = VFIO_REGION_INFO_FLAG_READ
+   | VFIO_REGION_INFO_FLAG_WRITE;
/* Only regions addressed with PAGE granularity can be MMAPed
 * securely. */
if (!(info.offset  ~PAGE_MASK)  !(info.size  ~PAGE_MASK))
@@ -153,13 +154,43 @@ static bool is_in_resource(struct resource res, u64 addr, 
u64 size)
 static ssize_t vfio_platform_read(void *device_data, char __user *buf,
 size_t count, loff_t *ppos)
 {
-   return 0;
+   struct vfio_platform_device *vdev = device_data;
+   struct device_node *of_node = vdev-pdev-dev.of_node;
+   struct resource res;
+   int i;
+
+   for (i = 0; !of_address_to_resource(of_node, i, res); i++) {
+   if (!is_in_resource(res, *ppos, count))
+   continue;
+
+   if (copy_to_user(buf, ppos, count))
+   return -EFAULT;
+   else
+   return count;
+   }
+
+   return -EINVAL;
 }
 
 static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
  size_t count, loff_t *ppos)
 {
-   return 0;
+   struct vfio_platform_device *vdev = device_data;
+   struct device_node *of_node = vdev-pdev-dev.of_node;
+   struct resource res;
+   int i;
+
+   for (i = 0; !of_address_to_resource(of_node, i, res); i++) {
+   if (!is_in_resource(res, *ppos, count))
+   continue;
+
+   if (copy_from_user(ppos, buf, count))
+   return -EFAULT;
+   else
+   return count;
+   }
+
+   return -EINVAL;
 }
 
 static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
-- 
1.8.1.2

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree based devices

2013-09-30 Thread Bhushan Bharat-R65777


 -Original Message-
 From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
 boun...@lists.linux-foundation.org] On Behalf Of Antonios Motakis
 Sent: Monday, September 30, 2013 8:59 PM
 To: kvm...@lists.cs.columbia.edu; alex.william...@redhat.com
 Cc: linux-samsung-...@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; 
 Yoder
 Stuart-B08248; iommu@lists.linux-foundation.org; Antonios Motakis;
 t...@virtualopensystems.com
 Subject: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree based
 devices
 
 Platform devices in the Linux kernel are usually managed by the DT interface.
 This patch forms the base to support these kind of devices with VFIO.
 
 Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com
 ---
  drivers/vfio/Kconfig |  11 +++
  drivers/vfio/Makefile|   1 +
  drivers/vfio/vfio_platform.c | 187 
 +++
  include/uapi/linux/vfio.h|   1 +
  4 files changed, 200 insertions(+)
  create mode 100644 drivers/vfio/vfio_platform.c
 
 diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 
 1f84eda..35254b7
 100644
 --- a/drivers/vfio/Kconfig
 +++ b/drivers/vfio/Kconfig
 @@ -13,4 +13,15 @@ menuconfig VFIO
 
 If you don't know what to do here, say N.
 
 +config VFIO_PLATFORM
 + tristate VFIO support for device tree based platform devices
 + depends on VFIO  EVENTFD  OF
 + help
 +   Support for platform devices with VFIO. This is required to make
 +   use of platform devices present on device tree nodes using the VFIO
 +   framework. Devices that are not described in the device tree cannot
 +   be used by this driver.
 +
 +   If you don't know what to do here, say N.
 +
  source drivers/vfio/pci/Kconfig
 diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile index
 2398d4a..575c8dd 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_PCI) += pci/
 +obj-$(CONFIG_VFIO_PLATFORM) += vfio_platform.o
 diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c new

We can make this parallel to PCI, something like 
drivers/vfio/platform/platform.c

-Bharat

 file mode 100644 index 000..b9686b0
 --- /dev/null
 +++ b/drivers/vfio/vfio_platform.c
 @@ -0,0 +1,187 @@
 +/*
 + * Copyright (C) 2013 - Virtual Open Systems
 + * Author: Antonios Motakis a.mota...@virtualopensystems.com
 + *
 + * 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.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 + */
 +
 +#include linux/device.h
 +#include linux/eventfd.h
 +#include linux/interrupt.h
 +#include linux/iommu.h
 +#include linux/module.h
 +#include linux/mutex.h
 +#include linux/notifier.h
 +#include linux/pm_runtime.h
 +#include linux/slab.h
 +#include linux/types.h
 +#include linux/uaccess.h
 +#include linux/vfio.h
 +
 +#define DRIVER_VERSION  0.1
 +#define DRIVER_AUTHOR   Antonios Motakis a.mota...@virtualopensystems.com
 +#define DRIVER_DESC VFIO Device Tree devices - User Level meta-driver
 +
 +struct vfio_platform_device {
 + struct platform_device  *pdev;
 +};
 +
 +static void vfio_platform_release(void *device_data) {
 + module_put(THIS_MODULE);
 +}
 +
 +static int vfio_platform_open(void *device_data) {
 + if (!try_module_get(THIS_MODULE))
 + return -ENODEV;
 +
 + return 0;
 +}
 +
 +static long vfio_platform_ioctl(void *device_data,
 +unsigned int cmd, unsigned long arg) {
 + struct vfio_platform_device *vdev = device_data;
 + unsigned long minsz;
 +
 + if (cmd == VFIO_DEVICE_GET_INFO) {
 + struct vfio_device_info info;
 +
 + minsz = offsetofend(struct vfio_device_info, num_irqs);
 +
 + if (copy_from_user(info, (void __user *)arg, minsz))
 + return -EFAULT;
 +
 + if (info.argsz  minsz)
 + return -EINVAL;
 +
 + info.flags = VFIO_DEVICE_FLAGS_PLATFORM;
 + info.num_regions = 0;
 + info.num_irqs = 0;
 +
 + return copy_to_user((void __user *)arg, info, minsz);
 +
 + } else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
 + return -EINVAL;
 +
 + else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
 + return -EINVAL;
 +
 + else if (cmd == 

RE: [PATCH 3/7] Return info for device and its memory regions and interrupts

2013-09-30 Thread Bhushan Bharat-R65777


 -Original Message-
 From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
 boun...@lists.linux-foundation.org] On Behalf Of Antonios Motakis
 Sent: Monday, September 30, 2013 8:59 PM
 To: kvm...@lists.cs.columbia.edu; alex.william...@redhat.com
 Cc: linux-samsung-...@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; 
 Yoder
 Stuart-B08248; iommu@lists.linux-foundation.org; Antonios Motakis;
 t...@virtualopensystems.com
 Subject: [PATCH 3/7] Return info for device and its memory regions and
 interrupts
 
 A VFIO userspace driver will start by opening the VFIO device that corresponds
 to an IOMMU group, and will use the ioctl interface to get the basic device
 info, such as number of memory regions and interrupts, and their properties.
 
 This patch implements the IOCTLs:
  - VFIO_DEVICE_GET_INFO
  - VFIO_DEVICE_GET_REGION_INFO
  - VFIO_DEVICE_GET_IRQ_INFO
 
 Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com
 ---
  drivers/vfio/vfio_platform.c | 60 
 ++--
  1 file changed, 53 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c index
 b9686b0..a0abcfa 100644
 --- a/drivers/vfio/vfio_platform.c
 +++ b/drivers/vfio/vfio_platform.c
 @@ -28,6 +28,10 @@
  #include linux/types.h
  #include linux/uaccess.h
  #include linux/vfio.h
 +#include linux/of.h
 +#include linux/of_address.h
 +#include linux/of_irq.h
 +#include linux/of_platform.h
 
  #define DRIVER_VERSION  0.1
  #define DRIVER_AUTHOR   Antonios Motakis a.mota...@virtualopensystems.com
 @@ -54,10 +58,13 @@ static long vfio_platform_ioctl(void *device_data,
  unsigned int cmd, unsigned long arg)  {
   struct vfio_platform_device *vdev = device_data;
 + struct device_node *of_node = vdev-pdev-dev.of_node;
   unsigned long minsz;
 
   if (cmd == VFIO_DEVICE_GET_INFO) {
   struct vfio_device_info info;
 + struct resource res;
 + int cnt = 0;
 
   minsz = offsetofend(struct vfio_device_info, num_irqs);
 
 @@ -68,18 +75,57 @@ static long vfio_platform_ioctl(void *device_data,
   return -EINVAL;
 
   info.flags = VFIO_DEVICE_FLAGS_PLATFORM;
 - info.num_regions = 0;
 - info.num_irqs = 0;
 +
 + while (!of_address_to_resource(of_node, cnt, res))
 + cnt++;
 +
 + info.num_regions = cnt;
 +
 + info.num_irqs = of_irq_count(of_node);
 
   return copy_to_user((void __user *)arg, info, minsz);
 
 - } else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
 - return -EINVAL;
 + } else if (cmd == VFIO_DEVICE_GET_REGION_INFO) {
 + struct vfio_region_info info;
 + struct resource res;
 
 - else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
 - return -EINVAL;
 + minsz = offsetofend(struct vfio_region_info, offset);
 +
 + if (copy_from_user(info, (void __user *)arg, minsz))
 + return -EFAULT;
 +
 + if (info.argsz  minsz)
 + return -EINVAL;
 +
 + if(of_address_to_resource(of_node, info.index, res))
 + return -EINVAL;
 +
 + info.offset = res.start;/* map phys addr with offset */
 + info.size = resource_size(res);
 + info.flags = 0;
 +
 + return copy_to_user((void __user *)arg, info, minsz);
 +
 + } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
 + struct vfio_irq_info info;
 + struct resource res;
 +
 + minsz = offsetofend(struct vfio_irq_info, count);
 +
 + if (copy_from_user(info, (void __user *)arg, minsz))
 + return -EFAULT;
 +
 + if (info.argsz  minsz)
 + return -EINVAL;
 +
 + of_irq_to_resource(of_node, info.index, res);

Why are we calling the above function if not using res?

 +
 + info.flags = 0;
 + info.count = 1;

I believe count here is number of interrupts, and we can have devices with more 
than 1 interrupt.

-Bharat

 +
 + return copy_to_user((void __user *)arg, info, minsz);
 
 - else if (cmd == VFIO_DEVICE_SET_IRQS)
 + } else if (cmd == VFIO_DEVICE_SET_IRQS)
   return -EINVAL;
 
   else if (cmd == VFIO_DEVICE_RESET)
 --
 1.8.1.2
 
 ___
 iommu mailing list
 iommu@lists.linux-foundation.org
 https://lists.linuxfoundation.org/mailman/listinfo/iommu


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

2013-09-30 Thread Alex Williamson
On Mon, 2013-09-30 at 17:28 +0200, Antonios Motakis wrote:
 This is a preview of the base work, towards VFIO support on ARM platforms
 with an IOMMU. It forms a base on to which to implement the functionality
 necessary to enable using device tree devices on ARM (and other platforms
 based on device trees) with VFIO.
 
 This patch series has been subjected to limited testing on the Arndale board
 (with the Exynos 5250 System MMU). More extensive testing will follow as more
 features are implemented.
 
 It depends on Cho KyongHo's patch series iommu/exynos: Fixes and Enhancements
 of System MMU driver with DT, applied on a Linux 3.10.1 kernel, and also my
 own iommu/exynos: add devices attached to the System MMU to an IOMMU group.
 Those patches are required at least in order to test the proposed module on
 Arndale.
 
 The API used is identical to the existing VFIO API that is also used with
 PCI devices. Only devices that include a basic set of IRQs and memory regions
 are targeted; devices with complicated relationships with other devices on the
 device tree are not taken into account at this stage.
 
 The following IOCTLs have been found to be working on the Arndale with no
 changes to VFIO:
  - VFIO_GET_API_VERSION
  - VFIO_CHECK_EXTENSION
 The TYPE1 fix proposed here enables the following IOCTLs:
  - VFIO_GROUP_GET_STATUS
  - VFIO_GROUP_SET_CONTAINER
  - VFIO_SET_IOMMU
  - VFIO_IOMMU_GET_INFO
  - VFIO_IOMMU_MAP_DMA
 The VFIO platform driver proposed here implements the following:
  - VFIO_GROUP_GET_DEVICE_FD
  - VFIO_DEVICE_GET_INFO
  - VFIO_DEVICE_GET_REGION_INFO
  - VFIO_DEVICE_GET_IRQ_INFO
 In addition, the VFIO platform driver implements the following through
 the VFIO device file descriptor:
  - MMAPing memory regions to the virtual address space of the VFIO user.
  - Read / write of memory regions directly through the file descriptor.
 
 What still needs to be done, includes:
  - IRQs / IRQFD support.
  - The API is not currently extended with device tree specific information.
  - Proper binding of the VFIO_DT driver to devices; currently to test the
driver, one has to edit the device tree and add vfio to the
compatible property. However Linux does not support OF drivers that
can be dynamically bound to any device.
  - More extensive testing with a functioning device doing DMA.
  - QEMU / KVM support.
 
 Antonios Motakis (7):
   VFIO_IOMMU_TYPE1 workaround to build for platform devices
   Initial skeleton of VFIO support for Device Tree based devices
   Return info for device and its memory regions and interrupts
   VFIO: DT: Support MMAP of MMIO regions
   VFIO: DT: Read and write support for the device fd
   VFIO: Update documentation for VFIO_IOMMU_TYPE1 driver
   VFIO: VFIO_PLATFORM: Update documentation for platform specific
 devices
 
  Documentation/vfio.txt  |  25 +++-
  drivers/vfio/Kconfig|  13 +-
  drivers/vfio/Makefile   |   1 +
  drivers/vfio/vfio_iommu_type1.c |  22 ++-
  drivers/vfio/vfio_platform.c| 308 
 
  include/uapi/linux/vfio.h   |   1 +
  6 files changed, 361 insertions(+), 9 deletions(-)
  create mode 100644 drivers/vfio/vfio_platform.c

Looks like a good first pass to me, I'll be interested to see what the
other platform device folks think.  Thanks,

Alex

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/arm-smmu: Clear global and context bank fault status registers

2013-09-30 Thread Will Deacon
On Mon, Sep 30, 2013 at 06:17:16PM +0100, Andreas Herrmann wrote:
 On Mon, Sep 30, 2013 at 12:06:15PM -0400, Will Deacon wrote:
  On Mon, Sep 30, 2013 at 02:56:21PM +0100, Andreas Herrmann wrote:
   
   After reset these registers have unknown values.
   This might cause problems when evaluating SMMU_GFSR and/or SMMU_CB_FSR
   in handlers for combined interrupts.
   
   Signed-off-by: Andreas Herrmann andreas.herrm...@calxeda.com
   ---
drivers/iommu/arm-smmu.c |   27 ---
1 file changed, 20 insertions(+), 7 deletions(-)
   
   diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
   index 579b6f8..cbbf597 100644
   --- a/drivers/iommu/arm-smmu.c
   +++ b/drivers/iommu/arm-smmu.c
   @@ -631,6 +631,12 @@ static irqreturn_t arm_smmu_global_fault(int irq, 
   void *dev)
 return IRQ_HANDLED;
}

   +static void arm_smmu_clear_cb_fsr(struct arm_smmu_device *smmu, u8 cbndx)
   +{
   + void __iomem *cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, 
   cbndx);
   + writel_relaxed(FSR_FAULT, cb_base + ARM_SMMU_CB_FSR);
   +}
  
  Hmm, why not just stick this in arm_smmu_init_context_bank...
 
 Because we should clear the FSR before we call request_irq.
 Otherwise we might handle interrupts although the context bank is not
 enabled.
 
 Moving request_irq after arm_smmu_init_context_bank is not optimal
 either. (We should have configured the context interrupt before
 translation is enabled. Otherwise it's possible to miss a fault.)

How would you miss a fault? If the device can start issuing transactions
before the SMMU has set up the mapping, then there's a race in the caller
code which we shouldn't attempt to resolve here.

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/arm-smmu: Clear global and context bank fault status registers

2013-09-30 Thread Andreas Herrmann
On Mon, Sep 30, 2013 at 02:30:06PM -0400, Will Deacon wrote:
 On Mon, Sep 30, 2013 at 06:17:16PM +0100, Andreas Herrmann wrote:
  On Mon, Sep 30, 2013 at 12:06:15PM -0400, Will Deacon wrote:
   On Mon, Sep 30, 2013 at 02:56:21PM +0100, Andreas Herrmann wrote:

After reset these registers have unknown values.
This might cause problems when evaluating SMMU_GFSR and/or SMMU_CB_FSR
in handlers for combined interrupts.

Signed-off-by: Andreas Herrmann andreas.herrm...@calxeda.com
---
 drivers/iommu/arm-smmu.c |   27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 579b6f8..cbbf597 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -631,6 +631,12 @@ static irqreturn_t arm_smmu_global_fault(int irq, 
void *dev)
return IRQ_HANDLED;
 }
 
+static void arm_smmu_clear_cb_fsr(struct arm_smmu_device *smmu, u8 
cbndx)
+{
+   void __iomem *cb_base = ARM_SMMU_CB_BASE(smmu) + 
ARM_SMMU_CB(smmu, cbndx);
+   writel_relaxed(FSR_FAULT, cb_base + ARM_SMMU_CB_FSR);
+}
   
   Hmm, why not just stick this in arm_smmu_init_context_bank...
  
  Because we should clear the FSR before we call request_irq.
  Otherwise we might handle interrupts although the context bank is not
  enabled.
  
  Moving request_irq after arm_smmu_init_context_bank is not optimal
  either. (We should have configured the context interrupt before
  translation is enabled. Otherwise it's possible to miss a fault.)
 
 How would you miss a fault?

 If the device can start issuing transactions before the SMMU has set
 up the mapping, then there's a race in the caller code which we
 shouldn't attempt to resolve here.

Broken device, broken driver code (maybe violating dma-api)
whatsoever. All that's needed is a device that is already doing DMA
when we enable SMMU handling for its transactions.
Yes, normally this should not happen. But if it happens we get a fault
and better should handle it.

I think, it's somehow logical to have fault handling set up before
fault reporting is switched on for a context bank.


Andreas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu