[PATCH v6 0/3] PSCI v0.2 support and DT bindings

[PATCH v6 0/3] PSCI v0.2 support and DT bindings

[Ashwin Chaugule]

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

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                         | 179 +++++++++++++++++++-----
 arch/arm/kernel/psci_smp.c                     |  21 +++
 arch/arm64/include/asm/psci.h                  |   2 +-
 arch/arm64/kernel/psci.c                       | 183 ++++++++++++++++++++-----
 include/uapi/linux/psci.h                      |   5 +
 7 files changed, 354 insertions(+), 78 deletions(-)

-- 
1.8.3.2

[PATCH v6 1/3] PSCI: Add initial support for PSCIv0.2 functions

[Ashwin Chaugule]

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>
---
 arch/arm/include/asm/psci.h   |   7 +-
 arch/arm/kernel/psci.c        | 179 ++++++++++++++++++++++++++++++++---------
 arch/arm64/include/asm/psci.h |   2 +-
 arch/arm64/kernel/psci.c      | 183 +++++++++++++++++++++++++++++++++---------
 4 files changed, 294 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..ead8cdb 100644
--- a/arch/arm/kernel/psci.c
+++ b/arch/arm/kernel/psci.c
@@ -17,6 +17,7 @@
 
 #include <linux/init.h>
 #include <linux/of.h>
+#include <uapi/linux/psci.h>
 
 #include <asm/compiler.h>
 #include <asm/errno.h>
@@ -27,53 +28,44 @@
 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 +102,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 +153,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;
 
-void __init psci_init(void)
+	fn = psci_function_id[PSCI_FN_AFFINITY_INFO];
+	err = invoke_psci_fn(fn, target_affinity, lowest_affinity_level, 0);
+	return err;
+}
+
+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;
+}
 
