Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1254564 > unrolled thread

[PATCH v3 0/7] VFIO platform reset module rework

Started byEric Auger <eric.auger@linaro.org>
First post2015-10-23 14:40 +0200
Last post2015-10-23 15:10 +0200
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/7] VFIO platform reset module rework Eric Auger <eric.auger@linaro.org> - 2015-10-23 14:40 +0200
    [PATCH v3 2/7] vfio: platform: add capability to register a reset function Eric Auger <eric.auger@linaro.org> - 2015-10-23 14:50 +0200
      Re: [PATCH v3 2/7] vfio: platform: add capability to register a reset function Arnd Bergmann <arnd@arndb.de> - 2015-10-23 15:10 +0200
        Re: [PATCH v3 2/7] vfio: platform: add capability to register a reset  function Eric Auger <eric.auger@linaro.org> - 2015-10-23 16:20 +0200
    [PATCH v3 1/7] vfio: platform: introduce vfio-platform-base module Eric Auger <eric.auger@linaro.org> - 2015-10-23 14:50 +0200
      Re: [PATCH v3 1/7] vfio: platform: introduce vfio-platform-base module Arnd Bergmann <arnd@arndb.de> - 2015-10-23 15:10 +0200

#1254564 — [PATCH v3 0/7] VFIO platform reset module rework

FromEric Auger <eric.auger@linaro.org>
Date2015-10-23 14:40 +0200
Subject[PATCH v3 0/7] VFIO platform reset module rework
Message-ID<qmJNw-1va-13@gated-at.bofh.it>
This series fixes the current implementation by getting rid of the
usage of __symbol_get which caused a compilation issue with
CONFIG_MODULES disabled. On top of this, the usage of MODULE_ALIAS makes
possible to add a new reset module without being obliged to update the
framework. The new implementation relies on the reset module registering
its reset function to the vfio-platform driver.

The series is available at

https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.3-rc6-rework-v3

Best Regards

Eric

v2 -> v3:
- use driver_mutex instead of reset_mutex
- style fixes: single mutex_unlock
- use static nodes; vfio_platform_register_reset now is a macro
- vfio_platform_reset_private.h removed since reset_module_(un)register
  disappear. No use of symbol_get anymore
- new patch introducing vfio-platform-base
- reset look-up moved back at vfio-platform probe time
- new patch featuring dev_info/dev_warn

v1 -> v2:
* in vfio_platform_common.c:
  - move reset lookup at load time and put reset at release: this is to
    prevent a race between the 2 load module loads
  - reset_list becomes static
  - vfio_platform_register/unregister_reset take a const char * as compat
  - fix node link
  - remove old combo struct and cleanup proto of vfio_platform_get_reset
  - add mutex to protect the reset list
* in calxeda xgmac reset module
  - introduce vfio_platform_reset_private.h
  - use module_vfio_reset_handler macro
  - do not export vfio_platform_calxedaxgmac_reset symbol anymore
  - add a pr_info to show the device is reset by vfio reset module


Eric Auger (7):
  vfio: platform: introduce vfio-platform-base module
  vfio: platform: add capability to register a reset function
  vfio: platform: introduce module_vfio_reset_handler macro
  vfio: platform: reset: calxedaxgmac: add reset function registration
  vfio: platform: add compat in vfio_platform_device
  vfio: platform: use list of registered reset function
  vfio: platform: add dev_info on device reset

 drivers/vfio/platform/Makefile                     |   6 +-
 .../platform/reset/vfio_platform_calxedaxgmac.c    |   5 +-
 drivers/vfio/platform/vfio_amba.c                  |   1 +
 drivers/vfio/platform/vfio_platform.c              |   1 +
 drivers/vfio/platform/vfio_platform_common.c       | 118 +++++++++++++++------
 drivers/vfio/platform/vfio_platform_private.h      |  40 ++++++-
 6 files changed, 129 insertions(+), 42 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1254566 — [PATCH v3 2/7] vfio: platform: add capability to register a reset function

