* [PATCH V2 1/3] vfio, platform: add support for ACPI while detecting the reset driver
[not found] <1457715275-11925-1-git-send-email-okaya@codeaurora.org>
@ 2016-03-11 16:54 ` Sinan Kaya
2016-03-11 16:54 ` [PATCH V2 2/3] vfio, platform: make reset driver a requirement Sinan Kaya
2016-03-11 16:54 ` [PATCH V2 3/3] vfio, platform: add QTI HIDMA reset driver Sinan Kaya
2 siblings, 0 replies; 10+ messages in thread
From: Sinan Kaya @ 2016-03-11 16:54 UTC (permalink / raw)
To: kvm, timur, cov, jcm, eric.auger
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Baptiste Reynal, Alex Williamson, Dan Carpenter, Arnd Bergmann,
linux-kernel
The code is using the compatible DT string to associate a reset driver with
the actual device itself. The compatible string does not exist on ACPI
based systems. HID is the unique identifier for a device driver instead.
The change allows a driver to register with DT compatible string or ACPI
HID and then match the object with one of these conditions.
Rules for loading the reset driver are as follow:
- ACPI HID needs match for ACPI systems
- DT compat needs to match for OF systems
Tested-by: Eric Auger <eric.auger@linaro.org> (device tree only)
Tested-by: Shanker Donthineni <shankerd@codeaurora.org> (ACPI only)
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
.../vfio/platform/reset/vfio_platform_amdxgbe.c | 4 +-
.../platform/reset/vfio_platform_calxedaxgmac.c | 4 +-
drivers/vfio/platform/vfio_platform_common.c | 107 +++++++++++++++++----
drivers/vfio/platform/vfio_platform_private.h | 43 +++++----
4 files changed, 120 insertions(+), 38 deletions(-)
diff --git a/drivers/vfio/platform/reset/vfio_platform_amdxgbe.c b/drivers/vfio/platform/reset/vfio_platform_amdxgbe.c
index d4030d0..6d87442 100644
--- a/drivers/vfio/platform/reset/vfio_platform_amdxgbe.c
+++ b/drivers/vfio/platform/reset/vfio_platform_amdxgbe.c
@@ -119,7 +119,9 @@ int vfio_platform_amdxgbe_reset(struct vfio_platform_device *vdev)
return 0;
}
-module_vfio_reset_handler("amd,xgbe-seattle-v1a", vfio_platform_amdxgbe_reset);
+module_vfio_reset_handler("amd,xgbe-seattle-v1a", NULL,
+ vfio_platform_amdxgbe_reset);
+MODULE_ALIAS_VFIO_PLATFORM_RESET("amd,xgbe-seattle-v1a");
MODULE_VERSION("0.1");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
index e3d3d94..952dd55 100644
--- a/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
+++ b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
@@ -77,7 +77,9 @@ int vfio_platform_calxedaxgmac_reset(struct vfio_platform_device *vdev)
return 0;
}
-module_vfio_reset_handler("calxeda,hb-xgmac", vfio_platform_calxedaxgmac_reset);
+module_vfio_reset_handler("calxeda,hb-xgmac", NULL,
+ vfio_platform_calxedaxgmac_reset);
+MODULE_ALIAS_VFIO_PLATFORM_RESET("calxeda,hb-xgmac");
MODULE_VERSION(DRIVER_VERSION);
MODULE_LICENSE("GPL v2");
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 418cdd9..b3f6ba2 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -13,6 +13,7 @@
*/
#include <linux/device.h>
+#include <linux/acpi.h>
#include <linux/iommu.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -31,14 +32,22 @@ static LIST_HEAD(reset_list);
static DEFINE_MUTEX(driver_lock);
static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
- struct module **module)
+ const char *acpihid, struct module **module)
{
struct vfio_platform_reset_node *iter;
vfio_platform_reset_fn_t reset_fn = NULL;
mutex_lock(&driver_lock);
list_for_each_entry(iter, &reset_list, link) {
- if (!strcmp(iter->compat, compat) &&
+ if (acpihid && iter->acpihid &&
+ !strcmp(iter->acpihid, acpihid) &&
+ try_module_get(iter->owner)) {
+ *module = iter->owner;
+ reset_fn = iter->reset;
+ break;
+ }
+ if (compat && iter->compat &&
+ !strcmp(iter->compat, compat) &&
try_module_get(iter->owner)) {
*module = iter->owner;
reset_fn = iter->reset;
@@ -49,15 +58,30 @@ static vfio_platform_reset_fn_t vfio_platform_lookup_reset(const char *compat,
return reset_fn;
}
-static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
+static int vfio_platform_get_reset(struct vfio_platform_device *vdev)
{
- vdev->reset = vfio_platform_lookup_reset(vdev->compat,
- &vdev->reset_module);
- if (!vdev->reset) {
- request_module("vfio-reset:%s", vdev->compat);
- vdev->reset = vfio_platform_lookup_reset(vdev->compat,
- &vdev->reset_module);
- }
+ int rc = -ENODEV;
+
+ vdev->reset = vfio_platform_lookup_reset(vdev->compat, vdev->acpihid,
+ &vdev->reset_module);
+ if (vdev->reset)
+ return 0;
+
+ if (vdev->acpihid)
+ rc = request_module("vfio-reset:%s", vdev->acpihid);
+
+ if (rc && vdev->compat)
+ rc = request_module("vfio-reset:%s", vdev->compat);
+
+ if (rc)
+ return rc;
+
+ vdev->reset = vfio_platform_lookup_reset(vdev->compat, vdev->acpihid,
+ &vdev->reset_module);
+ if (vdev->reset)
+ return 0;
+
+ return -ENODEV;
}
static void vfio_platform_put_reset(struct vfio_platform_device *vdev)
@@ -541,6 +565,46 @@ static const struct vfio_device_ops vfio_platform_ops = {
.mmap = vfio_platform_mmap,
};
+#ifdef CONFIG_ACPI
+int vfio_platform_probe_acpi(struct vfio_platform_device *vdev,
+ struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (!adev)
+ return -ENODEV;
+
+ vdev->acpihid = acpi_device_hid(adev);
+ if (!vdev->acpihid) {
+ pr_err("VFIO: cannot find ACPI HID for %s\n",
+ vdev->name);
+ return -ENODEV;
+ }
+ return 0;
+}
+#else
+int vfio_platform_probe_acpi(struct vfio_platform_device *vdev,
+ struct device *dev)
+{
+ return -EINVAL;
+}
+#endif
+
+int vfio_platform_probe_of(struct vfio_platform_device *vdev,
+ struct device *dev)
+{
+ int ret;
+
+ ret = device_property_read_string(dev, "compatible",
+ &vdev->compat);
+ if (ret) {
+ pr_err("VFIO: cannot retrieve compat for %s\n",
+ vdev->name);
+ return ret;
+ }
+ return 0;
+}
+
int vfio_platform_probe_common(struct vfio_platform_device *vdev,
struct device *dev)
{
@@ -550,14 +614,14 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
if (!vdev)
return -EINVAL;
- ret = device_property_read_string(dev, "compatible", &vdev->compat);
- if (ret) {
- pr_err("VFIO: cannot retrieve compat for %s\n", vdev->name);
- return -EINVAL;
- }
+ ret = vfio_platform_probe_acpi(vdev, dev);
+ if (ret)
+ ret = vfio_platform_probe_of(vdev, dev);
- vdev->device = dev;
+ if (ret)
+ return ret;
+ vdev->device = dev;
group = iommu_group_get(dev);
if (!group) {
pr_err("VFIO: No IOMMU group for device %s\n", vdev->name);
@@ -571,7 +635,6 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
}
vfio_platform_get_reset(vdev);
-
mutex_init(&vdev->igate);
return 0;
@@ -602,13 +665,21 @@ void __vfio_platform_register_reset(struct vfio_platform_reset_node *node)
EXPORT_SYMBOL_GPL(__vfio_platform_register_reset);
void vfio_platform_unregister_reset(const char *compat,
+ const char *acpihid,
vfio_platform_reset_fn_t fn)
{
struct vfio_platform_reset_node *iter, *temp;
mutex_lock(&driver_lock);
list_for_each_entry_safe(iter, temp, &reset_list, link) {
- if (!strcmp(iter->compat, compat) && (iter->reset == fn)) {
+ if (acpihid && iter->acpihid &&
+ !strcmp(iter->acpihid, acpihid) && (iter->reset == fn)) {
+ list_del(&iter->link);
+ break;
+ }
+
+ if (compat && iter->compat &&
+ !strcmp(iter->compat, compat) && (iter->reset == fn)) {
list_del(&iter->link);
break;
}
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 42816dd..1b95adf 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -58,6 +58,7 @@ struct vfio_platform_device {
struct mutex igate;
struct module *parent_module;
const char *compat;
+ const char *acpihid;
struct module *reset_module;
struct device *device;
@@ -79,6 +80,7 @@ typedef int (*vfio_platform_reset_fn_t)(struct vfio_platform_device *vdev);
struct vfio_platform_reset_node {
struct list_head link;
char *compat;
+ char *acpihid;
struct module *owner;
vfio_platform_reset_fn_t reset;
};
@@ -98,27 +100,32 @@ extern int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
extern void __vfio_platform_register_reset(struct vfio_platform_reset_node *n);
extern void vfio_platform_unregister_reset(const char *compat,
+ const char *acpihid,
vfio_platform_reset_fn_t fn);
-#define vfio_platform_register_reset(__compat, __reset) \
-static struct vfio_platform_reset_node __reset ## _node = { \
- .owner = THIS_MODULE, \
- .compat = __compat, \
- .reset = __reset, \
-}; \
+
+#define vfio_platform_register_reset(__compat, __acpihid, __reset) \
+static struct vfio_platform_reset_node __reset ## _node = { \
+ .owner = THIS_MODULE, \
+ .compat = __compat, \
+ .acpihid = __acpihid, \
+ .reset = __reset, \
+}; \
__vfio_platform_register_reset(&__reset ## _node)
-#define module_vfio_reset_handler(compat, reset) \
-MODULE_ALIAS("vfio-reset:" compat); \
-static int __init reset ## _module_init(void) \
-{ \
- vfio_platform_register_reset(compat, reset); \
- return 0; \
-}; \
-static void __exit reset ## _module_exit(void) \
-{ \
- vfio_platform_unregister_reset(compat, reset); \
-}; \
-module_init(reset ## _module_init); \
+#define MODULE_ALIAS_VFIO_PLATFORM_RESET(name) \
+ MODULE_ALIAS("vfio-reset:" name)
+
+#define module_vfio_reset_handler(compat, acpihid, reset) \
+static int __init reset ## _module_init(void) \
+{ \
+ vfio_platform_register_reset(compat, acpihid, reset); \
+ return 0; \
+}; \
+static void __exit reset ## _module_exit(void) \
+{ \
+ vfio_platform_unregister_reset(compat, acpihid, reset); \
+}; \
+module_init(reset ## _module_init); \
module_exit(reset ## _module_exit)
#endif /* VFIO_PLATFORM_PRIVATE_H */
--
1.8.2.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH V2 2/3] vfio, platform: make reset driver a requirement
[not found] <1457715275-11925-1-git-send-email-okaya@codeaurora.org>
2016-03-11 16:54 ` [PATCH V2 1/3] vfio, platform: add support for ACPI while detecting the reset driver Sinan Kaya
@ 2016-03-11 16:54 ` Sinan Kaya
2016-03-13 17:25 ` Sinan Kaya
2016-03-11 16:54 ` [PATCH V2 3/3] vfio, platform: add QTI HIDMA reset driver Sinan Kaya
2 siblings, 1 reply; 10+ messages in thread
From: Sinan Kaya @ 2016-03-11 16:54 UTC (permalink / raw)
To: kvm, timur, cov, jcm, eric.auger
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Baptiste Reynal, Alex Williamson, linux-kernel
The code was allowing platform devices to be used without a supporting VFIO
reset driver. The hardware can be left in some inconsistent state after a
guest machine abort.
The reset driver will put the hardware back to safe state and disable
interrupts before returning the control back to the host machine.
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/vfio_platform_common.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index b3f6ba2..5535fe1 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -165,12 +165,8 @@ static void vfio_platform_release(void *device_data)
mutex_lock(&driver_lock);
if (!(--vdev->refcnt)) {
- if (vdev->reset) {
- dev_info(vdev->device, "reset\n");
- vdev->reset(vdev);
- } else {
- dev_warn(vdev->device, "no reset function found!\n");
- }
+ dev_info(vdev->device, "reset\n");
+ vdev->reset(vdev);
vfio_platform_regions_cleanup(vdev);
vfio_platform_irq_cleanup(vdev);
}
@@ -634,7 +630,13 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
return ret;
}
- vfio_platform_get_reset(vdev);
+ ret = vfio_platform_get_reset(vdev);
+ if (ret) {
+ pr_err("vfio: no reset function found for device %s\n",
+ vdev->name);
+ iommu_group_put(group);
+ return ret;
+ }
mutex_init(&vdev->igate);
return 0;
--
1.8.2.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH V2 3/3] vfio, platform: add QTI HIDMA reset driver
[not found] <1457715275-11925-1-git-send-email-okaya@codeaurora.org>
2016-03-11 16:54 ` [PATCH V2 1/3] vfio, platform: add support for ACPI while detecting the reset driver Sinan Kaya
2016-03-11 16:54 ` [PATCH V2 2/3] vfio, platform: make reset driver a requirement Sinan Kaya
@ 2016-03-11 16:54 ` Sinan Kaya
2016-03-11 17:00 ` Sinan Kaya
2016-03-11 18:35 ` kbuild test robot
2 siblings, 2 replies; 10+ messages in thread
From: Sinan Kaya @ 2016-03-11 16:54 UTC (permalink / raw)
To: kvm, timur, cov, jcm, eric.auger
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel, Sinan Kaya,
Baptiste Reynal, Alex Williamson, Arnd Bergmann, linux-kernel
In situations where the userspace driver is stopped abnormally and the
VFIO platform device is released, the assigned HW device currently is
left running. As a consequence the HW device might continue issuing IRQs
and performing DMA accesses.
This patch is implementing a reset driver for HIDMA platform driver.
This gets called by the VFIO platform reset interface.
Reviewed-by: Eric Auger <eric.auger@linaro.org>
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/vfio/platform/reset/Kconfig | 9 ++
drivers/vfio/platform/reset/Makefile | 2 +
.../vfio/platform/reset/vfio_platform_qcomhidma.c | 101 +++++++++++++++++++++
3 files changed, 112 insertions(+)
create mode 100644 drivers/vfio/platform/reset/vfio_platform_qcomhidma.c
diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
index 70cccc5..d02b3b5 100644
--- a/drivers/vfio/platform/reset/Kconfig
+++ b/drivers/vfio/platform/reset/Kconfig
@@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
Enables the VFIO platform driver to handle reset for AMD XGBE
If you don't know what to do here, say N.
+
+config VFIO_PLATFORM_QCOMHIDMA_RESET
+ tristate "VFIO support for Qualcomm Technologies HIDMA reset"
+ depends on VFIO_PLATFORM
+ help
+ Enables the VFIO platform driver to handle reset for Qualcomm Technologies
+ HIDMA Channel.
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
index 93f4e23..ec7748a 100644
--- a/drivers/vfio/platform/reset/Makefile
+++ b/drivers/vfio/platform/reset/Makefile
@@ -1,7 +1,9 @@
vfio-platform-calxedaxgmac-y := vfio_platform_calxedaxgmac.o
vfio-platform-amdxgbe-y := vfio_platform_amdxgbe.o
+vfio-platform-qcomhidma-y := vfio_platform_qcomhidma.o
ccflags-y += -Idrivers/vfio/platform
obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
+obj-$(CONFIG_VFIO_PLATFORM_QCOMHIDMA_RESET) += vfio-platform-qcomhidma.o
obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
diff --git a/drivers/vfio/platform/reset/vfio_platform_qcomhidma.c b/drivers/vfio/platform/reset/vfio_platform_qcomhidma.c
new file mode 100644
index 0000000..103d481
--- /dev/null
+++ b/drivers/vfio/platform/reset/vfio_platform_qcomhidma.c
@@ -0,0 +1,101 @@
+/*
+ * Qualcomm Technologies HIDMA VFIO Reset Driver
+ *
+ * Copyright (c) 2016, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/device.h>
+#include <linux/iopoll.h>
+
+#include "vfio_platform_private.h"
+
+#define TRCA_CTRLSTS_OFFSET 0x000
+#define EVCA_CTRLSTS_OFFSET 0x000
+
+#define CH_CONTROL_MASK GENMASK(7, 0)
+#define CH_STATE_MASK GENMASK(7, 0)
+#define CH_STATE_BIT_POS 0x8
+
+#define HIDMA_CH_STATE(val) \
+ ((val >> CH_STATE_BIT_POS) & CH_STATE_MASK)
+
+#define EVCA_IRQ_EN_OFFSET 0x110
+
+#define CH_RESET 9
+#define CH_DISABLED 0
+
+int vfio_platform_qcomhidma_reset(struct vfio_platform_device *vdev)
+{
+ struct vfio_platform_region *trreg;
+ struct vfio_platform_region *evreg;
+ u32 val;
+ int ret;
+
+ if (vdev->num_regions != 2)
+ return -ENODEV;
+
+ trreg = &vdev->regions[0];
+ if (!trreg->ioaddr) {
+ trreg->ioaddr =
+ ioremap_nocache(trreg->addr, trreg->size);
+ if (!trreg->ioaddr)
+ return -ENOMEM;
+ }
+
+ evreg = &vdev->regions[1];
+ if (!evreg->ioaddr) {
+ evreg->ioaddr =
+ ioremap_nocache(evreg->addr, evreg->size);
+ if (!evreg->ioaddr)
+ return -ENOMEM;
+ }
+
+ /* disable IRQ */
+ writel(0, evreg->ioaddr + EVCA_IRQ_EN_OFFSET);
+
+ /* reset both transfer and event channels */
+ val = readl(trreg->ioaddr + TRCA_CTRLSTS_OFFSET);
+ val &= ~(CH_CONTROL_MASK << 16);
+ val |= CH_RESET << 16;
+ writel(val, trreg->ioaddr + TRCA_CTRLSTS_OFFSET);
+
+ ret = readl_poll_timeout(trreg->ioaddr + TRCA_CTRLSTS_OFFSET, val,
+ HIDMA_CH_STATE(val) == CH_DISABLED, 1000,
+ 10000);
+ if (ret)
+ return ret;
+
+ val = readl(evreg->ioaddr + EVCA_CTRLSTS_OFFSET);
+ val &= ~(CH_CONTROL_MASK << 16);
+ val |= CH_RESET << 16;
+ writel(val, evreg->ioaddr + EVCA_CTRLSTS_OFFSET);
+
+ ret = readl_poll_timeout(evreg->ioaddr + EVCA_CTRLSTS_OFFSET, val,
+ HIDMA_CH_STATE(val) == CH_DISABLED, 1000,
+ 10000);
+ if (ret)
+ return ret;
+
+ pr_info("HIDMA channel reset\n");
+ return 0;
+}
+module_vfio_reset_handler("qcom,hidma-1.0", "QCOM8061",
+ vfio_platform_qcomhidma_reset);
+VFIO_PLATFORM_MODULE_ALIAS("qcom,hidma-1.0");
+VFIO_PLATFORM_MODULE_ALIAS("QCOM8061");
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Reset support for Qualcomm Technologies HIDMA device");
--
1.8.2.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH V2 3/3] vfio, platform: add QTI HIDMA reset driver
2016-03-11 16:54 ` [PATCH V2 3/3] vfio, platform: add QTI HIDMA reset driver Sinan Kaya
@ 2016-03-11 17:00 ` Sinan Kaya
2016-03-11 18:35 ` kbuild test robot
1 sibling, 0 replies; 10+ messages in thread
From: Sinan Kaya @ 2016-03-11 17:00 UTC (permalink / raw)
To: kvm, timur, cov, jcm, eric.auger
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel,
Baptiste Reynal, Alex Williamson, Arnd Bergmann, linux-kernel
On 3/11/2016 11:54 AM, Sinan Kaya wrote:
> VFIO_PLATFORM_MODULE_ALIAS("qcom,hidma-1.0");
> +VFIO_PLATFORM_MODULE_ALIAS("QCOM8061");
I forgot to rename these to MODULE_ALIAS_VFIO_PLATFORM_RESET.
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2 3/3] vfio, platform: add QTI HIDMA reset driver
2016-03-11 16:54 ` [PATCH V2 3/3] vfio, platform: add QTI HIDMA reset driver Sinan Kaya
2016-03-11 17:00 ` Sinan Kaya
@ 2016-03-11 18:35 ` kbuild test robot
1 sibling, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2016-03-11 18:35 UTC (permalink / raw)
To: Sinan Kaya
Cc: kbuild-all, kvm, timur, cov, jcm, eric.auger, shankerd, vikrams,
marc.zyngier, mark.rutland, devicetree, vinod.koul, agross,
linux-arm-msm, linux-arm-kernel, Sinan Kaya, Baptiste Reynal,
Alex Williamson, Arnd Bergmann, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1688 bytes --]
Hi Sinan,
[auto build test ERROR on vfio/next]
[also build test ERROR on v4.5-rc7 next-20160311]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Sinan-Kaya/vfio-platform-add-HIDMA-and-ACPI-support/20160312-005737
base: https://github.com/awilliam/linux-vfio.git next
config: arm-allmodconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
>> drivers/vfio/platform/reset/vfio_platform_qcomhidma.c:97:28: error: expected declaration specifiers or '...' before string constant
VFIO_PLATFORM_MODULE_ALIAS("qcom,hidma-1.0");
^
drivers/vfio/platform/reset/vfio_platform_qcomhidma.c:98:28: error: expected declaration specifiers or '...' before string constant
VFIO_PLATFORM_MODULE_ALIAS("QCOM8061");
^
vim +97 drivers/vfio/platform/reset/vfio_platform_qcomhidma.c
91
92 pr_info("HIDMA channel reset\n");
93 return 0;
94 }
95 module_vfio_reset_handler("qcom,hidma-1.0", "QCOM8061",
96 vfio_platform_qcomhidma_reset);
> 97 VFIO_PLATFORM_MODULE_ALIAS("qcom,hidma-1.0");
98 VFIO_PLATFORM_MODULE_ALIAS("QCOM8061");
99
100 MODULE_LICENSE("GPL v2");
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 56252 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2 2/3] vfio, platform: make reset driver a requirement
2016-03-11 16:54 ` [PATCH V2 2/3] vfio, platform: make reset driver a requirement Sinan Kaya
@ 2016-03-13 17:25 ` Sinan Kaya
2016-03-16 4:36 ` Eric Auger
0 siblings, 1 reply; 10+ messages in thread
From: Sinan Kaya @ 2016-03-13 17:25 UTC (permalink / raw)
To: kvm, timur, cov, jcm, eric.auger
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel,
Baptiste Reynal, Alex Williamson, linux-kernel
On 3/11/2016 11:54 AM, Sinan Kaya wrote:
> The code was allowing platform devices to be used without a supporting VFIO
> reset driver. The hardware can be left in some inconsistent state after a
> guest machine abort.
>
> The reset driver will put the hardware back to safe state and disable
> interrupts before returning the control back to the host machine.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/vfio/platform/vfio_platform_common.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
I was looking at the code. It looks like this is going to break VFIO AMBA. The
common implementation is shared with AMBA and platform devices.
I couldn't see a reset function for AMBA devices.
Is there anyway to write reset function for it? I have no experience with
AMBA devices.
Would you include a reset needed flag and just not require it for AMBA?
(I honestly don't like this idea)
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2 2/3] vfio, platform: make reset driver a requirement
2016-03-13 17:25 ` Sinan Kaya
@ 2016-03-16 4:36 ` Eric Auger
2016-03-16 14:27 ` Sinan Kaya
0 siblings, 1 reply; 10+ messages in thread
From: Eric Auger @ 2016-03-16 4:36 UTC (permalink / raw)
To: Sinan Kaya, kvm, timur, cov, jcm
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel,
Baptiste Reynal, Alex Williamson, linux-kernel
Hi Sinan,
On 03/13/2016 06:25 PM, Sinan Kaya wrote:
> On 3/11/2016 11:54 AM, Sinan Kaya wrote:
>> The code was allowing platform devices to be used without a supporting VFIO
>> reset driver. The hardware can be left in some inconsistent state after a
>> guest machine abort.
>>
>> The reset driver will put the hardware back to safe state and disable
>> interrupts before returning the control back to the host machine.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>> drivers/vfio/platform/vfio_platform_common.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
> I was looking at the code. It looks like this is going to break VFIO AMBA. The
> common implementation is shared with AMBA and platform devices.
>
> I couldn't see a reset function for AMBA devices.
>
> Is there anyway to write reset function for it? I have no experience with
> AMBA devices.
To my knowledge only the PL330 DMA controller (drivers/dma/pl330.c) was
passthrough'ed at some point, rather for development purpose than
production purpose. This was done by Virtual Open Systems (ask Baptiste
for more details). But I don't think this is really used.
>
> Would you include a reset needed flag and just not require it for AMBA?
> (I honestly don't like this idea)
>
I think the requirement also makes sense for AMBA.
Maybe an option would be to add a module parameter that would allow to
use the vfio platform/amba driver without reset module (with explicit
opt-in from the user). Maybe this can be done later on.
FYI I will not have access to my mailbox until the end of the week.
Best Regards
Eric
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2 2/3] vfio, platform: make reset driver a requirement
2016-03-16 4:36 ` Eric Auger
@ 2016-03-16 14:27 ` Sinan Kaya
2016-03-27 15:21 ` Eric Auger
0 siblings, 1 reply; 10+ messages in thread
From: Sinan Kaya @ 2016-03-16 14:27 UTC (permalink / raw)
To: Eric Auger, kvm, timur, cov, jcm, Baptiste Reynal
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel,
Baptiste Reynal, Alex Williamson, linux-kernel
Hi Baptiste,
>> I couldn't see a reset function for AMBA devices.
>>
>> Is there anyway to write reset function for it? I have no experience with
>> AMBA devices.
> To my knowledge only the PL330 DMA controller (drivers/dma/pl330.c) was
> passthrough'ed at some point, rather for development purpose than
> production purpose. This was done by Virtual Open Systems (ask Baptiste
> for more details). But I don't think this is really used.
Are you OK with requiring reset driver for AMBA devices too? The PL330
driver won't work until a reset driver is submitted for it.
>>
>> Would you include a reset needed flag and just not require it for AMBA?
>> (I honestly don't like this idea)
>>
> I think the requirement also makes sense for AMBA.
>
> Maybe an option would be to add a module parameter that would allow to
> use the vfio platform/amba driver without reset module (with explicit
> opt-in from the user). Maybe this can be done later on.
I can see this flag useful for testing purposes but it should not be used in
production.
How about I add a module parameter which is not set by default? When set,
I don't perform the reset requirement check.
>
> FYI I will not have access to my mailbox until the end of the week.
OK, no rush.
>
> Best Regards
>
> Eric
>
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2 2/3] vfio, platform: make reset driver a requirement
2016-03-16 14:27 ` Sinan Kaya
@ 2016-03-27 15:21 ` Eric Auger
2016-03-27 17:55 ` Sinan Kaya
0 siblings, 1 reply; 10+ messages in thread
From: Eric Auger @ 2016-03-27 15:21 UTC (permalink / raw)
To: Sinan Kaya, kvm, timur, cov, jcm, Baptiste Reynal
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel,
Alex Williamson, linux-kernel
Hi Sinan,
On 03/16/2016 03:27 PM, Sinan Kaya wrote:
> Hi Baptiste,
>
>>> I couldn't see a reset function for AMBA devices.
>>>
>>> Is there anyway to write reset function for it? I have no experience with
>>> AMBA devices.
>> To my knowledge only the PL330 DMA controller (drivers/dma/pl330.c) was
>> passthrough'ed at some point, rather for development purpose than
>> production purpose. This was done by Virtual Open Systems (ask Baptiste
>> for more details). But I don't think this is really used.
>
> Are you OK with requiring reset driver for AMBA devices too? The PL330
> driver won't work until a reset driver is submitted for it.
>
>>>
>>> Would you include a reset needed flag and just not require it for AMBA?
>>> (I honestly don't like this idea)
>>>
>> I think the requirement also makes sense for AMBA.
>>
>> Maybe an option would be to add a module parameter that would allow to
>> use the vfio platform/amba driver without reset module (with explicit
>> opt-in from the user). Maybe this can be done later on.
>
> I can see this flag useful for testing purposes but it should not be used in
> production.
>
> How about I add a module parameter which is not set by default? When set,
> I don't perform the reset requirement check.
Yes that's what I meant
Best Regards
Eric
>
>>
>> FYI I will not have access to my mailbox until the end of the week.
>
> OK, no rush.
>
>>
>> Best Regards
>>
>> Eric
>>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2 2/3] vfio, platform: make reset driver a requirement
2016-03-27 15:21 ` Eric Auger
@ 2016-03-27 17:55 ` Sinan Kaya
0 siblings, 0 replies; 10+ messages in thread
From: Sinan Kaya @ 2016-03-27 17:55 UTC (permalink / raw)
To: Eric Auger, kvm, timur, cov, jcm, Baptiste Reynal
Cc: shankerd, vikrams, marc.zyngier, mark.rutland, devicetree,
vinod.koul, agross, linux-arm-msm, linux-arm-kernel,
Alex Williamson, linux-kernel
On 3/27/2016 11:21 AM, Eric Auger wrote:
>> How about I add a module parameter which is not set by default? When set,
>> > I don't perform the reset requirement check.
> Yes that's what I meant
>
> Best Regards
>
> Eric
Got it. I'll post something soon.
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-03-27 17:55 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1457715275-11925-1-git-send-email-okaya@codeaurora.org>
2016-03-11 16:54 ` [PATCH V2 1/3] vfio, platform: add support for ACPI while detecting the reset driver Sinan Kaya
2016-03-11 16:54 ` [PATCH V2 2/3] vfio, platform: make reset driver a requirement Sinan Kaya
2016-03-13 17:25 ` Sinan Kaya
2016-03-16 4:36 ` Eric Auger
2016-03-16 14:27 ` Sinan Kaya
2016-03-27 15:21 ` Eric Auger
2016-03-27 17:55 ` Sinan Kaya
2016-03-11 16:54 ` [PATCH V2 3/3] vfio, platform: add QTI HIDMA reset driver Sinan Kaya
2016-03-11 17:00 ` Sinan Kaya
2016-03-11 18:35 ` kbuild test robot
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).