-	pr_info("probing function IDs from device-tree\n");
+static int get_set_conduit_method(struct device_node *np)
+{
+	const char *method;
+
+	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 +190,85 @@ 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;
+}
+
+/*
+ * 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;
+
+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 +291,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..63a7685 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -18,6 +18,7 @@
 #include <linux/init.h>
 #include <linux/of.h>
 #include <linux/smp.h>
+#include <uapi/linux/psci.h>
 
 #include <asm/compiler.h>
 #include <asm/cpu_ops.h>
@@ -40,58 +41,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 +123,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 +174,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;
+}
 
-	pr_info("probing function IDs from device-tree\n");
+static int get_set_conduit_method(struct device_node *np)
+{
+	const char *method;
+
+	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 +211,85 @@ 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;
+}
+
+/*
+ * 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;
+
+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 +312,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

[PATCH v6 1/3] PSCI: Add initial support for PSCIv0.2 functions

[Anup Patel]

Hi Ashwin,

Please include system reboot & shutdown implementation
in this patch using PSCI v0.2 SYSTEM_OFF and
SYSTEM_RESET functions.

Regards,
Anup

[PATCH v6 1/3] PSCI: Add initial support for PSCIv0.2 functions

[Ashwin Chaugule]

On 21 April 2014 02:23, Anup Patel <anup.patel@linaro.org> wrote:
> Hi Ashwin,
>
> Please include system reboot & shutdown implementation
> in this patch using PSCI v0.2 SYSTEM_OFF and
> SYSTEM_RESET functions.

hm, I had thought these relied on having the (optional) MIGRATE
function. But thats not the case.

So, how about this..

---------------------8<---------------------

diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
index ead8cdb..5a54f2f 100644
--- a/arch/arm/kernel/psci.c
+++ b/arch/arm/kernel/psci.c
@@ -17,6 +17,8 @@

 #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>
@@ -24,6 +26,7 @@
 #include <asm/opcodes-sec.h>
 #include <asm/opcodes-virt.h>
 #include <asm/psci.h>
+#include <asm/system_misc.h>

 struct psci_operations psci_ops;

@@ -196,6 +199,16 @@ static int get_set_conduit_method(struct device_node *np)
  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_off(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.
@@ -249,6 +262,10 @@ static int psci_0_2_init(struct device_node *np)
  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_off;
+
 out_put_node:
  of_node_put(np);
  return err;
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 63a7685..583b7c3 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -18,6 +18,8 @@
 #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>
@@ -25,6 +27,7 @@
 #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
@@ -217,6 +220,16 @@ static int get_set_conduit_method(struct device_node *np)
  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_off(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.
@@ -270,6 +283,10 @@ static int psci_0_2_init(struct device_node *np)
  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_off;
+
 out_put_node:
  of_node_put(np);
  return err;

[PATCH v6 1/3] PSCI: Add initial support for PSCIv0.2 functions

[Anup Patel]

On Tue, Apr 22, 2014 at 11:02 PM, Ashwin Chaugule
<ashwin.chaugule@linaro.org> wrote:
> On 21 April 2014 02:23, Anup Patel <anup.patel@linaro.org> wrote:
>> Hi Ashwin,
>>
>> Please include system reboot & shutdown implementation
>> in this patch using PSCI v0.2 SYSTEM_OFF and
>> SYSTEM_RESET functions.
>
> hm, I had thought these relied on having the (optional) MIGRATE
> function. But thats not the case.
>
> So, how about this..
>
> ---------------------8<---------------------
>
> diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
> index ead8cdb..5a54f2f 100644
> --- a/arch/arm/kernel/psci.c
> +++ b/arch/arm/kernel/psci.c
> @@ -17,6 +17,8 @@
>
>  #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>
> @@ -24,6 +26,7 @@
>  #include <asm/opcodes-sec.h>
>  #include <asm/opcodes-virt.h>
>  #include <asm/psci.h>
> +#include <asm/system_misc.h>
>
>  struct psci_operations psci_ops;
>
> @@ -196,6 +199,16 @@ static int get_set_conduit_method(struct device_node *np)
>   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_off(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.
> @@ -249,6 +262,10 @@ static int psci_0_2_init(struct device_node *np)
>   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_off;
> +

Yes, I think this should work.

Thanks,
Anup

>  out_put_node:
>   of_node_put(np);
>   return err;
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index 63a7685..583b7c3 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -18,6 +18,8 @@
>  #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>
> @@ -25,6 +27,7 @@
>  #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
> @@ -217,6 +220,16 @@ static int get_set_conduit_method(struct device_node *np)
>   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_off(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.
> @@ -270,6 +283,10 @@ static int psci_0_2_init(struct device_node *np)
>   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_off;
> +
>  out_put_node:
>   of_node_put(np);
>   return err;
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v6 1/3] PSCI: Add initial support for PSCIv0.2 functions

[Rob Herring]

On Tue, Apr 22, 2014 at 12:32 PM, Ashwin Chaugule
<ashwin.chaugule@linaro.org> wrote:
> On 21 April 2014 02:23, Anup Patel <anup.patel@linaro.org> wrote:
>> Hi Ashwin,
>>
>> Please include system reboot & shutdown implementation
>> in this patch using PSCI v0.2 SYSTEM_OFF and
>> SYSTEM_RESET functions.
>
> hm, I had thought these relied on having the (optional) MIGRATE
> function. But thats not the case.
>
> So, how about this..
>
> ---------------------8<---------------------
>
> diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
> index ead8cdb..5a54f2f 100644
> --- a/arch/arm/kernel/psci.c
> +++ b/arch/arm/kernel/psci.c
> @@ -17,6 +17,8 @@
>
>  #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>
> @@ -24,6 +26,7 @@
>  #include <asm/opcodes-sec.h>
>  #include <asm/opcodes-virt.h>
>  #include <asm/psci.h>
> +#include <asm/system_misc.h>
>
>  struct psci_operations psci_ops;
>
> @@ -196,6 +199,16 @@ static int get_set_conduit_method(struct device_node *np)
>   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_off(void)

This is a bit terse. I would spell out system or use sys_poweroff
here. Otherwise,

Acked-by: Rob Herring <robh@kernel.org>

Rob

[PATCH v6 1/3] PSCI: Add initial support for PSCIv0.2 functions

[Ashwin Chaugule]

On 23 April 2014 09:16, Rob Herring <robherring2@gmail.com> wrote:
> On Tue, Apr 22, 2014 at 12:32 PM, Ashwin Chaugule
> <ashwin.chaugule@linaro.org> wrote:
>> On 21 April 2014 02:23, Anup Patel <anup.patel@linaro.org> wrote:
>>> Hi Ashwin,
>>>
>>> Please include system reboot & shutdown implementation
>>> in this patch using PSCI v0.2 SYSTEM_OFF and
>>> SYSTEM_RESET functions.
>>
>> hm, I had thought these relied on having the (optional) MIGRATE
>> function. But thats not the case.
>>
>> So, how about this..
>>
>> ---------------------8<---------------------
>>
>> diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
>> index ead8cdb..5a54f2f 100644
>> --- a/arch/arm/kernel/psci.c
>> +++ b/arch/arm/kernel/psci.c
>> @@ -17,6 +17,8 @@
>>
>>  #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>
>> @@ -24,6 +26,7 @@
>>  #include <asm/opcodes-sec.h>
>>  #include <asm/opcodes-virt.h>
>>  #include <asm/psci.h>
>> +#include <asm/system_misc.h>
>>
>>  struct psci_operations psci_ops;
>>
>> @@ -196,6 +199,16 @@ static int get_set_conduit_method(struct device_node *np)
>>   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_off(void)
>
> This is a bit terse. I would spell out system or use sys_poweroff
> here. Otherwise,
>
> Acked-by: Rob Herring <robh@kernel.org>
>

Thanks Rob. Are you okay with adding this Acked-by for this complete
patch [1/3]?

> Rob

[PATCH v6 1/3] PSCI: Add initial support for PSCIv0.2 functions

[Rob Herring]

On Wed, Apr 23, 2014 at 9:19 AM, Ashwin Chaugule
<ashwin.chaugule@linaro.org> wrote:
> On 23 April 2014 09:16, Rob Herring <robherring2@gmail.com> wrote:
>> On Tue, Apr 22, 2014 at 12:32 PM, Ashwin Chaugule
>> <ashwin.chaugule@linaro.org> wrote:
>>> On 21 April 2014 02:23, Anup Patel <anup.patel@linaro.org> wrote:
>>>> Hi Ashwin,
>>>>
>>>> Please include system reboot & shutdown implementation
>>>> in this patch using PSCI v0.2 SYSTEM_OFF and
>>>> SYSTEM_RESET functions.
>>>
>>> hm, I had thought these relied on having the (optional) MIGRATE
>>> function. But thats not the case.
>>>
>>> So, how about this..
>>>
>>> ---------------------8<---------------------
>>>
>>> diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
>>> index ead8cdb..5a54f2f 100644
>>> --- a/arch/arm/kernel/psci.c
>>> +++ b/arch/arm/kernel/psci.c
>>> @@ -17,6 +17,8 @@
>>>
>>>  #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>
>>> @@ -24,6 +26,7 @@
>>>  #include <asm/opcodes-sec.h>
>>>  #include <asm/opcodes-virt.h>
>>>  #include <asm/psci.h>
>>> +#include <asm/system_misc.h>
>>>
>>>  struct psci_operations psci_ops;
>>>
>>> @@ -196,6 +199,16 @@ static int get_set_conduit_method(struct device_node *np)
>>>   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_off(void)
>>
>> This is a bit terse. I would spell out system or use sys_poweroff
>> here. Otherwise,
>>
>> Acked-by: Rob Herring <robh@kernel.org>
>>
>
> Thanks Rob. Are you okay with adding this Acked-by for this complete
> patch [1/3]?

Yes. In fact, you can add Reviewed-by.

Rob

[PATCH v6 2/3] Documentation: devicetree: Add new binding for PSCIv0.2

[Ashwin Chaugule]

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

[PATCH v6 3/3] ARM: Check if a CPU has gone offline

[Ashwin Chaugule]

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 | 21 +++++++++++++++++++++
 include/uapi/linux/psci.h  |  5 +++++
 2 files changed, 26 insertions(+)

diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
index 570a48c..c6f1420 100644
--- a/arch/arm/kernel/psci_smp.c
+++ b/arch/arm/kernel/psci_smp.c
@@ -16,6 +16,7 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <linux/of.h>
+#include <uapi/linux/psci.h>
 
 #include <asm/psci.h>
 #include <asm/smp_plat.h>
@@ -66,6 +67,25 @@ 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;
+
+	if (!psci_ops.affinity_info)
+		return 1;
+
+	err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
+
+	if (err != PSCI_AFFINITY_INFO_RET_OFF) {
+		pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
+				cpu, err);
+		/* Make platform_cpu_kill() fail. */
+		return 0;
+	}
+	return 1;
+}
+
 #endif
 
 bool __init psci_smp_available(void)