FromEric Auger <eric.auger@linaro.org>
Date2015-10-23 14:50 +0200
Subject[PATCH v3 2/7] vfio: platform: add capability to register a reset function
Message-ID<qmJXc-1GX-9@gated-at.bofh.it>
In reply to#1254564
In preparation for subsequent changes in reset function lookup,
lets introduce a dynamic list of reset combos (compat string,
reset module, reset function). The list can be populated/voided with
two new functions, vfio_platform_register/unregister_reset. Those are
not yet used in this patch.

Signed-off-by: Eric Auger <eric.auger@linaro.org>

---

v2 -> v3:
- use goto out to have a single mutex_unlock
- implement vfio_platform_register_reset as a macro (suggested by Arnd)
- move reset_node struct declaration back to vfio_platform_private.h
- vfio_platform_unregister_reset does not return any value anymore

v1 -> v2:
- reset_list becomes static
- vfio_platform_register/unregister_reset take a const char * as compat
- fix node leak
- add reset_lock to protect the reset list manipulation
- move vfio_platform_reset_node declaration in vfio_platform_common.c
---
 drivers/vfio/platform/vfio_platform_common.c  | 26 ++++++++++++++++++++++++++
 drivers/vfio/platform/vfio_platform_private.h | 20 ++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 184e9d2..8eccd30 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -27,6 +27,7 @@
 #define DRIVER_AUTHOR   "Antonios Motakis <a.motakis@virtualopensystems.com>"
 #define DRIVER_DESC     "VFIO platform base module"
 
+static LIST_HEAD(reset_list);
 static DEFINE_MUTEX(driver_lock);
 
 static const struct vfio_platform_reset_combo reset_lookup_table[] = {
@@ -578,6 +579,31 @@ struct vfio_platform_device *vfio_platform_remove_common(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(vfio_platform_remove_common);
 
+int __vfio_platform_register_reset(struct vfio_platform_reset_node *node)
+{
+	mutex_lock(&driver_lock);
+	list_add(&node->link, &reset_list);
+	mutex_unlock(&driver_lock);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__vfio_platform_register_reset);
+
+void vfio_platform_unregister_reset(const char *compat)
+{
+	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)) {
+			list_del(&iter->link);
+			break;
+		}
+	}
+
+	mutex_unlock(&driver_lock);
+}
+EXPORT_SYMBOL_GPL(vfio_platform_unregister_reset);
+
 MODULE_VERSION(DRIVER_VERSION);
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR(DRIVER_AUTHOR);
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 7128690..277521a 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -71,6 +71,15 @@ struct vfio_platform_device {
 	int	(*reset)(struct vfio_platform_device *vdev);
 };
 
+typedef int (*vfio_platform_reset_fn_t)(struct vfio_platform_device *vdev);
+
+struct vfio_platform_reset_node {
+	struct list_head link;
+	char *compat;
+	struct module *owner;
+	vfio_platform_reset_fn_t reset;
+};
+
 struct vfio_platform_reset_combo {
 	const char *compat;
 	const char *reset_function_name;
@@ -90,4 +99,15 @@ extern int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
 					unsigned start, unsigned count,
 					void *data);
 
