linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 0/3] PSCI v0.2 support and DT bindings
@ 2014-05-07 14:27 Ashwin Chaugule
  2014-05-07 14:27 ` [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions Ashwin Chaugule
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Ashwin Chaugule @ 2014-05-07 14:27 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset introduces some functions as defined in the PSCI v0.2 spec
and a corresponding DT binding to differentiate it from previous PSCI versions.
Since PSCIv0.2 has standard function IDs, it is possible to statically assign them
rather than depend on values from DT.

This patchset depends on the PSCI header (/uapi/linux/psci.h) introduced by
http://www.spinics.net/lists/arm-kernel/msg319712.html which is now merged in the
ARM KVM tree next branch.


Changes in v9:
		-		Rebased on top of KVM "next" - https://git.kernel.org/cgit/linux/kernel/git/kvmarm/kvmarm.git/log/?h=next

Changes in v8:
		-		Replace msleep(100) with 10 x msleep(10)

Changes in v7:
		-       Add two new functions: SYSTEM_OFF and SYSTEM_RESET as defined in PSCIv0.2.
		-       Added a delay before calling cpu_kill to avoid race with cpu_die.

Changes in v6:
		-       rebase on top of uapi/linux/psci.h introduced by Anups patch
http://www.spinics.net/lists/arm-kernel/msg319712.html
		-       rebase on top of Catalins tree tip.

Changes in v5:
        -       Add uapi/linux/psci.h to kbuild
        -       ret with err if PSCI_ID_VERSION is not implemented.

Changes in v4:
        -       Correct copyright banner format.
        -       Check if PSCI Version ID is supported.
        -       Add all PSCI RET codes in uapi header.
        -       Explicitely ret 1 from psci_cpu_kill()

Changes in v3:
        -       Roll up common functionality for getting conduit method.
        -       Remove #defines for ARM32 and ARM64 in uapi/linux/psci.h
        -       Remove functions not supported by PSCI v0.1
        -       Misc cleanups.
        -       Add PSCI_AFFINITY_INFO return types in uapi header.
        -       Changed function names for PSCI v0.1 and PSCI v0.2
        -       Added copyright info to uapi header.
        -       Fixed args to affinity_info call.
        -       Fix typo in psci_init definition when PSCI is not defined.

Changes in v2:

        -       Add AFFINITY_INFO and MIGRATE_INFO_TYPE functions.
        -       Add binding Documentation.
        -       Add function to get PSCI version.
        -       Add common #defines in uapi/linux/psci.h
        -       Implement cpu_kill and check if CPU is dead via AFFINITY_INFO.
        -       Misc cleanups.

Changes in v1:

        -       Add new binding "arm, psci-0.2"
        -       Separate conduit and PSCI function assignment methods.

Ashwin Chaugule (3):
  PSCI: Add initial support for PSCIv0.2 functions
  Documentation: devicetree: Add new binding for PSCIv0.2
  ARM: Check if a CPU has gone offline

 Documentation/devicetree/bindings/arm/psci.txt |  35 ++++-
 arch/arm/include/asm/psci.h                    |   7 +-
 arch/arm/kernel/psci.c                         | 196 +++++++++++++++++++-----
 arch/arm/kernel/psci_smp.c                     |  31 ++++
 arch/arm64/include/asm/psci.h                  |   2 +-
 arch/arm64/kernel/psci.c                       | 200 ++++++++++++++++++++-----
 6 files changed, 393 insertions(+), 78 deletions(-)

-- 
1.8.3.2

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions
  2014-05-07 14:27 [PATCH v9 0/3] PSCI v0.2 support and DT bindings Ashwin Chaugule
@ 2014-05-07 14:27 ` Ashwin Chaugule
  2014-05-11  8:14   ` Anup Patel
  2014-05-12  9:10   ` Mark Rutland
  2014-05-07 14:27 ` [PATCH v9 2/3] Documentation: devicetree: Add new binding for PSCIv0.2 Ashwin Chaugule
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 14+ messages in thread
From: Ashwin Chaugule @ 2014-05-07 14:27 UTC (permalink / raw)
  To: linux-arm-kernel

The PSCIv0.2 spec defines standard values of function IDs
and introduces a few new functions. Detect version of PSCI
and appropriately select the right PSCI functions.

Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 arch/arm/include/asm/psci.h   |   7 +-
 arch/arm/kernel/psci.c        | 196 +++++++++++++++++++++++++++++++++--------
 arch/arm64/include/asm/psci.h |   2 +-
 arch/arm64/kernel/psci.c      | 200 ++++++++++++++++++++++++++++++++++--------
 4 files changed, 328 insertions(+), 77 deletions(-)

diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
index c4ae171..b93e34a 100644
--- a/arch/arm/include/asm/psci.h
+++ b/arch/arm/include/asm/psci.h
@@ -29,16 +29,19 @@ struct psci_operations {
 	int (*cpu_off)(struct psci_power_state state);
 	int (*cpu_on)(unsigned long cpuid, unsigned long entry_point);
 	int (*migrate)(unsigned long cpuid);
+	int (*affinity_info)(unsigned long target_affinity,
+			unsigned long lowest_affinity_level);
+	int (*migrate_info_type)(void);
 };
 
 extern struct psci_operations psci_ops;
 extern struct smp_operations psci_smp_ops;
 
 #ifdef CONFIG_ARM_PSCI
-void psci_init(void);
+int psci_init(void);
 bool psci_smp_available(void);
 #else
-static inline void psci_init(void) { }
+static inline int psci_init(void) { }
 static inline bool psci_smp_available(void) { return false; }
 #endif
 
diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
index 4693188..3775e62 100644
--- a/arch/arm/kernel/psci.c
+++ b/arch/arm/kernel/psci.c
@@ -17,63 +17,58 @@
 
 #include <linux/init.h>
 #include <linux/of.h>
+#include <linux/reboot.h>
+#include <linux/pm.h>
+#include <uapi/linux/psci.h>
 
 #include <asm/compiler.h>
 #include <asm/errno.h>
 #include <asm/opcodes-sec.h>
 #include <asm/opcodes-virt.h>
 #include <asm/psci.h>
+#include <asm/system_misc.h>
 
 struct psci_operations psci_ops;
 
 static int (*invoke_psci_fn)(u32, u32, u32, u32);
+typedef int (*psci_initcall_t)(const struct device_node *);
 
 enum psci_function {
 	PSCI_FN_CPU_SUSPEND,
 	PSCI_FN_CPU_ON,
 	PSCI_FN_CPU_OFF,
 	PSCI_FN_MIGRATE,
+	PSCI_FN_AFFINITY_INFO,
+	PSCI_FN_MIGRATE_INFO_TYPE,
 	PSCI_FN_MAX,
 };
 
 static u32 psci_function_id[PSCI_FN_MAX];
 
-#define PSCI_RET_SUCCESS		0
-#define PSCI_RET_EOPNOTSUPP		-1
-#define PSCI_RET_EINVAL			-2
-#define PSCI_RET_EPERM			-3
-
 static int psci_to_linux_errno(int errno)
 {
 	switch (errno) {
 	case PSCI_RET_SUCCESS:
 		return 0;
-	case PSCI_RET_EOPNOTSUPP:
+	case PSCI_RET_NOT_SUPPORTED:
 		return -EOPNOTSUPP;
-	case PSCI_RET_EINVAL:
+	case PSCI_RET_INVALID_PARAMS:
 		return -EINVAL;
-	case PSCI_RET_EPERM:
+	case PSCI_RET_DENIED:
 		return -EPERM;
 	};
 
 	return -EINVAL;
 }
 
-#define PSCI_POWER_STATE_ID_MASK	0xffff
-#define PSCI_POWER_STATE_ID_SHIFT	0
-#define PSCI_POWER_STATE_TYPE_MASK	0x1
-#define PSCI_POWER_STATE_TYPE_SHIFT	16
-#define PSCI_POWER_STATE_AFFL_MASK	0x3
-#define PSCI_POWER_STATE_AFFL_SHIFT	24
-
 static u32 psci_power_state_pack(struct psci_power_state state)
 {
-	return	((state.id & PSCI_POWER_STATE_ID_MASK)
-			<< PSCI_POWER_STATE_ID_SHIFT)	|
-		((state.type & PSCI_POWER_STATE_TYPE_MASK)
-			<< PSCI_POWER_STATE_TYPE_SHIFT)	|
-		((state.affinity_level & PSCI_POWER_STATE_AFFL_MASK)
-			<< PSCI_POWER_STATE_AFFL_SHIFT);
+	return	((state.id & PSCI_0_2_POWER_STATE_ID_MASK)
+			<< PSCI_0_2_POWER_STATE_ID_SHIFT)	|
+		((state.type & PSCI_0_2_POWER_STATE_TYPE_MASK)
+			<< PSCI_0_2_POWER_STATE_TYPE_SHIFT)	|
+		((state.affinity_level & PSCI_0_2_POWER_STATE_AFFL_MASK)
+			<< PSCI_0_2_POWER_STATE_AFFL_SHIFT);
 }
 
 /*
@@ -110,6 +105,14 @@ static noinline int __invoke_psci_fn_smc(u32 function_id, u32 arg0, u32 arg1,
 	return function_id;
 }
 
+static int psci_get_version(void)
+{
+	int err;
+
+	err = invoke_psci_fn(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
+	return err;
+}
+
 static int psci_cpu_suspend(struct psci_power_state state,
 			    unsigned long entry_point)
 {
@@ -153,26 +156,36 @@ static int psci_migrate(unsigned long cpuid)
 	return psci_to_linux_errno(err);
 }
 
-static const struct of_device_id psci_of_match[] __initconst = {
-	{ .compatible = "arm,psci",	},
-	{},
-};
+static int psci_affinity_info(unsigned long target_affinity,
+		unsigned long lowest_affinity_level)
+{
+	int err;
+	u32 fn;
+
+	fn = psci_function_id[PSCI_FN_AFFINITY_INFO];
+	err = invoke_psci_fn(fn, target_affinity, lowest_affinity_level, 0);
+	return err;
+}
 
-void __init psci_init(void)
+static int psci_migrate_info_type(void)
 {
-	struct device_node *np;
-	const char *method;
-	u32 id;
+	int err;
+	u32 fn;
 
-	np = of_find_matching_node(NULL, psci_of_match);
-	if (!np)
-		return;
+	fn = psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE];
+	err = invoke_psci_fn(fn, 0, 0, 0);
+	return err;
+}
+
+static int get_set_conduit_method(struct device_node *np)
+{
+	const char *method;
 
-	pr_info("probing function IDs from device-tree\n");
+	pr_info("probing for conduit method from DT.\n");
 
 	if (of_property_read_string(np, "method", &method)) {
-		pr_warning("missing \"method\" property\n");
-		goto out_put_node;
+		pr_warn("missing \"method\" property\n");
+		return -ENXIO;
 	}
 
 	if (!strcmp("hvc", method)) {
@@ -180,10 +193,99 @@ void __init psci_init(void)
 	} else if (!strcmp("smc", method)) {
 		invoke_psci_fn = __invoke_psci_fn_smc;
 	} else {
-		pr_warning("invalid \"method\" property: %s\n", method);
+		pr_warn("invalid \"method\" property: %s\n", method);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
+{
+	invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
+}
+
+static void psci_sys_poweroff(void)
+{
+	invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
+}
+
+/*
+ * PSCI Function IDs for v0.2+ are well defined so use
+ * standard values.
+ */
+static int psci_0_2_init(struct device_node *np)
+{
+	int err, ver;
+
+	err = get_set_conduit_method(np);
+
+	if (err)
+		goto out_put_node;
+
+	ver = psci_get_version();
+
+	if (ver == PSCI_RET_NOT_SUPPORTED) {
+		/* PSCI v0.2 mandates implementation of PSCI_ID_VERSION. */
+		pr_err("PSCI firmware does not comply with the v0.2 spec.\n");
+		err = -EOPNOTSUPP;
 		goto out_put_node;
+	} else {
+		pr_info("PSCIv%d.%d detected in firmware.\n",
+				PSCI_VERSION_MAJOR(ver),
+				PSCI_VERSION_MINOR(ver));
+
+		if (PSCI_VERSION_MAJOR(ver) == 0 &&
+				PSCI_VERSION_MINOR(ver) < 2) {
+			err = -EINVAL;
+			pr_err("Conflicting PSCI version detected.\n");
+			goto out_put_node;
+		}
 	}
 
+	pr_info("Using standard PSCI v0.2 function IDs\n");
+	psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN_CPU_SUSPEND;
+	psci_ops.cpu_suspend = psci_cpu_suspend;
+
+	psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
+	psci_ops.cpu_off = psci_cpu_off;
+
+	psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN_CPU_ON;
+	psci_ops.cpu_on = psci_cpu_on;
+
+	psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN_MIGRATE;
+	psci_ops.migrate = psci_migrate;
+
+	psci_function_id[PSCI_FN_AFFINITY_INFO] = PSCI_0_2_FN_AFFINITY_INFO;
+	psci_ops.affinity_info = psci_affinity_info;
+
+	psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE] =
+		PSCI_0_2_FN_MIGRATE_INFO_TYPE;
+	psci_ops.migrate_info_type = psci_migrate_info_type;
+
+	arm_pm_restart = psci_sys_reset;
+
+	pm_power_off = psci_sys_poweroff;
+
+out_put_node:
+	of_node_put(np);
+	return err;
+}
+
+/*
+ * PSCI < v0.2 get PSCI Function IDs via DT.
+ */
+static int psci_0_1_init(struct device_node *np)
+{
+	u32 id;
+	int err;
+
+	err = get_set_conduit_method(np);
+
+	if (err)
+		goto out_put_node;
+
+	pr_info("Using PSCI v0.1 Function IDs from DT\n");
+
 	if (!of_property_read_u32(np, "cpu_suspend", &id)) {
 		psci_function_id[PSCI_FN_CPU_SUSPEND] = id;
 		psci_ops.cpu_suspend = psci_cpu_suspend;
@@ -206,5 +308,25 @@ void __init psci_init(void)
 
 out_put_node:
 	of_node_put(np);
-	return;
+	return err;
+}
+
+static const struct of_device_id psci_of_match[] __initconst = {
+	{ .compatible = "arm,psci", .data = psci_0_1_init},
+	{ .compatible = "arm,psci-0.2", .data = psci_0_2_init},
+	{},
+};
+
+int __init psci_init(void)
+{
+	struct device_node *np;
+	const struct of_device_id *matched_np;
+	psci_initcall_t init_fn;
+
+	np = of_find_matching_node_and_match(NULL, psci_of_match, &matched_np);
+	if (!np)
+		return -ENODEV;
+
+	init_fn = (psci_initcall_t)matched_np->data;
+	return init_fn(np);
 }
diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h
index d15ab8b4..e5312ea 100644
--- a/arch/arm64/include/asm/psci.h
+++ b/arch/arm64/include/asm/psci.h
@@ -14,6 +14,6 @@
 #ifndef __ASM_PSCI_H
 #define __ASM_PSCI_H
 
-void psci_init(void);
+int psci_init(void);
 
 #endif /* __ASM_PSCI_H */
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index ea4828a..6045613 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -18,12 +18,16 @@
 #include <linux/init.h>
 #include <linux/of.h>
 #include <linux/smp.h>
+#include <linux/reboot.h>
+#include <linux/pm.h>
+#include <uapi/linux/psci.h>
 
 #include <asm/compiler.h>
 #include <asm/cpu_ops.h>
 #include <asm/errno.h>
 #include <asm/psci.h>
 #include <asm/smp_plat.h>
+#include <asm/system_misc.h>
 
 #define PSCI_POWER_STATE_TYPE_STANDBY		0
 #define PSCI_POWER_STATE_TYPE_POWER_DOWN	1
@@ -40,58 +44,52 @@ struct psci_operations {
 	int (*cpu_off)(struct psci_power_state state);
 	int (*cpu_on)(unsigned long cpuid, unsigned long entry_point);
 	int (*migrate)(unsigned long cpuid);
+	int (*affinity_info)(unsigned long target_affinity,
+			unsigned long lowest_affinity_level);
+	int (*migrate_info_type)(void);
 };
 
 static struct psci_operations psci_ops;
 
 static int (*invoke_psci_fn)(u64, u64, u64, u64);
+typedef int (*psci_initcall_t)(const struct device_node *);
 
 enum psci_function {
 	PSCI_FN_CPU_SUSPEND,
 	PSCI_FN_CPU_ON,
 	PSCI_FN_CPU_OFF,
 	PSCI_FN_MIGRATE,
+	PSCI_FN_AFFINITY_INFO,
+	PSCI_FN_MIGRATE_INFO_TYPE,
 	PSCI_FN_MAX,
 };
 
 static u32 psci_function_id[PSCI_FN_MAX];
 
-#define PSCI_RET_SUCCESS		0
-#define PSCI_RET_EOPNOTSUPP		-1
-#define PSCI_RET_EINVAL			-2
-#define PSCI_RET_EPERM			-3
-
 static int psci_to_linux_errno(int errno)
 {
 	switch (errno) {
 	case PSCI_RET_SUCCESS:
 		return 0;
-	case PSCI_RET_EOPNOTSUPP:
+	case PSCI_RET_NOT_SUPPORTED:
 		return -EOPNOTSUPP;
-	case PSCI_RET_EINVAL:
+	case PSCI_RET_INVALID_PARAMS:
 		return -EINVAL;
-	case PSCI_RET_EPERM:
+	case PSCI_RET_DENIED:
 		return -EPERM;
 	};
 
 	return -EINVAL;
 }
 
-#define PSCI_POWER_STATE_ID_MASK	0xffff
-#define PSCI_POWER_STATE_ID_SHIFT	0
-#define PSCI_POWER_STATE_TYPE_MASK	0x1
-#define PSCI_POWER_STATE_TYPE_SHIFT	16
-#define PSCI_POWER_STATE_AFFL_MASK	0x3
-#define PSCI_POWER_STATE_AFFL_SHIFT	24
-
 static u32 psci_power_state_pack(struct psci_power_state state)
 {
-	return	((state.id & PSCI_POWER_STATE_ID_MASK)
-			<< PSCI_POWER_STATE_ID_SHIFT)	|
-		((state.type & PSCI_POWER_STATE_TYPE_MASK)
-			<< PSCI_POWER_STATE_TYPE_SHIFT)	|
-		((state.affinity_level & PSCI_POWER_STATE_AFFL_MASK)
-			<< PSCI_POWER_STATE_AFFL_SHIFT);
+	return	((state.id & PSCI_0_2_POWER_STATE_ID_MASK)
+			<< PSCI_0_2_POWER_STATE_ID_SHIFT)	|
+		((state.type & PSCI_0_2_POWER_STATE_TYPE_MASK)
+			<< PSCI_0_2_POWER_STATE_TYPE_SHIFT)	|
+		((state.affinity_level & PSCI_0_2_POWER_STATE_AFFL_MASK)
+			<< PSCI_0_2_POWER_STATE_AFFL_SHIFT);
 }
 
 /*
@@ -128,6 +126,14 @@ static noinline int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
 	return function_id;
 }
 
+static int psci_get_version(void)
+{
+	int err;
+
+	err = invoke_psci_fn(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
+	return err;
+}
+
 static int psci_cpu_suspend(struct psci_power_state state,
 			    unsigned long entry_point)
 {
@@ -171,26 +177,36 @@ static int psci_migrate(unsigned long cpuid)
 	return psci_to_linux_errno(err);
 }
 
-static const struct of_device_id psci_of_match[] __initconst = {
-	{ .compatible = "arm,psci",	},
-	{},
-};
+static int psci_affinity_info(unsigned long target_affinity,
+		unsigned long lowest_affinity_level)
+{
+	int err;
+	u32 fn;
+
+	fn = psci_function_id[PSCI_FN_AFFINITY_INFO];
+	err = invoke_psci_fn(fn, target_affinity, lowest_affinity_level, 0);
+	return err;
+}
 
-void __init psci_init(void)
+static int psci_migrate_info_type(void)
 {
-	struct device_node *np;
-	const char *method;
-	u32 id;
+	int err;
+	u32 fn;
 
-	np = of_find_matching_node(NULL, psci_of_match);
-	if (!np)
-		return;
+	fn = psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE];
+	err = invoke_psci_fn(fn, 0, 0, 0);
+	return err;
+}
+
+static int get_set_conduit_method(struct device_node *np)
+{
+	const char *method;
 
-	pr_info("probing function IDs from device-tree\n");
+	pr_info("probing for conduit method from DT.\n");
 
 	if (of_property_read_string(np, "method", &method)) {
-		pr_warning("missing \"method\" property\n");
-		goto out_put_node;
+		pr_warn("missing \"method\" property\n");
+		return -ENXIO;
 	}
 
 	if (!strcmp("hvc", method)) {
@@ -198,10 +214,99 @@ void __init psci_init(void)
 	} else if (!strcmp("smc", method)) {
 		invoke_psci_fn = __invoke_psci_fn_smc;
 	} else {
-		pr_warning("invalid \"method\" property: %s\n", method);
+		pr_warn("invalid \"method\" property: %s\n", method);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
+{
+	invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
+}
+
+static void psci_sys_poweroff(void)
+{
+	invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
+}
+
+/*
+ * PSCI Function IDs for v0.2+ are well defined so use
+ * standard values.
+ */
+static int psci_0_2_init(struct device_node *np)
+{
+	int err, ver;
+
+	err = get_set_conduit_method(np);
+
+	if (err)
+		goto out_put_node;
+
+	ver = psci_get_version();
+
+	if (ver == PSCI_RET_NOT_SUPPORTED) {
+		/* PSCI v0.2 mandates implementation of PSCI_ID_VERSION. */
+		pr_err("PSCI firmware does not comply with the v0.2 spec.\n");
+		err = -EOPNOTSUPP;
 		goto out_put_node;
+	} else {
+		pr_info("PSCIv%d.%d detected in firmware.\n",
+				PSCI_VERSION_MAJOR(ver),
+				PSCI_VERSION_MINOR(ver));
+
+		if (PSCI_VERSION_MAJOR(ver) == 0 &&
+				PSCI_VERSION_MINOR(ver) < 2) {
+			err = -EINVAL;
+			pr_err("Conflicting PSCI version detected.\n");
+			goto out_put_node;
+		}
 	}
 
+	pr_info("Using standard PSCI v0.2 function IDs\n");
+	psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN64_CPU_SUSPEND;
+	psci_ops.cpu_suspend = psci_cpu_suspend;
+
+	psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
+	psci_ops.cpu_off = psci_cpu_off;
+
+	psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN64_CPU_ON;
+	psci_ops.cpu_on = psci_cpu_on;
+
+	psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN64_MIGRATE;
+	psci_ops.migrate = psci_migrate;
+
+	psci_function_id[PSCI_FN_AFFINITY_INFO] = PSCI_0_2_FN64_AFFINITY_INFO;
+	psci_ops.affinity_info = psci_affinity_info;
+
+	psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE] =
+		PSCI_0_2_FN_MIGRATE_INFO_TYPE;
+	psci_ops.migrate_info_type = psci_migrate_info_type;
+
+	arm_pm_restart = psci_sys_reset;
+
+	pm_power_off = psci_sys_poweroff;
+
+out_put_node:
+	of_node_put(np);
+	return err;
+}
+
+/*
+ * PSCI < v0.2 get PSCI Function IDs via DT.
+ */
+static int psci_0_1_init(struct device_node *np)
+{
+	u32 id;
+	int err;
+
+	err = get_set_conduit_method(np);
+
+	if (err)
+		goto out_put_node;
+
+	pr_info("Using PSCI v0.1 Function IDs from DT\n");
+
 	if (!of_property_read_u32(np, "cpu_suspend", &id)) {
 		psci_function_id[PSCI_FN_CPU_SUSPEND] = id;
 		psci_ops.cpu_suspend = psci_cpu_suspend;
@@ -224,7 +329,28 @@ void __init psci_init(void)
 
 out_put_node:
 	of_node_put(np);
-	return;
+	return err;
+}
+
+static const struct of_device_id psci_of_match[] __initconst = {
+	{ .compatible = "arm,psci",	.data = psci_0_1_init},
+	{ .compatible = "arm,psci-0.2",	.data = psci_0_2_init},
+	{},
+};
+
+int __init psci_init(void)
+{
+	struct device_node *np;
+	const struct of_device_id *matched_np;
+	psci_initcall_t init_fn;
+
+	np = of_find_matching_node_and_match(NULL, psci_of_match, &matched_np);
+
+	if (!np)
+		return -ENODEV;
+
+	init_fn = (psci_initcall_t)matched_np->data;
+	return init_fn(np);
 }
 
 #ifdef CONFIG_SMP
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v9 2/3] Documentation: devicetree: Add new binding for PSCIv0.2
  2014-05-07 14:27 [PATCH v9 0/3] PSCI v0.2 support and DT bindings Ashwin Chaugule
  2014-05-07 14:27 ` [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions Ashwin Chaugule
@ 2014-05-07 14:27 ` Ashwin Chaugule
  2014-05-12  8:54   ` Mark Rutland
  2014-05-07 14:27 ` [PATCH v9 3/3] ARM: Check if a CPU has gone offline Ashwin Chaugule
  2014-05-08  4:48 ` [PATCH v9 0/3] PSCI v0.2 support and DT bindings Ashwin Chaugule
  3 siblings, 1 reply; 14+ messages in thread
From: Ashwin Chaugule @ 2014-05-07 14:27 UTC (permalink / raw)
  To: linux-arm-kernel

The PSCI v0.2+ spec defines standard values for PSCI function IDs.
Add a new binding entry so that pre v0.2 implementations can
use DT entries for function IDs and v0.2+ implementations use
standard entries as defined by the PSCIv0.2 specification.

Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/arm/psci.txt | 35 +++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt
index 433afe9..d01a90b 100644
--- a/Documentation/devicetree/bindings/arm/psci.txt
+++ b/Documentation/devicetree/bindings/arm/psci.txt
@@ -21,7 +21,15 @@ to #0.
 
 Main node required properties:
 
- - compatible    : Must be "arm,psci"
+ - compatible    : should contain at least one of:
+
+				 * "arm,psci" : for implementations complying to PSCI versions prior to
+					0.2. For these cases function IDs must be provided.
+
+				 * "arm,psci-0.2" : for implementations complying to PSCI 0.2. Function
+					IDs are not required and should be ignored by an OS with PSCI 0.2
+					support, but are permitted to be present for compatibility with
+					existing software when "arm,psci" is later in the compatible list.
 
  - method        : The method of calling the PSCI firmware. Permitted
                    values are:
@@ -45,6 +53,8 @@ Main node optional properties:
 
 Example:
 
+Case 1: PSCI v0.1 only.
+
 	psci {
 		compatible	= "arm,psci";
 		method		= "smc";
@@ -53,3 +63,26 @@ Example:
 		cpu_on		= <0x95c10002>;
 		migrate		= <0x95c10003>;
 	};
+
+
+Case 2: PSCI v0.2 only
+
+	psci {
+		compatible	= "arm,psci-0.2";
+		method		= "smc";
+	};
+
+Case 3: PSCI v0.2 and PSCI v0.1.
+
+	As described above, for compatibility with existing kernels, the
+	hypervisor will likely want to provide IDs, e.g.
+
+	psci {
+		compatible = "arm,psci-0.2", "arm,psci";
+		method = "hvc";
+
+		cpu_on = < arbitrary value >;
+		cpu_off = < arbitrary value >;
+
+		...
+	};
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v9 3/3] ARM: Check if a CPU has gone offline
  2014-05-07 14:27 [PATCH v9 0/3] PSCI v0.2 support and DT bindings Ashwin Chaugule
  2014-05-07 14:27 ` [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions Ashwin Chaugule
  2014-05-07 14:27 ` [PATCH v9 2/3] Documentation: devicetree: Add new binding for PSCIv0.2 Ashwin Chaugule