@@ -78,5 +98,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
 };
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index a4136c3..857209b 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -74,4 +74,9 @@
 #define PSCI_RET_NOT_PRESENT			-7
 #define PSCI_RET_DISABLED			-8
 
+/* Return values from the PSCI_ID_AFFINITY_INFO Function. */
+#define PSCI_AFFINITY_INFO_RET_ON			0
+#define PSCI_AFFINITY_INFO_RET_OFF			1
+#define PSCI_AFFINITY_INFO_RET_ON_PENDING	2
+
 #endif /* _UAPI_LINUX_PSCI_H */
-- 
1.8.3.2

[PATCH v6 3/3] ARM: Check if a CPU has gone offline

[Mark Rutland]

On Thu, Apr 17, 2014 at 08:15:46PM +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 | 21 +++++++++++++++++++++
>  include/uapi/linux/psci.h  |  5 +++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
> index 570a48c..c6f1420 100644
> --- a/arch/arm/kernel/psci_smp.c
> +++ b/arch/arm/kernel/psci_smp.c
> @@ -16,6 +16,7 @@
>  #include <linux/init.h>
>  #include <linux/smp.h>
>  #include <linux/of.h>
> +#include <uapi/linux/psci.h>
>  
>  #include <asm/psci.h>
>  #include <asm/smp_plat.h>
> @@ -66,6 +67,25 @@ 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;
> +
> +	if (!psci_ops.affinity_info)
> +		return 1;
> +
> +	err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
> +
> +	if (err != PSCI_AFFINITY_INFO_RET_OFF) {
> +		pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
> +				cpu, err);
> +		/* Make platform_cpu_kill() fail. */
> +		return 0;
> +	}