+extern int __vfio_platform_register_reset(struct vfio_platform_reset_node *n);
+extern void vfio_platform_unregister_reset(const char *compat);
+
+#define vfio_platform_register_reset(__compat, __reset)		\
+static struct vfio_platform_reset_node __reset ## _node = {	\
+	.owner = THIS_MODULE,					\
+	.compat = __compat,					\
+	.reset = __reset,					\
+};								\
+__vfio_platform_register_reset(&__reset ## _node)
+
 #endif /* VFIO_PLATFORM_PRIVATE_H */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254587 — Re: [PATCH v3 2/7] vfio: platform: add capability to register a reset function

FromArnd Bergmann <arnd@arndb.de>
Date2015-10-23 15:10 +0200
SubjectRe: [PATCH v3 2/7] vfio: platform: add capability to register a reset function
Message-ID<qmKgz-2iZ-33@gated-at.bofh.it>
In reply to#1254566
On Friday 23 October 2015 14:37:10 Eric Auger wrote:
> +
> +void vfio_platform_unregister_reset(const char *compat)
> +{
> +       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)) {
> +                       list_del(&iter->link);
> +                       break;
> +               }
> +       }
> +
> +       mutex_unlock(&driver_lock);
> +}
> +EXPORT_SYMBOL_GPL(vfio_platform_unregister_reset);
> 

This is slightly unsafe in case you ever get two drivers that register
with the same compat string. If we care about that, we could pass
and compare both the string and the function pointer, or the
vfio_platform_reset_node.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254636 — Re: [PATCH v3 2/7] vfio: platform: add capability to register a reset function

FromEric Auger <eric.auger@linaro.org>
Date2015-10-23 16:20 +0200
SubjectRe: [PATCH v3 2/7] vfio: platform: add capability to register a reset function
Message-ID<qmLmi-3Q4-17@gated-at.bofh.it>
In reply to#1254587
On 10/23/2015 03:07 PM, Arnd Bergmann wrote:
> On Friday 23 October 2015 14:37:10 Eric Auger wrote:
>> +
>> +void vfio_platform_unregister_reset(const char *compat)
>> +{
>> +       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)) {
>> +                       list_del(&iter->link);
>> +                       break;
>> +               }
>> +       }
>> +
>> +       mutex_unlock(&driver_lock);
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_platform_unregister_reset);
>>
> 
> This is slightly unsafe in case you ever get two drivers that register
> with the same compat string. If we care about that, we could pass
> and compare both the string and the function pointer, or the
> vfio_platform_reset_node.
OK

Thanks

Eric
> 
> 	Arnd
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254574 — [PATCH v3 1/7] vfio: platform: introduce vfio-platform-base module

FromEric Auger <eric.auger@linaro.org>
Date2015-10-23 14:50 +0200
Subject[PATCH v3 1/7] vfio: platform: introduce vfio-platform-base module
Message-ID<qmJXd-1GX-35@gated-at.bofh.it>
In reply to#1254564
To prepare for vfio platform reset rework let's build
vfio_platform_common.c and vfio_platform_irq.c in a separate
module from vfio-platform and vfio-amba. This makes possible
to have separate module inits and works around a race between
platform driver init and vfio reset module init: that way we
make sure symbols exported by base are available when vfio-platform
driver gets probed.

The open/release being implemented in the base module, the ref
count is applied to the parent module instead.

Signed-off-by: Eric Auger <eric.auger@linaro.org>
Suggested-by: Arnd Bergmann <arnd@arndb.de>

---

v3: creation
---
 drivers/vfio/platform/Makefile                |  6 ++++--
 drivers/vfio/platform/vfio_amba.c             |  1 +
 drivers/vfio/platform/vfio_platform.c         |  1 +
 drivers/vfio/platform/vfio_platform_common.c  | 13 +++++++++++--
 drivers/vfio/platform/vfio_platform_private.h |  1 +
 5 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
index 9ce8afe..41a6224 100644
--- a/drivers/vfio/platform/Makefile
+++ b/drivers/vfio/platform/Makefile
@@ -1,10 +1,12 @@
-
-vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o
+vfio-platform-base-y := vfio_platform_common.o vfio_platform_irq.o
+vfio-platform-y := vfio_platform.o
 
 obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
+obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform-base.o
 obj-$(CONFIG_VFIO_PLATFORM) += reset/
 
 vfio-amba-y := vfio_amba.o
 
 obj-$(CONFIG_VFIO_AMBA) += vfio-amba.o