@ 2014-05-07 14:27 ` Ashwin Chaugule
  2014-05-12 10:16   ` Mark Rutland
  2014-05-08  4:48 ` [PATCH v9 0/3] PSCI v0.2 support and DT bindings Ashwin Chaugule
  3 siblings, 1 reply; 14+ messages in thread
From: Ashwin Chaugule @ 2014-05-07 14:27 UTC (permalink / raw)
  To: linux-arm-kernel

PSCIv0.2 adds a new function called AFFINITY_INFO, which
can be used to query if a specified CPU has actually gone
offline. Calling this function via cpu_kill ensures that
a CPU has quiesced after a call to cpu_die.

Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 arch/arm/kernel/psci_smp.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
index 570a48c..502e6cf 100644
--- a/arch/arm/kernel/psci_smp.c
+++ b/arch/arm/kernel/psci_smp.c
@@ -16,6 +16,8 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <linux/of.h>
+#include <linux/delay.h>
+#include <uapi/linux/psci.h>
 
 #include <asm/psci.h>
 #include <asm/smp_plat.h>
@@ -66,6 +68,34 @@ void __ref psci_cpu_die(unsigned int cpu)
        /* We should never return */
        panic("psci: cpu %d failed to shutdown\n", cpu);
 }
+
+int __ref psci_cpu_kill(unsigned int cpu)
+{
+	int err, i;
+
+	if (!psci_ops.affinity_info)
+		return 1;
+	/*
+	 * cpu_kill could race with cpu_die and we can
+	 * potentially end up declaring this cpu undead
+	 * while it is dying. So, try again a few times.
+	 */
+
+	for (i=0; i<10; i++) {
+		err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
+		if (err == PSCI_0_2_AFFINITY_LEVEL_OFF)
+			return 1;
+
+		msleep(10);
+		pr_info("Retrying again to check for CPU kill\n");
+	}
+
+	pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
+			cpu, err);
+	/* Make platform_cpu_kill() fail. */
+	return 0;
+}
+
 #endif
 
 bool __init psci_smp_available(void)
@@ -78,5 +108,6 @@ struct smp_operations __initdata psci_smp_ops = {
 	.smp_boot_secondary	= psci_boot_secondary,
 #ifdef CONFIG_HOTPLUG_CPU
 	.cpu_die		= psci_cpu_die,
+	.cpu_kill		= psci_cpu_kill,
 #endif
 };
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v9 0/3] PSCI v0.2 support and DT bindings
  2014-05-07 14:27 [PATCH v9 0/3] PSCI v0.2 support and DT bindings Ashwin Chaugule
                   ` (2 preceding siblings ...)
  2014-05-07 14:27 ` [PATCH v9 3/3] ARM: Check if a CPU has gone offline Ashwin Chaugule
@ 2014-05-08  4:48 ` Ashwin Chaugule
  3 siblings, 0 replies; 14+ messages in thread