We can race with the dying CPU here -- if we call AFFINITY_INFO before
the dying cpu is sufficiently far through its CPU_OFF call it won't
register as OFF. 

Could we poll here instead (with a reasonable limit on the number of
iterations)? That would enable us to not spuriously declare a CPU to be
dead when it happened to take slightly longer than we expect to turn
off.

Otherwise, this looks good. Thanks for implementing this :)

Cheers,
Mark.

[PATCH v6 3/3] ARM: Check if a CPU has gone offline

[Ashwin Chaugule]

Hi Mark,


On 17 April 2014 15:50, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Apr 17, 2014 at 08:15:46PM +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 | 21 +++++++++++++++++++++
>>  include/uapi/linux/psci.h  |  5 +++++
>>  2 files changed, 26 insertions(+)
>>
>> diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
>> index 570a48c..c6f1420 100644
>> --- a/arch/arm/kernel/psci_smp.c
>> +++ b/arch/arm/kernel/psci_smp.c
>> @@ -16,6 +16,7 @@
>>  #include <linux/init.h>
>>  #include <linux/smp.h>
>>  #include <linux/of.h>
>> +#include <uapi/linux/psci.h>
>>
>>  #include <asm/psci.h>
>>  #include <asm/smp_plat.h>
>> @@ -66,6 +67,25 @@ 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;
>> +
>> +     if (!psci_ops.affinity_info)
>> +             return 1;
>> +
>> +     err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
>> +
>> +     if (err != PSCI_AFFINITY_INFO_RET_OFF) {
>> +             pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
>> +                             cpu, err);
>> +             /* Make platform_cpu_kill() fail. */
>> +             return 0;
>> +     }
>
> We can race with the dying CPU here -- if we call AFFINITY_INFO before
> the dying cpu is sufficiently far through its CPU_OFF call it won't
> register as OFF.
>
> Could we poll here instead (with a reasonable limit on the number of
> iterations)? That would enable us to not spuriously declare a CPU to be
> dead when it happened to take slightly longer than we expect to turn
> off.

