From 9698cbf0bea6b9f5c3190ce97bdf8963c0148671 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Mon, 23 May 2016 17:47:30 +0800 Subject: vfio: platform: support No-IOMMU mode The vfio No-IOMMU mode was supported by this 'commit 03a76b60f8ba2797 ("vfio: Include No-IOMMU mode")', but it only support vfio-pci. Using vfio_iommu_group_get/put, but not iommu_group_get/put, the platform devices can be exposed to userspace with CONFIG_VFIO_NOIOMMU and the "enable_unsafe_noiommu_mode" option enabled. From 'commit 03a76b60f8ba2797 ("vfio: Include No-IOMMU mode")', "This should make it very clear that this mode is not safe. Additionally, CAP_SYS_RAWIO privileges are necessary to work with groups and containers using this mode. Groups making use of this support are named /dev/vfio/noiommu-$GROUP and can only make use of the special VFIO_NOIOMMU_IOMMU for the container. Use of this mode, specifically binding a device without a native IOMMU group to a VFIO bus driver will taint the kernel and should therefore not be considered supported." Signed-off-by: Peng Fan Cc: Eric Auger Cc: Baptiste Reynal Cc: Alex Williamson Reviewed-by: Eric Auger Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_common.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index e65b142d3422..993b2f932f80 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -561,7 +561,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, vdev->device = dev; - group = iommu_group_get(dev); + group = vfio_iommu_group_get(dev); if (!group) { pr_err("VFIO: No IOMMU group for device %s\n", vdev->name); return -EINVAL; @@ -569,7 +569,7 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, ret = vfio_add_group_dev(dev, &vfio_platform_ops, vdev); if (ret) { - iommu_group_put(group); + vfio_iommu_group_put(group, dev); return ret; } @@ -589,7 +589,7 @@ struct vfio_platform_device *vfio_platform_remove_common(struct device *dev) if (vdev) { vfio_platform_put_reset(vdev); - iommu_group_put(dev->iommu_group); + vfio_iommu_group_put(dev->iommu_group, dev); } return vdev; -- cgit v1.2.3 From 05f0c03fbac1819e86c9d5db4e208b68fc1b9b5e Mon Sep 17 00:00:00 2001 From: Yongji Xie Date: Thu, 30 Jun 2016 15:21:24 +0800 Subject: vfio-pci: Allow to mmap sub-page MMIO BARs if the mmio page is exclusive Current vfio-pci implementation disallows to mmap sub-page(size < PAGE_SIZE) MMIO BARs because these BARs' mmio page may be shared with other BARs. This will cause some performance issues when we passthrough a PCI device with this kind of BARs. Guest will be not able to handle the mmio accesses to the BARs which leads to mmio emulations in host. However, not all sub-page BARs will share page with other BARs. We should allow to mmap the sub-page MMIO BARs which we can make sure will not share page with other BARs. This patch adds support for this case. And we try to add a dummy resource to reserve the remainder of the page which hot-add device's BAR might be assigned into. But it's not necessary to handle the case when the BAR is not page aligned. Because we can't expect the BAR will be assigned into the same location in a page in guest when we passthrough the BAR. And it's hard to access this BAR in userspace because we have no way to get the BAR's location in a page. Signed-off-by: Yongji Xie Signed-off-by: Alex Williamson --- drivers/vfio/pci/vfio_pci.c | 88 ++++++++++++++++++++++++++++++++++--- drivers/vfio/pci/vfio_pci_private.h | 8 ++++ 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c index 188b1ff03f5f..d624a527777f 100644 --- a/drivers/vfio/pci/vfio_pci.c +++ b/drivers/vfio/pci/vfio_pci.c @@ -110,6 +110,74 @@ static inline bool vfio_pci_is_vga(struct pci_dev *pdev) return (pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA; } +static void vfio_pci_probe_mmaps(struct vfio_pci_device *vdev) +{ + struct resource *res; + int bar; + struct vfio_pci_dummy_resource *dummy_res; + + INIT_LIST_HEAD(&vdev->dummy_resources_list); + + for (bar = PCI_STD_RESOURCES; bar <= PCI_STD_RESOURCE_END; bar++) { + res = vdev->pdev->resource + bar; + + if (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP)) + goto no_mmap; + + if (!(res->flags & IORESOURCE_MEM)) + goto no_mmap; + + /* + * The PCI core shouldn't set up a resource with a + * type but zero size. But there may be bugs that + * cause us to do that. + */ + if (!resource_size(res)) + goto no_mmap; + + if (resource_size(res) >= PAGE_SIZE) { + vdev->bar_mmap_supported[bar] = true; + continue; + } + + if (!(res->start & ~PAGE_MASK)) { + /* + * Add a dummy resource to reserve the remainder + * of the exclusive page in case that hot-add + * device's bar is assigned into it. + */ + dummy_res = kzalloc(sizeof(*dummy_res), GFP_KERNEL); + if (dummy_res == NULL) + goto no_mmap; + + dummy_res->resource.name = "vfio sub-page reserved"; + dummy_res->resource.start = res->end + 1; + dummy_res->resource.end = res->start + PAGE_SIZE - 1; + dummy_res->resource.flags = res->flags; + if (request_resource(res->parent, + &dummy_res->resource)) { + kfree(dummy_res); + goto no_mmap; + } + dummy_res->index = bar; + list_add(&dummy_res->res_next, + &vdev->dummy_resources_list); + vdev->bar_mmap_supported[bar] = true; + continue; + } + /* + * Here we don't handle the case when the BAR is not page + * aligned because we can't expect the BAR will be + * assigned into the same location in a page in guest + * when we passthrough the BAR. And it's hard to access + * this BAR in userspace because we have no way to get + * the BAR's location in a page. + */ +no_mmap: + vdev->bar_mmap_supported[bar] = false; + } +} + static void vfio_pci_try_bus_reset(struct vfio_pci_device *vdev); static void vfio_pci_disable(struct vfio_pci_device *vdev); @@ -218,12 +286,15 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev) } } + vfio_pci_probe_mmaps(vdev); + return 0; } static void vfio_pci_disable(struct vfio_pci_device *vdev) { struct pci_dev *pdev = vdev->pdev; + struct vfio_pci_dummy_resource *dummy_res, *tmp; int i, bar; /* Stop the device from further DMA */ @@ -252,6 +323,13 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev) vdev->barmap[bar] = NULL; } + list_for_each_entry_safe(dummy_res, tmp, + &vdev->dummy_resources_list, res_next) { + list_del(&dummy_res->res_next); + release_resource(&dummy_res->resource); + kfree(dummy_res); + } + vdev->needs_reset = true; /* @@ -623,9 +701,7 @@ static long vfio_pci_ioctl(void *device_data, info.flags = VFIO_REGION_INFO_FLAG_READ | VFIO_REGION_INFO_FLAG_WRITE; - if (IS_ENABLED(CONFIG_VFIO_PCI_MMAP) && - pci_resource_flags(pdev, info.index) & - IORESOURCE_MEM && info.size >= PAGE_SIZE) { + if (vdev->bar_mmap_supported[info.index]) { info.flags |= VFIO_REGION_INFO_FLAG_MMAP; if (info.index == vdev->msix_bar) { ret = msix_sparse_mmap_cap(vdev, &caps); @@ -1049,16 +1125,16 @@ static int vfio_pci_mmap(void *device_data, struct vm_area_struct *vma) return -EINVAL; if (index >= VFIO_PCI_ROM_REGION_INDEX) return -EINVAL; - if (!(pci_resource_flags(pdev, index) & IORESOURCE_MEM)) + if (!vdev->bar_mmap_supported[index]) return -EINVAL; - phys_len = pci_resource_len(pdev, index); + phys_len = PAGE_ALIGN(pci_resource_len(pdev, index)); req_len = vma->vm_end - vma->vm_start; pgoff = vma->vm_pgoff & ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1); req_start = pgoff << PAGE_SHIFT; - if (phys_len < PAGE_SIZE || req_start + req_len > phys_len) + if (req_start + req_len > phys_len) return -EINVAL; if (index == vdev->msix_bar) { diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h index 016c14a1b454..2128de86c80d 100644 --- a/drivers/vfio/pci/vfio_pci_private.h +++ b/drivers/vfio/pci/vfio_pci_private.h @@ -57,9 +57,16 @@ struct vfio_pci_region { u32 flags; }; +struct vfio_pci_dummy_resource { + struct resource resource; + int index; + struct list_head res_next; +}; + struct vfio_pci_device { struct pci_dev *pdev; void __iomem *barmap[PCI_STD_RESOURCE_END + 1]; + bool bar_mmap_supported[PCI_STD_RESOURCE_END + 1]; u8 *pci_config_map; u8 *vconfig; struct perm_bits *msi_perm; @@ -88,6 +95,7 @@ struct vfio_pci_device { int refcnt; struct eventfd_ctx *err_trigger; struct eventfd_ctx *req_trigger; + struct list_head dummy_resources_list; }; #define is_intx(vdev) (vdev->irq_type == VFIO_PCI_INTX_IRQ_INDEX) -- cgit v1.2.3 From d370c917b9d4bef71e5d994aac5547f06f4dd76f Mon Sep 17 00:00:00 2001 From: Ilya Lesokhin Date: Thu, 14 Jul 2016 16:50:19 +0300 Subject: vfio: fix possible use after free of vfio group The vfio group should be released after the vfio_group_try_dissolve_container call. The code should not rely on someone else to hold a reference on the group. Signed-off-by: Ilya Lesokhin Signed-off-by: Alex Williamson --- drivers/vfio/vfio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 6fd6fa5469de..d1d70e0b011b 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -1711,8 +1711,8 @@ EXPORT_SYMBOL_GPL(vfio_group_get_external_user); void vfio_group_put_external_user(struct vfio_group *group) { - vfio_group_put(group); vfio_group_try_dissolve_container(group); + vfio_group_put(group); } EXPORT_SYMBOL_GPL(vfio_group_put_external_user); -- cgit v1.2.3 From 7aef80cf3187b76373359ed539e382eb34ebfb5d Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Tue, 19 Jul 2016 09:01:41 -0400 Subject: vfio: platform: rename reset function Renaming the reset function to of_reset as it is only used by the device tree based platforms. Signed-off-by: Sinan Kaya Reviewed-by: Eric Auger Reviewed-by: Baptiste Reynal Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_common.c | 30 +++++++++++++-------------- drivers/vfio/platform/vfio_platform_private.h | 6 +++--- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index 993b2f932f80..170479399cde 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -41,7 +41,7 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat, if (!strcmp(iter->compat, compat) && try_module_get(iter->owner)) { *module = iter->owner; - reset_fn = iter->reset; + reset_fn = iter->of_reset; break; } } @@ -51,18 +51,18 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat, static void vfio_platform_get_reset(struct vfio_platform_device *vdev) { - vdev->reset = vfio_platform_lookup_reset(vdev->compat, - &vdev->reset_module); - if (!vdev->reset) { + vdev->of_reset = vfio_platform_lookup_reset(vdev->compat, + &vdev->reset_module); + if (!vdev->of_reset) { request_module("vfio-reset:%s", vdev->compat); - vdev->reset = vfio_platform_lookup_reset(vdev->compat, - &vdev->reset_module); + vdev->of_reset = vfio_platform_lookup_reset(vdev->compat, + &vdev->reset_module); } } static void vfio_platform_put_reset(struct vfio_platform_device *vdev) { - if (vdev->reset) + if (vdev->of_reset) module_put(vdev->reset_module); } @@ -141,9 +141,9 @@ static void vfio_platform_release(void *device_data) mutex_lock(&driver_lock); if (!(--vdev->refcnt)) { - if (vdev->reset) { + if (vdev->of_reset) { dev_info(vdev->device, "reset\n"); - vdev->reset(vdev); + vdev->of_reset(vdev); } else { dev_warn(vdev->device, "no reset function found!\n"); } @@ -175,9 +175,9 @@ static int vfio_platform_open(void *device_data) if (ret) goto err_irq; - if (vdev->reset) { + if (vdev->of_reset) { dev_info(vdev->device, "reset\n"); - vdev->reset(vdev); + vdev->of_reset(vdev); } else { dev_warn(vdev->device, "no reset function found!\n"); } @@ -213,7 +213,7 @@ static long vfio_platform_ioctl(void *device_data, if (info.argsz < minsz) return -EINVAL; - if (vdev->reset) + if (vdev->of_reset) vdev->flags |= VFIO_DEVICE_FLAGS_RESET; info.flags = vdev->flags; info.num_regions = vdev->num_regions; @@ -312,8 +312,8 @@ static long vfio_platform_ioctl(void *device_data, return ret; } else if (cmd == VFIO_DEVICE_RESET) { - if (vdev->reset) - return vdev->reset(vdev); + if (vdev->of_reset) + return vdev->of_reset(vdev); else return -EINVAL; } @@ -611,7 +611,7 @@ void vfio_platform_unregister_reset(const char *compat, mutex_lock(&driver_lock); list_for_each_entry_safe(iter, temp, &reset_list, link) { - if (!strcmp(iter->compat, compat) && (iter->reset == fn)) { + if (!strcmp(iter->compat, compat) && (iter->of_reset == fn)) { list_del(&iter->link); break; } diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index 42816dd280cb..71ed7d1c1b7d 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -71,7 +71,7 @@ struct vfio_platform_device { struct resource* (*get_resource)(struct vfio_platform_device *vdev, int i); int (*get_irq)(struct vfio_platform_device *vdev, int i); - int (*reset)(struct vfio_platform_device *vdev); + int (*of_reset)(struct vfio_platform_device *vdev); }; typedef int (*vfio_platform_reset_fn_t)(struct vfio_platform_device *vdev); @@ -80,7 +80,7 @@ struct vfio_platform_reset_node { struct list_head link; char *compat; struct module *owner; - vfio_platform_reset_fn_t reset; + vfio_platform_reset_fn_t of_reset; }; extern int vfio_platform_probe_common(struct vfio_platform_device *vdev, @@ -103,7 +103,7 @@ extern void vfio_platform_unregister_reset(const char *compat, static struct vfio_platform_reset_node __reset ## _node = { \ .owner = THIS_MODULE, \ .compat = __compat, \ - .reset = __reset, \ + .of_reset = __reset, \ }; \ __vfio_platform_register_reset(&__reset ## _node) -- cgit v1.2.3 From f084aa749537b818d5b234448e960a59836b39e3 Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Tue, 19 Jul 2016 09:01:42 -0400 Subject: vfio: platform: move reset call to a common function The reset call sequence seems to replicate itself multiple times across the file. Grouping them together for maintenance reasons. Signed-off-by: Sinan Kaya Reviewed-by: Eric Auger Reviewed-by: Baptiste Reynal Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_common.c | 30 +++++++++++++--------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index 170479399cde..de419df768e1 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -134,6 +134,17 @@ static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev) kfree(vdev->regions); } +static int vfio_platform_call_reset(struct vfio_platform_device *vdev) +{ + if (vdev->of_reset) { + dev_info(vdev->device, "reset\n"); + return vdev->of_reset(vdev); + } + + dev_warn(vdev->device, "no reset function found!\n"); + return -EINVAL; +} + static void vfio_platform_release(void *device_data) { struct vfio_platform_device *vdev = device_data; @@ -141,12 +152,7 @@ static void vfio_platform_release(void *device_data) mutex_lock(&driver_lock); if (!(--vdev->refcnt)) { - if (vdev->of_reset) { - dev_info(vdev->device, "reset\n"); - vdev->of_reset(vdev); - } else { - dev_warn(vdev->device, "no reset function found!\n"); - } + vfio_platform_call_reset(vdev); vfio_platform_regions_cleanup(vdev); vfio_platform_irq_cleanup(vdev); } @@ -175,12 +181,7 @@ static int vfio_platform_open(void *device_data) if (ret) goto err_irq; - if (vdev->of_reset) { - dev_info(vdev->device, "reset\n"); - vdev->of_reset(vdev); - } else { - dev_warn(vdev->device, "no reset function found!\n"); - } + vfio_platform_call_reset(vdev); } vdev->refcnt++; @@ -312,10 +313,7 @@ static long vfio_platform_ioctl(void *device_data, return ret; } else if (cmd == VFIO_DEVICE_RESET) { - if (vdev->of_reset) - return vdev->of_reset(vdev); - else - return -EINVAL; + return vfio_platform_call_reset(vdev); } return -ENOTTY; -- cgit v1.2.3 From dc5542fb115a9576493831160e31d355b58541d7 Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Tue, 19 Jul 2016 09:01:43 -0400 Subject: vfio: platform: determine reset capability Creating a new function to determine if this driver supports reset function or not. This is an attempt to abstract device tree calls from the rest of the code. Signed-off-by: Sinan Kaya Reviewed-by: Eric Auger Reviewed-by: Baptiste Reynal Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_common.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index de419df768e1..307d61d2aea2 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -49,6 +49,11 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat, return reset_fn; } +static bool vfio_platform_has_reset(struct vfio_platform_device *vdev) +{ + return vdev->of_reset ? true : false; +} + static void vfio_platform_get_reset(struct vfio_platform_device *vdev) { vdev->of_reset = vfio_platform_lookup_reset(vdev->compat, @@ -214,7 +219,7 @@ static long vfio_platform_ioctl(void *device_data, if (info.argsz < minsz) return -EINVAL; - if (vdev->of_reset) + if (vfio_platform_has_reset(vdev)) vdev->flags |= VFIO_DEVICE_FLAGS_RESET; info.flags = vdev->flags; info.num_regions = vdev->num_regions; -- cgit v1.2.3 From a12a9368e192d56175e49d69360d861025a0f6f7 Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Tue, 19 Jul 2016 09:01:44 -0400 Subject: vfio: platform: add support for ACPI probe The code is using the compatible DT string to associate a reset driver with the actual device itself. The compatible string does not exist on ACPI based systems. HID is the unique identifier for a device driver instead. Signed-off-by: Sinan Kaya Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_common.c | 64 ++++++++++++++++++++++++--- drivers/vfio/platform/vfio_platform_private.h | 1 + 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index 307d61d2aea2..716f421d1b01 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -13,6 +13,7 @@ */ #include +#include #include #include #include @@ -49,6 +50,27 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat, return reset_fn; } +static int vfio_platform_acpi_probe(struct vfio_platform_device *vdev, + struct device *dev) +{ + struct acpi_device *adev; + + if (acpi_disabled) + return -ENOENT; + + adev = ACPI_COMPANION(dev); + if (!adev) { + pr_err("VFIO: ACPI companion device not found for %s\n", + vdev->name); + return -ENODEV; + } + +#ifdef CONFIG_ACPI + vdev->acpihid = acpi_device_hid(adev); +#endif + return WARN_ON(!vdev->acpihid) ? -EINVAL : 0; +} + static bool vfio_platform_has_reset(struct vfio_platform_device *vdev) { return vdev->of_reset ? true : false; @@ -547,6 +569,37 @@ static const struct vfio_device_ops vfio_platform_ops = { .mmap = vfio_platform_mmap, }; +int vfio_platform_of_probe(struct vfio_platform_device *vdev, + struct device *dev) +{ + int ret; + + ret = device_property_read_string(dev, "compatible", + &vdev->compat); + if (ret) + pr_err("VFIO: cannot retrieve compat for %s\n", + vdev->name); + + return ret; +} + +/* + * There can be two kernel build combinations. One build where + * ACPI is not selected in Kconfig and another one with the ACPI Kconfig. + * + * In the first case, vfio_platform_acpi_probe will return since + * acpi_disabled is 1. DT user will not see any kind of messages from + * ACPI. + * + * In the second case, both DT and ACPI is compiled in but the system is + * booting with any of these combinations. + * + * If the firmware is DT type, then acpi_disabled is 1. The ACPI probe routine + * terminates immediately without any messages. + * + * If the firmware is ACPI type, then acpi_disabled is 0. All other checks are + * valid checks. We cannot claim that this system is DT. + */ int vfio_platform_probe_common(struct vfio_platform_device *vdev, struct device *dev) { @@ -556,11 +609,12 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, if (!vdev) return -EINVAL; - ret = device_property_read_string(dev, "compatible", &vdev->compat); - if (ret) { - pr_err("VFIO: cannot retrieve compat for %s\n", vdev->name); - return -EINVAL; - } + ret = vfio_platform_acpi_probe(vdev, dev); + if (ret) + ret = vfio_platform_of_probe(vdev, dev); + + if (ret) + return ret; vdev->device = dev; diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index 71ed7d1c1b7d..ba9e4f8b0746 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -58,6 +58,7 @@ struct vfio_platform_device { struct mutex igate; struct module *parent_module; const char *compat; + const char *acpihid; struct module *reset_module; struct device *device; -- cgit v1.2.3 From 5afec27474fdc52e9d80b359ce10fab59c85d131 Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Tue, 19 Jul 2016 09:01:45 -0400 Subject: vfio: platform: add extra debug info argument to call reset Getting ready to bring out extra debug information to the caller so that more verbose information can be printed when an error is observed. Signed-off-by: Sinan Kaya Reviewed-by: Baptiste Reynal Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_common.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index 716f421d1b01..a9760c2e68ad 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -161,7 +161,8 @@ static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev) kfree(vdev->regions); } -static int vfio_platform_call_reset(struct vfio_platform_device *vdev) +static int vfio_platform_call_reset(struct vfio_platform_device *vdev, + const char **extra_dbg) { if (vdev->of_reset) { dev_info(vdev->device, "reset\n"); @@ -179,7 +180,7 @@ static void vfio_platform_release(void *device_data) mutex_lock(&driver_lock); if (!(--vdev->refcnt)) { - vfio_platform_call_reset(vdev); + vfio_platform_call_reset(vdev, NULL); vfio_platform_regions_cleanup(vdev); vfio_platform_irq_cleanup(vdev); } @@ -208,7 +209,7 @@ static int vfio_platform_open(void *device_data) if (ret) goto err_irq; - vfio_platform_call_reset(vdev); + vfio_platform_call_reset(vdev, NULL); } vdev->refcnt++; @@ -340,7 +341,7 @@ static long vfio_platform_ioctl(void *device_data, return ret; } else if (cmd == VFIO_DEVICE_RESET) { - return vfio_platform_call_reset(vdev); + return vfio_platform_call_reset(vdev, NULL); } return -ENOTTY; -- cgit v1.2.3 From d30daa33ec1d035acfdfc7662d7a5360592af44c Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Tue, 19 Jul 2016 09:01:46 -0400 Subject: vfio: platform: call _RST method when using ACPI The device tree code checks for the presence of a reset driver and calls the of_reset function pointer by looking up the reset driver as a module. ACPI defines _RST method to perform device level reset. After the _RST method is executed, the OS can resume using the device. _RST method is expected to stop DMA transfers and IRQs. This patch introduces two functions as vfio_platform_acpi_has_reset and vfio_platform_acpi_call_reset. The has reset method is used to declare reset capability via the ioctl flag VFIO_DEVICE_FLAGS_RESET. The call reset function is used to execute the _RST ACPI method. Signed-off-by: Sinan Kaya Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_common.c | 50 +++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index a9760c2e68ad..a15a69b52080 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -28,6 +28,8 @@ #define DRIVER_AUTHOR "Antonios Motakis " #define DRIVER_DESC "VFIO platform base module" +#define VFIO_PLATFORM_IS_ACPI(vdev) ((vdev)->acpihid != NULL) + static LIST_HEAD(reset_list); static DEFINE_MUTEX(driver_lock); @@ -71,13 +73,53 @@ static int vfio_platform_acpi_probe(struct vfio_platform_device *vdev, return WARN_ON(!vdev->acpihid) ? -EINVAL : 0; } +int vfio_platform_acpi_call_reset(struct vfio_platform_device *vdev, + const char **extra_dbg) +{ +#ifdef CONFIG_ACPI + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; + struct device *dev = vdev->device; + acpi_handle handle = ACPI_HANDLE(dev); + acpi_status acpi_ret; + + acpi_ret = acpi_evaluate_object(handle, "_RST", NULL, &buffer); + if (ACPI_FAILURE(acpi_ret)) { + if (extra_dbg) + *extra_dbg = acpi_format_exception(acpi_ret); + return -EINVAL; + } + + return 0; +#else + return -ENOENT; +#endif +} + +bool vfio_platform_acpi_has_reset(struct vfio_platform_device *vdev) +{ +#ifdef CONFIG_ACPI + struct device *dev = vdev->device; + acpi_handle handle = ACPI_HANDLE(dev); + + return acpi_has_method(handle, "_RST"); +#else + return false; +#endif +} + static bool vfio_platform_has_reset(struct vfio_platform_device *vdev) { + if (VFIO_PLATFORM_IS_ACPI(vdev)) + return vfio_platform_acpi_has_reset(vdev); + return vdev->of_reset ? true : false; } static void vfio_platform_get_reset(struct vfio_platform_device *vdev) { + if (VFIO_PLATFORM_IS_ACPI(vdev)) + return; + vdev->of_reset = vfio_platform_lookup_reset(vdev->compat, &vdev->reset_module); if (!vdev->of_reset) { @@ -89,6 +131,9 @@ static void vfio_platform_get_reset(struct vfio_platform_device *vdev) static void vfio_platform_put_reset(struct vfio_platform_device *vdev) { + if (VFIO_PLATFORM_IS_ACPI(vdev)) + return; + if (vdev->of_reset) module_put(vdev->reset_module); } @@ -164,7 +209,10 @@ static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev) static int vfio_platform_call_reset(struct vfio_platform_device *vdev, const char **extra_dbg) { - if (vdev->of_reset) { + if (VFIO_PLATFORM_IS_ACPI(vdev)) { + dev_info(vdev->device, "reset\n"); + return vfio_platform_acpi_call_reset(vdev, extra_dbg); + } else if (vdev->of_reset) { dev_info(vdev->device, "reset\n"); return vdev->of_reset(vdev); } -- cgit v1.2.3 From b5add544d677d36386c3559cf5db1485b59c4d7d Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Tue, 19 Jul 2016 09:01:47 -0400 Subject: vfio, platform: make reset driver a requirement by default The code was allowing platform devices to be used without a supporting VFIO reset driver. The hardware can be left in some inconsistent state after a guest machine abort. The reset driver will put the hardware back to safe state and disable interrupts before returning the control back to the host machine. Adding a new reset_required kernel module option to platform VFIO drivers. The default value is true for the DT and ACPI based drivers. The reset requirement value for AMBA drivers is set to false and is unchangeable to maintain the existing functionality. New requirements are: 1. A reset function needs to be implemented by the corresponding driver via DT/ACPI. 2. The reset function needs to be discovered via DT/ACPI. The probe of the driver will fail if any of the above conditions are not satisfied. Signed-off-by: Sinan Kaya Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_amba.c | 1 + drivers/vfio/platform/vfio_platform.c | 5 +++++ drivers/vfio/platform/vfio_platform_common.c | 15 +++++++++++---- drivers/vfio/platform/vfio_platform_private.h | 2 ++ 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c index a66479bd0edf..31372fbf6c5b 100644 --- a/drivers/vfio/platform/vfio_amba.c +++ b/drivers/vfio/platform/vfio_amba.c @@ -68,6 +68,7 @@ static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id) vdev->get_resource = get_amba_resource; vdev->get_irq = get_amba_irq; vdev->parent_module = THIS_MODULE; + vdev->reset_required = false; ret = vfio_platform_probe_common(vdev, &adev->dev); if (ret) { diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index b1cc3a768784..6561751a1063 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -23,6 +23,10 @@ #define DRIVER_AUTHOR "Antonios Motakis " #define DRIVER_DESC "VFIO for platform devices - User Level meta-driver" +static bool reset_required = true; +module_param(reset_required, bool, 0444); +MODULE_PARM_DESC(reset_required, "override reset requirement (default: 1)"); + /* probing devices from the linux platform bus */ static struct resource *get_platform_resource(struct vfio_platform_device *vdev, @@ -66,6 +70,7 @@ static int vfio_platform_probe(struct platform_device *pdev) vdev->get_resource = get_platform_resource; vdev->get_irq = get_platform_irq; vdev->parent_module = THIS_MODULE; + vdev->reset_required = reset_required; ret = vfio_platform_probe_common(vdev, &pdev->dev); if (ret) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index a15a69b52080..d8755d2c6053 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -115,10 +115,10 @@ static bool vfio_platform_has_reset(struct vfio_platform_device *vdev) return vdev->of_reset ? true : false; } -static void vfio_platform_get_reset(struct vfio_platform_device *vdev) +static int vfio_platform_get_reset(struct vfio_platform_device *vdev) { if (VFIO_PLATFORM_IS_ACPI(vdev)) - return; + return vfio_platform_acpi_has_reset(vdev) ? 0 : -ENOENT; vdev->of_reset = vfio_platform_lookup_reset(vdev->compat, &vdev->reset_module); @@ -127,6 +127,8 @@ static void vfio_platform_get_reset(struct vfio_platform_device *vdev) vdev->of_reset = vfio_platform_lookup_reset(vdev->compat, &vdev->reset_module); } + + return vdev->of_reset ? 0 : -ENOENT; } static void vfio_platform_put_reset(struct vfio_platform_device *vdev) @@ -667,6 +669,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, vdev->device = dev; + ret = vfio_platform_get_reset(vdev); + if (ret && vdev->reset_required) { + pr_err("vfio: no reset function found for device %s\n", + vdev->name); + return ret; + } + group = vfio_iommu_group_get(dev); if (!group) { pr_err("VFIO: No IOMMU group for device %s\n", vdev->name); @@ -679,8 +688,6 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev, return ret; } - vfio_platform_get_reset(vdev); - mutex_init(&vdev->igate); return 0; diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index ba9e4f8b0746..85ffe5d9d1ab 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -73,6 +73,8 @@ struct vfio_platform_device { (*get_resource)(struct vfio_platform_device *vdev, int i); int (*get_irq)(struct vfio_platform_device *vdev, int i); int (*of_reset)(struct vfio_platform_device *vdev); + + bool reset_required; }; typedef int (*vfio_platform_reset_fn_t)(struct vfio_platform_device *vdev); -- cgit v1.2.3 From e99442323f33470677deeed35619313b05837f35 Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Tue, 19 Jul 2016 09:01:48 -0400 Subject: vfio: platform: check reset call return code during open Open call is ignoring the return code from reset call and can potentially continue even though reset call failed. If reset_required module parameter is set, this patch is going to validate the return code and will abort open if reset fails. Signed-off-by: Sinan Kaya Reviewed-by: Baptiste Reynal Reviewed-by: Eric Auger Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_common.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index d8755d2c6053..389d6515bc09 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -251,6 +251,8 @@ static int vfio_platform_open(void *device_data) mutex_lock(&driver_lock); if (!vdev->refcnt) { + const char *extra_dbg = NULL; + ret = vfio_platform_regions_init(vdev); if (ret) goto err_reg; @@ -259,7 +261,12 @@ static int vfio_platform_open(void *device_data) if (ret) goto err_irq; - vfio_platform_call_reset(vdev, NULL); + ret = vfio_platform_call_reset(vdev, &extra_dbg); + if (ret && vdev->reset_required) { + dev_warn(vdev->device, "reset driver is required and reset call failed in open (%d) %s\n", + ret, extra_dbg ? extra_dbg : ""); + goto err_rst; + } } vdev->refcnt++; @@ -267,6 +274,8 @@ static int vfio_platform_open(void *device_data) mutex_unlock(&driver_lock); return 0; +err_rst: + vfio_platform_irq_cleanup(vdev); err_irq: vfio_platform_regions_cleanup(vdev); err_reg: -- cgit v1.2.3 From 0991bbdbf5b85bd14a26e783f087b0f2913c93b1 Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Tue, 19 Jul 2016 09:01:49 -0400 Subject: vfio: platform: check reset call return code during release Release call is ignoring the return code from reset call and can potentially continue even though reset call failed. If reset_required module parameter is set, this patch is going to validate the return code and will cause stack dump with WARN_ON and warn the user of failure. Signed-off-by: Sinan Kaya Reviewed-by: Eric Auger Reviewed-by: Baptiste Reynal Signed-off-by: Alex Williamson --- drivers/vfio/platform/vfio_platform_common.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index 389d6515bc09..1cf2d462b53d 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -230,7 +230,15 @@ static void vfio_platform_release(void *device_data) mutex_lock(&driver_lock); if (!(--vdev->refcnt)) { - vfio_platform_call_reset(vdev, NULL); + const char *extra_dbg = NULL; + int ret; + + ret = vfio_platform_call_reset(vdev, &extra_dbg); + if (ret && vdev->reset_required) { + dev_warn(vdev->device, "reset driver is required and reset call failed in release (%d) %s\n", + ret, extra_dbg ? extra_dbg : ""); + WARN_ON(1); + } vfio_platform_regions_cleanup(vdev); vfio_platform_irq_cleanup(vdev); } -- cgit v1.2.3