From: Ashwin Chaugule @ 2014-05-08  4:48 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Catalin,

On 7 May 2014 10:27, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
> This patchset introduces some functions as defined in the PSCI v0.2 spec
> and a corresponding DT binding to differentiate it from previous PSCI versions.
> Since PSCIv0.2 has standard function IDs, it is possible to statically assign them
> rather than depend on values from DT.
>
> This patchset depends on the PSCI header (/uapi/linux/psci.h) introduced by
> http://www.spinics.net/lists/arm-kernel/msg319712.html which is now merged in the
> ARM KVM tree next branch.
>
>
> Changes in v9:
>                 -               Rebased on top of KVM "next" - https://git.kernel.org/cgit/linux/kernel/git/kvmarm/kvmarm.git/log/?h=next
>
>
> Ashwin Chaugule (3):
>   PSCI: Add initial support for PSCIv0.2 functions
>   Documentation: devicetree: Add new binding for PSCIv0.2
>   ARM: Check if a CPU has gone offline
>
>  Documentation/devicetree/bindings/arm/psci.txt |  35 ++++-
>  arch/arm/include/asm/psci.h                    |   7 +-
>  arch/arm/kernel/psci.c                         | 196 +++++++++++++++++++-----
>  arch/arm/kernel/psci_smp.c                     |  31 ++++
>  arch/arm64/include/asm/psci.h                  |   2 +-
>  arch/arm64/kernel/psci.c                       | 200 ++++++++++++++++++++-----
>  6 files changed, 393 insertions(+), 78 deletions(-)
>

Can you please pick up these patches for the next merge window ?

Cheers,
Ashwin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions
  2014-05-07 14:27 ` [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions Ashwin Chaugule
@ 2014-05-11  8:14   ` Anup Patel
  2014-05-12 13:52     ` Ashwin Chaugule
  2014-05-12  9:10   ` Mark Rutland
  1 sibling, 1 reply; 14+ messages in thread
From: Anup Patel @ 2014-05-11  8:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, May 7, 2014 at 7:57 PM, Ashwin Chaugule
<ashwin.chaugule@linaro.org> wrote:
> The PSCIv0.2 spec defines standard values of function IDs
> and introduces a few new functions. Detect version of PSCI
> and appropriately select the right PSCI functions.
>
> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  arch/arm/include/asm/psci.h   |   7 +-
>  arch/arm/kernel/psci.c        | 196 +++++++++++++++++++++++++++++++++--------
>  arch/arm64/include/asm/psci.h |   2 +-
>  arch/arm64/kernel/psci.c      | 200 ++++++++++++++++++++++++++++++++++--------
>  4 files changed, 328 insertions(+), 77 deletions(-)
>
> diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> index c4ae171..b93e34a 100644
> --- a/arch/arm/include/asm/psci.h
> +++ b/arch/arm/include/asm/psci.h
> @@ -29,16 +29,19 @@ struct psci_operations {
>         int (*cpu_off)(struct psci_power_state state);
>         int (*cpu_on)(unsigned long cpuid, unsigned long entry_point);
>         int (*migrate)(unsigned long cpuid);
> +       int (*affinity_info)(unsigned long target_affinity,
> +                       unsigned long lowest_affinity_level);
> +       int (*migrate_info_type)(void);
>  };
>
>  extern struct psci_operations psci_ops;
>  extern struct smp_operations psci_smp_ops;
>
>  #ifdef CONFIG_ARM_PSCI
> -void psci_init(void);
> +int psci_init(void);
>  bool psci_smp_available(void);
>  #else
> -static inline void psci_init(void) { }
> +static inline int psci_init(void) { }
>  static inline bool psci_smp_available(void) { return false; }
>  #endif
>
> diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
> index 4693188..3775e62 100644
> --- a/arch/arm/kernel/psci.c
> +++ b/arch/arm/kernel/psci.c
> @@ -17,63 +17,58 @@
>
>  #include <linux/init.h>
>  #include <linux/of.h>
> +#include <linux/reboot.h>
> +#include <linux/pm.h>
> +#include <uapi/linux/psci.h>
>
>  #include <asm/compiler.h>
>  #include <asm/errno.h>
>  #include <asm/opcodes-sec.h>
>  #include <asm/opcodes-virt.h>
>  #include <asm/psci.h>
> +#include <asm/system_misc.h>
>
>  struct psci_operations psci_ops;
>
>  static int (*invoke_psci_fn)(u32, u32, u32, u32);
> +typedef int (*psci_initcall_t)(const struct device_node *);
>
>  enum psci_function {
>         PSCI_FN_CPU_SUSPEND,
>         PSCI_FN_CPU_ON,
>         PSCI_FN_CPU_OFF,
>         PSCI_FN_MIGRATE,
> +       PSCI_FN_AFFINITY_INFO,
> +       PSCI_FN_MIGRATE_INFO_TYPE,
>         PSCI_FN_MAX,
>  };
>
>  static u32 psci_function_id[PSCI_FN_MAX];
>
> -#define PSCI_RET_SUCCESS               0
> -#define PSCI_RET_EOPNOTSUPP            -1
> -#define PSCI_RET_EINVAL                        -2
> -#define PSCI_RET_EPERM                 -3
> -
>  static int psci_to_linux_errno(int errno)
>  {
>         switch (errno) {
>         case PSCI_RET_SUCCESS:
>                 return 0;
> -       case PSCI_RET_EOPNOTSUPP:
> +       case PSCI_RET_NOT_SUPPORTED:
>                 return -EOPNOTSUPP;
> -       case PSCI_RET_EINVAL:
> +       case PSCI_RET_INVALID_PARAMS:
>                 return -EINVAL;
> -       case PSCI_RET_EPERM:
> +       case PSCI_RET_DENIED:
>                 return -EPERM;
>         };
>
>         return -EINVAL;
>  }
>
> -#define PSCI_POWER_STATE_ID_MASK       0xffff
> -#define PSCI_POWER_STATE_ID_SHIFT      0
> -#define PSCI_POWER_STATE_TYPE_MASK     0x1
> -#define PSCI_POWER_STATE_TYPE_SHIFT    16
> -#define PSCI_POWER_STATE_AFFL_MASK     0x3
> -#define PSCI_POWER_STATE_AFFL_SHIFT    24
> -
>  static u32 psci_power_state_pack(struct psci_power_state state)
>  {
> -       return  ((state.id & PSCI_POWER_STATE_ID_MASK)
> -                       << PSCI_POWER_STATE_ID_SHIFT)   |
> -               ((state.type & PSCI_POWER_STATE_TYPE_MASK)
> -                       << PSCI_POWER_STATE_TYPE_SHIFT) |
> -               ((state.affinity_level & PSCI_POWER_STATE_AFFL_MASK)
> -                       << PSCI_POWER_STATE_AFFL_SHIFT);
> +       return  ((state.id & PSCI_0_2_POWER_STATE_ID_MASK)
> +                       << PSCI_0_2_POWER_STATE_ID_SHIFT)       |
> +               ((state.type & PSCI_0_2_POWER_STATE_TYPE_MASK)
> +                       << PSCI_0_2_POWER_STATE_TYPE_SHIFT)     |
> +               ((state.affinity_level & PSCI_0_2_POWER_STATE_AFFL_MASK)
> +                       << PSCI_0_2_POWER_STATE_AFFL_SHIFT);
>  }