+obj-$(CONFIG_VFIO_AMBA) += vfio-platform-base.o
 obj-$(CONFIG_VFIO_AMBA) += reset/
diff --git a/drivers/vfio/platform/vfio_amba.c b/drivers/vfio/platform/vfio_amba.c
index ff0331f..a66479b 100644
--- a/drivers/vfio/platform/vfio_amba.c
+++ b/drivers/vfio/platform/vfio_amba.c
@@ -67,6 +67,7 @@ static int vfio_amba_probe(struct amba_device *adev, const struct amba_id *id)
 	vdev->flags = VFIO_DEVICE_FLAGS_AMBA;
 	vdev->get_resource = get_amba_resource;
 	vdev->get_irq = get_amba_irq;
+	vdev->parent_module = THIS_MODULE;
 
 	ret = vfio_platform_probe_common(vdev, &adev->dev);
 	if (ret) {
diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
index cef645c..f1625dc 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -65,6 +65,7 @@ static int vfio_platform_probe(struct platform_device *pdev)
 	vdev->flags = VFIO_DEVICE_FLAGS_PLATFORM;
 	vdev->get_resource = get_platform_resource;
 	vdev->get_irq = get_platform_irq;
+	vdev->parent_module = THIS_MODULE;
 
 	ret = vfio_platform_probe_common(vdev, &pdev->dev);
 	if (ret)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index e43efb5..184e9d2 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -23,6 +23,10 @@
 
 #include "vfio_platform_private.h"
 
+#define DRIVER_VERSION  "0.10"
+#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis@virtualopensystems.com>"
+#define DRIVER_DESC     "VFIO platform base module"
+
 static DEFINE_MUTEX(driver_lock);
 
 static const struct vfio_platform_reset_combo reset_lookup_table[] = {
@@ -146,7 +150,7 @@ static void vfio_platform_release(void *device_data)
 
 	mutex_unlock(&driver_lock);
 
-	module_put(THIS_MODULE);
+	module_put(vdev->parent_module);
 }
 
 static int vfio_platform_open(void *device_data)
@@ -154,7 +158,7 @@ static int vfio_platform_open(void *device_data)
 	struct vfio_platform_device *vdev = device_data;
 	int ret;
 
-	if (!try_module_get(THIS_MODULE))
+	if (!try_module_get(vdev->parent_module))
 		return -ENODEV;
 
 	mutex_lock(&driver_lock);
@@ -573,3 +577,8 @@ struct vfio_platform_device *vfio_platform_remove_common(struct device *dev)
 	return vdev;
 }
 EXPORT_SYMBOL_GPL(vfio_platform_remove_common);
+
+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
index 1c9b3d5..7128690 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -56,6 +56,7 @@ struct vfio_platform_device {
 	u32				num_irqs;
 	int				refcnt;
 	struct mutex			igate;
+	struct module			*parent_module;
 
 	/*
 	 * These fields should be filled by the bus specific binder
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1254585 — Re: [PATCH v3 1/7] vfio: platform: introduce vfio-platform-base module

FromArnd Bergmann <arnd@arndb.de>
Date2015-10-23 15:10 +0200
SubjectRe: [PATCH v3 1/7] vfio: platform: introduce vfio-platform-base module
Message-ID<qmKgy-2iZ-25@gated-at.bofh.it>
In reply to#1254574
On Friday 23 October 2015 14:37:09 Eric Auger wrote:
> To prepare for vfio platform reset rework let's build
> vfio_platform_common.c and vfio_platform_irq.c in a separate
> module from vfio-platform and vfio-amba. This makes possible
> to have separate module inits and works around a race between
> platform driver init and vfio reset module init: that way we
> make sure symbols exported by base are available when vfio-platform
> driver gets probed.
> 
> The open/release being implemented in the base module, the ref
> count is applied to the parent module instead.
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> 
> 

Reviewed-by: Arnd Bergmann <arnd@arndb.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web