True. How about something like this?

 int __ref psci_cpu_kill(unsigned int cpu)
 {
-       int err;
+       int err, retries;

        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 retry a couple of times.
+        */
+retry:
        err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);

        if (err != PSCI_AFFINITY_INFO_RET_OFF) {
+               if (++retries < 3) {
+                       pr_info("Retrying check for CPU kill: %d\n", retries);
+                       goto retry;
+               }
                pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
                                cpu, err);
                /* Make platform_cpu_kill() fail. */



Cheers,
Ashwin

[PATCH v6 3/3] ARM: Check if a CPU has gone offline

[Ashwin Chaugule]

On 18 April 2014 11:21, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
> Hi Mark,
>
>
> On 17 April 2014 15:50, Mark Rutland <mark.rutland@arm.com> wrote:
>> On Thu, Apr 17, 2014 at 08:15:46PM +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 | 21 +++++++++++++++++++++
>>>  include/uapi/linux/psci.h  |  5 +++++
>>>  2 files changed, 26 insertions(+)
>>>
>>> diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
>>> index 570a48c..c6f1420 100644
>>> --- a/arch/arm/kernel/psci_smp.c
>>> +++ b/arch/arm/kernel/psci_smp.c
>>> @@ -16,6 +16,7 @@
>>>  #include <linux/init.h>
>>>  #include <linux/smp.h>
>>>  #include <linux/of.h>
>>> +#include <uapi/linux/psci.h>
>>>
>>>  #include <asm/psci.h>
>>>  #include <asm/smp_plat.h>
>>> @@ -66,6 +67,25 @@ 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;
>>> +
>>> +     if (!psci_ops.affinity_info)
>>> +             return 1;
>>> +
>>> +     err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
>>> +
>>> +     if (err != PSCI_AFFINITY_INFO_RET_OFF) {
>>> +             pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
>>> +                             cpu, err);
>>> +             /* Make platform_cpu_kill() fail. */
>>> +             return 0;
>>> +     }
>>
>> We can race with the dying CPU here -- if we call AFFINITY_INFO before
>> the dying cpu is sufficiently far through its CPU_OFF call it won't
>> register as OFF.
>>
>> Could we poll here instead (with a reasonable limit on the number of
>> iterations)? That would enable us to not spuriously declare a CPU to be
>> dead when it happened to take slightly longer than we expect to turn
>> off.
>
> True. How about something like this?
>
>  int __ref psci_cpu_kill(unsigned int cpu)
>  {
> -       int err;
> +       int err, retries;
>
>         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 retry a couple of times.
> +        */
> +retry:
>         err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
>
>         if (err != PSCI_AFFINITY_INFO_RET_OFF) {
> +               if (++retries < 3) {
> +                       pr_info("Retrying check for CPU kill: %d\n", retries);
> +                       goto retry;
> +               }
>                 pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
>                                 cpu, err);
>                 /* Make platform_cpu_kill() fail. */
>
>
>


Hi Rob, I've already got your Reviewed-by on this patch without this
"retry" thing. Are you okay with this as well? I can then roll it up
in one patch.

Cheers,
Ashwin

[PATCH v6 3/3] ARM: Check if a CPU has gone offline

[Rob Herring]

On Thu, Apr 24, 2014 at 9:33 AM, Ashwin Chaugule
<ashwin.chaugule@linaro.org> wrote:
> On 18 April 2014 11:21, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
>> Hi Mark,
>>
>>
>> On 17 April 2014 15:50, Mark Rutland <mark.rutland@arm.com> wrote:
>>> On Thu, Apr 17, 2014 at 08:15:46PM +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.

[...]

>>> We can race with the dying CPU here -- if we call AFFINITY_INFO before
>>> the dying cpu is sufficiently far through its CPU_OFF call it won't
>>> register as OFF.
>>>
>>> Could we poll here instead (with a reasonable limit on the number of
>>> iterations)? That would enable us to not spuriously declare a CPU to be
>>> dead when it happened to take slightly longer than we expect to turn
>>> off.
>>
>> True. How about something like this?
>>
>>  int __ref psci_cpu_kill(unsigned int cpu)
>>  {
>> -       int err;
>> +       int err, retries;
>>
>>         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 retry a couple of times.
>> +        */
>> +retry:
>>         err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);
>>
>>         if (err != PSCI_AFFINITY_INFO_RET_OFF) {
>> +               if (++retries < 3) {
>> +                       pr_info("Retrying check for CPU kill: %d\n", retries);
>> +                       goto retry;
>> +               }
>>                 pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
>>                                 cpu, err);
>>                 /* Make platform_cpu_kill() fail. */
>>
>>
>>
>
>
> Hi Rob, I've already got your Reviewed-by on this patch without this
> "retry" thing. Are you okay with this as well? I can then roll it up
> in one patch.

Yes. My only comment is I would perhaps add a sleep (or delay if this
context cannot sleep) on the retry. I'm not sure what I reasonable
time would be, but at least then you are waiting a defined amount of
time versus how long it takes this code to execute.

Rob

[PATCH v6 3/3] ARM: Check if a CPU has gone offline

[Ashwin Chaugule]

On 24 April 2014 11:11, Rob Herring <robherring2@gmail.com> wrote:
>>
>> Hi Rob, I've already got your Reviewed-by on this patch without this
>> "retry" thing. Are you okay with this as well? I can then roll it up
>> in one patch.
>
> Yes. My only comment is I would perhaps add a sleep (or delay if this
> context cannot sleep) on the retry. I'm not sure what I reasonable
> time would be, but at least then you are waiting a defined amount of
> time versus how long it takes this code to execute.


Yea, its tricky to get the delay right. Does this look better? The
caller has a wait_for_completion (which waits for 5secs max) in it, so
this context should be sleepable.

"100 msecs ought to be enough for anyone". ;)

diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
index 3a83dcf..94485e0 100644
--- a/arch/arm/kernel/psci_smp.c
+++ b/arch/arm/kernel/psci_smp.c
@@ -16,6 +16,7 @@
 #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>
@@ -70,22 +71,23 @@ void __ref psci_cpu_die(unsigned int cpu)

 int __ref psci_cpu_kill(unsigned int cpu)
 {
- int err, retries;
+ int err, retry = 0;

  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 retry a couple of times.
+ * while it is dying. So, try once more if it fails.
  */
-retry:
+retry_once:
  err = psci_ops.affinity_info(cpu_logical_map(cpu), 0);

  if (err != PSCI_AFFINITY_INFO_RET_OFF) {
- if (++retries < 3) {
- pr_info("Retrying check for CPU kill: %d\n", retries);
- goto retry;
+ if (!retry++) {
+ msleep(100);
+ pr_info("Retrying once more to check for CPU kill\n");
+ goto retry_once;
  }
  pr_err("psci: Cannot kill CPU:%d, psci ret val: %d\n",
  cpu, err);