As per updated PSCI_0_2_POWER_STATE_xxx defines, this should be:

return ((state.id << PSCI_0_2_POWER_STATE_ID_SHIFT)
          & PSCI_0_2_POWER_STATE_ID_MASK) |
          ((state.type << PSCI_0_2_POWER_STATE_TYPE_SHIFT)
          & PSCI_0_2_POWER_STATE_TYPE_MASK) |
          ((state.affinity_level << PSCI_0_2_POWER_STATE_AFFL_SHIFT)
          & PSCI_0_2_POWER_STATE_AFFL_MASK);

>
>  /*
> @@ -110,6 +105,14 @@ static noinline int __invoke_psci_fn_smc(u32 function_id, u32 arg0, u32 arg1,
>         return function_id;
>  }
>
> +static int psci_get_version(void)
> +{
> +       int err;
> +
> +       err = invoke_psci_fn(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> +       return err;
> +}
> +
>  static int psci_cpu_suspend(struct psci_power_state state,
>                             unsigned long entry_point)
>  {
> @@ -153,26 +156,36 @@ static int psci_migrate(unsigned long cpuid)
>         return psci_to_linux_errno(err);
>  }
>
> -static const struct of_device_id psci_of_match[] __initconst = {
> -       { .compatible = "arm,psci",     },
> -       {},
> -};
> +static int psci_affinity_info(unsigned long target_affinity,
> +               unsigned long lowest_affinity_level)
> +{
> +       int err;
> +       u32 fn;
> +
> +       fn = psci_function_id[PSCI_FN_AFFINITY_INFO];
> +       err = invoke_psci_fn(fn, target_affinity, lowest_affinity_level, 0);
> +       return err;
> +}
>
> -void __init psci_init(void)
> +static int psci_migrate_info_type(void)
>  {
> -       struct device_node *np;
> -       const char *method;
> -       u32 id;
> +       int err;
> +       u32 fn;
>
> -       np = of_find_matching_node(NULL, psci_of_match);
> -       if (!np)
> -               return;
> +       fn = psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE];
> +       err = invoke_psci_fn(fn, 0, 0, 0);
> +       return err;
> +}
> +
> +static int get_set_conduit_method(struct device_node *np)
> +{
> +       const char *method;
>
> -       pr_info("probing function IDs from device-tree\n");
> +       pr_info("probing for conduit method from DT.\n");
>
>         if (of_property_read_string(np, "method", &method)) {
> -               pr_warning("missing \"method\" property\n");
> -               goto out_put_node;
> +               pr_warn("missing \"method\" property\n");
> +               return -ENXIO;
>         }
>
>         if (!strcmp("hvc", method)) {
> @@ -180,10 +193,99 @@ void __init psci_init(void)
>         } else if (!strcmp("smc", method)) {
>                 invoke_psci_fn = __invoke_psci_fn_smc;
>         } else {
> -               pr_warning("invalid \"method\" property: %s\n", method);
> +               pr_warn("invalid \"method\" property: %s\n", method);
> +               return -EINVAL;
> +       }
> +       return 0;
> +}
> +
> +static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
> +{
> +       invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> +}
> +
> +static void psci_sys_poweroff(void)
> +{
> +       invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
> +}
> +
> +/*
> + * PSCI Function IDs for v0.2+ are well defined so use
> + * standard values.
> + */
> +static int psci_0_2_init(struct device_node *np)
> +{
> +       int err, ver;
> +
> +       err = get_set_conduit_method(np);
> +
> +       if (err)
> +               goto out_put_node;
> +
> +       ver = psci_get_version();
> +
> +       if (ver == PSCI_RET_NOT_SUPPORTED) {
> +               /* PSCI v0.2 mandates implementation of PSCI_ID_VERSION. */
> +               pr_err("PSCI firmware does not comply with the v0.2 spec.\n");
> +               err = -EOPNOTSUPP;
>                 goto out_put_node;
> +       } else {
> +               pr_info("PSCIv%d.%d detected in firmware.\n",
> +                               PSCI_VERSION_MAJOR(ver),
> +                               PSCI_VERSION_MINOR(ver));
> +
> +               if (PSCI_VERSION_MAJOR(ver) == 0 &&
> +                               PSCI_VERSION_MINOR(ver) < 2) {
> +                       err = -EINVAL;
> +                       pr_err("Conflicting PSCI version detected.\n");
> +                       goto out_put_node;
> +               }
>         }
>
> +       pr_info("Using standard PSCI v0.2 function IDs\n");
> +       psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN_CPU_SUSPEND;
> +       psci_ops.cpu_suspend = psci_cpu_suspend;
> +
> +       psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
> +       psci_ops.cpu_off = psci_cpu_off;
> +
> +       psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN_CPU_ON;
> +       psci_ops.cpu_on = psci_cpu_on;
> +
> +       psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN_MIGRATE;
> +       psci_ops.migrate = psci_migrate;
> +
> +       psci_function_id[PSCI_FN_AFFINITY_INFO] = PSCI_0_2_FN_AFFINITY_INFO;
> +       psci_ops.affinity_info = psci_affinity_info;
> +
> +       psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE] =
> +               PSCI_0_2_FN_MIGRATE_INFO_TYPE;
> +       psci_ops.migrate_info_type = psci_migrate_info_type;
> +
> +       arm_pm_restart = psci_sys_reset;
> +
> +       pm_power_off = psci_sys_poweroff;
> +
> +out_put_node:
> +       of_node_put(np);
> +       return err;
> +}
> +
> +/*
> + * PSCI < v0.2 get PSCI Function IDs via DT.
> + */
> +static int psci_0_1_init(struct device_node *np)
> +{
> +       u32 id;
> +       int err;
> +
> +       err = get_set_conduit_method(np);
> +
> +       if (err)
> +               goto out_put_node;
> +
> +       pr_info("Using PSCI v0.1 Function IDs from DT\n");
> +
>         if (!of_property_read_u32(np, "cpu_suspend", &id)) {
>                 psci_function_id[PSCI_FN_CPU_SUSPEND] = id;
>                 psci_ops.cpu_suspend = psci_cpu_suspend;
> @@ -206,5 +308,25 @@ void __init psci_init(void)
>
>  out_put_node:
>         of_node_put(np);
> -       return;
> +       return err;
> +}
> +
> +static const struct of_device_id psci_of_match[] __initconst = {
> +       { .compatible = "arm,psci", .data = psci_0_1_init},
> +       { .compatible = "arm,psci-0.2", .data = psci_0_2_init},
> +       {},
> +};
> +
> +int __init psci_init(void)
> +{
> +       struct device_node *np;
> +       const struct of_device_id *matched_np;
> +       psci_initcall_t init_fn;
> +
> +       np = of_find_matching_node_and_match(NULL, psci_of_match, &matched_np);
> +       if (!np)
> +               return -ENODEV;
> +
> +       init_fn = (psci_initcall_t)matched_np->data;
> +       return init_fn(np);
>  }
> diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h
> index d15ab8b4..e5312ea 100644
> --- a/arch/arm64/include/asm/psci.h
> +++ b/arch/arm64/include/asm/psci.h
> @@ -14,6 +14,6 @@
>  #ifndef __ASM_PSCI_H
>  #define __ASM_PSCI_H
>
> -void psci_init(void);
> +int psci_init(void);
>
>  #endif /* __ASM_PSCI_H */
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index ea4828a..6045613 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -18,12 +18,16 @@
>  #include <linux/init.h>
>  #include <linux/of.h>
>  #include <linux/smp.h>
> +#include <linux/reboot.h>
> +#include <linux/pm.h>
> +#include <uapi/linux/psci.h>
>
>  #include <asm/compiler.h>
>  #include <asm/cpu_ops.h>
>  #include <asm/errno.h>
>  #include <asm/psci.h>
>  #include <asm/smp_plat.h>
> +#include <asm/system_misc.h>
>
>  #define PSCI_POWER_STATE_TYPE_STANDBY          0
>  #define PSCI_POWER_STATE_TYPE_POWER_DOWN       1
> @@ -40,58 +44,52 @@ struct psci_operations {
>         int (*cpu_off)(struct psci_power_state state);
>         int (*cpu_on)(unsigned long cpuid, unsigned long entry_point);
>         int (*migrate)(unsigned long cpuid);
> +       int (*affinity_info)(unsigned long target_affinity,
> +                       unsigned long lowest_affinity_level);
> +       int (*migrate_info_type)(void);
>  };
>
>  static struct psci_operations psci_ops;
>
>  static int (*invoke_psci_fn)(u64, u64, u64, u64);
> +typedef int (*psci_initcall_t)(const struct device_node *);
>
>  enum psci_function {
>         PSCI_FN_CPU_SUSPEND,
>         PSCI_FN_CPU_ON,
>         PSCI_FN_CPU_OFF,
>         PSCI_FN_MIGRATE,
> +       PSCI_FN_AFFINITY_INFO,
> +       PSCI_FN_MIGRATE_INFO_TYPE,
>         PSCI_FN_MAX,
>  };
>
>  static u32 psci_function_id[PSCI_FN_MAX];
>
> -#define PSCI_RET_SUCCESS               0
> -#define PSCI_RET_EOPNOTSUPP            -1
> -#define PSCI_RET_EINVAL                        -2
> -#define PSCI_RET_EPERM                 -3
> -
>  static int psci_to_linux_errno(int errno)
>  {
>         switch (errno) {
>         case PSCI_RET_SUCCESS:
>                 return 0;
> -       case PSCI_RET_EOPNOTSUPP:
> +       case PSCI_RET_NOT_SUPPORTED:
>                 return -EOPNOTSUPP;
> -       case PSCI_RET_EINVAL:
> +       case PSCI_RET_INVALID_PARAMS:
>                 return -EINVAL;
> -       case PSCI_RET_EPERM:
> +       case PSCI_RET_DENIED:
>                 return -EPERM;
>         };
>
>         return -EINVAL;
>  }
>
> -#define PSCI_POWER_STATE_ID_MASK       0xffff
> -#define PSCI_POWER_STATE_ID_SHIFT      0
> -#define PSCI_POWER_STATE_TYPE_MASK     0x1
> -#define PSCI_POWER_STATE_TYPE_SHIFT    16
> -#define PSCI_POWER_STATE_AFFL_MASK     0x3
> -#define PSCI_POWER_STATE_AFFL_SHIFT    24
> -
>  static u32 psci_power_state_pack(struct psci_power_state state)
>  {
> -       return  ((state.id & PSCI_POWER_STATE_ID_MASK)
> -                       << PSCI_POWER_STATE_ID_SHIFT)   |
> -               ((state.type & PSCI_POWER_STATE_TYPE_MASK)
> -                       << PSCI_POWER_STATE_TYPE_SHIFT) |
> -               ((state.affinity_level & PSCI_POWER_STATE_AFFL_MASK)
> -                       << PSCI_POWER_STATE_AFFL_SHIFT);
> +       return  ((state.id & PSCI_0_2_POWER_STATE_ID_MASK)
> +                       << PSCI_0_2_POWER_STATE_ID_SHIFT)       |
> +               ((state.type & PSCI_0_2_POWER_STATE_TYPE_MASK)
> +                       << PSCI_0_2_POWER_STATE_TYPE_SHIFT)     |
> +               ((state.affinity_level & PSCI_0_2_POWER_STATE_AFFL_MASK)
> +                       << PSCI_0_2_POWER_STATE_AFFL_SHIFT);
>  }

Same as above.

>
>  /*
> @@ -128,6 +126,14 @@ static noinline int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
>         return function_id;
>  }
>
> +static int psci_get_version(void)
> +{
> +       int err;
> +
> +       err = invoke_psci_fn(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> +       return err;
> +}
> +
>  static int psci_cpu_suspend(struct psci_power_state state,
>                             unsigned long entry_point)
>  {
> @@ -171,26 +177,36 @@ static int psci_migrate(unsigned long cpuid)
>         return psci_to_linux_errno(err);
>  }
>
> -static const struct of_device_id psci_of_match[] __initconst = {
> -       { .compatible = "arm,psci",     },
> -       {},
> -};
> +static int psci_affinity_info(unsigned long target_affinity,
> +               unsigned long lowest_affinity_level)
> +{
> +       int err;
> +       u32 fn;
> +
> +       fn = psci_function_id[PSCI_FN_AFFINITY_INFO];
> +       err = invoke_psci_fn(fn, target_affinity, lowest_affinity_level, 0);
> +       return err;
> +}
>
> -void __init psci_init(void)
> +static int psci_migrate_info_type(void)
>  {
> -       struct device_node *np;
> -       const char *method;
> -       u32 id;
> +       int err;
> +       u32 fn;
>
> -       np = of_find_matching_node(NULL, psci_of_match);
> -       if (!np)
> -               return;
> +       fn = psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE];
> +       err = invoke_psci_fn(fn, 0, 0, 0);
> +       return err;
> +}
> +
> +static int get_set_conduit_method(struct device_node *np)
> +{
> +       const char *method;
>
> -       pr_info("probing function IDs from device-tree\n");
> +       pr_info("probing for conduit method from DT.\n");
>
>         if (of_property_read_string(np, "method", &method)) {
> -               pr_warning("missing \"method\" property\n");
> -               goto out_put_node;
> +               pr_warn("missing \"method\" property\n");
> +               return -ENXIO;
>         }
>
>         if (!strcmp("hvc", method)) {
> @@ -198,10 +214,99 @@ void __init psci_init(void)
>         } else if (!strcmp("smc", method)) {
>                 invoke_psci_fn = __invoke_psci_fn_smc;
>         } else {
> -               pr_warning("invalid \"method\" property: %s\n", method);
> +               pr_warn("invalid \"method\" property: %s\n", method);
> +               return -EINVAL;
> +       }
> +       return 0;
> +}
> +
> +static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
> +{
> +       invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> +}
> +
> +static void psci_sys_poweroff(void)
> +{
> +       invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
> +}
> +
> +/*
> + * PSCI Function IDs for v0.2+ are well defined so use
> + * standard values.
> + */
> +static int psci_0_2_init(struct device_node *np)
> +{
> +       int err, ver;
> +
> +       err = get_set_conduit_method(np);
> +
> +       if (err)
> +               goto out_put_node;
> +
> +       ver = psci_get_version();
> +
> +       if (ver == PSCI_RET_NOT_SUPPORTED) {
> +               /* PSCI v0.2 mandates implementation of PSCI_ID_VERSION. */
> +               pr_err("PSCI firmware does not comply with the v0.2 spec.\n");
> +               err = -EOPNOTSUPP;
>                 goto out_put_node;
> +       } else {
> +               pr_info("PSCIv%d.%d detected in firmware.\n",
> +                               PSCI_VERSION_MAJOR(ver),
> +                               PSCI_VERSION_MINOR(ver));
> +
> +               if (PSCI_VERSION_MAJOR(ver) == 0 &&
> +                               PSCI_VERSION_MINOR(ver) < 2) {
> +                       err = -EINVAL;
> +                       pr_err("Conflicting PSCI version detected.\n");
> +                       goto out_put_node;
> +               }
>         }
>
> +       pr_info("Using standard PSCI v0.2 function IDs\n");
> +       psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN64_CPU_SUSPEND;
> +       psci_ops.cpu_suspend = psci_cpu_suspend;
> +
> +       psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
> +       psci_ops.cpu_off = psci_cpu_off;
> +
> +       psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN64_CPU_ON;
> +       psci_ops.cpu_on = psci_cpu_on;
> +
> +       psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN64_MIGRATE;
> +       psci_ops.migrate = psci_migrate;
> +
> +       psci_function_id[PSCI_FN_AFFINITY_INFO] = PSCI_0_2_FN64_AFFINITY_INFO;
> +       psci_ops.affinity_info = psci_affinity_info;
> +
> +       psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE] =
> +               PSCI_0_2_FN_MIGRATE_INFO_TYPE;
> +       psci_ops.migrate_info_type = psci_migrate_info_type;
> +
> +       arm_pm_restart = psci_sys_reset;
> +
> +       pm_power_off = psci_sys_poweroff;
> +
> +out_put_node:
> +       of_node_put(np);
> +       return err;
> +}
> +
> +/*
> + * PSCI < v0.2 get PSCI Function IDs via DT.
> + */
> +static int psci_0_1_init(struct device_node *np)
> +{
> +       u32 id;
> +       int err;
> +
> +       err = get_set_conduit_method(np);
> +
> +       if (err)
> +               goto out_put_node;
> +
> +       pr_info("Using PSCI v0.1 Function IDs from DT\n");
> +
>         if (!of_property_read_u32(np, "cpu_suspend", &id)) {
>                 psci_function_id[PSCI_FN_CPU_SUSPEND] = id;
>                 psci_ops.cpu_suspend = psci_cpu_suspend;
> @@ -224,7 +329,28 @@ void __init psci_init(void)
>
>  out_put_node:
>         of_node_put(np);
> -       return;
> +       return err;
> +}
> +
> +static const struct of_device_id psci_of_match[] __initconst = {
> +       { .compatible = "arm,psci",     .data = psci_0_1_init},
> +       { .compatible = "arm,psci-0.2", .data = psci_0_2_init},
> +       {},
> +};
> +
> +int __init psci_init(void)
> +{
> +       struct device_node *np;
> +       const struct of_device_id *matched_np;
> +       psci_initcall_t init_fn;
> +
> +       np = of_find_matching_node_and_match(NULL, psci_of_match, &matched_np);
> +
> +       if (!np)
> +               return -ENODEV;
> +
> +       init_fn = (psci_initcall_t)matched_np->data;
> +       return init_fn(np);
>  }
>
>  #ifdef CONFIG_SMP
> --
> 1.8.3.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Regards,
Anup

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v9 2/3] Documentation: devicetree: Add new binding for PSCIv0.2
  2014-05-07 14:27 ` [PATCH v9 2/3] Documentation: devicetree: Add new binding for PSCIv0.2 Ashwin Chaugule
@ 2014-05-12  8:54   ` Mark Rutland
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2014-05-12  8:54 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Ashwin,

