* [RFC PATCH v5 00/11] VFIO support for platform devices @ 2014-04-28 15:52 Antonios Motakis [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: kvm-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A This patch series aims to implement VFIO support for platform devices that reside behind an IOMMU. Examples of such devices are devices behind an ARM SMMU, or behind a Samsung Exynos System MMU. This v5 of the VFIO_PLATFORN patch series includes a number of fixes and improvements over the features already implemented in the series. The next version v6 of the VFIO_PLATFORM patch series is planned to introduce more complete functionality with more comprehensive eventfd support. The API used is based on 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 complex relationships with other devices on a device tree are not taken into account at this stage. A copy with all the dependencies applied can be cloned from branch vfio-platform-v5 at git-9UaJU3cA/F/QT0dZR+AlfA@public.gmane.org:virtualopensystems/linux-kvm-arm.git For those who want to apply manually, these patches are based on Alex Williamson's 'next' branch available from https://github.com/awilliam/linux-vfio which includes the multi domain support patches for VFIO_IOMMU_TYPE1. Kim Phillip's driver_override patch for platform bus devices is also included here for convenience. The following IOCTLs have been found to be working on FastModels with an ARM SMMU (MMU400). Testing was based on the ARM PL330 DMA Controller featured on those models. - VFIO_GET_API_VERSION - VFIO_CHECK_EXTENSION - VFIO_GROUP_GET_STATUS - VFIO_GROUP_SET_CONTAINER - VFIO_SET_IOMMU - VFIO_IOMMU_GET_INFO - VFIO_IOMMU_MAP_DMA For this ioctl specifically, a new flag has been added: VFIO_DMA_MAP_FLAG_EXEC. This flag is taken into account on systems with an ARM SMMU. The availability of this flag is exposed via the capability VFIO_IOMMU_PROT_EXEC. 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 - VFIO_DEVICE_SET_IRQS IRQs are implemented partially using this ioctl. Handling incoming interrupts with an eventfd is supported, as is masking and unmasking. Level sensitive interrupts are automasked. What is not implemented is masking/unmasking via eventfd. More comprehensive support for eventfd functionality is planned for v6. 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: - Eventfd for masking/unmasking (IRQFD and IOEVENTFD from the KVM side) - Extend the driver and API for device tree metadata - Support ARM AMBA devices natively - Device specific functionality (e.g. VFIO_DEVICE_RESET) - IOMMUs with nested page tables (Stage 1 & 2 translation on ARM SMMUs) Changes since v4: - Use static offsets for each region in the VFIO device fd - Include patch in the series for the ARM SMMU to expose IOMMU_EXEC availability via IOMMU_CAP_DMA_EXEC - Rebased on VFIO multi domain support: - IOMMU_EXEC is now available if at least one IOMMU in the container supports it - Expose IOMMU_EXEC if available via the capability VFIO_IOMMU_PROT_EXEC - Some bug fixes Changes since v3: - Use Kim Phillips' driver_probe_device() Changes since v2: - Fixed Read/Write and MMAP on device regions - Removed dependency on Device Tree - Interrupts support - Interrupt masking/unmasking - Automask level sensitive interrupts - Introduced VFIO_DMA_MAP_FLAG_EXEC - Code clean ups Antonios Motakis (10): ARM SMMU: Add capability IOMMU_CAP_DMA_EXEC VFIO_IOMMU_TYPE1 for platform bus devices on ARM VFIO_IOMMU_TYPE1: Introduce the VFIO_DMA_MAP_FLAG_EXEC flag VFIO_PLATFORM: Initial skeleton of VFIO support for platform devices VFIO_PLATFORM: Return info for device and its memory mapped IO regions VFIO_PLATFORM: Read and write support for the device fd VFIO_PLATFORM: Support MMAP of MMIO regions VFIO_PLATFORM: Return IRQ info VFIO_PLATFORM: Initial interrupts support VFIO_PLATFORM: Support for maskable and automasked interrupts Kim Phillips (1): driver core: platform: add device binding path 'driver_override' Documentation/ABI/testing/sysfs-bus-platform | 20 ++ drivers/base/platform.c | 46 +++ drivers/iommu/arm-smmu.c | 2 + drivers/vfio/Kconfig | 3 +- drivers/vfio/Makefile | 1 + drivers/vfio/platform/Kconfig | 9 + drivers/vfio/platform/Makefile | 4 + drivers/vfio/platform/vfio_platform.c | 436 ++++++++++++++++++++++++++ drivers/vfio/platform/vfio_platform_irq.c | 289 +++++++++++++++++ drivers/vfio/platform/vfio_platform_private.h | 59 ++++ drivers/vfio/vfio_iommu_type1.c | 39 ++- include/linux/iommu.h | 5 +- include/linux/platform_device.h | 1 + include/uapi/linux/vfio.h | 3 + 14 files changed, 910 insertions(+), 7 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-platform create mode 100644 drivers/vfio/platform/Kconfig create mode 100644 drivers/vfio/platform/Makefile create mode 100644 drivers/vfio/platform/vfio_platform.c create mode 100644 drivers/vfio/platform/vfio_platform_irq.c create mode 100644 drivers/vfio/platform/vfio_platform_private.h -- 1.8.3.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* [RFC PATCH v5 01/11] driver core: platform: add device binding path 'driver_override' [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 15:52 ` Antonios Motakis 2014-05-21 0:25 ` [RFC PATCH v5_v2 " Kim Phillips 2014-06-03 0:42 ` [PATCH] " Kim Phillips 2014-04-28 15:52 ` [RFC PATCH v5 02/11] ARM SMMU: Add capability IOMMU_CAP_DMA_EXEC Antonios Motakis ` (9 subsequent siblings) 10 siblings, 2 replies; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: open list, kvm-u79uwXL29TY76Z2rM5mHXA, open list:DOCUMENTATION, Greg Kroah-Hartman, Johan Hovold, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, Rob Herring, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Rob Landley, Grant Likely, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, Libo Chen, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A From: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> Needed by platform device drivers, such as the vfio-platform driver [1], in order to bypass the existing OF, ACPI, id_table and name string matches, and successfully be able to be bound to any device, like so: echo vfio-platform > /sys/bus/platform/devices/fff51000.ethernet/driver_override echo fff51000.ethernet > /sys/bus/platform/devices/fff51000.ethernet/driver/unbind echo fff51000.ethernet > /sys/bus/platform/drivers_probe This mimics "PCI: Introduce new device binding path using pci_dev.driver_override" [2], which is an interface enhancement for more deterministic PCI device binding, e.g., when in the presence of hotplug. [1] http://lkml.iu.edu/hypermail/linux/kernel/1402.1/00177.html [2] http://lists-archives.com/linux-kernel/28030441-pci-introduce-new-device-binding-path-using-pci_dev-driver_override.html Suggested-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> --- Documentation/ABI/testing/sysfs-bus-platform | 20 ++++++++++++ drivers/base/platform.c | 46 ++++++++++++++++++++++++++++ include/linux/platform_device.h | 1 + 3 files changed, 67 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-platform diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform new file mode 100644 index 0000000..5172a61 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-platform @@ -0,0 +1,20 @@ +What: /sys/bus/platform/devices/.../driver_override +Date: April 2014 +Contact: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> +Description: + This file allows the driver for a device to be specified which + will override standard OF, ACPI, ID table, and name matching. + When specified, only a driver with a name matching the value + written to driver_override will have an opportunity to bind + to the device. The override is specified by writing a string + to the driver_override file (echo vfio-platform > \ + driver_override) and may be cleared with an empty string + (echo > driver_override). This returns the device to standard + matching rules binding. Writing to driver_override does not + automatically unbind the device from its current driver or make + any attempt to automatically load the specified driver. If no + driver with a matching name is currently loaded in the kernel, + the device will not bind to any driver. This also allows + devices to opt-out of driver binding using a driver_override + name such as "none". Only a single driver may be specified in + the override, there is no support for parsing delimiters. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index bc78848..6db9fc2 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -22,6 +22,7 @@ #include <linux/pm_runtime.h> #include <linux/idr.h> #include <linux/acpi.h> +#include <linux/limits.h> #include "base.h" #include "power/power.h" @@ -693,8 +694,49 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a, } static DEVICE_ATTR_RO(modalias); +static ssize_t driver_override_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct platform_device *pdev = to_platform_device(dev); + char *driver_override, *old = pdev->driver_override, *cp; + + if (count > PATH_MAX) + return -EINVAL; + + driver_override = kstrndup(buf, count, GFP_KERNEL); + if (!driver_override) + return -ENOMEM; + + cp = strchr(driver_override, '\n'); + if (cp) + *cp = '\0'; + + if (strlen(driver_override)) { + pdev->driver_override = driver_override; + } else { + kfree(driver_override); + pdev->driver_override = NULL; + } + + kfree(old); + + return count; +} + +static ssize_t driver_override_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct platform_device *pdev = to_platform_device(dev); + + return sprintf(buf, "%s\n", pdev->driver_override); +} +static DEVICE_ATTR_RW(driver_override); + + static struct attribute *platform_dev_attrs[] = { &dev_attr_modalias.attr, + &dev_attr_driver_override.attr, NULL, }; ATTRIBUTE_GROUPS(platform_dev); @@ -750,6 +792,10 @@ static int platform_match(struct device *dev, struct device_driver *drv) struct platform_device *pdev = to_platform_device(dev); struct platform_driver *pdrv = to_platform_driver(drv); + /* When driver_override is set, only bind to the matching driver */ + if (pdev->driver_override) + return !strcmp(pdev->driver_override, drv->name); + /* Attempt an OF style match first */ if (of_driver_match_device(dev, drv)) return 1; diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 16f6654..153d303 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -28,6 +28,7 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; + char *driver_override; /* Driver name to force a match */ /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [RFC PATCH v5_v2 01/11] driver core: platform: add device binding path 'driver_override' 2014-04-28 15:52 ` [RFC PATCH v5 01/11] driver core: platform: add device binding path 'driver_override' Antonios Motakis @ 2014-05-21 0:25 ` Kim Phillips [not found] ` <20140520192537.bab9fa2088c1cd5da3f92639-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-06-03 0:42 ` [PATCH] " Kim Phillips 1 sibling, 1 reply; 42+ messages in thread From: Kim Phillips @ 2014-05-21 0:25 UTC (permalink / raw) To: a.motakis Cc: alex.williamson, kvmarm, iommu, linux-kernel, kvm, linux-doc, gregkh, jhovold, will.deacon, a.rigo, rob.herring, stuart.yoder, rob, grant.likely, tech, clbchenlibo.chen, christoffer.dall, eric.auger, Julien Grall, Ian Campbell, Stefano Stabellini, Alexander Graf From: Kim Phillips <kim.phillips@freescale.com> Needed by platform device drivers, such as the vfio-platform driver later in series, in order to bypass the existing OF, ACPI, id_table and name string matches, and successfully be able to be bound to any device, like so: echo vfio-platform > /sys/bus/platform/devices/fff51000.ethernet/driver_override echo fff51000.ethernet > /sys/bus/platform/devices/fff51000.ethernet/driver/unbind echo fff51000.ethernet > /sys/bus/platform/drivers_probe This mimics "PCI: Introduce new device binding path using pci_dev.driver_override" [1], which is an interface enhancement for more deterministic PCI device binding, e.g., when in the presence of hotplug. [1] https://lists.cs.columbia.edu/pipermail/kvmarm/2014-May/009527.html Suggested-by: Alex Williamson <alex.williamson@redhat.com> Signed-off-by: Kim Phillips <kim.phillips@freescale.com> --- changes in v2 patch of v5 of this patchseries: - rebased onto today's Linus' ToT - added kfree to match PCI counterpart fix, as Alex Williamson just posted a v3 of the patch (thanks Christoffer for the notification) - in the commit text, replaced vfio platform driver reference with 'later in series', and updated the PCI version mailing list reference to the v3 version. Is it safe to assume this patch will continue to as part of the VFIO platform driver patchseries, and be submitted by Antonis? If so, can we start collecting some {Reviewed,Acked}-bys? Thanks, Kim. Documentation/ABI/testing/sysfs-bus-platform | 20 ++++++++++++ drivers/base/platform.c | 47 ++++++++++++++++++++++++++++ include/linux/platform_device.h | 1 + 3 files changed, 68 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-platform diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform new file mode 100644 index 0000000..5172a61 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-platform @@ -0,0 +1,20 @@ +What: /sys/bus/platform/devices/.../driver_override +Date: April 2014 +Contact: Kim Phillips <kim.phillips@freescale.com> +Description: + This file allows the driver for a device to be specified which + will override standard OF, ACPI, ID table, and name matching. + When specified, only a driver with a name matching the value + written to driver_override will have an opportunity to bind + to the device. The override is specified by writing a string + to the driver_override file (echo vfio-platform > \ + driver_override) and may be cleared with an empty string + (echo > driver_override). This returns the device to standard + matching rules binding. Writing to driver_override does not + automatically unbind the device from its current driver or make + any attempt to automatically load the specified driver. If no + driver with a matching name is currently loaded in the kernel, + the device will not bind to any driver. This also allows + devices to opt-out of driver binding using a driver_override + name such as "none". Only a single driver may be specified in + the override, there is no support for parsing delimiters. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 5b47210..4f47563 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -23,6 +23,7 @@ #include <linux/pm_runtime.h> #include <linux/idr.h> #include <linux/acpi.h> +#include <linux/limits.h> #include "base.h" #include "power/power.h" @@ -188,6 +189,7 @@ static void platform_device_release(struct device *dev) kfree(pa->pdev.dev.platform_data); kfree(pa->pdev.mfd_cell); kfree(pa->pdev.resource); + kfree(pa->pdev.driver_override); kfree(pa); } @@ -695,8 +697,49 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a, } static DEVICE_ATTR_RO(modalias); +static ssize_t driver_override_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct platform_device *pdev = to_platform_device(dev); + char *driver_override, *old = pdev->driver_override, *cp; + + if (count > PATH_MAX) + return -EINVAL; + + driver_override = kstrndup(buf, count, GFP_KERNEL); + if (!driver_override) + return -ENOMEM; + + cp = strchr(driver_override, '\n'); + if (cp) + *cp = '\0'; + + if (strlen(driver_override)) { + pdev->driver_override = driver_override; + } else { + kfree(driver_override); + pdev->driver_override = NULL; + } + + kfree(old); + + return count; +} + +static ssize_t driver_override_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct platform_device *pdev = to_platform_device(dev); + + return sprintf(buf, "%s\n", pdev->driver_override); +} +static DEVICE_ATTR_RW(driver_override); + + static struct attribute *platform_dev_attrs[] = { &dev_attr_modalias.attr, + &dev_attr_driver_override.attr, NULL, }; ATTRIBUTE_GROUPS(platform_dev); @@ -752,6 +795,10 @@ static int platform_match(struct device *dev, struct device_driver *drv) struct platform_device *pdev = to_platform_device(dev); struct platform_driver *pdrv = to_platform_driver(drv); + /* When driver_override is set, only bind to the matching driver */ + if (pdev->driver_override) + return !strcmp(pdev->driver_override, drv->name); + /* Attempt an OF style match first */ if (of_driver_match_device(dev, drv)) return 1; diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 16f6654..153d303 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -28,6 +28,7 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; + char *driver_override; /* Driver name to force a match */ /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- 1.9.3 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <20140520192537.bab9fa2088c1cd5da3f92639-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* Re: [RFC PATCH v5_v2 01/11] driver core: platform: add device binding path 'driver_override' [not found] ` <20140520192537.bab9fa2088c1cd5da3f92639-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> @ 2014-05-29 19:43 ` Alex Williamson [not found] ` <1401392619.2412.102.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-05-30 0:36 ` Stuart Yoder 1 sibling, 1 reply; 42+ messages in thread From: Alex Williamson @ 2014-05-29 19:43 UTC (permalink / raw) To: Kim Phillips Cc: kvm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, clbchenlibo.chen-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8, stuart.yoder-KZfg59tc24xl57MIdRCFDg, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, eric.auger-QSEj5FYQhm4dnm+yROfE0A, jhovold-Re5JQEeQqe8AvxtiuMwx3w, Alexander Graf, grant.likely-QSEj5FYQhm4dnm+yROfE0A, Ian Campbell, Stefano Stabellini, Julien Grall, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ, a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, rob-VoJi6FS/r0vR7s880joybQ, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Tue, 2014-05-20 at 19:25 -0500, Kim Phillips wrote: > From: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> > > Needed by platform device drivers, such as the vfio-platform driver > later in series, in order to bypass the existing OF, ACPI, id_table and > name string matches, and successfully be able to be bound to any > device, like so: > > echo vfio-platform > /sys/bus/platform/devices/fff51000.ethernet/driver_override > echo fff51000.ethernet > /sys/bus/platform/devices/fff51000.ethernet/driver/unbind > echo fff51000.ethernet > /sys/bus/platform/drivers_probe > > This mimics "PCI: Introduce new device binding path using > pci_dev.driver_override" [1], which is an interface enhancement > for more deterministic PCI device binding, e.g., when in the > presence of hotplug. > > [1] https://lists.cs.columbia.edu/pipermail/kvmarm/2014-May/009527.html > > Suggested-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> Looks largely identical to the PCI version of the same that has been accepted for v3.16 and ack'd by GregKH. Reviewed-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > --- > changes in v2 patch of v5 of this patchseries: > - rebased onto today's Linus' ToT > - added kfree to match PCI counterpart fix, as Alex Williamson > just posted a v3 of the patch (thanks Christoffer for the > notification) > - in the commit text, replaced vfio platform driver reference with > 'later in series', and updated the PCI version mailing list reference > to the v3 version. > > Is it safe to assume this patch will continue to as part of the VFIO > platform driver patchseries, and be submitted by Antonis? If so, can > we start collecting some {Reviewed,Acked}-bys? Thanks, Kim. > > Documentation/ABI/testing/sysfs-bus-platform | 20 ++++++++++++ > drivers/base/platform.c | 47 ++++++++++++++++++++++++++++ > include/linux/platform_device.h | 1 + > 3 files changed, 68 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-platform > > diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform > new file mode 100644 > index 0000000..5172a61 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-platform > @@ -0,0 +1,20 @@ > +What: /sys/bus/platform/devices/.../driver_override > +Date: April 2014 > +Contact: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> > +Description: > + This file allows the driver for a device to be specified which > + will override standard OF, ACPI, ID table, and name matching. > + When specified, only a driver with a name matching the value > + written to driver_override will have an opportunity to bind > + to the device. The override is specified by writing a string > + to the driver_override file (echo vfio-platform > \ > + driver_override) and may be cleared with an empty string > + (echo > driver_override). This returns the device to standard > + matching rules binding. Writing to driver_override does not > + automatically unbind the device from its current driver or make > + any attempt to automatically load the specified driver. If no > + driver with a matching name is currently loaded in the kernel, > + the device will not bind to any driver. This also allows > + devices to opt-out of driver binding using a driver_override > + name such as "none". Only a single driver may be specified in > + the override, there is no support for parsing delimiters. > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index 5b47210..4f47563 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -23,6 +23,7 @@ > #include <linux/pm_runtime.h> > #include <linux/idr.h> > #include <linux/acpi.h> > +#include <linux/limits.h> > > #include "base.h" > #include "power/power.h" > @@ -188,6 +189,7 @@ static void platform_device_release(struct device *dev) > kfree(pa->pdev.dev.platform_data); > kfree(pa->pdev.mfd_cell); > kfree(pa->pdev.resource); > + kfree(pa->pdev.driver_override); > kfree(pa); > } > > @@ -695,8 +697,49 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a, > } > static DEVICE_ATTR_RO(modalias); > > +static ssize_t driver_override_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + char *driver_override, *old = pdev->driver_override, *cp; > + > + if (count > PATH_MAX) > + return -EINVAL; > + > + driver_override = kstrndup(buf, count, GFP_KERNEL); > + if (!driver_override) > + return -ENOMEM; > + > + cp = strchr(driver_override, '\n'); > + if (cp) > + *cp = '\0'; > + > + if (strlen(driver_override)) { > + pdev->driver_override = driver_override; > + } else { > + kfree(driver_override); > + pdev->driver_override = NULL; > + } > + > + kfree(old); > + > + return count; > +} > + > +static ssize_t driver_override_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + > + return sprintf(buf, "%s\n", pdev->driver_override); > +} > +static DEVICE_ATTR_RW(driver_override); > + > + > static struct attribute *platform_dev_attrs[] = { > &dev_attr_modalias.attr, > + &dev_attr_driver_override.attr, > NULL, > }; > ATTRIBUTE_GROUPS(platform_dev); > @@ -752,6 +795,10 @@ static int platform_match(struct device *dev, struct device_driver *drv) > struct platform_device *pdev = to_platform_device(dev); > struct platform_driver *pdrv = to_platform_driver(drv); > > + /* When driver_override is set, only bind to the matching driver */ > + if (pdev->driver_override) > + return !strcmp(pdev->driver_override, drv->name); > + > /* Attempt an OF style match first */ > if (of_driver_match_device(dev, drv)) > return 1; > diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h > index 16f6654..153d303 100644 > --- a/include/linux/platform_device.h > +++ b/include/linux/platform_device.h > @@ -28,6 +28,7 @@ struct platform_device { > struct resource *resource; > > const struct platform_device_id *id_entry; > + char *driver_override; /* Driver name to force a match */ > > /* MFD cell pointer */ > struct mfd_cell *mfd_cell; ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1401392619.2412.102.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>]
* Re: [RFC PATCH v5_v2 01/11] driver core: platform: add device binding path 'driver_override' [not found] ` <1401392619.2412.102.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> @ 2014-05-29 21:24 ` Alexander Graf 0 siblings, 0 replies; 42+ messages in thread From: Alexander Graf @ 2014-05-29 21:24 UTC (permalink / raw) To: Alex Williamson, Kim Phillips Cc: Julien Grall, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Ian Campbell, kvm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jhovold-Re5JQEeQqe8AvxtiuMwx3w, eric.auger-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, clbchenlibo.chen-hv44wF8Li93QT0dZR+AlfA, rob-VoJi6FS/r0vR7s880joybQ, grant.likely-QSEj5FYQhm4dnm+yROfE0A, a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, Stefano Stabellini, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On 29.05.14 21:43, Alex Williamson wrote: > On Tue, 2014-05-20 at 19:25 -0500, Kim Phillips wrote: >> From: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> >> >> Needed by platform device drivers, such as the vfio-platform driver >> later in series, in order to bypass the existing OF, ACPI, id_table and >> name string matches, and successfully be able to be bound to any >> device, like so: >> >> echo vfio-platform > /sys/bus/platform/devices/fff51000.ethernet/driver_override >> echo fff51000.ethernet > /sys/bus/platform/devices/fff51000.ethernet/driver/unbind >> echo fff51000.ethernet > /sys/bus/platform/drivers_probe >> >> This mimics "PCI: Introduce new device binding path using >> pci_dev.driver_override" [1], which is an interface enhancement >> for more deterministic PCI device binding, e.g., when in the >> presence of hotplug. >> >> [1] https://lists.cs.columbia.edu/pipermail/kvmarm/2014-May/009527.html >> >> Suggested-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> > Looks largely identical to the PCI version of the same that has been > accepted for v3.16 and ack'd by GregKH. > > Reviewed-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Yup, would be great to have feature parity for device binding on platform and PCI. Reviewed-by: Alexander Graf <agraf-l3A5Bk7waGM@public.gmane.org> Alex ^ permalink raw reply [flat|nested] 42+ messages in thread
* RE: [RFC PATCH v5_v2 01/11] driver core: platform: add device binding path 'driver_override' [not found] ` <20140520192537.bab9fa2088c1cd5da3f92639-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-05-29 19:43 ` Alex Williamson @ 2014-05-30 0:36 ` Stuart Yoder 1 sibling, 0 replies; 42+ messages in thread From: Stuart Yoder @ 2014-05-30 0:36 UTC (permalink / raw) To: Kim Phillips, a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org Cc: Alexander Graf, Julien Grall, Ian Campbell, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org, jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, will.deacon-5wv7dgnIgG8@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, clbchenlibo.chen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org, rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org, grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, Stefano Stabellini, kvmarm-FPEHb7Xf0XX1GAl8QSHYmA > -----Original Message----- > From: Kim Phillips [mailto:kim.phillips-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org] > Sent: Tuesday, May 20, 2014 7:26 PM > To: a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org > Cc: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org; > iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; > kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; > gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org; jhovold-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; will.deacon-5wv7dgnIgG8@public.gmane.org; > a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org; rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org; Yoder Stuart- > B08248; rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org; grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; > tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org; clbchenlibo.chen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org; > christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; Julien Grall; Ian > Campbell; Stefano Stabellini; Alexander Graf > Subject: [RFC PATCH v5_v2 01/11] driver core: platform: add device > binding path 'driver_override' > > From: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> > > Needed by platform device drivers, such as the vfio-platform driver > later in series, in order to bypass the existing OF, ACPI, id_table and > name string matches, and successfully be able to be bound to any > device, like so: > > echo vfio-platform > > /sys/bus/platform/devices/fff51000.ethernet/driver_override > echo fff51000.ethernet > > /sys/bus/platform/devices/fff51000.ethernet/driver/unbind > echo fff51000.ethernet > /sys/bus/platform/drivers_probe > > This mimics "PCI: Introduce new device binding path using > pci_dev.driver_override" [1], which is an interface enhancement > for more deterministic PCI device binding, e.g., when in the > presence of hotplug. > > [1] https://lists.cs.columbia.edu/pipermail/kvmarm/2014-May/009527.html > > Suggested-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> > --- > changes in v2 patch of v5 of this patchseries: > - rebased onto today's Linus' ToT > - added kfree to match PCI counterpart fix, as Alex Williamson > just posted a v3 of the patch (thanks Christoffer for the > notification) > - in the commit text, replaced vfio platform driver reference with > 'later in series', and updated the PCI version mailing list reference > to the v3 version. Reviewed-by: Stuart Yoder <stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org> Stuart ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] driver core: platform: add device binding path 'driver_override' 2014-04-28 15:52 ` [RFC PATCH v5 01/11] driver core: platform: add device binding path 'driver_override' Antonios Motakis 2014-05-21 0:25 ` [RFC PATCH v5_v2 " Kim Phillips @ 2014-06-03 0:42 ` Kim Phillips [not found] ` <20140602194258.5135925bf3c71b9c010fb409-KZfg59tc24xl57MIdRCFDg@public.gmane.org> 1 sibling, 1 reply; 42+ messages in thread From: Kim Phillips @ 2014-06-03 0:42 UTC (permalink / raw) To: a.motakis Cc: alex.williamson, kvmarm, iommu, linux-kernel, kvm, linux-doc, gregkh, jhovold, will.deacon, a.rigo, rob.herring, stuart.yoder, rob, grant.likely, tech, clbchenlibo.chen, christoffer.dall, eric.auger, Julien Grall, Ian Campbell, Stefano Stabellini, Alexander Graf Needed by platform device drivers, such as the upcoming vfio-platform driver, in order to bypass the existing OF, ACPI, id_table and name string matches, and successfully be able to be bound to any device, like so: echo vfio-platform > /sys/bus/platform/devices/fff51000.ethernet/driver_override echo fff51000.ethernet > /sys/bus/platform/devices/fff51000.ethernet/driver/unbind echo fff51000.ethernet > /sys/bus/platform/drivers_probe This mimics "PCI: Introduce new device binding path using pci_dev.driver_override", which is an interface enhancement for more deterministic PCI device binding, e.g., when in the presence of hotplug. Reviewed-by: Alex Williamson <alex.williamson@redhat.com> Reviewed-by: Alexander Graf <agraf@suse.de> Reviewed-by: Stuart Yoder <stuart.yoder@freescale.com> Signed-off-by: Kim Phillips <kim.phillips@freescale.com> --- Greg, This is largely identical to the PCI version of the same that has been accepted for v3.16 and ack'd by you: https://lists.cs.columbia.edu/pipermail/kvmarm/2014-May/009674.html and applied to Bjorn Helgaas' PCI tree: https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=782a985d7af26db39e86070d28f987cad21313c0 You are the platform driver core maintainer: can you apply this to your driver-core tree now? Thanks, Kim changes since previous version (v2 of the v5 vfio-platform driver series): - commit text: replaced vfio platform driver reference with 'upcoming', and removed the PCI version mailing list reference since it has now been accepted. - added Alex W., Alex G., and Stuart's Reviewed-by's. changes in v2 patch of v5 of this patchseries: - rebased onto today's Linus' ToT - added kfree to match PCI counterpart fix, as Alex Williamson just posted a v3 of the patch (thanks Christoffer for the notification) - in the commit text, replaced vfio platform driver reference with 'later in series', and updated the PCI version mailing list reference to the v3 version. Documentation/ABI/testing/sysfs-bus-platform | 20 ++++++++++++ drivers/base/platform.c | 47 ++++++++++++++++++++++++++++ include/linux/platform_device.h | 1 + 3 files changed, 68 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-platform diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform new file mode 100644 index 0000000..5172a61 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-platform @@ -0,0 +1,20 @@ +What: /sys/bus/platform/devices/.../driver_override +Date: April 2014 +Contact: Kim Phillips <kim.phillips@freescale.com> +Description: + This file allows the driver for a device to be specified which + will override standard OF, ACPI, ID table, and name matching. + When specified, only a driver with a name matching the value + written to driver_override will have an opportunity to bind + to the device. The override is specified by writing a string + to the driver_override file (echo vfio-platform > \ + driver_override) and may be cleared with an empty string + (echo > driver_override). This returns the device to standard + matching rules binding. Writing to driver_override does not + automatically unbind the device from its current driver or make + any attempt to automatically load the specified driver. If no + driver with a matching name is currently loaded in the kernel, + the device will not bind to any driver. This also allows + devices to opt-out of driver binding using a driver_override + name such as "none". Only a single driver may be specified in + the override, there is no support for parsing delimiters. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 5b47210..4f47563 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -23,6 +23,7 @@ #include <linux/pm_runtime.h> #include <linux/idr.h> #include <linux/acpi.h> +#include <linux/limits.h> #include "base.h" #include "power/power.h" @@ -188,6 +189,7 @@ static void platform_device_release(struct device *dev) kfree(pa->pdev.dev.platform_data); kfree(pa->pdev.mfd_cell); kfree(pa->pdev.resource); + kfree(pa->pdev.driver_override); kfree(pa); } @@ -695,8 +697,49 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a, } static DEVICE_ATTR_RO(modalias); +static ssize_t driver_override_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct platform_device *pdev = to_platform_device(dev); + char *driver_override, *old = pdev->driver_override, *cp; + + if (count > PATH_MAX) + return -EINVAL; + + driver_override = kstrndup(buf, count, GFP_KERNEL); + if (!driver_override) + return -ENOMEM; + + cp = strchr(driver_override, '\n'); + if (cp) + *cp = '\0'; + + if (strlen(driver_override)) { + pdev->driver_override = driver_override; + } else { + kfree(driver_override); + pdev->driver_override = NULL; + } + + kfree(old); + + return count; +} + +static ssize_t driver_override_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct platform_device *pdev = to_platform_device(dev); + + return sprintf(buf, "%s\n", pdev->driver_override); +} +static DEVICE_ATTR_RW(driver_override); + + static struct attribute *platform_dev_attrs[] = { &dev_attr_modalias.attr, + &dev_attr_driver_override.attr, NULL, }; ATTRIBUTE_GROUPS(platform_dev); @@ -752,6 +795,10 @@ static int platform_match(struct device *dev, struct device_driver *drv) struct platform_device *pdev = to_platform_device(dev); struct platform_driver *pdrv = to_platform_driver(drv); + /* When driver_override is set, only bind to the matching driver */ + if (pdev->driver_override) + return !strcmp(pdev->driver_override, drv->name); + /* Attempt an OF style match first */ if (of_driver_match_device(dev, drv)) return 1; diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h index 16f6654..153d303 100644 --- a/include/linux/platform_device.h +++ b/include/linux/platform_device.h @@ -28,6 +28,7 @@ struct platform_device { struct resource *resource; const struct platform_device_id *id_entry; + char *driver_override; /* Driver name to force a match */ /* MFD cell pointer */ struct mfd_cell *mfd_cell; -- 1.9.3 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <20140602194258.5135925bf3c71b9c010fb409-KZfg59tc24xl57MIdRCFDg@public.gmane.org>]
* Re: [PATCH] driver core: platform: add device binding path 'driver_override' [not found] ` <20140602194258.5135925bf3c71b9c010fb409-KZfg59tc24xl57MIdRCFDg@public.gmane.org> @ 2014-06-03 4:28 ` Greg KH [not found] ` <20140603042842.GD14668-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> 2014-07-08 22:37 ` Greg KH 1 sibling, 1 reply; 42+ messages in thread From: Greg KH @ 2014-06-03 4:28 UTC (permalink / raw) To: Kim Phillips Cc: kvm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, clbchenlibo.chen-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8, stuart.yoder-KZfg59tc24xl57MIdRCFDg, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, eric.auger-QSEj5FYQhm4dnm+yROfE0A, jhovold-Re5JQEeQqe8AvxtiuMwx3w, Alexander Graf, grant.likely-QSEj5FYQhm4dnm+yROfE0A, Ian Campbell, Stefano Stabellini, Julien Grall, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ, a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, rob-VoJi6FS/r0vR7s880joybQ, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, Jun 02, 2014 at 07:42:58PM -0500, Kim Phillips wrote: > Needed by platform device drivers, such as the upcoming > vfio-platform driver, in order to bypass the existing OF, ACPI, > id_table and name string matches, and successfully be able to be > bound to any device, like so: > > echo vfio-platform > /sys/bus/platform/devices/fff51000.ethernet/driver_override > echo fff51000.ethernet > /sys/bus/platform/devices/fff51000.ethernet/driver/unbind > echo fff51000.ethernet > /sys/bus/platform/drivers_probe > > This mimics "PCI: Introduce new device binding path using > pci_dev.driver_override", which is an interface enhancement > for more deterministic PCI device binding, e.g., when in the > presence of hotplug. > > Reviewed-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > Reviewed-by: Alexander Graf <agraf-l3A5Bk7waGM@public.gmane.org> > Reviewed-by: Stuart Yoder <stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org> > Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> > --- > Greg, > > This is largely identical to the PCI version of the same that has > been accepted for v3.16 and ack'd by you: > > https://lists.cs.columbia.edu/pipermail/kvmarm/2014-May/009674.html > > and applied to Bjorn Helgaas' PCI tree: > > https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=782a985d7af26db39e86070d28f987cad21313c0 > > You are the platform driver core maintainer: can you apply this to > your driver-core tree now? Yes, I will after this merge window ends, it's too late for 3.16-rc1 with the window opening up a week early, sorry. greg k-h ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <20140603042842.GD14668-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] driver core: platform: add device binding path 'driver_override' [not found] ` <20140603042842.GD14668-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> @ 2014-06-26 1:08 ` Kim Phillips 2014-07-07 21:19 ` Kim Phillips 1 sibling, 0 replies; 42+ messages in thread From: Kim Phillips @ 2014-06-26 1:08 UTC (permalink / raw) To: Greg KH Cc: kvm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, clbchenlibo.chen-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8, stuart.yoder-KZfg59tc24xl57MIdRCFDg, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, eric.auger-QSEj5FYQhm4dnm+yROfE0A, jhovold-Re5JQEeQqe8AvxtiuMwx3w, Alexander Graf, grant.likely-QSEj5FYQhm4dnm+yROfE0A, Ian Campbell, Stefano Stabellini, Julien Grall, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ, a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, rob-VoJi6FS/r0vR7s880joybQ, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2 Jun 2014 21:28:42 -0700 Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote: > On Mon, Jun 02, 2014 at 07:42:58PM -0500, Kim Phillips wrote: > > You are the platform driver core maintainer: can you apply this to > > your driver-core tree now? > > Yes, I will after this merge window ends, it's too late for 3.16-rc1 > with the window opening up a week early, sorry. How about now? fwiw, I just checked: it still applies cleanly. Thanks, Kim ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] driver core: platform: add device binding path 'driver_override' [not found] ` <20140603042842.GD14668-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> 2014-06-26 1:08 ` Kim Phillips @ 2014-07-07 21:19 ` Kim Phillips 1 sibling, 0 replies; 42+ messages in thread From: Kim Phillips @ 2014-07-07 21:19 UTC (permalink / raw) To: Greg KH Cc: kvm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, clbchenlibo.chen-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8, stuart.yoder-KZfg59tc24xl57MIdRCFDg, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, eric.auger-QSEj5FYQhm4dnm+yROfE0A, jhovold-Re5JQEeQqe8AvxtiuMwx3w, Alexander Graf, grant.likely-QSEj5FYQhm4dnm+yROfE0A, Ian Campbell, Stefano Stabellini, Julien Grall, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ, a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, rob-VoJi6FS/r0vR7s880joybQ, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2 Jun 2014 21:28:42 -0700 Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote: > On Mon, Jun 02, 2014 at 07:42:58PM -0500, Kim Phillips wrote: > > You are the platform driver core maintainer: can you apply this to > > your driver-core tree now? > > Yes, I will after this merge window ends, it's too late for 3.16-rc1 > with the window opening up a week early, sorry. Hi Greg, The merge window has ended, and I see commit activity on your driver-core tree's driver-core-next branch, but this patch is still missing: Do you think you can get around to applying it before the next merge window? Thanks, Kim ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH] driver core: platform: add device binding path 'driver_override' [not found] ` <20140602194258.5135925bf3c71b9c010fb409-KZfg59tc24xl57MIdRCFDg@public.gmane.org> 2014-06-03 4:28 ` Greg KH @ 2014-07-08 22:37 ` Greg KH 1 sibling, 0 replies; 42+ messages in thread From: Greg KH @ 2014-07-08 22:37 UTC (permalink / raw) To: Kim Phillips Cc: kvm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, clbchenlibo.chen-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8, stuart.yoder-KZfg59tc24xl57MIdRCFDg, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, eric.auger-QSEj5FYQhm4dnm+yROfE0A, jhovold-Re5JQEeQqe8AvxtiuMwx3w, Alexander Graf, grant.likely-QSEj5FYQhm4dnm+yROfE0A, Ian Campbell, Stefano Stabellini, Julien Grall, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ, a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, rob-VoJi6FS/r0vR7s880joybQ, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, Jun 02, 2014 at 07:42:58PM -0500, Kim Phillips wrote: > Needed by platform device drivers, such as the upcoming > vfio-platform driver, in order to bypass the existing OF, ACPI, > id_table and name string matches, and successfully be able to be > bound to any device, like so: > > echo vfio-platform > /sys/bus/platform/devices/fff51000.ethernet/driver_override > echo fff51000.ethernet > /sys/bus/platform/devices/fff51000.ethernet/driver/unbind > echo fff51000.ethernet > /sys/bus/platform/drivers_probe > > This mimics "PCI: Introduce new device binding path using > pci_dev.driver_override", which is an interface enhancement > for more deterministic PCI device binding, e.g., when in the > presence of hotplug. > > Reviewed-by: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > Reviewed-by: Alexander Graf <agraf-l3A5Bk7waGM@public.gmane.org> > Reviewed-by: Stuart Yoder <stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org> > Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org> > --- > Greg, > > This is largely identical to the PCI version of the same that has > been accepted for v3.16 and ack'd by you: > > https://lists.cs.columbia.edu/pipermail/kvmarm/2014-May/009674.html > > and applied to Bjorn Helgaas' PCI tree: > > https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=782a985d7af26db39e86070d28f987cad21313c0 > > You are the platform driver core maintainer: can you apply this to > your driver-core tree now? Sorry for the very long delay, it's now merged in my tree. Thanks for being persistant. greg k-h ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH v5 02/11] ARM SMMU: Add capability IOMMU_CAP_DMA_EXEC [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 01/11] driver core: platform: add device binding path 'driver_override' Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis [not found] ` <1398700371-20096-3-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 03/11] VFIO_IOMMU_TYPE1 for platform bus devices on ARM Antonios Motakis ` (8 subsequent siblings) 10 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: Shuah Khan, kvm-u79uwXL29TY76Z2rM5mHXA, Alexey Kardashevskiy, open list, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, moderated list:ARM SMMU DRIVER, Varun Sethi, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A The ARM SMMU can take an IOMMU_EXEC protection flag in addition to IOMMU_READ and IOMMU_WRITE. Expose this as an IOMMU capability. Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/iommu/arm-smmu.c | 2 ++ include/linux/iommu.h | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 1d9ab39..abf802f 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1509,6 +1509,8 @@ static int arm_smmu_domain_has_cap(struct iommu_domain *domain, if (smmu_domain->root_cfg.smmu->features & ARM_SMMU_FEAT_COHERENT_WALK) caps |= IOMMU_CAP_CACHE_COHERENCY; + caps |= IOMMU_CAP_DMA_EXEC; + return !!(cap & caps); } diff --git a/include/linux/iommu.h b/include/linux/iommu.h index b96a5b2..4f547f3 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -57,8 +57,9 @@ struct iommu_domain { struct iommu_domain_geometry geometry; }; -#define IOMMU_CAP_CACHE_COHERENCY 0x1 -#define IOMMU_CAP_INTR_REMAP 0x2 /* isolates device intrs */ +#define IOMMU_CAP_CACHE_COHERENCY (1 << 0) +#define IOMMU_CAP_INTR_REMAP (1 << 1) /* isolates device intrs */ +#define IOMMU_CAP_DMA_EXEC (1 << 2) /* EXEC protection flag */ /* * Following constraints are specifc to FSL_PAMUV1: -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-3-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* Re: [RFC PATCH v5 02/11] ARM SMMU: Add capability IOMMU_CAP_DMA_EXEC [not found] ` <1398700371-20096-3-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 19:37 ` Will Deacon [not found] ` <20140428193709.GE22135-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Will Deacon @ 2014-04-28 19:37 UTC (permalink / raw) To: Antonios Motakis Cc: Shuah Khan, stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alexey Kardashevskiy, open list, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, moderated list:ARM SMMU DRIVER, Varun Sethi, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org On Mon, Apr 28, 2014 at 04:52:42PM +0100, Antonios Motakis wrote: > The ARM SMMU can take an IOMMU_EXEC protection flag in addition to > IOMMU_READ and IOMMU_WRITE. Expose this as an IOMMU capability. The other way of handling this would be to negate the capability and advertise a NOEXEC cap instead. That would need the IOMMU_EXEC flag to become IOMMU_NOEXEC and the ARM SMMU driver updating accordingly, but it might make more sense if people don't object to mixing positive and negative logic in the IOMMU_* flags. Any thoughts? Will ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <20140428193709.GE22135-5wv7dgnIgG8@public.gmane.org>]
* Re: [RFC PATCH v5 02/11] ARM SMMU: Add capability IOMMU_CAP_DMA_EXEC [not found] ` <20140428193709.GE22135-5wv7dgnIgG8@public.gmane.org> @ 2014-04-28 20:20 ` Alex Williamson 2014-04-29 9:49 ` Antonios Motakis 2014-04-29 9:52 ` Antonios Motakis 2 siblings, 0 replies; 42+ messages in thread From: Alex Williamson @ 2014-04-28 20:20 UTC (permalink / raw) To: Will Deacon Cc: Shuah Khan, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alexey Kardashevskiy, open list, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, moderated list:ARM SMMU DRIVER, Varun Sethi, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org On Mon, 2014-04-28 at 20:37 +0100, Will Deacon wrote: > On Mon, Apr 28, 2014 at 04:52:42PM +0100, Antonios Motakis wrote: > > The ARM SMMU can take an IOMMU_EXEC protection flag in addition to > > IOMMU_READ and IOMMU_WRITE. Expose this as an IOMMU capability. > > The other way of handling this would be to negate the capability and > advertise a NOEXEC cap instead. That would need the IOMMU_EXEC flag to > become IOMMU_NOEXEC and the ARM SMMU driver updating accordingly, but it > might make more sense if people don't object to mixing positive and negative > logic in the IOMMU_* flags. > > Any thoughts? A benefit of doing that would be that the flag becomes enforceable. As written in this draft, if a user does not specify EXEC, the mapping may or may not be executable, depending on the IOMMU capability (assuming that if EXEC is not supported that it follows READ). If the flag changes to NOEXEC, then all the domains in the container should support it or else the mapping should fail. We could also avoid the test in vfio code when doing a mapping and just let the IOMMU driver fail the map if NOEXEC is unsupported. Thanks, Alex ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC PATCH v5 02/11] ARM SMMU: Add capability IOMMU_CAP_DMA_EXEC [not found] ` <20140428193709.GE22135-5wv7dgnIgG8@public.gmane.org> 2014-04-28 20:20 ` Alex Williamson @ 2014-04-29 9:49 ` Antonios Motakis 2014-04-29 9:52 ` Antonios Motakis 2 siblings, 0 replies; 42+ messages in thread From: Antonios Motakis @ 2014-04-29 9:49 UTC (permalink / raw) To: Will Deacon Cc: Shuah Khan, stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alexey Kardashevskiy, open list, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, moderated list:ARM SMMU DRIVER, Varun Sethi, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org [-- Attachment #1.1: Type: text/plain, Size: 908 bytes --] I agree, that would be simpler for the user. As it is now we have to worry about whether the EXEC flac is available just to use the PL330, with NOEXEC the device would always be usable by default. On Mon, Apr 28, 2014 at 9:37 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote: > On Mon, Apr 28, 2014 at 04:52:42PM +0100, Antonios Motakis wrote: > > The ARM SMMU can take an IOMMU_EXEC protection flag in addition to > > IOMMU_READ and IOMMU_WRITE. Expose this as an IOMMU capability. > > The other way of handling this would be to negate the capability and > advertise a NOEXEC cap instead. That would need the IOMMU_EXEC flag to > become IOMMU_NOEXEC and the ARM SMMU driver updating accordingly, but it > might make more sense if people don't object to mixing positive and > negative > logic in the IOMMU_* flags. > > Any thoughts? > > Will > -- Antonios Motakis Virtual Open Systems [-- Attachment #1.2: Type: text/html, Size: 1387 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [RFC PATCH v5 02/11] ARM SMMU: Add capability IOMMU_CAP_DMA_EXEC [not found] ` <20140428193709.GE22135-5wv7dgnIgG8@public.gmane.org> 2014-04-28 20:20 ` Alex Williamson 2014-04-29 9:49 ` Antonios Motakis @ 2014-04-29 9:52 ` Antonios Motakis 2 siblings, 0 replies; 42+ messages in thread From: Antonios Motakis @ 2014-04-29 9:52 UTC (permalink / raw) To: Will Deacon Cc: Shuah Khan, stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alexey Kardashevskiy, open list, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, moderated list:ARM SMMU DRIVER, Varun Sethi, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org On Mon, Apr 28, 2014 at 9:37 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote: > On Mon, Apr 28, 2014 at 04:52:42PM +0100, Antonios Motakis wrote: >> The ARM SMMU can take an IOMMU_EXEC protection flag in addition to >> IOMMU_READ and IOMMU_WRITE. Expose this as an IOMMU capability. > > The other way of handling this would be to negate the capability and > advertise a NOEXEC cap instead. That would need the IOMMU_EXEC flag to > become IOMMU_NOEXEC and the ARM SMMU driver updating accordingly, but it > might make more sense if people don't object to mixing positive and negative > logic in the IOMMU_* flags. > > Any thoughts? > I agree, that would be simpler for the user. As it is now we have to worry about whether the EXEC flac is available just to use the PL330, with NOEXEC the device would always be usable by default. > Will -- Antonios Motakis Virtual Open Systems ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH v5 03/11] VFIO_IOMMU_TYPE1 for platform bus devices on ARM [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 01/11] driver core: platform: add device binding path 'driver_override' Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 02/11] ARM SMMU: Add capability IOMMU_CAP_DMA_EXEC Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis [not found] ` <1398700371-20096-4-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 04/11] VFIO_IOMMU_TYPE1: Introduce the VFIO_DMA_MAP_FLAG_EXEC flag Antonios Motakis ` (7 subsequent siblings) 10 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: kvm-u79uwXL29TY76Z2rM5mHXA, open list, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A This allows to make use of the VFIO_IOMMU_TYPE1 driver with platform devices on ARM in addition to PCI. This is required in order to use the Exynos SMMU, or ARM SMMU driver with VFIO_IOMMU_TYPE1. Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/vfio/Kconfig | 2 +- drivers/vfio/vfio_iommu_type1.c | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index af7b204..3a598ed 100644 --- a/drivers/vfio/Kconfig +++ b/drivers/vfio/Kconfig @@ -11,7 +11,7 @@ config VFIO_IOMMU_SPAPR_TCE 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 select VFIO_IOMMU_SPAPR_TCE if (PPC_POWERNV || PPC_PSERIES) select ANON_INODES help diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index 1f90344..4dc989d 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -30,6 +30,7 @@ #include <linux/iommu.h> #include <linux/module.h> #include <linux/mm.h> +#include <linux/pci.h> /* pci_bus_type */ #include <linux/rbtree.h> #include <linux/sched.h> #include <linux/slab.h> @@ -721,13 +722,15 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, INIT_LIST_HEAD(&domain->group_list); list_add(&group->next, &domain->group_list); - if (!allow_unsafe_interrupts && +#ifdef CONFIG_PCI + if (bus == &pci_bus_type && !allow_unsafe_interrupts && !iommu_domain_has_cap(domain->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__); ret = -EPERM; goto out_detach; } +#endif if (iommu_domain_has_cap(domain->domain, IOMMU_CAP_CACHE_COHERENCY)) domain->prot |= IOMMU_CACHE; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-4-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* Re: [RFC PATCH v5 03/11] VFIO_IOMMU_TYPE1 for platform bus devices on ARM [not found] ` <1398700371-20096-4-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 16:43 ` Alex Williamson [not found] ` <1398703421.24318.262.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Alex Williamson @ 2014-04-28 16:43 UTC (permalink / raw) To: Antonios Motakis Cc: kvm-u79uwXL29TY76Z2rM5mHXA, open list, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > This allows to make use of the VFIO_IOMMU_TYPE1 driver with platform > devices on ARM in addition to PCI. This is required in order to use the > Exynos SMMU, or ARM SMMU driver with VFIO_IOMMU_TYPE1. > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > --- > drivers/vfio/Kconfig | 2 +- > drivers/vfio/vfio_iommu_type1.c | 5 ++++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > index af7b204..3a598ed 100644 > --- a/drivers/vfio/Kconfig > +++ b/drivers/vfio/Kconfig > @@ -11,7 +11,7 @@ config VFIO_IOMMU_SPAPR_TCE > 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 > select VFIO_IOMMU_SPAPR_TCE if (PPC_POWERNV || PPC_PSERIES) > select ANON_INODES > help > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 1f90344..4dc989d 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -30,6 +30,7 @@ > #include <linux/iommu.h> > #include <linux/module.h> > #include <linux/mm.h> > +#include <linux/pci.h> /* pci_bus_type */ > #include <linux/rbtree.h> > #include <linux/sched.h> > #include <linux/slab.h> > @@ -721,13 +722,15 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > INIT_LIST_HEAD(&domain->group_list); > list_add(&group->next, &domain->group_list); > > - if (!allow_unsafe_interrupts && > +#ifdef CONFIG_PCI > + if (bus == &pci_bus_type && !allow_unsafe_interrupts && > !iommu_domain_has_cap(domain->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__); > ret = -EPERM; > goto out_detach; > } > +#endif > > if (iommu_domain_has_cap(domain->domain, IOMMU_CAP_CACHE_COHERENCY)) > domain->prot |= IOMMU_CACHE; This is not a PCI specific requirement. Anything that can support MSI needs an IOMMU that can provide isolation for both DMA and interrupts. I think the IOMMU should still be telling us that it has this feature. Thanks, Alex ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1398703421.24318.262.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>]
* Re: [RFC PATCH v5 03/11] VFIO_IOMMU_TYPE1 for platform bus devices on ARM [not found] ` <1398703421.24318.262.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> @ 2014-04-28 19:19 ` Will Deacon [not found] ` <20140428191920.GC22135-5wv7dgnIgG8@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Will Deacon @ 2014-04-28 19:19 UTC (permalink / raw) To: Alex Williamson Cc: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, marc.zyngier-5wv7dgnIgG8, open list, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org Hi Alex, On Mon, Apr 28, 2014 at 05:43:41PM +0100, Alex Williamson wrote: > On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > > This allows to make use of the VFIO_IOMMU_TYPE1 driver with platform > > devices on ARM in addition to PCI. This is required in order to use the > > Exynos SMMU, or ARM SMMU driver with VFIO_IOMMU_TYPE1. [...] > > @@ -721,13 +722,15 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > > INIT_LIST_HEAD(&domain->group_list); > > list_add(&group->next, &domain->group_list); > > > > - if (!allow_unsafe_interrupts && > > +#ifdef CONFIG_PCI > > + if (bus == &pci_bus_type && !allow_unsafe_interrupts && > > !iommu_domain_has_cap(domain->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__); > > ret = -EPERM; > > goto out_detach; > > } > > +#endif > > > > if (iommu_domain_has_cap(domain->domain, IOMMU_CAP_CACHE_COHERENCY)) > > domain->prot |= IOMMU_CACHE; > > This is not a PCI specific requirement. Anything that can support MSI > needs an IOMMU that can provide isolation for both DMA and interrupts. > I think the IOMMU should still be telling us that it has this feature. Please excuse any ignorance on part here (I'm not at all familiar with the Intel IOMMU), but shouldn't this really be a property of the interrupt controller itself? On ARM with GICv3, there is a separate block called the ITS (interrupt translation service) which is part of the interrupt controller. The ITS provides a doorbell page which the SMMU can map into a guest operating system to provide MSI for passthrough devices, but this isn't something the SMMU is aware of -- it will just see the iommu_map request for a non-cacheable mapping. Will ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <20140428191920.GC22135-5wv7dgnIgG8@public.gmane.org>]
* Re: [RFC PATCH v5 03/11] VFIO_IOMMU_TYPE1 for platform bus devices on ARM [not found] ` <20140428191920.GC22135-5wv7dgnIgG8@public.gmane.org> @ 2014-04-28 20:08 ` Alex Williamson [not found] ` <1398715690.24318.321.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Alex Williamson @ 2014-04-28 20:08 UTC (permalink / raw) To: Will Deacon Cc: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, marc.zyngier-5wv7dgnIgG8, open list, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org On Mon, 2014-04-28 at 20:19 +0100, Will Deacon wrote: > Hi Alex, > > On Mon, Apr 28, 2014 at 05:43:41PM +0100, Alex Williamson wrote: > > On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > > > This allows to make use of the VFIO_IOMMU_TYPE1 driver with platform > > > devices on ARM in addition to PCI. This is required in order to use the > > > Exynos SMMU, or ARM SMMU driver with VFIO_IOMMU_TYPE1. > > [...] > > > > @@ -721,13 +722,15 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > > > INIT_LIST_HEAD(&domain->group_list); > > > list_add(&group->next, &domain->group_list); > > > > > > - if (!allow_unsafe_interrupts && > > > +#ifdef CONFIG_PCI > > > + if (bus == &pci_bus_type && !allow_unsafe_interrupts && > > > !iommu_domain_has_cap(domain->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__); > > > ret = -EPERM; > > > goto out_detach; > > > } > > > +#endif > > > > > > if (iommu_domain_has_cap(domain->domain, IOMMU_CAP_CACHE_COHERENCY)) > > > domain->prot |= IOMMU_CACHE; > > > > This is not a PCI specific requirement. Anything that can support MSI > > needs an IOMMU that can provide isolation for both DMA and interrupts. > > I think the IOMMU should still be telling us that it has this feature. > > Please excuse any ignorance on part here (I'm not at all familiar with the > Intel IOMMU), but shouldn't this really be a property of the interrupt > controller itself? On ARM with GICv3, there is a separate block called the > ITS (interrupt translation service) which is part of the interrupt > controller. The ITS provides a doorbell page which the SMMU can map into a > guest operating system to provide MSI for passthrough devices, but this > isn't something the SMMU is aware of -- it will just see the iommu_map > request for a non-cacheable mapping. Hi Will, I don't know the history of why this is an IOMMU domain capability on x86, it's sort of a paradox. An MSI from a device is conceptually just a DMA write and is therefore logically co-located in the IOMMU hardware, but x86 doesn't allow it to be mapped via the IOMMU API interfaces. For compatibility, interrupt remapping support is buried deep in the request_irq interface and effectively invisible other than having this path to query it. Therefore this flag is effectively just saying "MSI isolation support is present and enabled". IOW, the host is protected from interrupt injection attacks from malicious devices. If there is some property of your platform that makes this always the case, then the IOMMU driver can always export this capability as true. With PCI, MSI is configured via spec defined configuration space registers, so we emulate these registers and prevent user access to them so that we don't need to allow the user a way to setup an interrupt remapping entry. It's done for them via request_irq. IIRC, the Freescale devices have a limited number of MSI pages and can therefore create some instances with isolation while others may require sharing. In that case I would expect this flag to indicate whether the domain has an exclusive or shared page. In any case, I suspect keying on the bus_type here is not the correct way to go. Thanks, Alex ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1398715690.24318.321.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>]
* Re: [RFC PATCH v5 03/11] VFIO_IOMMU_TYPE1 for platform bus devices on ARM [not found] ` <1398715690.24318.321.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> @ 2014-04-30 13:08 ` Will Deacon 0 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2014-04-30 13:08 UTC (permalink / raw) To: Alex Williamson Cc: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Marc Zyngier, open list, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, stuart.yoder-KZfg59tc24xl57MIdRCFDg@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org On Mon, Apr 28, 2014 at 09:08:10PM +0100, Alex Williamson wrote: > On Mon, 2014-04-28 at 20:19 +0100, Will Deacon wrote: > > Please excuse any ignorance on part here (I'm not at all familiar with the > > Intel IOMMU), but shouldn't this really be a property of the interrupt > > controller itself? On ARM with GICv3, there is a separate block called the > > ITS (interrupt translation service) which is part of the interrupt > > controller. The ITS provides a doorbell page which the SMMU can map into a > > guest operating system to provide MSI for passthrough devices, but this > > isn't something the SMMU is aware of -- it will just see the iommu_map > > request for a non-cacheable mapping. > > I don't know the history of why this is an IOMMU domain capability on > x86, it's sort of a paradox. An MSI from a device is conceptually just > a DMA write and is therefore logically co-located in the IOMMU hardware, > but x86 doesn't allow it to be mapped via the IOMMU API interfaces. For > compatibility, interrupt remapping support is buried deep in the > request_irq interface and effectively invisible other than having this > path to query it. Therefore this flag is effectively just saying "MSI > isolation support is present and enabled". IOW, the host is protected > from interrupt injection attacks from malicious devices. If there is > some property of your platform that makes this always the case, then the > IOMMU driver can always export this capability as true. Thanks for the explanation. On ARM, the SMMU does indeed see the MSI write just like a normal write, so it can be mapped via iommu_map() to point at the interrupt controller doorbell page. I guess that means we can enable this capability for all MSI-capable devices upstream of the SMMU, providing that the IRQ controller doesn't have any horrible quirks. > With PCI, MSI is configured via spec defined configuration space > registers, so we emulate these registers and prevent user access to them > so that we don't need to allow the user a way to setup an interrupt > remapping entry. It's done for them via request_irq. > > IIRC, the Freescale devices have a limited number of MSI pages and can > therefore create some instances with isolation while others may require > sharing. In that case I would expect this flag to indicate whether the > domain has an exclusive or shared page. > > In any case, I suspect keying on the bus_type here is not the correct > way to go. Thanks, Agreed, I was more intrigued by the meaning of the flag. Thanks, Will ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH v5 04/11] VFIO_IOMMU_TYPE1: Introduce the VFIO_DMA_MAP_FLAG_EXEC flag [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> ` (2 preceding siblings ...) 2014-04-28 15:52 ` [RFC PATCH v5 03/11] VFIO_IOMMU_TYPE1 for platform bus devices on ARM Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis [not found] ` <1398700371-20096-5-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 05/11] VFIO_PLATFORM: Initial skeleton of VFIO support for platform devices Antonios Motakis ` (6 subsequent siblings) 10 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: kvm-u79uwXL29TY76Z2rM5mHXA, open list, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A The ARM SMMU driver expects the IOMMU_EXEC flag, otherwise it will set the page tables for a device as XN (execute never). This affects devices such as the ARM PL330 DMA Controller, which fails to operate if the XN flag is set on the memory it tries to fetch its instructions from. We introduce the VFIO_DMA_MAP_FLAG_EXEC to VFIO, and use it in VFIO_IOMMU_TYPE1 to set the IOMMU_EXEC flag. This way the user can control whether the XN flag will be set on the requested mappings. If the IOMMU_EXEC flag is available for at least one IOMMU of a container, the new capability VFIO_IOMMU_PROT_EXEC will be exposed. Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/vfio/vfio_iommu_type1.c | 34 +++++++++++++++++++++++++++++++--- include/uapi/linux/vfio.h | 2 ++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index 4dc989d..6ce32bf 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -498,12 +498,15 @@ static int map_try_harder(struct vfio_domain *domain, dma_addr_t iova, } static int vfio_iommu_map(struct vfio_iommu *iommu, dma_addr_t iova, - unsigned long pfn, long npage, int prot) + unsigned long pfn, long npage, int prot, bool exec) { struct vfio_domain *d; int ret; list_for_each_entry(d, &iommu->domain_list, next) { + if (exec && iommu_domain_has_cap(d->domain, IOMMU_CAP_DMA_EXEC)) + prot |= IOMMU_EXEC; + ret = iommu_map(d->domain, iova, (phys_addr_t)pfn << PAGE_SHIFT, npage << PAGE_SHIFT, prot | d->prot); if (ret) { @@ -530,6 +533,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, size_t size = map->size; long npage; int ret = 0, prot = 0; + bool prot_exec = false; uint64_t mask; struct vfio_dma *dma; unsigned long pfn; @@ -543,6 +547,8 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, prot |= IOMMU_WRITE; if (map->flags & VFIO_DMA_MAP_FLAG_READ) prot |= IOMMU_READ; + if (map->flags & VFIO_DMA_MAP_FLAG_EXEC) + prot_exec = true; if (!prot) return -EINVAL; /* No READ/WRITE? */ @@ -595,7 +601,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, } /* Map it! */ - ret = vfio_iommu_map(iommu, iova, pfn, npage, prot); + ret = vfio_iommu_map(iommu, iova, pfn, npage, prot, prot_exec); if (ret) { vfio_unpin_pages(pfn, npage, prot, true); break; @@ -887,6 +893,23 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu) return ret; } +static int vfio_domains_have_iommu_exec(struct vfio_iommu *iommu) +{ + struct vfio_domain *d; + int ret = 0; + + mutex_lock(&iommu->lock); + list_for_each_entry(d, &iommu->domain_list, next) { + if (iommu_domain_has_cap(d->domain, IOMMU_CAP_DMA_EXEC)) { + ret = 1; + break; + } + } + mutex_unlock(&iommu->lock); + + return ret; +} + static long vfio_iommu_type1_ioctl(void *iommu_data, unsigned int cmd, unsigned long arg) { @@ -902,6 +925,10 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, if (!iommu) return 0; return vfio_domains_have_iommu_cache(iommu); + case VFIO_IOMMU_PROT_EXEC: + if (!iommu) + return 0; + return vfio_domains_have_iommu_exec(iommu); default: return 0; } @@ -925,7 +952,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, } else if (cmd == VFIO_IOMMU_MAP_DMA) { struct vfio_iommu_type1_dma_map map; uint32_t mask = VFIO_DMA_MAP_FLAG_READ | - VFIO_DMA_MAP_FLAG_WRITE; + VFIO_DMA_MAP_FLAG_WRITE | + VFIO_DMA_MAP_FLAG_EXEC; minsz = offsetofend(struct vfio_iommu_type1_dma_map, size); diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index cb9023d..0847b29 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -29,6 +29,7 @@ * capability is subject to change as groups are added or removed. */ #define VFIO_DMA_CC_IOMMU 4 +#define VFIO_IOMMU_PROT_EXEC 5 /* * The IOCTL interface is designed for extensibility by embedding the @@ -398,6 +399,7 @@ struct vfio_iommu_type1_dma_map { __u32 flags; #define VFIO_DMA_MAP_FLAG_READ (1 << 0) /* readable from device */ #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1) /* writable from device */ +#define VFIO_DMA_MAP_FLAG_EXEC (1 << 2) /* executable from device */ __u64 vaddr; /* Process virtual address */ __u64 iova; /* IO virtual address */ __u64 size; /* Size of mapping (bytes) */ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-5-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* Re: [RFC PATCH v5 04/11] VFIO_IOMMU_TYPE1: Introduce the VFIO_DMA_MAP_FLAG_EXEC flag [not found] ` <1398700371-20096-5-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 16:53 ` Alex Williamson [not found] ` <1398704036.24318.269.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Alex Williamson @ 2014-04-28 16:53 UTC (permalink / raw) To: Antonios Motakis Cc: kvm-u79uwXL29TY76Z2rM5mHXA, open list, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > The ARM SMMU driver expects the IOMMU_EXEC flag, otherwise it will > set the page tables for a device as XN (execute never). This affects > devices such as the ARM PL330 DMA Controller, which fails to operate > if the XN flag is set on the memory it tries to fetch its instructions > from. > > We introduce the VFIO_DMA_MAP_FLAG_EXEC to VFIO, and use it in > VFIO_IOMMU_TYPE1 to set the IOMMU_EXEC flag. This way the user can > control whether the XN flag will be set on the requested mappings. If > the IOMMU_EXEC flag is available for at least one IOMMU of a container, > the new capability VFIO_IOMMU_PROT_EXEC will be exposed. > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > --- > drivers/vfio/vfio_iommu_type1.c | 34 +++++++++++++++++++++++++++++++--- > include/uapi/linux/vfio.h | 2 ++ > 2 files changed, 33 insertions(+), 3 deletions(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 4dc989d..6ce32bf 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -498,12 +498,15 @@ static int map_try_harder(struct vfio_domain *domain, dma_addr_t iova, > } > > static int vfio_iommu_map(struct vfio_iommu *iommu, dma_addr_t iova, > - unsigned long pfn, long npage, int prot) > + unsigned long pfn, long npage, int prot, bool exec) > { > struct vfio_domain *d; > int ret; > > list_for_each_entry(d, &iommu->domain_list, next) { > + if (exec && iommu_domain_has_cap(d->domain, IOMMU_CAP_DMA_EXEC)) > + prot |= IOMMU_EXEC; > + > ret = iommu_map(d->domain, iova, (phys_addr_t)pfn << PAGE_SHIFT, > npage << PAGE_SHIFT, prot | d->prot); > if (ret) { > @@ -530,6 +533,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, > size_t size = map->size; > long npage; > int ret = 0, prot = 0; > + bool prot_exec = false; > uint64_t mask; > struct vfio_dma *dma; > unsigned long pfn; > @@ -543,6 +547,8 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, > prot |= IOMMU_WRITE; > if (map->flags & VFIO_DMA_MAP_FLAG_READ) > prot |= IOMMU_READ; > + if (map->flags & VFIO_DMA_MAP_FLAG_EXEC) > + prot_exec = true; Is it really necessary for this to be a new parameter rather than prot | = IOMMU_EXEC here? It could simply be masked out in vfio_iommu_map() when not supported. When IOMMU_CAP_DMA_EXEC is not supported, does IOMMU_EXEC effectively follow IOMMU_READ? If so, does that imply we need to error on a request for (IOMMU_WRITE | IOMMU_EXEC)? Thanks, Alex > > if (!prot) > return -EINVAL; /* No READ/WRITE? */ > @@ -595,7 +601,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, > } > > /* Map it! */ > - ret = vfio_iommu_map(iommu, iova, pfn, npage, prot); > + ret = vfio_iommu_map(iommu, iova, pfn, npage, prot, prot_exec); > if (ret) { > vfio_unpin_pages(pfn, npage, prot, true); > break; > @@ -887,6 +893,23 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu) > return ret; > } > > +static int vfio_domains_have_iommu_exec(struct vfio_iommu *iommu) > +{ > + struct vfio_domain *d; > + int ret = 0; > + > + mutex_lock(&iommu->lock); > + list_for_each_entry(d, &iommu->domain_list, next) { > + if (iommu_domain_has_cap(d->domain, IOMMU_CAP_DMA_EXEC)) { > + ret = 1; > + break; > + } > + } > + mutex_unlock(&iommu->lock); > + > + return ret; > +} > + > static long vfio_iommu_type1_ioctl(void *iommu_data, > unsigned int cmd, unsigned long arg) > { > @@ -902,6 +925,10 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, > if (!iommu) > return 0; > return vfio_domains_have_iommu_cache(iommu); > + case VFIO_IOMMU_PROT_EXEC: > + if (!iommu) > + return 0; > + return vfio_domains_have_iommu_exec(iommu); > default: > return 0; > } > @@ -925,7 +952,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, > } else if (cmd == VFIO_IOMMU_MAP_DMA) { > struct vfio_iommu_type1_dma_map map; > uint32_t mask = VFIO_DMA_MAP_FLAG_READ | > - VFIO_DMA_MAP_FLAG_WRITE; > + VFIO_DMA_MAP_FLAG_WRITE | > + VFIO_DMA_MAP_FLAG_EXEC; > > minsz = offsetofend(struct vfio_iommu_type1_dma_map, size); > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > index cb9023d..0847b29 100644 > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -29,6 +29,7 @@ > * capability is subject to change as groups are added or removed. > */ > #define VFIO_DMA_CC_IOMMU 4 > +#define VFIO_IOMMU_PROT_EXEC 5 > > /* > * The IOCTL interface is designed for extensibility by embedding the > @@ -398,6 +399,7 @@ struct vfio_iommu_type1_dma_map { > __u32 flags; > #define VFIO_DMA_MAP_FLAG_READ (1 << 0) /* readable from device */ > #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1) /* writable from device */ > +#define VFIO_DMA_MAP_FLAG_EXEC (1 << 2) /* executable from device */ > __u64 vaddr; /* Process virtual address */ > __u64 iova; /* IO virtual address */ > __u64 size; /* Size of mapping (bytes) */ ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1398704036.24318.269.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>]
* Re: [RFC PATCH v5 04/11] VFIO_IOMMU_TYPE1: Introduce the VFIO_DMA_MAP_FLAG_EXEC flag [not found] ` <1398704036.24318.269.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> @ 2014-04-29 13:16 ` Antonios Motakis 0 siblings, 0 replies; 42+ messages in thread From: Antonios Motakis @ 2014-04-29 13:16 UTC (permalink / raw) To: Alex Williamson Cc: KVM devel mailing list, open list, Will Deacon, alvise rigo, Stuart Yoder, Linux IOMMU, VirtualOpenSystems Technical Team, kvm-arm, Christoffer Dall On Mon, Apr 28, 2014 at 6:53 PM, Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: >> The ARM SMMU driver expects the IOMMU_EXEC flag, otherwise it will >> set the page tables for a device as XN (execute never). This affects >> devices such as the ARM PL330 DMA Controller, which fails to operate >> if the XN flag is set on the memory it tries to fetch its instructions >> from. >> >> We introduce the VFIO_DMA_MAP_FLAG_EXEC to VFIO, and use it in >> VFIO_IOMMU_TYPE1 to set the IOMMU_EXEC flag. This way the user can >> control whether the XN flag will be set on the requested mappings. If >> the IOMMU_EXEC flag is available for at least one IOMMU of a container, >> the new capability VFIO_IOMMU_PROT_EXEC will be exposed. >> >> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> >> --- >> drivers/vfio/vfio_iommu_type1.c | 34 +++++++++++++++++++++++++++++++--- >> include/uapi/linux/vfio.h | 2 ++ >> 2 files changed, 33 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c >> index 4dc989d..6ce32bf 100644 >> --- a/drivers/vfio/vfio_iommu_type1.c >> +++ b/drivers/vfio/vfio_iommu_type1.c >> @@ -498,12 +498,15 @@ static int map_try_harder(struct vfio_domain *domain, dma_addr_t iova, >> } >> >> static int vfio_iommu_map(struct vfio_iommu *iommu, dma_addr_t iova, >> - unsigned long pfn, long npage, int prot) >> + unsigned long pfn, long npage, int prot, bool exec) >> { >> struct vfio_domain *d; >> int ret; >> >> list_for_each_entry(d, &iommu->domain_list, next) { >> + if (exec && iommu_domain_has_cap(d->domain, IOMMU_CAP_DMA_EXEC)) >> + prot |= IOMMU_EXEC; >> + >> ret = iommu_map(d->domain, iova, (phys_addr_t)pfn << PAGE_SHIFT, >> npage << PAGE_SHIFT, prot | d->prot); >> if (ret) { >> @@ -530,6 +533,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, >> size_t size = map->size; >> long npage; >> int ret = 0, prot = 0; >> + bool prot_exec = false; >> uint64_t mask; >> struct vfio_dma *dma; >> unsigned long pfn; >> @@ -543,6 +547,8 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, >> prot |= IOMMU_WRITE; >> if (map->flags & VFIO_DMA_MAP_FLAG_READ) >> prot |= IOMMU_READ; >> + if (map->flags & VFIO_DMA_MAP_FLAG_EXEC) >> + prot_exec = true; > > Is it really necessary for this to be a new parameter rather than prot | > = IOMMU_EXEC here? It could simply be masked out in vfio_iommu_map() > when not supported. It seemed more readable to me at that point to mask it in, but if you prefer we can mask it out instead. If we replace EXEC with NOEXEC as suggested in the other thread, then we might just check if all domains in the container support the flag and avoid the trouble altogether. > When IOMMU_CAP_DMA_EXEC is not supported, does > IOMMU_EXEC effectively follow IOMMU_READ? If so, does that imply we > need to error on a request for (IOMMU_WRITE | IOMMU_EXEC)? Thanks, > This is probably a good idea, I imagine there won't ever be a platform where EXEC without READ makes sense. > Alex > >> >> if (!prot) >> return -EINVAL; /* No READ/WRITE? */ >> @@ -595,7 +601,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu, >> } >> >> /* Map it! */ >> - ret = vfio_iommu_map(iommu, iova, pfn, npage, prot); >> + ret = vfio_iommu_map(iommu, iova, pfn, npage, prot, prot_exec); >> if (ret) { >> vfio_unpin_pages(pfn, npage, prot, true); >> break; >> @@ -887,6 +893,23 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu) >> return ret; >> } >> >> +static int vfio_domains_have_iommu_exec(struct vfio_iommu *iommu) >> +{ >> + struct vfio_domain *d; >> + int ret = 0; >> + >> + mutex_lock(&iommu->lock); >> + list_for_each_entry(d, &iommu->domain_list, next) { >> + if (iommu_domain_has_cap(d->domain, IOMMU_CAP_DMA_EXEC)) { >> + ret = 1; >> + break; >> + } >> + } >> + mutex_unlock(&iommu->lock); >> + >> + return ret; >> +} >> + >> static long vfio_iommu_type1_ioctl(void *iommu_data, >> unsigned int cmd, unsigned long arg) >> { >> @@ -902,6 +925,10 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, >> if (!iommu) >> return 0; >> return vfio_domains_have_iommu_cache(iommu); >> + case VFIO_IOMMU_PROT_EXEC: >> + if (!iommu) >> + return 0; >> + return vfio_domains_have_iommu_exec(iommu); >> default: >> return 0; >> } >> @@ -925,7 +952,8 @@ static long vfio_iommu_type1_ioctl(void *iommu_data, >> } else if (cmd == VFIO_IOMMU_MAP_DMA) { >> struct vfio_iommu_type1_dma_map map; >> uint32_t mask = VFIO_DMA_MAP_FLAG_READ | >> - VFIO_DMA_MAP_FLAG_WRITE; >> + VFIO_DMA_MAP_FLAG_WRITE | >> + VFIO_DMA_MAP_FLAG_EXEC; >> >> minsz = offsetofend(struct vfio_iommu_type1_dma_map, size); >> >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h >> index cb9023d..0847b29 100644 >> --- a/include/uapi/linux/vfio.h >> +++ b/include/uapi/linux/vfio.h >> @@ -29,6 +29,7 @@ >> * capability is subject to change as groups are added or removed. >> */ >> #define VFIO_DMA_CC_IOMMU 4 >> +#define VFIO_IOMMU_PROT_EXEC 5 >> >> /* >> * The IOCTL interface is designed for extensibility by embedding the >> @@ -398,6 +399,7 @@ struct vfio_iommu_type1_dma_map { >> __u32 flags; >> #define VFIO_DMA_MAP_FLAG_READ (1 << 0) /* readable from device */ >> #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1) /* writable from device */ >> +#define VFIO_DMA_MAP_FLAG_EXEC (1 << 2) /* executable from device */ >> __u64 vaddr; /* Process virtual address */ >> __u64 iova; /* IO virtual address */ >> __u64 size; /* Size of mapping (bytes) */ > > > -- Antonios Motakis Virtual Open Systems ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH v5 05/11] VFIO_PLATFORM: Initial skeleton of VFIO support for platform devices [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> ` (3 preceding siblings ...) 2014-04-28 15:52 ` [RFC PATCH v5 04/11] VFIO_IOMMU_TYPE1: Introduce the VFIO_DMA_MAP_FLAG_EXEC flag Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis [not found] ` <1398700371-20096-6-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 06/11] VFIO_PLATFORM: Return info for device and its memory mapped IO regions Antonios Motakis ` (5 subsequent siblings) 10 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: Mark Rutland, open list, kvm-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A This patch forms the skeleton for platform devices support with VFIO. Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/vfio/Kconfig | 1 + drivers/vfio/Makefile | 1 + drivers/vfio/platform/Kconfig | 9 ++ drivers/vfio/platform/Makefile | 4 + drivers/vfio/platform/vfio_platform.c | 172 ++++++++++++++++++++++++++ drivers/vfio/platform/vfio_platform_private.h | 22 ++++ include/uapi/linux/vfio.h | 1 + 7 files changed, 210 insertions(+) create mode 100644 drivers/vfio/platform/Kconfig create mode 100644 drivers/vfio/platform/Makefile create mode 100644 drivers/vfio/platform/vfio_platform.c create mode 100644 drivers/vfio/platform/vfio_platform_private.h diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 3a598ed..a484d72 100644 --- a/drivers/vfio/Kconfig +++ b/drivers/vfio/Kconfig @@ -21,3 +21,4 @@ menuconfig VFIO If you don't know what to do here, say N. source "drivers/vfio/pci/Kconfig" +source "drivers/vfio/platform/Kconfig" diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile index 72bfabc..b5e4a33 100644 --- a/drivers/vfio/Makefile +++ b/drivers/vfio/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_VFIO) += vfio.o obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o obj-$(CONFIG_VFIO_PCI) += pci/ +obj-$(CONFIG_VFIO_PLATFORM) += platform/ diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig new file mode 100644 index 0000000..42b0022 --- /dev/null +++ b/drivers/vfio/platform/Kconfig @@ -0,0 +1,9 @@ +config VFIO_PLATFORM + tristate "VFIO support for platform devices" + depends on VFIO && EVENTFD + help + Support for platform devices with VFIO. This is required to make + use of platform devices present on the system using the VFIO + framework. + + If you don't know what to do here, say N. diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile new file mode 100644 index 0000000..df3a014 --- /dev/null +++ b/drivers/vfio/platform/Makefile @@ -0,0 +1,4 @@ + +vfio-platform-y := vfio_platform.o + +obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c new file mode 100644 index 0000000..1661746 --- /dev/null +++ b/drivers/vfio/platform/vfio_platform.c @@ -0,0 +1,172 @@ +/* + * Copyright (C) 2013 - Virtual Open Systems + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> + * + * 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. + */ + +#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> +#include <linux/io.h> +#include <linux/platform_device.h> +#include <linux/irq.h> + +#include "vfio_platform_private.h" + +#define DRIVER_VERSION "0.5" +#define DRIVER_AUTHOR "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>" +#define DRIVER_DESC "VFIO for platform devices - User Level meta-driver" + +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", + .open = vfio_platform_open, + .release = vfio_platform_release, + .ioctl = vfio_platform_ioctl, + .read = vfio_platform_read, + .write = vfio_platform_write, + .mmap = vfio_platform_mmap, +}; + +static int vfio_platform_probe(struct platform_device *pdev) +{ + struct vfio_platform_device *vdev; + struct iommu_group *group; + int ret; + + group = iommu_group_get(&pdev->dev); + if (!group) { + pr_err("VFIO: No IOMMU group for device %s\n", pdev->name); + return -EINVAL; + } + + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); + if (!vdev) { + iommu_group_put(group); + return -ENOMEM; + } + + vdev->pdev = pdev; + + ret = vfio_add_group_dev(&pdev->dev, &vfio_platform_ops, vdev); + if (ret) { + iommu_group_put(group); + kfree(vdev); + } + + return ret; +} + +static int vfio_platform_remove(struct platform_device *pdev) +{ + struct vfio_platform_device *vdev; + + vdev = vfio_del_group_dev(&pdev->dev); + if (!vdev) + return -EINVAL; + + iommu_group_put(pdev->dev.iommu_group); + kfree(vdev); + + return 0; +} + +static struct platform_driver vfio_platform_driver = { + .probe = vfio_platform_probe, + .remove = vfio_platform_remove, + .driver = { + .name = "vfio-platform", + .owner = THIS_MODULE, + }, +}; + +module_platform_driver(vfio_platform_driver); + +MODULE_VERSION(DRIVER_VERSION); +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR(DRIVER_AUTHOR); +MODULE_DESCRIPTION(DRIVER_DESC); diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h new file mode 100644 index 0000000..4ae88f8 --- /dev/null +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2013 - Virtual Open Systems + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> + * + * 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. + */ + +#ifndef VFIO_PLATFORM_PRIVATE_H +#define VFIO_PLATFORM_PRIVATE_H + +struct vfio_platform_device { + struct platform_device *pdev; +}; + +#endif /* VFIO_PLATFORM_PRIVATE_H */ diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 0847b29..b1a8de8 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -155,6 +155,7 @@ struct vfio_device_info { __u32 flags; #define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */ #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */ +#define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ __u32 num_regions; /* Max region index + 1 */ __u32 num_irqs; /* Max IRQ index + 1 */ }; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-6-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* Re: [RFC PATCH v5 05/11] VFIO_PLATFORM: Initial skeleton of VFIO support for platform devices [not found] ` <1398700371-20096-6-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 17:04 ` Alex Williamson [not found] ` <1398704642.24318.273.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Alex Williamson @ 2014-04-28 17:04 UTC (permalink / raw) To: Antonios Motakis Cc: Mark Rutland, open list, kvm-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > This patch forms the skeleton for platform devices support with VFIO. > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > --- > drivers/vfio/Kconfig | 1 + > drivers/vfio/Makefile | 1 + > drivers/vfio/platform/Kconfig | 9 ++ > drivers/vfio/platform/Makefile | 4 + > drivers/vfio/platform/vfio_platform.c | 172 ++++++++++++++++++++++++++ > drivers/vfio/platform/vfio_platform_private.h | 22 ++++ > include/uapi/linux/vfio.h | 1 + > 7 files changed, 210 insertions(+) > create mode 100644 drivers/vfio/platform/Kconfig > create mode 100644 drivers/vfio/platform/Makefile > create mode 100644 drivers/vfio/platform/vfio_platform.c > create mode 100644 drivers/vfio/platform/vfio_platform_private.h > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > index 3a598ed..a484d72 100644 > --- a/drivers/vfio/Kconfig > +++ b/drivers/vfio/Kconfig > @@ -21,3 +21,4 @@ menuconfig VFIO > If you don't know what to do here, say N. > > source "drivers/vfio/pci/Kconfig" > +source "drivers/vfio/platform/Kconfig" > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > index 72bfabc..b5e4a33 100644 > --- a/drivers/vfio/Makefile > +++ b/drivers/vfio/Makefile > @@ -2,3 +2,4 @@ obj-$(CONFIG_VFIO) += vfio.o > obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o > obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o > obj-$(CONFIG_VFIO_PCI) += pci/ > +obj-$(CONFIG_VFIO_PLATFORM) += platform/ > diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig > new file mode 100644 > index 0000000..42b0022 > --- /dev/null > +++ b/drivers/vfio/platform/Kconfig > @@ -0,0 +1,9 @@ > +config VFIO_PLATFORM > + tristate "VFIO support for platform devices" > + depends on VFIO && EVENTFD VFIO_PCI also depends on PCI, is there no CONFIG_PLATFORM? What benefit would I get from this if I enable it on my x86 box? > + help > + Support for platform devices with VFIO. This is required to make > + use of platform devices present on the system using the VFIO > + framework. > + > + If you don't know what to do here, say N. > diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile > new file mode 100644 > index 0000000..df3a014 > --- /dev/null > +++ b/drivers/vfio/platform/Makefile > @@ -0,0 +1,4 @@ > + > +vfio-platform-y := vfio_platform.o > + > +obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c > new file mode 100644 > index 0000000..1661746 > --- /dev/null > +++ b/drivers/vfio/platform/vfio_platform.c > @@ -0,0 +1,172 @@ > +/* > + * Copyright (C) 2013 - Virtual Open Systems > + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > + * > + * 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. > + */ > + > +#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> > +#include <linux/io.h> > +#include <linux/platform_device.h> > +#include <linux/irq.h> > + > +#include "vfio_platform_private.h" > + > +#define DRIVER_VERSION "0.5" > +#define DRIVER_AUTHOR "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>" > +#define DRIVER_DESC "VFIO for platform devices - User Level meta-driver" > + > +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; > +} I know these are just stubs, but I suspect they should probably return error. Thanks, Alex > + > +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", > + .open = vfio_platform_open, > + .release = vfio_platform_release, > + .ioctl = vfio_platform_ioctl, > + .read = vfio_platform_read, > + .write = vfio_platform_write, > + .mmap = vfio_platform_mmap, > +}; > + > +static int vfio_platform_probe(struct platform_device *pdev) > +{ > + struct vfio_platform_device *vdev; > + struct iommu_group *group; > + int ret; > + > + group = iommu_group_get(&pdev->dev); > + if (!group) { > + pr_err("VFIO: No IOMMU group for device %s\n", pdev->name); > + return -EINVAL; > + } > + > + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); > + if (!vdev) { > + iommu_group_put(group); > + return -ENOMEM; > + } > + > + vdev->pdev = pdev; > + > + ret = vfio_add_group_dev(&pdev->dev, &vfio_platform_ops, vdev); > + if (ret) { > + iommu_group_put(group); > + kfree(vdev); > + } > + > + return ret; > +} > + > +static int vfio_platform_remove(struct platform_device *pdev) > +{ > + struct vfio_platform_device *vdev; > + > + vdev = vfio_del_group_dev(&pdev->dev); > + if (!vdev) > + return -EINVAL; > + > + iommu_group_put(pdev->dev.iommu_group); > + kfree(vdev); > + > + return 0; > +} > + > +static struct platform_driver vfio_platform_driver = { > + .probe = vfio_platform_probe, > + .remove = vfio_platform_remove, > + .driver = { > + .name = "vfio-platform", > + .owner = THIS_MODULE, > + }, > +}; > + > +module_platform_driver(vfio_platform_driver); > + > +MODULE_VERSION(DRIVER_VERSION); > +MODULE_LICENSE("GPL v2"); > +MODULE_AUTHOR(DRIVER_AUTHOR); > +MODULE_DESCRIPTION(DRIVER_DESC); > diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h > new file mode 100644 > index 0000000..4ae88f8 > --- /dev/null > +++ b/drivers/vfio/platform/vfio_platform_private.h > @@ -0,0 +1,22 @@ > +/* > + * Copyright (C) 2013 - Virtual Open Systems > + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > + * > + * 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. > + */ > + > +#ifndef VFIO_PLATFORM_PRIVATE_H > +#define VFIO_PLATFORM_PRIVATE_H > + > +struct vfio_platform_device { > + struct platform_device *pdev; > +}; > + > +#endif /* VFIO_PLATFORM_PRIVATE_H */ > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > index 0847b29..b1a8de8 100644 > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -155,6 +155,7 @@ struct vfio_device_info { > __u32 flags; > #define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */ > #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */ > +#define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ > __u32 num_regions; /* Max region index + 1 */ > __u32 num_irqs; /* Max IRQ index + 1 */ > }; ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1398704642.24318.273.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>]
* Re: [RFC PATCH v5 05/11] VFIO_PLATFORM: Initial skeleton of VFIO support for platform devices [not found] ` <1398704642.24318.273.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> @ 2014-04-29 13:23 ` Antonios Motakis 0 siblings, 0 replies; 42+ messages in thread From: Antonios Motakis @ 2014-04-29 13:23 UTC (permalink / raw) To: Alex Williamson Cc: Mark Rutland, open list, KVM devel mailing list, Catalin Marinas, Will Deacon, alvise rigo, Stuart Yoder, Linux IOMMU, VirtualOpenSystems Technical Team, kvm-arm, Christoffer Dall On Mon, Apr 28, 2014 at 7:04 PM, Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: >> This patch forms the skeleton for platform devices support with VFIO. >> >> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> >> --- >> drivers/vfio/Kconfig | 1 + >> drivers/vfio/Makefile | 1 + >> drivers/vfio/platform/Kconfig | 9 ++ >> drivers/vfio/platform/Makefile | 4 + >> drivers/vfio/platform/vfio_platform.c | 172 ++++++++++++++++++++++++++ >> drivers/vfio/platform/vfio_platform_private.h | 22 ++++ >> include/uapi/linux/vfio.h | 1 + >> 7 files changed, 210 insertions(+) >> create mode 100644 drivers/vfio/platform/Kconfig >> create mode 100644 drivers/vfio/platform/Makefile >> create mode 100644 drivers/vfio/platform/vfio_platform.c >> create mode 100644 drivers/vfio/platform/vfio_platform_private.h >> >> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig >> index 3a598ed..a484d72 100644 >> --- a/drivers/vfio/Kconfig >> +++ b/drivers/vfio/Kconfig >> @@ -21,3 +21,4 @@ menuconfig VFIO >> If you don't know what to do here, say N. >> >> source "drivers/vfio/pci/Kconfig" >> +source "drivers/vfio/platform/Kconfig" >> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile >> index 72bfabc..b5e4a33 100644 >> --- a/drivers/vfio/Makefile >> +++ b/drivers/vfio/Makefile >> @@ -2,3 +2,4 @@ obj-$(CONFIG_VFIO) += vfio.o >> obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o >> obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o >> obj-$(CONFIG_VFIO_PCI) += pci/ >> +obj-$(CONFIG_VFIO_PLATFORM) += platform/ >> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig >> new file mode 100644 >> index 0000000..42b0022 >> --- /dev/null >> +++ b/drivers/vfio/platform/Kconfig >> @@ -0,0 +1,9 @@ >> +config VFIO_PLATFORM >> + tristate "VFIO support for platform devices" >> + depends on VFIO && EVENTFD > > VFIO_PCI also depends on PCI, is there no CONFIG_PLATFORM? What benefit > would I get from this if I enable it on my x86 box? That's a good point. Unfortunately I don't know of an obvious config we can depend on, other than known target platforms (i.e. ARM). Useless on an x86 PC, but maybe someone has a weird x86 based non-PC based architecture somewhere :) > >> + help >> + Support for platform devices with VFIO. This is required to make >> + use of platform devices present on the system using the VFIO >> + framework. >> + >> + If you don't know what to do here, say N. >> diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile >> new file mode 100644 >> index 0000000..df3a014 >> --- /dev/null >> +++ b/drivers/vfio/platform/Makefile >> @@ -0,0 +1,4 @@ >> + >> +vfio-platform-y := vfio_platform.o >> + >> +obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o >> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c >> new file mode 100644 >> index 0000000..1661746 >> --- /dev/null >> +++ b/drivers/vfio/platform/vfio_platform.c >> @@ -0,0 +1,172 @@ >> +/* >> + * Copyright (C) 2013 - Virtual Open Systems >> + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> >> + * >> + * 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. >> + */ >> + >> +#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> >> +#include <linux/io.h> >> +#include <linux/platform_device.h> >> +#include <linux/irq.h> >> + >> +#include "vfio_platform_private.h" >> + >> +#define DRIVER_VERSION "0.5" >> +#define DRIVER_AUTHOR "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>" >> +#define DRIVER_DESC "VFIO for platform devices - User Level meta-driver" >> + >> +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; >> +} > > I know these are just stubs, but I suspect they should probably return > error. Thanks, > Ok! > Alex > >> + >> +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", >> + .open = vfio_platform_open, >> + .release = vfio_platform_release, >> + .ioctl = vfio_platform_ioctl, >> + .read = vfio_platform_read, >> + .write = vfio_platform_write, >> + .mmap = vfio_platform_mmap, >> +}; >> + >> +static int vfio_platform_probe(struct platform_device *pdev) >> +{ >> + struct vfio_platform_device *vdev; >> + struct iommu_group *group; >> + int ret; >> + >> + group = iommu_group_get(&pdev->dev); >> + if (!group) { >> + pr_err("VFIO: No IOMMU group for device %s\n", pdev->name); >> + return -EINVAL; >> + } >> + >> + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); >> + if (!vdev) { >> + iommu_group_put(group); >> + return -ENOMEM; >> + } >> + >> + vdev->pdev = pdev; >> + >> + ret = vfio_add_group_dev(&pdev->dev, &vfio_platform_ops, vdev); >> + if (ret) { >> + iommu_group_put(group); >> + kfree(vdev); >> + } >> + >> + return ret; >> +} >> + >> +static int vfio_platform_remove(struct platform_device *pdev) >> +{ >> + struct vfio_platform_device *vdev; >> + >> + vdev = vfio_del_group_dev(&pdev->dev); >> + if (!vdev) >> + return -EINVAL; >> + >> + iommu_group_put(pdev->dev.iommu_group); >> + kfree(vdev); >> + >> + return 0; >> +} >> + >> +static struct platform_driver vfio_platform_driver = { >> + .probe = vfio_platform_probe, >> + .remove = vfio_platform_remove, >> + .driver = { >> + .name = "vfio-platform", >> + .owner = THIS_MODULE, >> + }, >> +}; >> + >> +module_platform_driver(vfio_platform_driver); >> + >> +MODULE_VERSION(DRIVER_VERSION); >> +MODULE_LICENSE("GPL v2"); >> +MODULE_AUTHOR(DRIVER_AUTHOR); >> +MODULE_DESCRIPTION(DRIVER_DESC); >> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h >> new file mode 100644 >> index 0000000..4ae88f8 >> --- /dev/null >> +++ b/drivers/vfio/platform/vfio_platform_private.h >> @@ -0,0 +1,22 @@ >> +/* >> + * Copyright (C) 2013 - Virtual Open Systems >> + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> >> + * >> + * 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. >> + */ >> + >> +#ifndef VFIO_PLATFORM_PRIVATE_H >> +#define VFIO_PLATFORM_PRIVATE_H >> + >> +struct vfio_platform_device { >> + struct platform_device *pdev; >> +}; >> + >> +#endif /* VFIO_PLATFORM_PRIVATE_H */ >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h >> index 0847b29..b1a8de8 100644 >> --- a/include/uapi/linux/vfio.h >> +++ b/include/uapi/linux/vfio.h >> @@ -155,6 +155,7 @@ struct vfio_device_info { >> __u32 flags; >> #define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */ >> #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */ >> +#define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */ >> __u32 num_regions; /* Max region index + 1 */ >> __u32 num_irqs; /* Max IRQ index + 1 */ >> }; > > > -- Antonios Motakis Virtual Open Systems ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH v5 06/11] VFIO_PLATFORM: Return info for device and its memory mapped IO regions [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> ` (4 preceding siblings ...) 2014-04-28 15:52 ` [RFC PATCH v5 05/11] VFIO_PLATFORM: Initial skeleton of VFIO support for platform devices Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis [not found] ` <1398700371-20096-7-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 07/11] VFIO_PLATFORM: Read and write support for the device fd Antonios Motakis ` (4 subsequent siblings) 10 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: Mark Rutland, open list, kvm-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A 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 enables the IOCTLs: - VFIO_DEVICE_GET_INFO - VFIO_DEVICE_GET_REGION_INFO IRQ info is provided by one of the latter patches. Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/vfio/platform/vfio_platform.c | 77 ++++++++++++++++++++++++--- drivers/vfio/platform/vfio_platform_private.h | 17 ++++++ 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 1661746..5430cbe 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -34,15 +34,62 @@ #define DRIVER_AUTHOR "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>" #define DRIVER_DESC "VFIO for platform devices - User Level meta-driver" +static int vfio_platform_regions_init(struct vfio_platform_device *vdev) +{ + int cnt = 0, i; + + while (platform_get_resource(vdev->pdev, IORESOURCE_MEM, cnt)) + cnt++; + + vdev->num_regions = cnt; + + vdev->region = kzalloc(sizeof(struct vfio_platform_region) * cnt, + GFP_KERNEL); + if (!vdev->region) + return -ENOMEM; + + for (i = 0; i < cnt; i++) { + struct vfio_platform_region region; + struct resource *res = + platform_get_resource(vdev->pdev, IORESOURCE_MEM, i); + + region.addr = res->start; + region.size = resource_size(res); + region.flags = 0; + + vdev->region[i] = region; + } + + return 0; +} + +static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev) +{ + kfree(vdev->region); +} + static void vfio_platform_release(void *device_data) { + struct vfio_platform_device *vdev = device_data; + + vfio_platform_regions_cleanup(vdev); + module_put(THIS_MODULE); } static int vfio_platform_open(void *device_data) { - if (!try_module_get(THIS_MODULE)) + struct vfio_platform_device *vdev = device_data; + int ret; + + ret = vfio_platform_regions_init(vdev); + if (ret) + return ret; + + if (!try_module_get(THIS_MODULE)) { + vfio_platform_regions_cleanup(vdev); return -ENODEV; + } return 0; } @@ -65,18 +112,36 @@ static long vfio_platform_ioctl(void *device_data, return -EINVAL; info.flags = VFIO_DEVICE_FLAGS_PLATFORM; - info.num_regions = 0; + info.num_regions = vdev->num_regions; 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_REGION_INFO) { + struct vfio_region_info info; + + 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 (info.index >= vdev->num_regions) + return -EINVAL; + + /* map offset to the physical address */ + info.offset = VFIO_PLATFORM_INDEX_TO_OFFSET(info.index); + info.size = vdev->region[info.index].size; + info.flags = vdev->region[info.index].flags; + + return copy_to_user((void __user *)arg, &info, minsz); - else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) + } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) { return -EINVAL; - else if (cmd == VFIO_DEVICE_SET_IRQS) + } else if (cmd == VFIO_DEVICE_SET_IRQS) return -EINVAL; else if (cmd == VFIO_DEVICE_RESET) diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index 4ae88f8..3448f918 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -15,8 +15,25 @@ #ifndef VFIO_PLATFORM_PRIVATE_H #define VFIO_PLATFORM_PRIVATE_H +#define VFIO_PLATFORM_OFFSET_SHIFT 40 +#define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 1) + +#define VFIO_PLATFORM_OFFSET_TO_INDEX(off) \ + (off >> VFIO_PLATFORM_OFFSET_SHIFT) + +#define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \ + ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT) + +struct vfio_platform_region { + u64 addr; + resource_size_t size; + u32 flags; +}; + struct vfio_platform_device { struct platform_device *pdev; + struct vfio_platform_region *region; + u32 num_regions; }; #endif /* VFIO_PLATFORM_PRIVATE_H */ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-7-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* Re: [RFC PATCH v5 06/11] VFIO_PLATFORM: Return info for device and its memory mapped IO regions [not found] ` <1398700371-20096-7-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 17:16 ` Alex Williamson [not found] ` <1398705389.24318.279.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Alex Williamson @ 2014-04-28 17:16 UTC (permalink / raw) To: Antonios Motakis Cc: Mark Rutland, open list, kvm-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > 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 enables the IOCTLs: > - VFIO_DEVICE_GET_INFO > - VFIO_DEVICE_GET_REGION_INFO > > IRQ info is provided by one of the latter patches. > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > --- > drivers/vfio/platform/vfio_platform.c | 77 ++++++++++++++++++++++++--- > drivers/vfio/platform/vfio_platform_private.h | 17 ++++++ > 2 files changed, 88 insertions(+), 6 deletions(-) > > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c > index 1661746..5430cbe 100644 > --- a/drivers/vfio/platform/vfio_platform.c > +++ b/drivers/vfio/platform/vfio_platform.c > @@ -34,15 +34,62 @@ > #define DRIVER_AUTHOR "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>" > #define DRIVER_DESC "VFIO for platform devices - User Level meta-driver" > > +static int vfio_platform_regions_init(struct vfio_platform_device *vdev) > +{ > + int cnt = 0, i; > + > + while (platform_get_resource(vdev->pdev, IORESOURCE_MEM, cnt)) > + cnt++; > + > + vdev->num_regions = cnt; > + > + vdev->region = kzalloc(sizeof(struct vfio_platform_region) * cnt, > + GFP_KERNEL); > + if (!vdev->region) Should vdev->num_regions be cleared here or set at the end to avoid possibly walking a null pointer later? > + return -ENOMEM; > + > + for (i = 0; i < cnt; i++) { > + struct vfio_platform_region region; > + struct resource *res = > + platform_get_resource(vdev->pdev, IORESOURCE_MEM, i); > + > + region.addr = res->start; > + region.size = resource_size(res); > + region.flags = 0; > + > + vdev->region[i] = region; nit, the local variable with copy at the end seems rather unnecessary here. > + } > + > + return 0; > +} > + > +static void vfio_platform_regions_cleanup(struct vfio_platform_device *vdev) > +{ > + kfree(vdev->region); Makes me nervous again that we have vdev->num_regions still set to a value. Maybe just paranoia. > +} > + > static void vfio_platform_release(void *device_data) > { > + struct vfio_platform_device *vdev = device_data; > + > + vfio_platform_regions_cleanup(vdev); > + > module_put(THIS_MODULE); > } > > static int vfio_platform_open(void *device_data) > { > - if (!try_module_get(THIS_MODULE)) > + struct vfio_platform_device *vdev = device_data; > + int ret; > + > + ret = vfio_platform_regions_init(vdev); > + if (ret) > + return ret; > + > + if (!try_module_get(THIS_MODULE)) { > + vfio_platform_regions_cleanup(vdev); > return -ENODEV; > + } Getting a reference to the module seems like it should be step 1 here. Thanks, Alex > > return 0; > } > @@ -65,18 +112,36 @@ static long vfio_platform_ioctl(void *device_data, > return -EINVAL; > > info.flags = VFIO_DEVICE_FLAGS_PLATFORM; > - info.num_regions = 0; > + info.num_regions = vdev->num_regions; > 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_REGION_INFO) { > + struct vfio_region_info info; > + > + 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 (info.index >= vdev->num_regions) > + return -EINVAL; > + > + /* map offset to the physical address */ > + info.offset = VFIO_PLATFORM_INDEX_TO_OFFSET(info.index); > + info.size = vdev->region[info.index].size; > + info.flags = vdev->region[info.index].flags; > + > + return copy_to_user((void __user *)arg, &info, minsz); > > - else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) > + } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) { > return -EINVAL; > > - else if (cmd == VFIO_DEVICE_SET_IRQS) > + } else if (cmd == VFIO_DEVICE_SET_IRQS) > return -EINVAL; > > else if (cmd == VFIO_DEVICE_RESET) > diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h > index 4ae88f8..3448f918 100644 > --- a/drivers/vfio/platform/vfio_platform_private.h > +++ b/drivers/vfio/platform/vfio_platform_private.h > @@ -15,8 +15,25 @@ > #ifndef VFIO_PLATFORM_PRIVATE_H > #define VFIO_PLATFORM_PRIVATE_H > > +#define VFIO_PLATFORM_OFFSET_SHIFT 40 > +#define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 1) > + > +#define VFIO_PLATFORM_OFFSET_TO_INDEX(off) \ > + (off >> VFIO_PLATFORM_OFFSET_SHIFT) > + > +#define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \ > + ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT) > + > +struct vfio_platform_region { > + u64 addr; > + resource_size_t size; > + u32 flags; > +}; > + > struct vfio_platform_device { > struct platform_device *pdev; > + struct vfio_platform_region *region; > + u32 num_regions; > }; > > #endif /* VFIO_PLATFORM_PRIVATE_H */ ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1398705389.24318.279.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>]
* Re: [RFC PATCH v5 06/11] VFIO_PLATFORM: Return info for device and its memory mapped IO regions [not found] ` <1398705389.24318.279.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> @ 2014-05-02 16:55 ` Antonios Motakis 0 siblings, 0 replies; 42+ messages in thread From: Antonios Motakis @ 2014-05-02 16:55 UTC (permalink / raw) To: Alex Williamson Cc: Mark Rutland, open list, KVM devel mailing list, Catalin Marinas, Will Deacon, alvise rigo, Stuart Yoder, Linux IOMMU, VirtualOpenSystems Technical Team, kvm-arm, Christoffer Dall [-- Attachment #1.1: Type: text/plain, Size: 6579 bytes --] On Mon, Apr 28, 2014 at 7:16 PM, Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org > wrote: > On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > > 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 enables the IOCTLs: > > - VFIO_DEVICE_GET_INFO > > - VFIO_DEVICE_GET_REGION_INFO > > > > IRQ info is provided by one of the latter patches. > > > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > > --- > > drivers/vfio/platform/vfio_platform.c | 77 > ++++++++++++++++++++++++--- > > drivers/vfio/platform/vfio_platform_private.h | 17 ++++++ > > 2 files changed, 88 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/vfio/platform/vfio_platform.c > b/drivers/vfio/platform/vfio_platform.c > > index 1661746..5430cbe 100644 > > --- a/drivers/vfio/platform/vfio_platform.c > > +++ b/drivers/vfio/platform/vfio_platform.c > > @@ -34,15 +34,62 @@ > > #define DRIVER_AUTHOR "Antonios Motakis < > a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>" > > #define DRIVER_DESC "VFIO for platform devices - User Level > meta-driver" > > > > +static int vfio_platform_regions_init(struct vfio_platform_device *vdev) > > +{ > > + int cnt = 0, i; > > + > > + while (platform_get_resource(vdev->pdev, IORESOURCE_MEM, cnt)) > > + cnt++; > > + > > + vdev->num_regions = cnt; > > + > > + vdev->region = kzalloc(sizeof(struct vfio_platform_region) * cnt, > > + GFP_KERNEL); > > + if (!vdev->region) > > Should vdev->num_regions be cleared here or set at the end to avoid > possibly walking a null pointer later? > > Ack. > > + return -ENOMEM; > > + > > + for (i = 0; i < cnt; i++) { > > + struct vfio_platform_region region; > > + struct resource *res = > > + platform_get_resource(vdev->pdev, IORESOURCE_MEM, > i); > > + > > + region.addr = res->start; > > + region.size = resource_size(res); > > + region.flags = 0; > > + > > + vdev->region[i] = region; > > nit, the local variable with copy at the end seems rather unnecessary > here. > > I'm doing it purely for readability reasons, the compiler should be able to do the right thing anyway. But I am not married to the idea of course. > > + } > > + > > + return 0; > > +} > > + > > +static void vfio_platform_regions_cleanup(struct vfio_platform_device > *vdev) > > +{ > > + kfree(vdev->region); > > Makes me nervous again that we have vdev->num_regions still set to a > value. Maybe just paranoia. > Definitely not paranoia, I will reset the num_regions here. > > > +} > > + > > static void vfio_platform_release(void *device_data) > > { > > + struct vfio_platform_device *vdev = device_data; > > + > > + vfio_platform_regions_cleanup(vdev); > > + > > module_put(THIS_MODULE); > > } > > > > static int vfio_platform_open(void *device_data) > > { > > - if (!try_module_get(THIS_MODULE)) > > + struct vfio_platform_device *vdev = device_data; > > + int ret; > > + > > + ret = vfio_platform_regions_init(vdev); > > + if (ret) > > + return ret; > > + > > + if (!try_module_get(THIS_MODULE)) { > > + vfio_platform_regions_cleanup(vdev); > > return -ENODEV; > > + } > > Getting a reference to the module seems like it should be step 1 here. > Thanks, > > Ack. > Alex > > > > > return 0; > > } > > @@ -65,18 +112,36 @@ static long vfio_platform_ioctl(void *device_data, > > return -EINVAL; > > > > info.flags = VFIO_DEVICE_FLAGS_PLATFORM; > > - info.num_regions = 0; > > + info.num_regions = vdev->num_regions; > > 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_REGION_INFO) { > > + struct vfio_region_info info; > > + > > + 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 (info.index >= vdev->num_regions) > > + return -EINVAL; > > + > > + /* map offset to the physical address */ > > + info.offset = VFIO_PLATFORM_INDEX_TO_OFFSET(info.index); > > + info.size = vdev->region[info.index].size; > > + info.flags = vdev->region[info.index].flags; > > + > > + return copy_to_user((void __user *)arg, &info, minsz); > > > > - else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) > > + } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) { > > return -EINVAL; > > > > - else if (cmd == VFIO_DEVICE_SET_IRQS) > > + } else if (cmd == VFIO_DEVICE_SET_IRQS) > > return -EINVAL; > > > > else if (cmd == VFIO_DEVICE_RESET) > > diff --git a/drivers/vfio/platform/vfio_platform_private.h > b/drivers/vfio/platform/vfio_platform_private.h > > index 4ae88f8..3448f918 100644 > > --- a/drivers/vfio/platform/vfio_platform_private.h > > +++ b/drivers/vfio/platform/vfio_platform_private.h > > @@ -15,8 +15,25 @@ > > #ifndef VFIO_PLATFORM_PRIVATE_H > > #define VFIO_PLATFORM_PRIVATE_H > > > > +#define VFIO_PLATFORM_OFFSET_SHIFT 40 > > +#define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << > VFIO_PLATFORM_OFFSET_SHIFT) - 1) > > + > > +#define VFIO_PLATFORM_OFFSET_TO_INDEX(off) \ > > + (off >> VFIO_PLATFORM_OFFSET_SHIFT) > > + > > +#define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \ > > + ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT) > > + > > +struct vfio_platform_region { > > + u64 addr; > > + resource_size_t size; > > + u32 flags; > > +}; > > + > > struct vfio_platform_device { > > struct platform_device *pdev; > > + struct vfio_platform_region *region; > > + u32 num_regions; > > }; > > > > #endif /* VFIO_PLATFORM_PRIVATE_H */ > > > > -- Antonios Motakis Virtual Open Systems [-- Attachment #1.2: Type: text/html, Size: 9376 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH v5 07/11] VFIO_PLATFORM: Read and write support for the device fd [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> ` (5 preceding siblings ...) 2014-04-28 15:52 ` [RFC PATCH v5 06/11] VFIO_PLATFORM: Return info for device and its memory mapped IO regions Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis [not found] ` <1398700371-20096-8-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 08/11] VFIO_PLATFORM: Support MMAP of MMIO regions Antonios Motakis ` (3 subsequent siblings) 10 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: kvm-u79uwXL29TY76Z2rM5mHXA, open list, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A 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.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/vfio/platform/vfio_platform.c | 110 +++++++++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 3 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 5430cbe..7c01ced 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -55,7 +55,8 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev) region.addr = res->start; region.size = resource_size(res); - region.flags = 0; + region.flags = VFIO_REGION_INFO_FLAG_READ + | VFIO_REGION_INFO_FLAG_WRITE; vdev->region[i] = region; } @@ -153,13 +154,116 @@ static long vfio_platform_ioctl(void *device_data, 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; + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos); + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK; + unsigned int done = 0; + void __iomem *io; + + if (index >= vdev->num_regions) + return -EINVAL; + + io = ioremap_nocache(vdev->region[index].addr, + vdev->region[index].size); + + while (count) { + size_t filled; + + if (count >= 4 && !(off % 4)) { + u32 val; + + val = ioread32(io + off); + if (copy_to_user(buf, &val, 4)) + goto err; + + filled = 4; + } else if (count >= 2 && !(off % 2)) { + u16 val; + + val = ioread16(io + off); + if (copy_to_user(buf, &val, 2)) + goto err; + + filled = 2; + } else { + u8 val; + + val = ioread8(io + off); + if (copy_to_user(buf, &val, 1)) + goto err; + + filled = 1; + } + + + count -= filled; + done += filled; + off += filled; + buf += filled; + } + + iounmap(io); + return done; +err: + iounmap(io); + return -EFAULT; } 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; + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos); + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK; + unsigned int done = 0; + void __iomem *io; + + if (index >= vdev->num_regions) + return -EINVAL; + + io = ioremap_nocache(vdev->region[index].addr, + vdev->region[index].size); + + while (count) { + size_t filled; + + if (count >= 4 && !(off % 4)) { + u32 val; + + if (copy_from_user(&val, buf, 4)) + goto err; + iowrite32(val, io + off); + + filled = 4; + } else if (count >= 2 && !(off % 2)) { + u16 val; + + if (copy_from_user(&val, buf, 2)) + goto err; + iowrite16(val, io + off); + + filled = 2; + } else { + u8 val; + + if (copy_from_user(&val, buf, 1)) + goto err; + iowrite8(val, io + off); + + filled = 1; + } + + count -= filled; + done += filled; + off += filled; + buf += filled; + } + + iounmap(io); + return done; +err: + iounmap(io); + return -EFAULT; } static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma) -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-8-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* Re: [RFC PATCH v5 07/11] VFIO_PLATFORM: Read and write support for the device fd [not found] ` <1398700371-20096-8-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 17:24 ` Alex Williamson [not found] ` <1398705857.24318.284.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Alex Williamson @ 2014-04-28 17:24 UTC (permalink / raw) To: Antonios Motakis Cc: kvm-u79uwXL29TY76Z2rM5mHXA, open list, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > 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. If there are regions we cannot mmap for security, why do we provide full read/write access to them? > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > --- > drivers/vfio/platform/vfio_platform.c | 110 +++++++++++++++++++++++++++++++++- > 1 file changed, 107 insertions(+), 3 deletions(-) > > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c > index 5430cbe..7c01ced 100644 > --- a/drivers/vfio/platform/vfio_platform.c > +++ b/drivers/vfio/platform/vfio_platform.c > @@ -55,7 +55,8 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev) > > region.addr = res->start; > region.size = resource_size(res); > - region.flags = 0; > + region.flags = VFIO_REGION_INFO_FLAG_READ > + | VFIO_REGION_INFO_FLAG_WRITE; > > vdev->region[i] = region; > } > @@ -153,13 +154,116 @@ static long vfio_platform_ioctl(void *device_data, > 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; > + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos); > + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK; > + unsigned int done = 0; > + void __iomem *io; > + > + if (index >= vdev->num_regions) > + return -EINVAL; > + > + io = ioremap_nocache(vdev->region[index].addr, > + vdev->region[index].size); I haven't looked at ioremap on arm, but if it's remotely non-trivially, it's probably a good idea to do this only on first access, cache it, and unmap it on region cleanup. Thanks, Alex > + > + while (count) { > + size_t filled; > + > + if (count >= 4 && !(off % 4)) { > + u32 val; > + > + val = ioread32(io + off); > + if (copy_to_user(buf, &val, 4)) > + goto err; > + > + filled = 4; > + } else if (count >= 2 && !(off % 2)) { > + u16 val; > + > + val = ioread16(io + off); > + if (copy_to_user(buf, &val, 2)) > + goto err; > + > + filled = 2; > + } else { > + u8 val; > + > + val = ioread8(io + off); > + if (copy_to_user(buf, &val, 1)) > + goto err; > + > + filled = 1; > + } > + > + > + count -= filled; > + done += filled; > + off += filled; > + buf += filled; > + } > + > + iounmap(io); > + return done; > +err: > + iounmap(io); > + return -EFAULT; > } > > 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; > + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos); > + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK; > + unsigned int done = 0; > + void __iomem *io; > + > + if (index >= vdev->num_regions) > + return -EINVAL; > + > + io = ioremap_nocache(vdev->region[index].addr, > + vdev->region[index].size); > + > + while (count) { > + size_t filled; > + > + if (count >= 4 && !(off % 4)) { > + u32 val; > + > + if (copy_from_user(&val, buf, 4)) > + goto err; > + iowrite32(val, io + off); > + > + filled = 4; > + } else if (count >= 2 && !(off % 2)) { > + u16 val; > + > + if (copy_from_user(&val, buf, 2)) > + goto err; > + iowrite16(val, io + off); > + > + filled = 2; > + } else { > + u8 val; > + > + if (copy_from_user(&val, buf, 1)) > + goto err; > + iowrite8(val, io + off); > + > + filled = 1; > + } > + > + count -= filled; > + done += filled; > + off += filled; > + buf += filled; > + } > + > + iounmap(io); > + return done; > +err: > + iounmap(io); > + return -EFAULT; > } > > static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma) ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1398705857.24318.284.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>]
* Re: [RFC PATCH v5 07/11] VFIO_PLATFORM: Read and write support for the device fd [not found] ` <1398705857.24318.284.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> @ 2014-04-29 13:27 ` Antonios Motakis 0 siblings, 0 replies; 42+ messages in thread From: Antonios Motakis @ 2014-04-29 13:27 UTC (permalink / raw) To: Alex Williamson Cc: KVM devel mailing list, open list, Will Deacon, alvise rigo, Stuart Yoder, Linux IOMMU, VirtualOpenSystems Technical Team, kvm-arm, Christoffer Dall On Mon, Apr 28, 2014 at 7:24 PM, Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: >> 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. > > If there are regions we cannot mmap for security, why do we provide full > read/write access to them? What I refer to here is to the problem of mmaping an MMIO region that is not page-sized. For example if it is just a small number of registers which are not aligned on a page boundary, mmaping them would still make the whole page accessible to the guest, with unpredictable outcomes. But in these cases read/write is still OK. > >> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> >> --- >> drivers/vfio/platform/vfio_platform.c | 110 +++++++++++++++++++++++++++++++++- >> 1 file changed, 107 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c >> index 5430cbe..7c01ced 100644 >> --- a/drivers/vfio/platform/vfio_platform.c >> +++ b/drivers/vfio/platform/vfio_platform.c >> @@ -55,7 +55,8 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev) >> >> region.addr = res->start; >> region.size = resource_size(res); >> - region.flags = 0; >> + region.flags = VFIO_REGION_INFO_FLAG_READ >> + | VFIO_REGION_INFO_FLAG_WRITE; >> >> vdev->region[i] = region; >> } >> @@ -153,13 +154,116 @@ static long vfio_platform_ioctl(void *device_data, >> 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; >> + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos); >> + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK; >> + unsigned int done = 0; >> + void __iomem *io; >> + >> + if (index >= vdev->num_regions) >> + return -EINVAL; >> + >> + io = ioremap_nocache(vdev->region[index].addr, >> + vdev->region[index].size); > > I haven't looked at ioremap on arm, but if it's remotely non-trivially, > it's probably a good idea to do this only on first access, cache it, and > unmap it on region cleanup. Thanks, Ack. > > Alex > >> + >> + while (count) { >> + size_t filled; >> + >> + if (count >= 4 && !(off % 4)) { >> + u32 val; >> + >> + val = ioread32(io + off); >> + if (copy_to_user(buf, &val, 4)) >> + goto err; >> + >> + filled = 4; >> + } else if (count >= 2 && !(off % 2)) { >> + u16 val; >> + >> + val = ioread16(io + off); >> + if (copy_to_user(buf, &val, 2)) >> + goto err; >> + >> + filled = 2; >> + } else { >> + u8 val; >> + >> + val = ioread8(io + off); >> + if (copy_to_user(buf, &val, 1)) >> + goto err; >> + >> + filled = 1; >> + } >> + >> + >> + count -= filled; >> + done += filled; >> + off += filled; >> + buf += filled; >> + } >> + >> + iounmap(io); >> + return done; >> +err: >> + iounmap(io); >> + return -EFAULT; >> } >> >> 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; >> + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos); >> + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK; >> + unsigned int done = 0; >> + void __iomem *io; >> + >> + if (index >= vdev->num_regions) >> + return -EINVAL; >> + >> + io = ioremap_nocache(vdev->region[index].addr, >> + vdev->region[index].size); >> + >> + while (count) { >> + size_t filled; >> + >> + if (count >= 4 && !(off % 4)) { >> + u32 val; >> + >> + if (copy_from_user(&val, buf, 4)) >> + goto err; >> + iowrite32(val, io + off); >> + >> + filled = 4; >> + } else if (count >= 2 && !(off % 2)) { >> + u16 val; >> + >> + if (copy_from_user(&val, buf, 2)) >> + goto err; >> + iowrite16(val, io + off); >> + >> + filled = 2; >> + } else { >> + u8 val; >> + >> + if (copy_from_user(&val, buf, 1)) >> + goto err; >> + iowrite8(val, io + off); >> + >> + filled = 1; >> + } >> + >> + count -= filled; >> + done += filled; >> + off += filled; >> + buf += filled; >> + } >> + >> + iounmap(io); >> + return done; >> +err: >> + iounmap(io); >> + return -EFAULT; >> } >> >> static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma) > > > -- Antonios Motakis Virtual Open Systems ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH v5 08/11] VFIO_PLATFORM: Support MMAP of MMIO regions [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> ` (6 preceding siblings ...) 2014-04-28 15:52 ` [RFC PATCH v5 07/11] VFIO_PLATFORM: Read and write support for the device fd Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis [not found] ` <1398700371-20096-9-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 09/11] VFIO_PLATFORM: Return IRQ info Antonios Motakis ` (2 subsequent siblings) 10 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: kvm-u79uwXL29TY76Z2rM5mHXA, open list, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A Allow to memory map the MMIO regions of the device so userspace can directly access them. Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/vfio/platform/vfio_platform.c | 40 ++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 7c01ced..37beff3 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -58,6 +58,11 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev) region.flags = VFIO_REGION_INFO_FLAG_READ | VFIO_REGION_INFO_FLAG_WRITE; + /* Only regions addressed with PAGE granularity can be MMAPed + * securely. */ + if (!(region.addr & ~PAGE_MASK) && !(region.size & ~PAGE_MASK)) + region.flags |= VFIO_REGION_INFO_FLAG_MMAP; + vdev->region[i] = region; } @@ -268,7 +273,40 @@ err: static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma) { - return -EINVAL; + struct vfio_platform_device *vdev = device_data; + unsigned int index; + u64 req_len, pgoff, req_start; + struct vfio_platform_region region; + + index = vma->vm_pgoff >> (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT); + + if (vma->vm_end < vma->vm_start) + return -EINVAL; + if ((vma->vm_flags & VM_SHARED) == 0) + return -EINVAL; + if (index >= vdev->num_regions) + return -EINVAL; + if (vma->vm_start & ~PAGE_MASK) + return -EINVAL; + if (vma->vm_end & ~PAGE_MASK) + return -EINVAL; + + region = vdev->region[index]; + + req_len = vma->vm_end - vma->vm_start; + pgoff = vma->vm_pgoff & + ((1U << (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT)) - 1); + req_start = pgoff << PAGE_SHIFT; + + if (region.size < PAGE_SIZE || req_start + req_len > region.size) + return -EINVAL; + + vma->vm_private_data = vdev; + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); + vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff; + + return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, + req_len, vma->vm_page_prot); } static const struct vfio_device_ops vfio_platform_ops = { -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-9-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* Re: [RFC PATCH v5 08/11] VFIO_PLATFORM: Support MMAP of MMIO regions [not found] ` <1398700371-20096-9-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 17:27 ` Alex Williamson [not found] ` <1398706042.24318.287.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Alex Williamson @ 2014-04-28 17:27 UTC (permalink / raw) To: Antonios Motakis Cc: kvm-u79uwXL29TY76Z2rM5mHXA, open list, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > Allow to memory map the MMIO regions of the device so userspace can > directly access them. > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > --- > drivers/vfio/platform/vfio_platform.c | 40 ++++++++++++++++++++++++++++++++++- > 1 file changed, 39 insertions(+), 1 deletion(-) > > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c > index 7c01ced..37beff3 100644 > --- a/drivers/vfio/platform/vfio_platform.c > +++ b/drivers/vfio/platform/vfio_platform.c > @@ -58,6 +58,11 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev) > region.flags = VFIO_REGION_INFO_FLAG_READ > | VFIO_REGION_INFO_FLAG_WRITE; > > + /* Only regions addressed with PAGE granularity can be MMAPed > + * securely. */ > + if (!(region.addr & ~PAGE_MASK) && !(region.size & ~PAGE_MASK)) > + region.flags |= VFIO_REGION_INFO_FLAG_MMAP; > + Ok, so the security is only that we need page alignment. Maybe that could be made more clear in the previous patch. PCI has regions like the MSI-X table that we need to protect from userspace access beyond page alignment requirements. Thanks, Alex > vdev->region[i] = region; > } > > @@ -268,7 +273,40 @@ err: > > static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma) > { > - return -EINVAL; > + struct vfio_platform_device *vdev = device_data; > + unsigned int index; > + u64 req_len, pgoff, req_start; > + struct vfio_platform_region region; > + > + index = vma->vm_pgoff >> (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT); > + > + if (vma->vm_end < vma->vm_start) > + return -EINVAL; > + if ((vma->vm_flags & VM_SHARED) == 0) > + return -EINVAL; > + if (index >= vdev->num_regions) > + return -EINVAL; > + if (vma->vm_start & ~PAGE_MASK) > + return -EINVAL; > + if (vma->vm_end & ~PAGE_MASK) > + return -EINVAL; > + > + region = vdev->region[index]; > + > + req_len = vma->vm_end - vma->vm_start; > + pgoff = vma->vm_pgoff & > + ((1U << (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT)) - 1); > + req_start = pgoff << PAGE_SHIFT; > + > + if (region.size < PAGE_SIZE || req_start + req_len > region.size) > + return -EINVAL; > + > + vma->vm_private_data = vdev; > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > + vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff; > + > + return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, > + req_len, vma->vm_page_prot); > } > > static const struct vfio_device_ops vfio_platform_ops = { ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1398706042.24318.287.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>]
* Re: [RFC PATCH v5 08/11] VFIO_PLATFORM: Support MMAP of MMIO regions [not found] ` <1398706042.24318.287.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> @ 2014-04-29 13:28 ` Antonios Motakis 0 siblings, 0 replies; 42+ messages in thread From: Antonios Motakis @ 2014-04-29 13:28 UTC (permalink / raw) To: Alex Williamson Cc: KVM devel mailing list, open list, Will Deacon, alvise rigo, Stuart Yoder, Linux IOMMU, VirtualOpenSystems Technical Team, kvm-arm, Christoffer Dall On Mon, Apr 28, 2014 at 7:27 PM, Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: >> Allow to memory map the MMIO regions of the device so userspace can >> directly access them. >> >> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> >> --- >> drivers/vfio/platform/vfio_platform.c | 40 ++++++++++++++++++++++++++++++++++- >> 1 file changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c >> index 7c01ced..37beff3 100644 >> --- a/drivers/vfio/platform/vfio_platform.c >> +++ b/drivers/vfio/platform/vfio_platform.c >> @@ -58,6 +58,11 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev) >> region.flags = VFIO_REGION_INFO_FLAG_READ >> | VFIO_REGION_INFO_FLAG_WRITE; >> >> + /* Only regions addressed with PAGE granularity can be MMAPed >> + * securely. */ >> + if (!(region.addr & ~PAGE_MASK) && !(region.size & ~PAGE_MASK)) >> + region.flags |= VFIO_REGION_INFO_FLAG_MMAP; >> + > > Ok, so the security is only that we need page alignment. Maybe that > could be made more clear in the previous patch. PCI has regions like > the MSI-X table that we need to protect from userspace access beyond > page alignment requirements. Thanks, Ack, I will make the previous description more clear. > > Alex > >> vdev->region[i] = region; >> } >> >> @@ -268,7 +273,40 @@ err: >> >> static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma) >> { >> - return -EINVAL; >> + struct vfio_platform_device *vdev = device_data; >> + unsigned int index; >> + u64 req_len, pgoff, req_start; >> + struct vfio_platform_region region; >> + >> + index = vma->vm_pgoff >> (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT); >> + >> + if (vma->vm_end < vma->vm_start) >> + return -EINVAL; >> + if ((vma->vm_flags & VM_SHARED) == 0) >> + return -EINVAL; >> + if (index >= vdev->num_regions) >> + return -EINVAL; >> + if (vma->vm_start & ~PAGE_MASK) >> + return -EINVAL; >> + if (vma->vm_end & ~PAGE_MASK) >> + return -EINVAL; >> + >> + region = vdev->region[index]; >> + >> + req_len = vma->vm_end - vma->vm_start; >> + pgoff = vma->vm_pgoff & >> + ((1U << (VFIO_PLATFORM_OFFSET_SHIFT - PAGE_SHIFT)) - 1); >> + req_start = pgoff << PAGE_SHIFT; >> + >> + if (region.size < PAGE_SIZE || req_start + req_len > region.size) >> + return -EINVAL; >> + >> + vma->vm_private_data = vdev; >> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); >> + vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff; >> + >> + return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, >> + req_len, vma->vm_page_prot); >> } >> >> static const struct vfio_device_ops vfio_platform_ops = { > > > -- Antonios Motakis Virtual Open Systems ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH v5 09/11] VFIO_PLATFORM: Return IRQ info [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> ` (7 preceding siblings ...) 2014-04-28 15:52 ` [RFC PATCH v5 08/11] VFIO_PLATFORM: Support MMAP of MMIO regions Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis [not found] ` <1398700371-20096-10-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 10/11] VFIO_PLATFORM: Initial interrupts support Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 11/11] VFIO_PLATFORM: Support for maskable and automasked interrupts Antonios Motakis 10 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: Mark Rutland, open list, kvm-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A Return information for the interrupts exposed by the device. This patch extends VFIO_DEVICE_GET_INFO with the number of IRQs and enables VFIO_DEVICE_GET_IRQ_INFO Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/vfio/platform/Makefile | 2 +- drivers/vfio/platform/vfio_platform.c | 35 +++++++++++++-- drivers/vfio/platform/vfio_platform_irq.c | 63 +++++++++++++++++++++++++++ drivers/vfio/platform/vfio_platform_private.h | 11 +++++ 4 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 drivers/vfio/platform/vfio_platform_irq.c diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile index df3a014..2c53327 100644 --- a/drivers/vfio/platform/Makefile +++ b/drivers/vfio/platform/Makefile @@ -1,4 +1,4 @@ -vfio-platform-y := vfio_platform.o +vfio-platform-y := vfio_platform.o vfio_platform_irq.o obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 37beff3..2e16595 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -79,6 +79,7 @@ static void vfio_platform_release(void *device_data) struct vfio_platform_device *vdev = device_data; vfio_platform_regions_cleanup(vdev); + vfio_platform_irq_cleanup(vdev); module_put(THIS_MODULE); } @@ -92,12 +93,22 @@ static int vfio_platform_open(void *device_data) if (ret) return ret; + ret = vfio_platform_irq_init(vdev); + if (ret) + goto err_irq; + if (!try_module_get(THIS_MODULE)) { - vfio_platform_regions_cleanup(vdev); - return -ENODEV; + ret = -ENODEV; + goto err_mod; } return 0; + +err_mod: + vfio_platform_irq_cleanup(vdev); +err_irq: + vfio_platform_regions_cleanup(vdev); + return ret; } static long vfio_platform_ioctl(void *device_data, @@ -119,7 +130,7 @@ static long vfio_platform_ioctl(void *device_data, info.flags = VFIO_DEVICE_FLAGS_PLATFORM; info.num_regions = vdev->num_regions; - info.num_irqs = 0; + info.num_irqs = vdev->num_irqs; return copy_to_user((void __user *)arg, &info, minsz); @@ -145,7 +156,23 @@ static long vfio_platform_ioctl(void *device_data, return copy_to_user((void __user *)arg, &info, minsz); } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) { - return -EINVAL; + struct vfio_irq_info info; + + minsz = offsetofend(struct vfio_irq_info, count); + + if (copy_from_user(&info, (void __user *)arg, minsz)) + return -EFAULT; + + if (info.argsz < minsz) + return -EINVAL; + + if (info.index >= vdev->num_irqs) + return -EINVAL; + + info.flags = vdev->irq[info.index].flags; + info.count = vdev->irq[info.index].count; + + return copy_to_user((void __user *)arg, &info, minsz); } else if (cmd == VFIO_DEVICE_SET_IRQS) return -EINVAL; diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c new file mode 100644 index 0000000..075c401 --- /dev/null +++ b/drivers/vfio/platform/vfio_platform_irq.c @@ -0,0 +1,63 @@ +/* + * VFIO platform devices interrupt handling + * + * Copyright (C) 2013 - Virtual Open Systems + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> + * + * 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. + */ + +#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> +#include <linux/platform_device.h> +#include <linux/irq.h> + +#include "vfio_platform_private.h" + +int vfio_platform_irq_init(struct vfio_platform_device *vdev) +{ + int cnt = 0, i; + + while (platform_get_irq(vdev->pdev, cnt) > 0) + cnt++; + + vdev->num_irqs = cnt; + + vdev->irq = kzalloc(sizeof(struct vfio_platform_irq) * vdev->num_irqs, + GFP_KERNEL); + if (!vdev->irq) + return -ENOMEM; + + for (i = 0; i < cnt; i++) { + struct vfio_platform_irq irq; + + irq.flags = 0; + irq.count = 1; + + vdev->irq[i] = irq; + } + + return 0; +} + +void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev) +{ + kfree(vdev->irq); +} diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index 3448f918..9776cff 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -24,6 +24,11 @@ #define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \ ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT) +struct vfio_platform_irq { + u32 flags; + u32 count; +}; + struct vfio_platform_region { u64 addr; resource_size_t size; @@ -34,6 +39,12 @@ struct vfio_platform_device { struct platform_device *pdev; struct vfio_platform_region *region; u32 num_regions; + struct vfio_platform_irq *irq; + u32 num_irqs; }; +extern int vfio_platform_irq_init(struct vfio_platform_device *vdev); + +extern void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev); + #endif /* VFIO_PLATFORM_PRIVATE_H */ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-10-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* Re: [RFC PATCH v5 09/11] VFIO_PLATFORM: Return IRQ info [not found] ` <1398700371-20096-10-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 17:33 ` Alex Williamson 0 siblings, 0 replies; 42+ messages in thread From: Alex Williamson @ 2014-04-28 17:33 UTC (permalink / raw) To: Antonios Motakis Cc: Mark Rutland, open list, kvm-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > Return information for the interrupts exposed by the device. > This patch extends VFIO_DEVICE_GET_INFO with the number of IRQs > and enables VFIO_DEVICE_GET_IRQ_INFO > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > --- > drivers/vfio/platform/Makefile | 2 +- > drivers/vfio/platform/vfio_platform.c | 35 +++++++++++++-- > drivers/vfio/platform/vfio_platform_irq.c | 63 +++++++++++++++++++++++++++ > drivers/vfio/platform/vfio_platform_private.h | 11 +++++ > 4 files changed, 106 insertions(+), 5 deletions(-) > create mode 100644 drivers/vfio/platform/vfio_platform_irq.c > > diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile > index df3a014..2c53327 100644 > --- a/drivers/vfio/platform/Makefile > +++ b/drivers/vfio/platform/Makefile > @@ -1,4 +1,4 @@ > > -vfio-platform-y := vfio_platform.o > +vfio-platform-y := vfio_platform.o vfio_platform_irq.o > > obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c > index 37beff3..2e16595 100644 > --- a/drivers/vfio/platform/vfio_platform.c > +++ b/drivers/vfio/platform/vfio_platform.c > @@ -79,6 +79,7 @@ static void vfio_platform_release(void *device_data) > struct vfio_platform_device *vdev = device_data; > > vfio_platform_regions_cleanup(vdev); > + vfio_platform_irq_cleanup(vdev); > > module_put(THIS_MODULE); > } > @@ -92,12 +93,22 @@ static int vfio_platform_open(void *device_data) > if (ret) > return ret; > > + ret = vfio_platform_irq_init(vdev); > + if (ret) > + goto err_irq; > + > if (!try_module_get(THIS_MODULE)) { > - vfio_platform_regions_cleanup(vdev); > - return -ENODEV; > + ret = -ENODEV; > + goto err_mod; > } Same comments from REGION_INFO patch apply here. Ordering above, paranoia about keeping num_irqs consistent with allocation, nit about local variable use in initialization loop. Thanks, Alex > > return 0; > + > +err_mod: > + vfio_platform_irq_cleanup(vdev); > +err_irq: > + vfio_platform_regions_cleanup(vdev); > + return ret; > } > > static long vfio_platform_ioctl(void *device_data, > @@ -119,7 +130,7 @@ static long vfio_platform_ioctl(void *device_data, > > info.flags = VFIO_DEVICE_FLAGS_PLATFORM; > info.num_regions = vdev->num_regions; > - info.num_irqs = 0; > + info.num_irqs = vdev->num_irqs; > > return copy_to_user((void __user *)arg, &info, minsz); > > @@ -145,7 +156,23 @@ static long vfio_platform_ioctl(void *device_data, > return copy_to_user((void __user *)arg, &info, minsz); > > } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) { > - return -EINVAL; > + struct vfio_irq_info info; > + > + minsz = offsetofend(struct vfio_irq_info, count); > + > + if (copy_from_user(&info, (void __user *)arg, minsz)) > + return -EFAULT; > + > + if (info.argsz < minsz) > + return -EINVAL; > + > + if (info.index >= vdev->num_irqs) > + return -EINVAL; > + > + info.flags = vdev->irq[info.index].flags; > + info.count = vdev->irq[info.index].count; > + > + return copy_to_user((void __user *)arg, &info, minsz); > > } else if (cmd == VFIO_DEVICE_SET_IRQS) > return -EINVAL; > diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c > new file mode 100644 > index 0000000..075c401 > --- /dev/null > +++ b/drivers/vfio/platform/vfio_platform_irq.c > @@ -0,0 +1,63 @@ > +/* > + * VFIO platform devices interrupt handling > + * > + * Copyright (C) 2013 - Virtual Open Systems > + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > + * > + * 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. > + */ > + > +#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> > +#include <linux/platform_device.h> > +#include <linux/irq.h> > + > +#include "vfio_platform_private.h" > + > +int vfio_platform_irq_init(struct vfio_platform_device *vdev) > +{ > + int cnt = 0, i; > + > + while (platform_get_irq(vdev->pdev, cnt) > 0) > + cnt++; > + > + vdev->num_irqs = cnt; > + > + vdev->irq = kzalloc(sizeof(struct vfio_platform_irq) * vdev->num_irqs, > + GFP_KERNEL); > + if (!vdev->irq) > + return -ENOMEM; > + > + for (i = 0; i < cnt; i++) { > + struct vfio_platform_irq irq; > + > + irq.flags = 0; > + irq.count = 1; > + > + vdev->irq[i] = irq; > + } > + > + return 0; > +} > + > +void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev) > +{ > + kfree(vdev->irq); > +} > diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h > index 3448f918..9776cff 100644 > --- a/drivers/vfio/platform/vfio_platform_private.h > +++ b/drivers/vfio/platform/vfio_platform_private.h > @@ -24,6 +24,11 @@ > #define VFIO_PLATFORM_INDEX_TO_OFFSET(index) \ > ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT) > > +struct vfio_platform_irq { > + u32 flags; > + u32 count; > +}; > + > struct vfio_platform_region { > u64 addr; > resource_size_t size; > @@ -34,6 +39,12 @@ struct vfio_platform_device { > struct platform_device *pdev; > struct vfio_platform_region *region; > u32 num_regions; > + struct vfio_platform_irq *irq; > + u32 num_irqs; > }; > > +extern int vfio_platform_irq_init(struct vfio_platform_device *vdev); > + > +extern void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev); > + > #endif /* VFIO_PLATFORM_PRIVATE_H */ ^ permalink raw reply [flat|nested] 42+ messages in thread
* [RFC PATCH v5 10/11] VFIO_PLATFORM: Initial interrupts support [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> ` (8 preceding siblings ...) 2014-04-28 15:52 ` [RFC PATCH v5 09/11] VFIO_PLATFORM: Return IRQ info Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 11/11] VFIO_PLATFORM: Support for maskable and automasked interrupts Antonios Motakis 10 siblings, 0 replies; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: Mark Rutland, open list, kvm-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A This patch allows to set an eventfd for a patform device's interrupt, and also to trigger the interrupt eventfd from userspace for testing. Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/vfio/platform/vfio_platform.c | 36 +++++++- drivers/vfio/platform/vfio_platform_irq.c | 123 +++++++++++++++++++++++++- drivers/vfio/platform/vfio_platform_private.h | 7 ++ 3 files changed, 162 insertions(+), 4 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c index 2e16595..8aa5d10 100644 --- a/drivers/vfio/platform/vfio_platform.c +++ b/drivers/vfio/platform/vfio_platform.c @@ -174,10 +174,40 @@ static long vfio_platform_ioctl(void *device_data, return copy_to_user((void __user *)arg, &info, minsz); - } else if (cmd == VFIO_DEVICE_SET_IRQS) - return -EINVAL; + } else if (cmd == VFIO_DEVICE_SET_IRQS) { + struct vfio_irq_set hdr; + int ret = 0; + + minsz = offsetofend(struct vfio_irq_set, count); + + if (copy_from_user(&hdr, (void __user *)arg, minsz)) + return -EFAULT; + + if (hdr.argsz < minsz) + return -EINVAL; + + if (hdr.index >= vdev->num_irqs) + return -EINVAL; + + if (hdr.start != 0 || hdr.count > 1) + return -EINVAL; + + if (hdr.count == 0 && + (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE) || + !(hdr.flags & VFIO_IRQ_SET_ACTION_TRIGGER))) + return -EINVAL; + + if (hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK | + VFIO_IRQ_SET_ACTION_TYPE_MASK)) + return -EINVAL; + + ret = vfio_platform_set_irqs_ioctl(vdev, hdr.flags, hdr.index, + hdr.start, hdr.count, + (void *)arg+minsz); + + return ret; - else if (cmd == VFIO_DEVICE_RESET) + } else if (cmd == VFIO_DEVICE_RESET) return -EINVAL; return -ENOTTY; diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c index 075c401..433edc1 100644 --- a/drivers/vfio/platform/vfio_platform_irq.c +++ b/drivers/vfio/platform/vfio_platform_irq.c @@ -31,6 +31,9 @@ #include "vfio_platform_private.h" +static int vfio_set_trigger(struct vfio_platform_device *vdev, + int index, int fd); + int vfio_platform_irq_init(struct vfio_platform_device *vdev) { int cnt = 0, i; @@ -47,9 +50,11 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev) for (i = 0; i < cnt; i++) { struct vfio_platform_irq irq; + int hwirq = platform_get_irq(vdev->pdev, i); - irq.flags = 0; + irq.flags = VFIO_IRQ_INFO_EVENTFD; irq.count = 1; + irq.hwirq = hwirq; vdev->irq[i] = irq; } @@ -59,5 +64,121 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev) void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev) { + int i; + + for (i = 0; i < vdev->num_irqs; i++) + vfio_set_trigger(vdev, i, -1); + kfree(vdev->irq); } + +static irqreturn_t vfio_irq_handler(int irq, void *dev_id) +{ + struct eventfd_ctx *trigger = dev_id; + + eventfd_signal(trigger, 1); + + return IRQ_HANDLED; +} + +static int vfio_set_trigger(struct vfio_platform_device *vdev, + int index, int fd) +{ + struct vfio_platform_irq *irq = &vdev->irq[index]; + struct eventfd_ctx *trigger; + int ret; + + if (irq->trigger) { + free_irq(irq->hwirq, irq); + kfree(irq->name); + eventfd_ctx_put(irq->trigger); + irq->trigger = NULL; + } + + if (fd < 0) /* Disable only */ + return 0; + + irq->name = kasprintf(GFP_KERNEL, "vfio-irq[%d](%s)", + irq->hwirq, vdev->pdev->name); + if (!irq->name) + return -ENOMEM; + + trigger = eventfd_ctx_fdget(fd); + if (IS_ERR(trigger)) { + kfree(irq->name); + return PTR_ERR(trigger); + } + + irq->trigger = trigger; + + ret = request_irq(irq->hwirq, vfio_irq_handler, 0, irq->name, irq); + if (ret) { + kfree(irq->name); + eventfd_ctx_put(trigger); + irq->trigger = NULL; + return ret; + } + + return 0; +} + +static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev, + unsigned index, unsigned start, + unsigned count, uint32_t flags, void *data) +{ + struct vfio_platform_irq *irq = &vdev->irq[index]; + uint8_t arr; + int32_t fd; + + switch (flags & VFIO_IRQ_SET_DATA_TYPE_MASK) { + case VFIO_IRQ_SET_DATA_NONE: + if (count == 0) + return vfio_set_trigger(vdev, index, -1); + + vfio_irq_handler(irq->hwirq, irq); + return 0; + + case VFIO_IRQ_SET_DATA_BOOL: + if (copy_from_user(&arr, data, sizeof(uint8_t))) + return -EFAULT; + + if (arr == 0x1) { + vfio_irq_handler(irq->hwirq, irq); + return 0; + } + + return -EINVAL; + + case VFIO_IRQ_SET_DATA_EVENTFD: + if (copy_from_user(&fd, data, sizeof(int32_t))) + return -EFAULT; + + return vfio_set_trigger(vdev, index, fd); + } + + return -EFAULT; +} + +int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev, + uint32_t flags, unsigned index, unsigned start, + unsigned count, void *data) +{ + int (*func)(struct vfio_platform_device *vdev, unsigned index, + unsigned start, unsigned count, uint32_t flags, + void *data) = NULL; + + switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) { + case VFIO_IRQ_SET_ACTION_MASK: + case VFIO_IRQ_SET_ACTION_UNMASK: + /* XXX not implemented */ + break; + case VFIO_IRQ_SET_ACTION_TRIGGER: + func = vfio_platform_set_irq_trigger; + break; + } + + if (!func) + return -ENOTTY; + + return func(vdev, index, start, count, flags, data); +} diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index 9776cff..e21c15a 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -25,8 +25,11 @@ ((u64)(index) << VFIO_PLATFORM_OFFSET_SHIFT) struct vfio_platform_irq { + struct eventfd_ctx *trigger; u32 flags; u32 count; + int hwirq; + char *name; }; struct vfio_platform_region { @@ -47,4 +50,8 @@ extern int vfio_platform_irq_init(struct vfio_platform_device *vdev); extern void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev); +extern int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev, + uint32_t flags, unsigned index, unsigned start, + unsigned count, void *data); + #endif /* VFIO_PLATFORM_PRIVATE_H */ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [RFC PATCH v5 11/11] VFIO_PLATFORM: Support for maskable and automasked interrupts [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> ` (9 preceding siblings ...) 2014-04-28 15:52 ` [RFC PATCH v5 10/11] VFIO_PLATFORM: Initial interrupts support Antonios Motakis @ 2014-04-28 15:52 ` Antonios Motakis [not found] ` <1398700371-20096-12-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 10 siblings, 1 reply; 42+ messages in thread From: Antonios Motakis @ 2014-04-28 15:52 UTC (permalink / raw) To: alex.williamson-H+wXaHxf7aLQT0dZR+AlfA, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: Mark Rutland, open list, kvm-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, Antonios Motakis, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A Adds support to mask interrupts, and also for automasked interrupts. Level sensitive interrupts are exposed as automasked interrupts and are masked and disabled automatically when they fire. Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> --- drivers/vfio/platform/vfio_platform_irq.c | 117 ++++++++++++++++++++++++-- drivers/vfio/platform/vfio_platform_private.h | 2 + 2 files changed, 113 insertions(+), 6 deletions(-) diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c index 433edc1..e38982f 100644 --- a/drivers/vfio/platform/vfio_platform_irq.c +++ b/drivers/vfio/platform/vfio_platform_irq.c @@ -52,9 +52,16 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev) struct vfio_platform_irq irq; int hwirq = platform_get_irq(vdev->pdev, i); - irq.flags = VFIO_IRQ_INFO_EVENTFD; + spin_lock_init(&irq.lock); + + irq.flags = VFIO_IRQ_INFO_EVENTFD | VFIO_IRQ_INFO_MASKABLE; + + if (irq_get_trigger_type(hwirq) & IRQ_TYPE_LEVEL_MASK) + irq.flags |= VFIO_IRQ_INFO_AUTOMASKED; + irq.count = 1; irq.hwirq = hwirq; + irq.masked = false; vdev->irq[i] = irq; } @@ -66,19 +73,39 @@ void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev) { int i; - for (i = 0; i < vdev->num_irqs; i++) + for (i = 0; i < vdev->num_irqs; i++) { vfio_set_trigger(vdev, i, -1); + if (vdev->irq[i].masked) + enable_irq(vdev->irq[i].hwirq); + } + kfree(vdev->irq); } static irqreturn_t vfio_irq_handler(int irq, void *dev_id) { - struct eventfd_ctx *trigger = dev_id; + struct vfio_platform_irq *irq_ctx = dev_id; + unsigned long flags; + int ret = IRQ_NONE; + + spin_lock_irqsave(&irq_ctx->lock, flags); + + if (!irq_ctx->masked) { + ret = IRQ_HANDLED; + + if (irq_ctx->flags & VFIO_IRQ_INFO_AUTOMASKED) { + disable_irq_nosync(irq_ctx->hwirq); + irq_ctx->masked = true; + } + } - eventfd_signal(trigger, 1); + spin_unlock_irqrestore(&irq_ctx->lock, flags); - return IRQ_HANDLED; + if (ret == IRQ_HANDLED) + eventfd_signal(irq_ctx->trigger, 1); + + return ret; } static int vfio_set_trigger(struct vfio_platform_device *vdev, @@ -159,6 +186,82 @@ static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev, return -EFAULT; } +static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev, + unsigned index, unsigned start, + unsigned count, uint32_t flags, void *data) +{ + uint8_t arr; + + if (start != 0 || count != 1) + return -EINVAL; + + switch (flags & VFIO_IRQ_SET_DATA_TYPE_MASK) { + case VFIO_IRQ_SET_DATA_BOOL: + if (copy_from_user(&arr, data, sizeof(uint8_t))) + return -EFAULT; + + if (arr != 0x1) + return -EINVAL; + + case VFIO_IRQ_SET_DATA_NONE: + + spin_lock_irq(&vdev->irq[index].lock); + + if (vdev->irq[index].masked) { + enable_irq(vdev->irq[index].hwirq); + vdev->irq[index].masked = false; + } + + spin_unlock_irq(&vdev->irq[index].lock); + + return 0; + + case VFIO_IRQ_SET_DATA_EVENTFD: /* XXX not implemented yet */ + default: + return -ENOTTY; + } + + return 0; +} + +static int vfio_platform_set_irq_mask(struct vfio_platform_device *vdev, + unsigned index, unsigned start, + unsigned count, uint32_t flags, void *data) +{ + uint8_t arr; + + if (start != 0 || count != 1) + return -EINVAL; + + switch (flags & VFIO_IRQ_SET_DATA_TYPE_MASK) { + case VFIO_IRQ_SET_DATA_BOOL: + if (copy_from_user(&arr, data, sizeof(uint8_t))) + return -EFAULT; + + if (arr != 0x1) + return -EINVAL; + + case VFIO_IRQ_SET_DATA_NONE: + + spin_lock_irq(&vdev->irq[index].lock); + + if (!vdev->irq[index].masked) { + disable_irq(vdev->irq[index].hwirq); + vdev->irq[index].masked = true; + } + + spin_unlock_irq(&vdev->irq[index].lock); + + return 0; + + case VFIO_IRQ_SET_DATA_EVENTFD: /* XXX not implemented yet */ + default: + return -ENOTTY; + } + + return 0; +} + int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev, uint32_t flags, unsigned index, unsigned start, unsigned count, void *data) @@ -169,8 +272,10 @@ int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev, switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) { case VFIO_IRQ_SET_ACTION_MASK: + func = vfio_platform_set_irq_mask; + break; case VFIO_IRQ_SET_ACTION_UNMASK: - /* XXX not implemented */ + func = vfio_platform_set_irq_unmask; break; case VFIO_IRQ_SET_ACTION_TRIGGER: func = vfio_platform_set_irq_trigger; diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h index e21c15a..e283171 100644 --- a/drivers/vfio/platform/vfio_platform_private.h +++ b/drivers/vfio/platform/vfio_platform_private.h @@ -30,6 +30,8 @@ struct vfio_platform_irq { u32 count; int hwirq; char *name; + bool masked; + spinlock_t lock; }; struct vfio_platform_region { -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <1398700371-20096-12-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>]
* Re: [RFC PATCH v5 11/11] VFIO_PLATFORM: Support for maskable and automasked interrupts [not found] ` <1398700371-20096-12-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> @ 2014-04-28 17:46 ` Alex Williamson [not found] ` <1398707198.24318.293.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Alex Williamson @ 2014-04-28 17:46 UTC (permalink / raw) To: Antonios Motakis Cc: Mark Rutland, open list, kvm-u79uwXL29TY76Z2rM5mHXA, Catalin Marinas, will.deacon-5wv7dgnIgG8, a.rigo-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, stuart.yoder-KZfg59tc24xl57MIdRCFDg, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J, kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, christoffer.dall-QSEj5FYQhm4dnm+yROfE0A On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > Adds support to mask interrupts, and also for automasked interrupts. > Level sensitive interrupts are exposed as automasked interrupts and > are masked and disabled automatically when they fire. > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > --- > drivers/vfio/platform/vfio_platform_irq.c | 117 ++++++++++++++++++++++++-- > drivers/vfio/platform/vfio_platform_private.h | 2 + > 2 files changed, 113 insertions(+), 6 deletions(-) > > diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c > index 433edc1..e38982f 100644 > --- a/drivers/vfio/platform/vfio_platform_irq.c > +++ b/drivers/vfio/platform/vfio_platform_irq.c > @@ -52,9 +52,16 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev) > struct vfio_platform_irq irq; > int hwirq = platform_get_irq(vdev->pdev, i); > > - irq.flags = VFIO_IRQ_INFO_EVENTFD; > + spin_lock_init(&irq.lock); > + > + irq.flags = VFIO_IRQ_INFO_EVENTFD | VFIO_IRQ_INFO_MASKABLE; > + > + if (irq_get_trigger_type(hwirq) & IRQ_TYPE_LEVEL_MASK) > + irq.flags |= VFIO_IRQ_INFO_AUTOMASKED; > + > irq.count = 1; > irq.hwirq = hwirq; > + irq.masked = false; > > vdev->irq[i] = irq; > } > @@ -66,19 +73,39 @@ void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev) > { > int i; > > - for (i = 0; i < vdev->num_irqs; i++) > + for (i = 0; i < vdev->num_irqs; i++) { > vfio_set_trigger(vdev, i, -1); > > + if (vdev->irq[i].masked) > + enable_irq(vdev->irq[i].hwirq); This looks suspicious. set_trigger(,, -1) calls free_irq() and here we enable_irq(). Shouldn't the nexe user's call to request_irq() be sufficient to re-enable it? Thanks, Alex > + } > + > kfree(vdev->irq); > } > > static irqreturn_t vfio_irq_handler(int irq, void *dev_id) > { > - struct eventfd_ctx *trigger = dev_id; > + struct vfio_platform_irq *irq_ctx = dev_id; > + unsigned long flags; > + int ret = IRQ_NONE; > + > + spin_lock_irqsave(&irq_ctx->lock, flags); > + > + if (!irq_ctx->masked) { > + ret = IRQ_HANDLED; > + > + if (irq_ctx->flags & VFIO_IRQ_INFO_AUTOMASKED) { > + disable_irq_nosync(irq_ctx->hwirq); > + irq_ctx->masked = true; > + } > + } > > - eventfd_signal(trigger, 1); > + spin_unlock_irqrestore(&irq_ctx->lock, flags); > > - return IRQ_HANDLED; > + if (ret == IRQ_HANDLED) > + eventfd_signal(irq_ctx->trigger, 1); > + > + return ret; > } > > static int vfio_set_trigger(struct vfio_platform_device *vdev, > @@ -159,6 +186,82 @@ static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev, > return -EFAULT; > } > > +static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev, > + unsigned index, unsigned start, > + unsigned count, uint32_t flags, void *data) > +{ > + uint8_t arr; > + > + if (start != 0 || count != 1) > + return -EINVAL; > + > + switch (flags & VFIO_IRQ_SET_DATA_TYPE_MASK) { > + case VFIO_IRQ_SET_DATA_BOOL: > + if (copy_from_user(&arr, data, sizeof(uint8_t))) > + return -EFAULT; > + > + if (arr != 0x1) > + return -EINVAL; > + > + case VFIO_IRQ_SET_DATA_NONE: > + > + spin_lock_irq(&vdev->irq[index].lock); > + > + if (vdev->irq[index].masked) { > + enable_irq(vdev->irq[index].hwirq); > + vdev->irq[index].masked = false; > + } > + > + spin_unlock_irq(&vdev->irq[index].lock); > + > + return 0; > + > + case VFIO_IRQ_SET_DATA_EVENTFD: /* XXX not implemented yet */ > + default: > + return -ENOTTY; > + } > + > + return 0; > +} > + > +static int vfio_platform_set_irq_mask(struct vfio_platform_device *vdev, > + unsigned index, unsigned start, > + unsigned count, uint32_t flags, void *data) > +{ > + uint8_t arr; > + > + if (start != 0 || count != 1) > + return -EINVAL; > + > + switch (flags & VFIO_IRQ_SET_DATA_TYPE_MASK) { > + case VFIO_IRQ_SET_DATA_BOOL: > + if (copy_from_user(&arr, data, sizeof(uint8_t))) > + return -EFAULT; > + > + if (arr != 0x1) > + return -EINVAL; > + > + case VFIO_IRQ_SET_DATA_NONE: > + > + spin_lock_irq(&vdev->irq[index].lock); > + > + if (!vdev->irq[index].masked) { > + disable_irq(vdev->irq[index].hwirq); > + vdev->irq[index].masked = true; > + } > + > + spin_unlock_irq(&vdev->irq[index].lock); > + > + return 0; > + > + case VFIO_IRQ_SET_DATA_EVENTFD: /* XXX not implemented yet */ > + default: > + return -ENOTTY; > + } > + > + return 0; > +} > + > int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev, > uint32_t flags, unsigned index, unsigned start, > unsigned count, void *data) > @@ -169,8 +272,10 @@ int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev, > > switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) { > case VFIO_IRQ_SET_ACTION_MASK: > + func = vfio_platform_set_irq_mask; > + break; > case VFIO_IRQ_SET_ACTION_UNMASK: > - /* XXX not implemented */ > + func = vfio_platform_set_irq_unmask; > break; > case VFIO_IRQ_SET_ACTION_TRIGGER: > func = vfio_platform_set_irq_trigger; > diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h > index e21c15a..e283171 100644 > --- a/drivers/vfio/platform/vfio_platform_private.h > +++ b/drivers/vfio/platform/vfio_platform_private.h > @@ -30,6 +30,8 @@ struct vfio_platform_irq { > u32 count; > int hwirq; > char *name; > + bool masked; > + spinlock_t lock; > }; > > struct vfio_platform_region { ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <1398707198.24318.293.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>]
* Re: [RFC PATCH v5 11/11] VFIO_PLATFORM: Support for maskable and automasked interrupts [not found] ` <1398707198.24318.293.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> @ 2014-05-02 17:25 ` Antonios Motakis 0 siblings, 0 replies; 42+ messages in thread From: Antonios Motakis @ 2014-05-02 17:25 UTC (permalink / raw) To: Alex Williamson Cc: Mark Rutland, open list, KVM devel mailing list, Catalin Marinas, Will Deacon, alvise rigo, Stuart Yoder, Linux IOMMU, VirtualOpenSystems Technical Team, kvm-arm, Christoffer Dall On Mon, Apr 28, 2014 at 7:46 PM, Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > > On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote: > > Adds support to mask interrupts, and also for automasked interrupts. > > Level sensitive interrupts are exposed as automasked interrupts and > > are masked and disabled automatically when they fire. > > > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> > > --- > > drivers/vfio/platform/vfio_platform_irq.c | 117 ++++++++++++++++++++++++-- > > drivers/vfio/platform/vfio_platform_private.h | 2 + > > 2 files changed, 113 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c > > index 433edc1..e38982f 100644 > > --- a/drivers/vfio/platform/vfio_platform_irq.c > > +++ b/drivers/vfio/platform/vfio_platform_irq.c > > @@ -52,9 +52,16 @@ int vfio_platform_irq_init(struct vfio_platform_device *vdev) > > struct vfio_platform_irq irq; > > int hwirq = platform_get_irq(vdev->pdev, i); > > > > - irq.flags = VFIO_IRQ_INFO_EVENTFD; > > + spin_lock_init(&irq.lock); > > + > > + irq.flags = VFIO_IRQ_INFO_EVENTFD | VFIO_IRQ_INFO_MASKABLE; > > + > > + if (irq_get_trigger_type(hwirq) & IRQ_TYPE_LEVEL_MASK) > > + irq.flags |= VFIO_IRQ_INFO_AUTOMASKED; > > + > > irq.count = 1; > > irq.hwirq = hwirq; > > + irq.masked = false; > > > > vdev->irq[i] = irq; > > } > > @@ -66,19 +73,39 @@ void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev) > > { > > int i; > > > > - for (i = 0; i < vdev->num_irqs; i++) > > + for (i = 0; i < vdev->num_irqs; i++) { > > vfio_set_trigger(vdev, i, -1); > > > > + if (vdev->irq[i].masked) > > + enable_irq(vdev->irq[i].hwirq); > > This looks suspicious. set_trigger(,, -1) calls free_irq() and here we > enable_irq(). Shouldn't the nexe user's call to request_irq() be > sufficient to re-enable it? Thanks, > This is actually called when we unbind from the device, so it is indeed not needed. > Alex > ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2014-07-08 22:37 UTC | newest] Thread overview: 42+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-28 15:52 [RFC PATCH v5 00/11] VFIO support for platform devices Antonios Motakis [not found] ` <1398700371-20096-1-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 15:52 ` [RFC PATCH v5 01/11] driver core: platform: add device binding path 'driver_override' Antonios Motakis 2014-05-21 0:25 ` [RFC PATCH v5_v2 " Kim Phillips [not found] ` <20140520192537.bab9fa2088c1cd5da3f92639-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2014-05-29 19:43 ` Alex Williamson [not found] ` <1401392619.2412.102.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-05-29 21:24 ` Alexander Graf 2014-05-30 0:36 ` Stuart Yoder 2014-06-03 0:42 ` [PATCH] " Kim Phillips [not found] ` <20140602194258.5135925bf3c71b9c010fb409-KZfg59tc24xl57MIdRCFDg@public.gmane.org> 2014-06-03 4:28 ` Greg KH [not found] ` <20140603042842.GD14668-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> 2014-06-26 1:08 ` Kim Phillips 2014-07-07 21:19 ` Kim Phillips 2014-07-08 22:37 ` Greg KH 2014-04-28 15:52 ` [RFC PATCH v5 02/11] ARM SMMU: Add capability IOMMU_CAP_DMA_EXEC Antonios Motakis [not found] ` <1398700371-20096-3-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 19:37 ` Will Deacon [not found] ` <20140428193709.GE22135-5wv7dgnIgG8@public.gmane.org> 2014-04-28 20:20 ` Alex Williamson 2014-04-29 9:49 ` Antonios Motakis 2014-04-29 9:52 ` Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 03/11] VFIO_IOMMU_TYPE1 for platform bus devices on ARM Antonios Motakis [not found] ` <1398700371-20096-4-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 16:43 ` Alex Williamson [not found] ` <1398703421.24318.262.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-04-28 19:19 ` Will Deacon [not found] ` <20140428191920.GC22135-5wv7dgnIgG8@public.gmane.org> 2014-04-28 20:08 ` Alex Williamson [not found] ` <1398715690.24318.321.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-04-30 13:08 ` Will Deacon 2014-04-28 15:52 ` [RFC PATCH v5 04/11] VFIO_IOMMU_TYPE1: Introduce the VFIO_DMA_MAP_FLAG_EXEC flag Antonios Motakis [not found] ` <1398700371-20096-5-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 16:53 ` Alex Williamson [not found] ` <1398704036.24318.269.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-04-29 13:16 ` Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 05/11] VFIO_PLATFORM: Initial skeleton of VFIO support for platform devices Antonios Motakis [not found] ` <1398700371-20096-6-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 17:04 ` Alex Williamson [not found] ` <1398704642.24318.273.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-04-29 13:23 ` Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 06/11] VFIO_PLATFORM: Return info for device and its memory mapped IO regions Antonios Motakis [not found] ` <1398700371-20096-7-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 17:16 ` Alex Williamson [not found] ` <1398705389.24318.279.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-05-02 16:55 ` Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 07/11] VFIO_PLATFORM: Read and write support for the device fd Antonios Motakis [not found] ` <1398700371-20096-8-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 17:24 ` Alex Williamson [not found] ` <1398705857.24318.284.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-04-29 13:27 ` Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 08/11] VFIO_PLATFORM: Support MMAP of MMIO regions Antonios Motakis [not found] ` <1398700371-20096-9-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 17:27 ` Alex Williamson [not found] ` <1398706042.24318.287.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-04-29 13:28 ` Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 09/11] VFIO_PLATFORM: Return IRQ info Antonios Motakis [not found] ` <1398700371-20096-10-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 17:33 ` Alex Williamson 2014-04-28 15:52 ` [RFC PATCH v5 10/11] VFIO_PLATFORM: Initial interrupts support Antonios Motakis 2014-04-28 15:52 ` [RFC PATCH v5 11/11] VFIO_PLATFORM: Support for maskable and automasked interrupts Antonios Motakis [not found] ` <1398700371-20096-12-git-send-email-a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> 2014-04-28 17:46 ` Alex Williamson [not found] ` <1398707198.24318.293.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org> 2014-05-02 17:25 ` Antonios Motakis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).