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(-)
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
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 >; + + ... + };
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 };
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
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@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Regards, Anup
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.
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
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); }
/*
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 theCPU 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
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.
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
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:
- 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.
- 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
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:
- 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.
- 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.
-
Anup Patel -
Ashwin Chaugule -
Catalin Marinas -
Mark Rutland