On Wed, May 07, 2014 at 03:27:26PM +0100, Ashwin Chaugule wrote:
> The PSCI v0.2+ spec defines standard values for PSCI function IDs.
> Add a new binding entry so that pre v0.2 implementations can
> use DT entries for function IDs and v0.2+ implementations use
> standard entries as defined by the PSCIv0.2 specification.
> 
> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
> Acked-by: Rob Herring <robh@kernel.org>

This looks good to me as a binding, and my prior comments all seem to
have been addressed. So:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

I have one minor comment regarding the final example. It would be nice
to be a little more explicit about the compatibility story there for
those people who are likely to skim the body of the document and only
read the examples.

[...]

> +Case 3: PSCI v0.2 and PSCI v0.1.
> +
> +	As described above, for compatibility with existing kernels, the
> +	hypervisor will likely want to provide IDs, e.g.

All we'd need here is to point out that the IDs will be ignored by a
PSCI 0.2 capable OS, which will assume the standard PSCI 0.2 IDs.

Could we replace the above with something like:

A DTB may provide IDs for use by kernels without PSCI 0.2 support,
enabling firmware and hypervisors to support existing and new kernels.
These IDs will be ignored by kernels with PSCI 0.2 support, which will
use the standard PSCI 0.2 IDs exclusively.

> +
> +	psci {
> +		compatible = "arm,psci-0.2", "arm,psci";
> +		method = "hvc";
> +
> +		cpu_on = < arbitrary value >;
> +		cpu_off = < arbitrary value >;
> +
> +		...
> +	};

Cheers,
Mark.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions
  2014-05-07 14:27 ` [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions Ashwin Chaugule
  2014-05-11  8:14   ` Anup Patel
@ 2014-05-12  9:10   ` Mark Rutland
  2014-05-12 14:02     ` Ashwin Chaugule
  1 sibling, 1 reply; 14+ messages in thread
From: Mark Rutland @ 2014-05-12  9:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Ashwin,

On Wed, May 07, 2014 at 03:27:25PM +0100, Ashwin Chaugule wrote:
> The PSCIv0.2 spec defines standard values of function IDs
> and introduces a few new functions. Detect version of PSCI
> and appropriately select the right PSCI functions.
>
> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>

Other than those issues pointed out by Anup, this looks good to me,
though I'd like to see the cpu_kill callbacks for arm (and arm64) go
though at the same time (possibly folded in) so that implementations are
exercised from the start and we don't have to add properties to later
bodge around any current brokenness.

Have these patches been tested against an implementation?

Which tree are you planning on merging this through? Do we need to split
the arm and arm64 parts?

Cheers,
Mark.

