Re: [PATCH 3/9] ARM: dma-mapping: Always pass proper prot flags to iommu_map()
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
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
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
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
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
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
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
-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
-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)
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
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
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