> ---
>  arch/arm/include/asm/psci.h   |   7 +-
>  arch/arm/kernel/psci.c        | 196 +++++++++++++++++++++++++++++++++--------
>  arch/arm64/include/asm/psci.h |   2 +-
>  arch/arm64/kernel/psci.c      | 200 ++++++++++++++++++++++++++++++++++--------
>  4 files changed, 328 insertions(+), 77 deletions(-)
>
> diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> index c4ae171..b93e34a 100644
> --- a/arch/arm/include/asm/psci.h
> +++ b/arch/arm/include/asm/psci.h
> @@ -29,16 +29,19 @@ struct psci_operations {
>         int (*cpu_off)(struct psci_power_state state);
>         int (*cpu_on)(unsigned long cpuid, unsigned long entry_point);
>         int (*migrate)(unsigned long cpuid);
> +       int (*affinity_info)(unsigned long target_affinity,
> +                       unsigned long lowest_affinity_level);
> +       int (*migrate_info_type)(void);
>  };
>
>  extern struct psci_operations psci_ops;
>  extern struct smp_operations psci_smp_ops;
>
>  #ifdef CONFIG_ARM_PSCI
> -void psci_init(void);
> +int psci_init(void);
>  bool psci_smp_available(void);
>  #else
> -static inline void psci_init(void) { }
> +static inline int psci_init(void) { }
>  static inline bool psci_smp_available(void) { return false; }
>  #endif
>
> diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
> index 4693188..3775e62 100644
> --- a/arch/arm/kernel/psci.c
> +++ b/arch/arm/kernel/psci.c
> @@ -17,63 +17,58 @@
>
>  #include <linux/init.h>
>  #include <linux/of.h>
> +#include <linux/reboot.h>
> +#include <linux/pm.h>
> +#include <uapi/linux/psci.h>
>
>  #include <asm/compiler.h>
>  #include <asm/errno.h>
>  #include <asm/opcodes-sec.h>
>  #include <asm/opcodes-virt.h>
>  #include <asm/psci.h>
> +#include <asm/system_misc.h>
>
>  struct psci_operations psci_ops;
>
>  static int (*invoke_psci_fn)(u32, u32, u32, u32);
> +typedef int (*psci_initcall_t)(const struct device_node *);
>
>  enum psci_function {
>         PSCI_FN_CPU_SUSPEND,
>         PSCI_FN_CPU_ON,
>         PSCI_FN_CPU_OFF,
>         PSCI_FN_MIGRATE,
> +       PSCI_FN_AFFINITY_INFO,
> +       PSCI_FN_MIGRATE_INFO_TYPE,
>         PSCI_FN_MAX,
>  };
>
>  static u32 psci_function_id[PSCI_FN_MAX];
>
> -#define PSCI_RET_SUCCESS               0
> -#define PSCI_RET_EOPNOTSUPP            -1
> -#define PSCI_RET_EINVAL                        -2
> -#define PSCI_RET_EPERM                 -3
> -
>  static int psci_to_linux_errno(int errno)
>  {
>         switch (errno) {
>         case PSCI_RET_SUCCESS:
>                 return 0;
> -       case PSCI_RET_EOPNOTSUPP:
> +       case PSCI_RET_NOT_SUPPORTED:
>                 return -EOPNOTSUPP;
> -       case PSCI_RET_EINVAL:
> +       case PSCI_RET_INVALID_PARAMS:
>                 return -EINVAL;
> -       case PSCI_RET_EPERM:
> +       case PSCI_RET_DENIED:
>                 return -EPERM;
>         };
>
>         return -EINVAL;
>  }
>
> -#define PSCI_POWER_STATE_ID_MASK       0xffff
> -#define PSCI_POWER_STATE_ID_SHIFT      0
> -#define PSCI_POWER_STATE_TYPE_MASK     0x1
> -#define PSCI_POWER_STATE_TYPE_SHIFT    16
> -#define PSCI_POWER_STATE_AFFL_MASK     0x3
> -#define PSCI_POWER_STATE_AFFL_SHIFT    24
> -
>  static u32 psci_power_state_pack(struct psci_power_state state)
>  {
> -       return  ((state.id & PSCI_POWER_STATE_ID_MASK)
> -                       << PSCI_POWER_STATE_ID_SHIFT)   |
> -               ((state.type & PSCI_POWER_STATE_TYPE_MASK)
> -                       << PSCI_POWER_STATE_TYPE_SHIFT) |
> -               ((state.affinity_level & PSCI_POWER_STATE_AFFL_MASK)
> -                       << PSCI_POWER_STATE_AFFL_SHIFT);
> +       return  ((state.id & PSCI_0_2_POWER_STATE_ID_MASK)
> +                       << PSCI_0_2_POWER_STATE_ID_SHIFT)       |
> +               ((state.type & PSCI_0_2_POWER_STATE_TYPE_MASK)
> +                       << PSCI_0_2_POWER_STATE_TYPE_SHIFT)     |
> +               ((state.affinity_level & PSCI_0_2_POWER_STATE_AFFL_MASK)
> +                       << PSCI_0_2_POWER_STATE_AFFL_SHIFT);
>  }
>
>  /*
> @@ -110,6 +105,14 @@ static noinline int __invoke_psci_fn_smc(u32 function_id, u32 arg0, u32 arg1,
>         return function_id;
>  }
>
> +static int psci_get_version(void)
> +{
> +       int err;
> +
> +       err = invoke_psci_fn(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> +       return err;
> +}
> +
>  static int psci_cpu_suspend(struct psci_power_state state,
>                             unsigned long entry_point)
>  {
> @@ -153,26 +156,36 @@ static int psci_migrate(unsigned long cpuid)
>         return psci_to_linux_errno(err);
>  }
>
> -static const struct of_device_id psci_of_match[] __initconst = {
> -       { .compatible = "arm,psci",     },
> -       {},
> -};
> +static int psci_affinity_info(unsigned long target_affinity,
> +               unsigned long lowest_affinity_level)
> +{
> +       int err;
> +       u32 fn;
> +
> +       fn = psci_function_id[PSCI_FN_AFFINITY_INFO];
> +       err = invoke_psci_fn(fn, target_affinity, lowest_affinity_level, 0);
> +       return err;
> +}
>
> -void __init psci_init(void)
> +static int psci_migrate_info_type(void)
>  {
> -       struct device_node *np;
> -       const char *method;
> -       u32 id;
> +       int err;
> +       u32 fn;
>
> -       np = of_find_matching_node(NULL, psci_of_match);
> -       if (!np)
> -               return;
> +       fn = psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE];
> +       err = invoke_psci_fn(fn, 0, 0, 0);
> +       return err;
> +}
> +
> +static int get_set_conduit_method(struct device_node *np)
> +{
> +       const char *method;
>
> -       pr_info("probing function IDs from device-tree\n");
> +       pr_info("probing for conduit method from DT.\n");
>
>         if (of_property_read_string(np, "method", &method)) {
> -               pr_warning("missing \"method\" property\n");
> -               goto out_put_node;
> +               pr_warn("missing \"method\" property\n");
> +               return -ENXIO;
>         }
>
>         if (!strcmp("hvc", method)) {
> @@ -180,10 +193,99 @@ void __init psci_init(void)
>         } else if (!strcmp("smc", method)) {
>                 invoke_psci_fn = __invoke_psci_fn_smc;
>         } else {
> -               pr_warning("invalid \"method\" property: %s\n", method);
> +               pr_warn("invalid \"method\" property: %s\n", method);
> +               return -EINVAL;
> +       }
> +       return 0;
> +}
> +
> +static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
> +{
> +       invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> +}
> +
> +static void psci_sys_poweroff(void)
> +{
> +       invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
> +}
> +
> +/*
> + * PSCI Function IDs for v0.2+ are well defined so use
> + * standard values.
> + */
> +static int psci_0_2_init(struct device_node *np)
> +{
> +       int err, ver;
> +
> +       err = get_set_conduit_method(np);
> +
> +       if (err)
> +               goto out_put_node;
> +
> +       ver = psci_get_version();
> +
> +       if (ver == PSCI_RET_NOT_SUPPORTED) {
> +               /* PSCI v0.2 mandates implementation of PSCI_ID_VERSION. */
> +               pr_err("PSCI firmware does not comply with the v0.2 spec.\n");
> +               err = -EOPNOTSUPP;
>                 goto out_put_node;
> +       } else {
> +               pr_info("PSCIv%d.%d detected in firmware.\n",
> +                               PSCI_VERSION_MAJOR(ver),
> +                               PSCI_VERSION_MINOR(ver));
> +
> +               if (PSCI_VERSION_MAJOR(ver) == 0 &&
> +                               PSCI_VERSION_MINOR(ver) < 2) {
> +                       err = -EINVAL;
> +                       pr_err("Conflicting PSCI version detected.\n");
> +                       goto out_put_node;
> +               }
>         }
>
> +       pr_info("Using standard PSCI v0.2 function IDs\n");
> +       psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN_CPU_SUSPEND;
> +       psci_ops.cpu_suspend = psci_cpu_suspend;
> +
> +       psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
> +       psci_ops.cpu_off = psci_cpu_off;
> +
> +       psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN_CPU_ON;
> +       psci_ops.cpu_on = psci_cpu_on;
> +
> +       psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN_MIGRATE;
> +       psci_ops.migrate = psci_migrate;
> +
> +       psci_function_id[PSCI_FN_AFFINITY_INFO] = PSCI_0_2_FN_AFFINITY_INFO;
> +       psci_ops.affinity_info = psci_affinity_info;
> +
> +       psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE] =
> +               PSCI_0_2_FN_MIGRATE_INFO_TYPE;
> +       psci_ops.migrate_info_type = psci_migrate_info_type;
> +
> +       arm_pm_restart = psci_sys_reset;
> +
> +       pm_power_off = psci_sys_poweroff;
> +
> +out_put_node:
> +       of_node_put(np);
> +       return err;
> +}
> +
> +/*
> + * PSCI < v0.2 get PSCI Function IDs via DT.
> + */
> +static int psci_0_1_init(struct device_node *np)
> +{
> +       u32 id;
> +       int err;
> +
> +       err = get_set_conduit_method(np);
> +
> +       if (err)
> +               goto out_put_node;
> +
> +       pr_info("Using PSCI v0.1 Function IDs from DT\n");
> +
>         if (!of_property_read_u32(np, "cpu_suspend", &id)) {
>                 psci_function_id[PSCI_FN_CPU_SUSPEND] = id;
>                 psci_ops.cpu_suspend = psci_cpu_suspend;
> @@ -206,5 +308,25 @@ void __init psci_init(void)
>
>  out_put_node:
>         of_node_put(np);
> -       return;
> +       return err;
> +}
> +
> +static const struct of_device_id psci_of_match[] __initconst = {
> +       { .compatible = "arm,psci", .data = psci_0_1_init},
> +       { .compatible = "arm,psci-0.2", .data = psci_0_2_init},
> +       {},
> +};
> +
> +int __init psci_init(void)
> +{
> +       struct device_node *np;
> +       const struct of_device_id *matched_np;
> +       psci_initcall_t init_fn;
> +
> +       np = of_find_matching_node_and_match(NULL, psci_of_match, &matched_np);
> +       if (!np)
> +               return -ENODEV;
> +
> +       init_fn = (psci_initcall_t)matched_np->data;
> +       return init_fn(np);
>  }
> diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h
> index d15ab8b4..e5312ea 100644
> --- a/arch/arm64/include/asm/psci.h
> +++ b/arch/arm64/include/asm/psci.h
> @@ -14,6 +14,6 @@
>  #ifndef __ASM_PSCI_H
>  #define __ASM_PSCI_H
>
> -void psci_init(void);
> +int psci_init(void);
>
>  #endif /* __ASM_PSCI_H */
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index ea4828a..6045613 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -18,12 +18,16 @@
>  #include <linux/init.h>
>  #include <linux/of.h>
>  #include <linux/smp.h>
> +#include <linux/reboot.h>
> +#include <linux/pm.h>
> +#include <uapi/linux/psci.h>
>
>  #include <asm/compiler.h>
>  #include <asm/cpu_ops.h>
>  #include <asm/errno.h>
>  #include <asm/psci.h>
>  #include <asm/smp_plat.h>
> +#include <asm/system_misc.h>
>
>  #define PSCI_POWER_STATE_TYPE_STANDBY          0
>  #define PSCI_POWER_STATE_TYPE_POWER_DOWN       1
> @@ -40,58 +44,52 @@ struct psci_operations {
>         int (*cpu_off)(struct psci_power_state state);
>         int (*cpu_on)(unsigned long cpuid, unsigned long entry_point);
>         int (*migrate)(unsigned long cpuid);
> +       int (*affinity_info)(unsigned long target_affinity,
> +                       unsigned long lowest_affinity_level);
> +       int (*migrate_info_type)(void);
>  };
>
>  static struct psci_operations psci_ops;
>
>  static int (*invoke_psci_fn)(u64, u64, u64, u64);
> +typedef int (*psci_initcall_t)(const struct device_node *);
>
>  enum psci_function {
>         PSCI_FN_CPU_SUSPEND,
>         PSCI_FN_CPU_ON,
>         PSCI_FN_CPU_OFF,
>         PSCI_FN_MIGRATE,
> +       PSCI_FN_AFFINITY_INFO,
> +       PSCI_FN_MIGRATE_INFO_TYPE,
>         PSCI_FN_MAX,
>  };
>
>  static u32 psci_function_id[PSCI_FN_MAX];
>
> -#define PSCI_RET_SUCCESS               0
> -#define PSCI_RET_EOPNOTSUPP            -1
> -#define PSCI_RET_EINVAL                        -2
> -#define PSCI_RET_EPERM                 -3
> -
>  static int psci_to_linux_errno(int errno)
>  {
>         switch (errno) {
>         case PSCI_RET_SUCCESS:
>                 return 0;
> -       case PSCI_RET_EOPNOTSUPP:
> +       case PSCI_RET_NOT_SUPPORTED:
>                 return -EOPNOTSUPP;
> -       case PSCI_RET_EINVAL:
> +       case PSCI_RET_INVALID_PARAMS:
>                 return -EINVAL;
> -       case PSCI_RET_EPERM:
> +       case PSCI_RET_DENIED:
>                 return -EPERM;
>         };
>
>         return -EINVAL;
>  }
>
> -#define PSCI_POWER_STATE_ID_MASK       0xffff
> -#define PSCI_POWER_STATE_ID_SHIFT      0
> -#define PSCI_POWER_STATE_TYPE_MASK     0x1
> -#define PSCI_POWER_STATE_TYPE_SHIFT    16
> -#define PSCI_POWER_STATE_AFFL_MASK     0x3
> -#define PSCI_POWER_STATE_AFFL_SHIFT    24
> -
>  static u32 psci_power_state_pack(struct psci_power_state state)
>  {
> -       return  ((state.id & PSCI_POWER_STATE_ID_MASK)
> -                       << PSCI_POWER_STATE_ID_SHIFT)   |
> -               ((state.type & PSCI_POWER_STATE_TYPE_MASK)
> -                       << PSCI_POWER_STATE_TYPE_SHIFT) |
> -               ((state.affinity_level & PSCI_POWER_STATE_AFFL_MASK)
> -                       << PSCI_POWER_STATE_AFFL_SHIFT);
> +       return  ((state.id & PSCI_0_2_POWER_STATE_ID_MASK)
> +                       << PSCI_0_2_POWER_STATE_ID_SHIFT)       |
> +               ((state.type & PSCI_0_2_POWER_STATE_TYPE_MASK)
> +                       << PSCI_0_2_POWER_STATE_TYPE_SHIFT)     |
> +               ((state.affinity_level & PSCI_0_2_POWER_STATE_AFFL_MASK)
> +                       << PSCI_0_2_POWER_STATE_AFFL_SHIFT);
>  }
>
>  /*
> @@ -128,6 +126,14 @@ static noinline int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
>         return function_id;
>  }
>
> +static int psci_get_version(void)
> +{
> +       int err;
> +
> +       err = invoke_psci_fn(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
> +       return err;
> +}
> +
>  static int psci_cpu_suspend(struct psci_power_state state,
>                             unsigned long entry_point)
>  {
> @@ -171,26 +177,36 @@ static int psci_migrate(unsigned long cpuid)
>         return psci_to_linux_errno(err);
>  }
>
> -static const struct of_device_id psci_of_match[] __initconst = {
> -       { .compatible = "arm,psci",     },
> -       {},
> -};
> +static int psci_affinity_info(unsigned long target_affinity,
> +               unsigned long lowest_affinity_level)
> +{
> +       int err;
> +       u32 fn;
> +
> +       fn = psci_function_id[PSCI_FN_AFFINITY_INFO];
> +       err = invoke_psci_fn(fn, target_affinity, lowest_affinity_level, 0);
> +       return err;
> +}
>
> -void __init psci_init(void)
> +static int psci_migrate_info_type(void)
>  {
> -       struct device_node *np;
> -       const char *method;
> -       u32 id;
> +       int err;
> +       u32 fn;
>
> -       np = of_find_matching_node(NULL, psci_of_match);
> -       if (!np)
> -               return;
> +       fn = psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE];
> +       err = invoke_psci_fn(fn, 0, 0, 0);
> +       return err;
> +}
> +
> +static int get_set_conduit_method(struct device_node *np)
> +{
> +       const char *method;
>
> -       pr_info("probing function IDs from device-tree\n");
> +       pr_info("probing for conduit method from DT.\n");
>
>         if (of_property_read_string(np, "method", &method)) {
> -               pr_warning("missing \"method\" property\n");
> -               goto out_put_node;
> +               pr_warn("missing \"method\" property\n");
> +               return -ENXIO;
>         }
>
>         if (!strcmp("hvc", method)) {
> @@ -198,10 +214,99 @@ void __init psci_init(void)
>         } else if (!strcmp("smc", method)) {
>                 invoke_psci_fn = __invoke_psci_fn_smc;
>         } else {
> -               pr_warning("invalid \"method\" property: %s\n", method);
> +               pr_warn("invalid \"method\" property: %s\n", method);
> +               return -EINVAL;
> +       }
> +       return 0;
> +}
> +
> +static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
> +{
> +       invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> +}
> +
> +static void psci_sys_poweroff(void)
> +{
> +       invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
> +}
> +
> +/*
> + * PSCI Function IDs for v0.2+ are well defined so use
> + * standard values.
> + */
> +static int psci_0_2_init(struct device_node *np)
> +{
> +       int err, ver;
> +
> +       err = get_set_conduit_method(np);
> +
> +       if (err)
> +               goto out_put_node;
> +
> +       ver = psci_get_version();
> +
> +       if (ver == PSCI_RET_NOT_SUPPORTED) {
> +               /* PSCI v0.2 mandates implementation of PSCI_ID_VERSION. */
> +               pr_err("PSCI firmware does not comply with the v0.2 spec.\n");
> +               err = -EOPNOTSUPP;
>                 goto out_put_node;
> +       } else {
> +               pr_info("PSCIv%d.%d detected in firmware.\n",
> +                               PSCI_VERSION_MAJOR(ver),
> +                               PSCI_VERSION_MINOR(ver));
> +
> +               if (PSCI_VERSION_MAJOR(ver) == 0 &&
> +                               PSCI_VERSION_MINOR(ver) < 2) {
> +                       err = -EINVAL;
> +                       pr_err("Conflicting PSCI version detected.\n");
> +                       goto out_put_node;
> +               }
>         }
>
> +       pr_info("Using standard PSCI v0.2 function IDs\n");
> +       psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN64_CPU_SUSPEND;
> +       psci_ops.cpu_suspend = psci_cpu_suspend;
> +
> +       psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
> +       psci_ops.cpu_off = psci_cpu_off;
> +
> +       psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN64_CPU_ON;
> +       psci_ops.cpu_on = psci_cpu_on;
> +
> +       psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN64_MIGRATE;
> +       psci_ops.migrate = psci_migrate;
> +
> +       psci_function_id[PSCI_FN_AFFINITY_INFO] = PSCI_0_2_FN64_AFFINITY_INFO;
> +       psci_ops.affinity_info = psci_affinity_info;
> +
> +       psci_function_id[PSCI_FN_MIGRATE_INFO_TYPE] =
> +               PSCI_0_2_FN_MIGRATE_INFO_TYPE;
> +       psci_ops.migrate_info_type = psci_migrate_info_type;
> +
> +       arm_pm_restart = psci_sys_reset;
> +
> +       pm_power_off = psci_sys_poweroff;
> +
> +out_put_node:
> +       of_node_put(np);
> +       return err;
> +}
> +
> +/*
> + * PSCI < v0.2 get PSCI Function IDs via DT.
> + */
> +static int psci_0_1_init(struct device_node *np)
> +{
> +       u32 id;
> +       int err;
> +
> +       err = get_set_conduit_method(np);
> +
> +       if (err)
> +               goto out_put_node;
> +
> +       pr_info("Using PSCI v0.1 Function IDs from DT\n");
> +
>         if (!of_property_read_u32(np, "cpu_suspend", &id)) {
>                 psci_function_id[PSCI_FN_CPU_SUSPEND] = id;
>                 psci_ops.cpu_suspend = psci_cpu_suspend;
> @@ -224,7 +329,28 @@ void __init psci_init(void)
>
>  out_put_node:
>         of_node_put(np);
> -       return;
> +       return err;
> +}
> +
> +static const struct of_device_id psci_of_match[] __initconst = {
> +       { .compatible = "arm,psci",     .data = psci_0_1_init},
> +       { .compatible = "arm,psci-0.2", .data = psci_0_2_init},
> +       {},
> +};
> +
> +int __init psci_init(void)
> +{
> +       struct device_node *np;
> +       const struct of_device_id *matched_np;
> +       psci_initcall_t init_fn;
> +
> +       np = of_find_matching_node_and_match(NULL, psci_of_match, &matched_np);
> +
> +       if (!np)
> +               return -ENODEV;
> +
> +       init_fn = (psci_initcall_t)matched_np->data;
> +       return init_fn(np);
>  }
>
>  #ifdef CONFIG_SMP
> --
> 1.8.3.2
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v9 3/3] ARM: Check if a CPU has gone offline
  2014-05-07 14:27 ` [PATCH v9 3/3] ARM: Check if a CPU has gone offline Ashwin Chaugule
@ 2014-05-12 10:16   ` Mark Rutland
  2014-05-12 13:49     ` Ashwin Chaugule
  0 siblings, 1 reply; 14+ messages in thread
From: Mark Rutland @ 2014-05-12 10:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Ashwin,

This looks mostly good to me, with a couple of issues below.

On Wed, May 07, 2014 at 03:27:27PM +0100, Ashwin Chaugule wrote:
> PSCIv0.2 adds a new function called AFFINITY_INFO, which
> can be used to query if a specified CPU has actually gone
> offline. Calling this function via cpu_kill ensures that
> a CPU has quiesced after a call to cpu_die.
> 
> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  arch/arm/kernel/psci_smp.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)

[...]

> +int __ref psci_cpu_kill(unsigned int cpu)
> +{
> +	int err, i;
> +
> +	if (!psci_ops.affinity_info)
> +		return 1;
> +	/*
> +	 * cpu_kill could race with cpu_die and we can
> +	 * potentially end up declaring this cpu undead
> +	 * while it is dying. So, try again a few times.
> +	 */
> +
> +	for (i=0; i<10; i++) {

Nit: please place spaces around the binary operators:

	for (i = 0; i < 10; i++) {

> +		err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
> +		if (err == PSCI_0_2_AFFINITY_LEVEL_OFF)
> +			return 1;
> +
> +		msleep(10);
> +		pr_info("Retrying again to check for CPU kill\n");

I don't think we need to print on each iteration -- that's going to spam
the log if we race and/or slow things down.

> +	}
> +
> +	pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
> +			cpu, err);

This message might be a little confusing as we're testing that the CPU
is dead rather than explicitly killing it. The "psci:" prefix is
redundant thanks to pr_fmt and CPU:%d is a little odd.

How about:

pr_warn("CPU%d may not have shut down cleanly (AFFINITY_INFO reports %d)\n",
	cpu, err);

> +	/* Make platform_cpu_kill() fail. */
> +	return 0;
> +}

[...]

Could you implement this for arm64 as well? We don't currently have a
cpu_kill callback on cpu_operations, but the (compile-tested only) patch
below should remedy that.

Cheers,
Mark.

---->8----
>From 8f81dc38a7a2b7b61165d350dff5cdcaa7248081 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Mon, 12 May 2014 10:39:07 +0100
Subject: [PATCH] arm64: cpu_ops: add cpu_kill callback

Currently there is a window between a CPU announcing that it is dead and
said CPU leaving the kernel. If data or instructions are clobbered (as
happens on a kexec) within this window the CPU might begin doing
arbitrary bad things.

In some of these cases (e.g. on those platforms with PSCI 0.2) we can
query the firmware to discover when the CPU has really left the kernel,
closing the race.

This patch adds a cpu_kill callback for use in those cases, which may
bew used to synchronise with the dying CPU.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/cpu_ops.h |  2 ++
 arch/arm64/kernel/smp.c          | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/arch/arm64/include/asm/cpu_ops.h b/arch/arm64/include/asm/cpu_ops.h
index 1524130..c7c7442 100644
--- a/arch/arm64/include/asm/cpu_ops.h
+++ b/arch/arm64/include/asm/cpu_ops.h
@@ -39,6 +39,7 @@ struct device_node;
  * 		from the cpu to be killed.
  * @cpu_die:	Makes a cpu leave the kernel. Must not fail. Called from the
  *		cpu being killed.
+ * @cpu_kill:	Ensures a cpu has left the kernel. Called from another cpu.
  * @cpu_suspend: Suspends a cpu and saves the required context. May fail owing
  *               to wrong parameters or error conditions. Called from the
  *               CPU being suspended. Must be called with IRQs disabled.
@@ -52,6 +53,7 @@ struct cpu_operations {
 #ifdef CONFIG_HOTPLUG_CPU
 	int		(*cpu_disable)(unsigned int cpu);
 	void		(*cpu_die)(unsigned int cpu);
+	int		(*cpu_kill)(unsigned int cpu);
 #endif
 #ifdef CONFIG_ARM64_CPU_SUSPEND
 	int		(*cpu_suspend)(unsigned long);
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 7cfb92a..4844628 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -220,6 +220,19 @@ int __cpu_disable(void)
 	return 0;
 }
 
+static int op_cpu_kill(unsigned int cpu)
+{
+	/*
+	 * If we have no means of synchronising with the dying CPU, then assume
+	 * that it is really dead. We can only wait for an arbitrary length of
+	 * time and hope that it's dead, so let's skip the wait and just hope.
+	 */
+	if (!cpu_ops[cpu]->cpu_kill)
+		return 0;
+
+	return cpu_ops[cpu]->cpu_kill(cpu);
+}
+
 static DECLARE_COMPLETION(cpu_died);
 
 /*
@@ -233,6 +246,15 @@ void __cpu_die(unsigned int cpu)
 		return;
 	}
 	pr_notice("CPU%u: shutdown\n", cpu);
+
+	/*
+	 * Now that the dying CPU is beyond the point of no return w.r.t.
+	 * in-kernel synchronisation, try to get the firwmare to help us to
+	 * verify that it has really left the kernel before we consider
+	 * clobbering anything it might still be using.
+	 */
+	if (op_cpu_kill(cpu))
+		pr_warn("CPU%d may not have shut down cleanly\n", cpu);
 }
 
 /*
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v9 3/3] ARM: Check if a CPU has gone offline
  2014-05-12 10:16   ` Mark Rutland
@ 2014-05-12 13:49     ` Ashwin Chaugule
  0 siblings, 0 replies; 14+ messages in thread
From: Ashwin Chaugule @ 2014-05-12 13:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 12 May 2014 06:16, Mark Rutland <mark.rutland@arm.com> wrote:
>> +     for (i=0; i<10; i++) {
>
> Nit: please place spaces around the binary operators:
>
>         for (i = 0; i < 10; i++) {

Ok.

>
>> +             err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
>> +             if (err == PSCI_0_2_AFFINITY_LEVEL_OFF)
>> +                     return 1;
>> +
>> +             msleep(10);
>> +             pr_info("Retrying again to check for CPU kill\n");
>
> I don't think we need to print on each iteration -- that's going to spam
> the log if we race and/or slow things down.

I think its better than having a single print which could be easily
hidden by other dmesg noise. This shouldn't (hopefully) take all 10
iterations in practice and if it does then the bunch of printks should
be easy to identify and indicate that the CPU did not die. Come to
think of it, I should probably add a print when it does die, to make
it clear after a few (if any) retries.

>
>> +     }
>> +
>> +     pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
>> +                     cpu, err);
>
> This message might be a little confusing as we're testing that the CPU
> is dead rather than explicitly killing it. The "psci:" prefix is
> redundant thanks to pr_fmt and CPU:%d is a little odd.
>
> How about:
>
> pr_warn("CPU%d may not have shut down cleanly (AFFINITY_INFO reports %d)\n",
>         cpu, err);

Ok.

>
>> +     /* Make platform_cpu_kill() fail. */
>> +     return 0;
>> +}
>
> [...]
>
> Could you implement this for arm64 as well? We don't currently have a
> cpu_kill callback on cpu_operations, but the (compile-tested only) patch
> below should remedy that.
>
> Cheers,
> Mark.
>
> ---->8----
> From 8f81dc38a7a2b7b61165d350dff5cdcaa7248081 Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> Date: Mon, 12 May 2014 10:39:07 +0100
> Subject: [PATCH] arm64: cpu_ops: add cpu_kill callback
>
> Currently there is a window between a CPU announcing that it is dead and
> said CPU leaving the kernel. If data or instructions are clobbered (as
> happens on a kexec) within this window the CPU might begin doing
> arbitrary bad things.
>
> In some of these cases (e.g. on those platforms with PSCI 0.2) we can
> query the firmware to discover when the CPU has really left the kernel,
> closing the race.
>
> This patch adds a cpu_kill callback for use in those cases, which may
> bew used to synchronise with the dying CPU.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/include/asm/cpu_ops.h |  2 ++
>  arch/arm64/kernel/smp.c          | 22 ++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/arch/arm64/include/asm/cpu_ops.h b/arch/arm64/include/asm/cpu_ops.h
> index 1524130..c7c7442 100644
> --- a/arch/arm64/include/asm/cpu_ops.h
> +++ b/arch/arm64/include/asm/cpu_ops.h
> @@ -39,6 +39,7 @@ struct device_node;
>   *             from the cpu to be killed.
>   * @cpu_die:   Makes a cpu leave the kernel. Must not fail. Called from the
>   *             cpu being killed.
> + * @cpu_kill:  Ensures a cpu has left the kernel. Called from another cpu.
>   * @cpu_suspend: Suspends a cpu and saves the required context. May fail owing
>   *               to wrong parameters or error conditions. Called from the
>   *               CPU being suspended. Must be called with IRQs disabled.
> @@ -52,6 +53,7 @@ struct cpu_operations {
>  #ifdef CONFIG_HOTPLUG_CPU
>         int             (*cpu_disable)(unsigned int cpu);
>         void            (*cpu_die)(unsigned int cpu);
> +       int             (*cpu_kill)(unsigned int cpu);
>  #endif
>  #ifdef CONFIG_ARM64_CPU_SUSPEND
>         int             (*cpu_suspend)(unsigned long);
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 7cfb92a..4844628 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -220,6 +220,19 @@ int __cpu_disable(void)
>         return 0;
>  }
>
> +static int op_cpu_kill(unsigned int cpu)
> +{
> +       /*
> +        * If we have no means of synchronising with the dying CPU, then assume
> +        * that it is really dead. We can only wait for an arbitrary length of
> +        * time and hope that it's dead, so let's skip the wait and just hope.
> +        */
> +       if (!cpu_ops[cpu]->cpu_kill)
> +               return 0;
> +
> +       return cpu_ops[cpu]->cpu_kill(cpu);
> +}
> +
>  static DECLARE_COMPLETION(cpu_died);
>
>  /*
> @@ -233,6 +246,15 @@ void __cpu_die(unsigned int cpu)
>                 return;
>         }
>         pr_notice("CPU%u: shutdown\n", cpu);
> +
> +       /*
> +        * Now that the dying CPU is beyond the point of no return w.r.t.
> +        * in-kernel synchronisation, try to get the firwmare to help us to
> +        * verify that it has really left the kernel before we consider
> +        * clobbering anything it might still be using.
> +        */
> +       if (op_cpu_kill(cpu))
> +               pr_warn("CPU%d may not have shut down cleanly\n", cpu);
>  }
>
>  /*
> --
> 1.8.1.1
>
Your patch looks good to me and I can fold it in with the ARM patch
with your signature. However, I dont have a way to test any of this
besides booting it up on the FVP. So, if you've got any way to run
this, that'd be great!
Thanks for the review!

Cheers,
Ashwin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions
  2014-05-11  8:14   ` Anup Patel
@ 2014-05-12 13:52     ` Ashwin Chaugule
  0 siblings, 0 replies; 14+ messages in thread
From: Ashwin Chaugule @ 2014-05-12 13:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 11 May 2014 04:14, Anup Patel <anup@brainfault.org> wrote:
>>  static u32 psci_power_state_pack(struct psci_power_state state)
>>  {
>> -       return  ((state.id & PSCI_POWER_STATE_ID_MASK)
>> -                       << PSCI_POWER_STATE_ID_SHIFT)   |
>> -               ((state.type & PSCI_POWER_STATE_TYPE_MASK)
>> -                       << PSCI_POWER_STATE_TYPE_SHIFT) |
>> -               ((state.affinity_level & PSCI_POWER_STATE_AFFL_MASK)
>> -                       << PSCI_POWER_STATE_AFFL_SHIFT);
>> +       return  ((state.id & PSCI_0_2_POWER_STATE_ID_MASK)
>> +                       << PSCI_0_2_POWER_STATE_ID_SHIFT)       |
>> +               ((state.type & PSCI_0_2_POWER_STATE_TYPE_MASK)
>> +                       << PSCI_0_2_POWER_STATE_TYPE_SHIFT)     |
>> +               ((state.affinity_level & PSCI_0_2_POWER_STATE_AFFL_MASK)
>> +                       << PSCI_0_2_POWER_STATE_AFFL_SHIFT);
>>  }
>
> As per updated PSCI_0_2_POWER_STATE_xxx defines, this should be:
>
> return ((state.id << PSCI_0_2_POWER_STATE_ID_SHIFT)
>           & PSCI_0_2_POWER_STATE_ID_MASK) |
>           ((state.type << PSCI_0_2_POWER_STATE_TYPE_SHIFT)
>           & PSCI_0_2_POWER_STATE_TYPE_MASK) |
>           ((state.affinity_level << PSCI_0_2_POWER_STATE_AFFL_SHIFT)
>           & PSCI_0_2_POWER_STATE_AFFL_MASK);
>

Argh! Thanks for noticing it anyway.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions
  2014-05-12  9:10   ` Mark Rutland
@ 2014-05-12 14:02     ` Ashwin Chaugule
  2014-05-13 17:09       ` Ashwin Chaugule
  0 siblings, 1 reply; 14+ messages in thread
From: Ashwin Chaugule @ 2014-05-12 14:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 12 May 2014 05:10, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Ashwin,
>
> On Wed, May 07, 2014 at 03:27:25PM +0100, Ashwin Chaugule wrote:
>> The PSCIv0.2 spec defines standard values of function IDs
>> and introduces a few new functions. Detect version of PSCI
>> and appropriately select the right PSCI functions.
>>
>> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>
> Other than those issues pointed out by Anup, this looks good to me,
> though I'd like to see the cpu_kill callbacks for arm (and arm64) go
> though at the same time (possibly folded in) so that implementations are
> exercised from the start and we don't have to add properties to later
> bodge around any current brokenness.

Yea. I'll fold it in the next revision.

>
> Have these patches been tested against an implementation?

Only boot tested on the ARMv8 FVP.

>
> Which tree are you planning on merging this through? Do we need to split
> the arm and arm64 parts?

IIRC all this PSCI work originally came via Catalins tree(?). But now,
these patches depend on a header introduced by Anups KVM patchset
which is merged in -
https://git.kernel.org/cgit/linux/kernel/git/kvmarm/kvmarm.git/log/?h=next

In a previous discussion (everyone here is CC'd) with Christoffer and
Paolo the 2nd option of the following was chosen:

1) Catalin can apply "Add common header for PSCI related defines", or
Ashwin can resend his series with the patch at the beginning.
Duplicate commits are fine, especially for seldom-modified files where
they do not cause conflicts.

2) Send Anup's patchset as a pull request to both me and Catalin,
relative to v3.16-rc1, both of us can apply it and Ashwin's series can
be commit on top.


To keep it simple it'd be better if it all goes via one tree, but if
there are objections or other suggestions, I'm happy to split this up.

Thanks,
Ashwin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions
  2014-05-12 14:02     ` Ashwin Chaugule
@ 2014-05-13 17:09       ` Ashwin Chaugule
  2014-05-13 21:29         ` Catalin Marinas
  0 siblings, 1 reply; 14+ messages in thread
From: Ashwin Chaugule @ 2014-05-13 17:09 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Catalin,

On 12 May 2014 10:02, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
>>
>> Which tree are you planning on merging this through? Do we need to split
>> the arm and arm64 parts?
>
> IIRC all this PSCI work originally came via Catalins tree(?). But now,
> these patches depend on a header introduced by Anups KVM patchset
> which is merged in -
> https://git.kernel.org/cgit/linux/kernel/git/kvmarm/kvmarm.git/log/?h=next
>
> In a previous discussion (everyone here is CC'd) with Christoffer and
> Paolo the 2nd option of the following was chosen:
>
> 1) Catalin can apply "Add common header for PSCI related defines", or
> Ashwin can resend his series with the patch at the beginning.
> Duplicate commits are fine, especially for seldom-modified files where
> they do not cause conflicts.
>
> 2) Send Anup's patchset as a pull request to both me and Catalin,
> relative to v3.16-rc1, both of us can apply it and Ashwin's series can
> be commit on top.
>
>
> To keep it simple it'd be better if it all goes via one tree, but if
> there are objections or other suggestions, I'm happy to split this up.

What is the preferred method to upstream this patchset? Would you
recommend this be split into ARM and ARM64 variants, or can it all go
via the ARM64 tree? I will then respin these accordingly, along with
the feedback from Anup and Mark.

Thanks,
Ashwin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions
  2014-05-13 17:09       ` Ashwin Chaugule
@ 2014-05-13 21:29         ` Catalin Marinas
  0 siblings, 0 replies; 14+ messages in thread
From: Catalin Marinas @ 2014-05-13 21:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 13, 2014 at 06:09:00PM +0100, Ashwin Chaugule wrote:
> On 12 May 2014 10:02, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
> >>
> >> Which tree are you planning on merging this through? Do we need to split
> >> the arm and arm64 parts?
> >
> > IIRC all this PSCI work originally came via Catalins tree(?). But now,
> > these patches depend on a header introduced by Anups KVM patchset
> > which is merged in -
> > https://git.kernel.org/cgit/linux/kernel/git/kvmarm/kvmarm.git/log/?h=next
> >
> > In a previous discussion (everyone here is CC'd) with Christoffer and
> > Paolo the 2nd option of the following was chosen:
> >
> > 1) Catalin can apply "Add common header for PSCI related defines", or
> > Ashwin can resend his series with the patch at the beginning.
> > Duplicate commits are fine, especially for seldom-modified files where
> > they do not cause conflicts.
> >
> > 2) Send Anup's patchset as a pull request to both me and Catalin,
> > relative to v3.16-rc1, both of us can apply it and Ashwin's series can
> > be commit on top.
> >
> > To keep it simple it'd be better if it all goes via one tree, but if
> > there are objections or other suggestions, I'm happy to split this up.
> 
> What is the preferred method to upstream this patchset? Would you
> recommend this be split into ARM and ARM64 variants, or can it all go
> via the ARM64 tree? I will then respin these accordingly, along with
> the feedback from Anup and Mark.

I don't mind via which tree they go in, it can even be the KVM tree if
there are dependencies, with my Ack on them (once you address the
comments). But please note that you need Acks from Russell as well since
that's touching arch/arm code.

-- 
Catalin

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-05-13 21:29 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-07 14:27 [PATCH v9 0/3] PSCI v0.2 support and DT bindings Ashwin Chaugule
2014-05-07 14:27 ` [PATCH v9 1/3] PSCI: Add initial support for PSCIv0.2 functions Ashwin Chaugule
2014-05-11  8:14   ` Anup Patel
2014-05-12 13:52     ` Ashwin Chaugule
2014-05-12  9:10   ` Mark Rutland
2014-05-12 14:02     ` Ashwin Chaugule
2014-05-13 17:09       ` Ashwin Chaugule
2014-05-13 21:29         ` Catalin Marinas
2014-05-07 14:27 ` [PATCH v9 2/3] Documentation: devicetree: Add new binding for PSCIv0.2 Ashwin Chaugule
2014-05-12  8:54   ` Mark Rutland
2014-05-07 14:27 ` [PATCH v9 3/3] ARM: Check if a CPU has gone offline Ashwin Chaugule
2014-05-12 10:16   ` Mark Rutland
2014-05-12 13:49     ` Ashwin Chaugule
2014-05-08  4:48 ` [PATCH v9 0/3] PSCI v0.2 support and DT bindings Ashwin Chaugule

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).