* [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support
@ 2015-07-08 17:16 Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 1/5] drivers: firmware: psci: add INVALID_ADDRESS return value Lorenzo Pieralisi
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-08 17:16 UTC (permalink / raw)
To: linux-arm-kernel
This series is a v2 of a previous posting:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/347352.html
v1 -> v2:
- rebased against v4.2-rc1
- added version to 1.0 specific macros
The PSCI v1.0 specification[1] introduces a brand new set of features
(ie OS initiated mode and system suspend being the most notable ones)
and provides updates to the PSCI 0.2 specification, keeping backward
compatibility.
PSCI v1.0 applies minor changes to function return codes and function
behaviour (ie AFFINITY_INFO requirements, INVALID_ADDRESS return value)
and introduces a new power_state parameter format (extended stateid)
that is probeable with the newly introduced PSCI_FEATURES call.
This series upgrades the current kernel PSCI layer implementation with
a set of patches that make the kernel PSCI v1.0 compliant.
The series augments the PSCI firmware layer with a hook to retrieve
the features for a specific PSCI function (ie based on the PSCI_FEATURES
call) and uses it to detect the power_state parameter format, updating
the power_state parameter handling functions accordingly.
In order to prevent firmware interfaces mismatch, a new compatible
string is added to the DT bindings to characterize a 1.0 compliant
firmware interface.
The series is built on top of M.Rutland's patch series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/353795.html
Tested on a Juno board.
[1] http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
Lorenzo Pieralisi (5):
drivers: firmware: psci: add INVALID_ADDRESS return value
drivers: firmware: psci: move power_state handling to generic code
drivers: firmware: psci: add PSCI_FEATURES call
drivers: firmware: psci: add extended stateid power_state support
drivers: firmware: psci: add PSCI v1.0 DT bindings
Documentation/devicetree/bindings/arm/psci.txt | 6 +++
arch/arm64/kernel/psci.c | 14 -------
drivers/firmware/psci.c | 53 ++++++++++++++++++++++++++
include/linux/psci.h | 2 +
include/uapi/linux/psci.h | 15 ++++++++
5 files changed, 76 insertions(+), 14 deletions(-)
--
2.2.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/5] drivers: firmware: psci: add INVALID_ADDRESS return value
2015-07-08 17:16 [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
@ 2015-07-08 17:16 ` Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 2/5] drivers: firmware: psci: move power_state handling to generic code Lorenzo Pieralisi
` (4 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-08 17:16 UTC (permalink / raw)
To: linux-arm-kernel
PSCI 1.0 introduces the INVALID_ADDRESS return value for functions
that take an address as input parameter (eg CPU_SUSPEND).
This patch adds INVALID_ADDRESS return value to kernel code and
updates the PSCI to linux error conversion to take it into account.
The kernel error value associated to INVALID_ADDRESS is set to
the error returned when the PSCI error code is INVALID_PARAMETERS
to comply with current call sites expected return value, given
that the kernel at present has no use for the additional error
information reported.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
drivers/firmware/psci.c | 1 +
include/uapi/linux/psci.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 81b70da..62d9bc1 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -78,6 +78,7 @@ static int psci_to_linux_errno(int errno)
case PSCI_RET_NOT_SUPPORTED:
return -EOPNOTSUPP;
case PSCI_RET_INVALID_PARAMS:
+ case PSCI_RET_INVALID_ADDRESS:
return -EINVAL;
case PSCI_RET_DENIED:
return -EPERM;
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 310d83e..64469e6 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -86,5 +86,6 @@
#define PSCI_RET_INTERNAL_FAILURE -6
#define PSCI_RET_NOT_PRESENT -7
#define PSCI_RET_DISABLED -8
+#define PSCI_RET_INVALID_ADDRESS -9
#endif /* _UAPI_LINUX_PSCI_H */
--
2.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/5] drivers: firmware: psci: move power_state handling to generic code
2015-07-08 17:16 [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 1/5] drivers: firmware: psci: add INVALID_ADDRESS return value Lorenzo Pieralisi
@ 2015-07-08 17:16 ` Lorenzo Pieralisi
2015-07-09 13:39 ` Catalin Marinas
2015-07-08 17:16 ` [PATCH v2 3/5] drivers: firmware: psci: add PSCI_FEATURES call Lorenzo Pieralisi
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-08 17:16 UTC (permalink / raw)
To: linux-arm-kernel
Functions implemented on arm64 to check if a power_state parameter
is valid and if the power_state implies context loss are not
arm64 specific and should be moved to generic code so that they
can be reused on arm systems too.
This patch moves the functions handling the power_state parameter
to generic PSCI firmware layer code.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
arch/arm64/kernel/psci.c | 14 --------------
drivers/firmware/psci.c | 15 +++++++++++++++
include/linux/psci.h | 2 ++
3 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 51fd15a..4be5972 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -30,20 +30,6 @@
#include <asm/smp_plat.h>
#include <asm/suspend.h>
-static bool psci_power_state_loses_context(u32 state)
-{
- return state & PSCI_0_2_POWER_STATE_TYPE_MASK;
-}
-
-static bool psci_power_state_is_valid(u32 state)
-{
- const u32 valid_mask = PSCI_0_2_POWER_STATE_ID_MASK |
- PSCI_0_2_POWER_STATE_TYPE_MASK |
- PSCI_0_2_POWER_STATE_AFFL_MASK;
-
- return !(state & ~valid_mask);
-}
-
static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
static int __maybe_unused cpu_psci_cpu_init_idle(unsigned int cpu)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 62d9bc1..6bd013d 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -70,6 +70,21 @@ enum psci_function {
static u32 psci_function_id[PSCI_FN_MAX];
+#define PSCI_0_2_POWER_STATE_MASK \
+ (PSCI_0_2_POWER_STATE_ID_MASK | \
+ PSCI_0_2_POWER_STATE_TYPE_MASK | \
+ PSCI_0_2_POWER_STATE_AFFL_MASK)
+
+bool psci_power_state_loses_context(u32 state)
+{
+ return state & PSCI_0_2_POWER_STATE_TYPE_MASK;
+}
+
+bool psci_power_state_is_valid(u32 state)
+{
+ return !(state & ~PSCI_0_2_POWER_STATE_MASK);
+}
+
static int psci_to_linux_errno(int errno)
{
switch (errno) {
diff --git a/include/linux/psci.h b/include/linux/psci.h
index a682fcc..12c4865 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -21,6 +21,8 @@
#define PSCI_POWER_STATE_TYPE_POWER_DOWN 1
bool psci_tos_resident_on(int cpu);
+bool psci_power_state_loses_context(u32 state);
+bool psci_power_state_is_valid(u32 state);
struct psci_operations {
int (*cpu_suspend)(u32 state, unsigned long entry_point);
--
2.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 3/5] drivers: firmware: psci: add PSCI_FEATURES call
2015-07-08 17:16 [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 1/5] drivers: firmware: psci: add INVALID_ADDRESS return value Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 2/5] drivers: firmware: psci: move power_state handling to generic code Lorenzo Pieralisi
@ 2015-07-08 17:16 ` Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support Lorenzo Pieralisi
` (2 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-08 17:16 UTC (permalink / raw)
To: linux-arm-kernel
PSCI v1.0 introduces a PSCI_FEATURES call that allows to probe for
features related to a specific function identifier.
This patch adds PSCI_FEATURES support to the PSCI firmware layer.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
drivers/firmware/psci.c | 6 ++++++
include/uapi/linux/psci.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 6bd013d..24ef3a8 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -197,6 +197,12 @@ static void psci_sys_poweroff(void)
invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
}
+static int __init psci_features(u32 psci_func_id)
+{
+ return invoke_psci_fn(PSCI_1_0_FN_PSCI_FEATURES,
+ psci_func_id, 0, 0);
+}
+
/*
* Detect the presence of a resident Trusted OS which may cause CPU_OFF to
* return DENIED (which would be fatal).
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 64469e6..187b828d 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -46,6 +46,8 @@
#define PSCI_0_2_FN64_MIGRATE PSCI_0_2_FN64(5)
#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU PSCI_0_2_FN64(7)
+#define PSCI_1_0_FN_PSCI_FEATURES PSCI_0_2_FN(10)
+
/* PSCI v0.2 power state encoding for CPU_SUSPEND function */
#define PSCI_0_2_POWER_STATE_ID_MASK 0xffff
#define PSCI_0_2_POWER_STATE_ID_SHIFT 0
--
2.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support
2015-07-08 17:16 [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
` (2 preceding siblings ...)
2015-07-08 17:16 ` [PATCH v2 3/5] drivers: firmware: psci: add PSCI_FEATURES call Lorenzo Pieralisi
@ 2015-07-08 17:16 ` Lorenzo Pieralisi
2015-10-22 22:07 ` Kevin Hilman
2015-07-08 17:16 ` [PATCH v2 5/5] drivers: firmware: psci: add PSCI v1.0 DT bindings Lorenzo Pieralisi
2015-09-14 13:35 ` [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
5 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-08 17:16 UTC (permalink / raw)
To: linux-arm-kernel
PSCI v1.0 augmented the power_state parameter format specification
(extended stateid) and introduced a way to probe it through the
PSCI_FEATURES interface.
This patch implements code that detects the power_state format at
run-time through the PSCI_FEATURES interface, so that the power_state
argument can be properly detected and validated in the kernel according
to the information provided through firmware.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
drivers/firmware/psci.c | 34 ++++++++++++++++++++++++++++++++--
include/uapi/linux/psci.h | 12 ++++++++++++
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 24ef3a8..bd2ba5b 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -75,14 +75,34 @@ static u32 psci_function_id[PSCI_FN_MAX];
PSCI_0_2_POWER_STATE_TYPE_MASK | \
PSCI_0_2_POWER_STATE_AFFL_MASK)
+#define PSCI_1_0_EXT_POWER_STATE_MASK \
+ (PSCI_1_0_EXT_POWER_STATE_ID_MASK | \
+ PSCI_1_0_EXT_POWER_STATE_TYPE_MASK)
+
+static u32 psci_cpu_suspend_feature;
+
+static inline bool psci_has_ext_power_state(void)
+{
+ return psci_cpu_suspend_feature &
+ PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK;
+}
+
bool psci_power_state_loses_context(u32 state)
{
- return state & PSCI_0_2_POWER_STATE_TYPE_MASK;
+ const u32 mask = psci_has_ext_power_state() ?
+ PSCI_1_0_EXT_POWER_STATE_TYPE_MASK :
+ PSCI_0_2_POWER_STATE_TYPE_MASK;
+
+ return state & mask;
}
bool psci_power_state_is_valid(u32 state)
{
- return !(state & ~PSCI_0_2_POWER_STATE_MASK);
+ const u32 valid_mask = psci_has_ext_power_state() ?
+ PSCI_1_0_EXT_POWER_STATE_MASK :
+ PSCI_0_2_POWER_STATE_MASK;
+
+ return !(state & ~valid_mask);
}
static int psci_to_linux_errno(int errno)
@@ -203,6 +223,14 @@ static int __init psci_features(u32 psci_func_id)
psci_func_id, 0, 0);
}
+static void __init psci_init_cpu_suspend(void)
+{
+ int feature = psci_features(psci_function_id[PSCI_FN_CPU_SUSPEND]);
+
+ if (feature != PSCI_RET_NOT_SUPPORTED)
+ psci_cpu_suspend_feature = feature;
+}
+
/*
* Detect the presence of a resident Trusted OS which may cause CPU_OFF to
* return DENIED (which would be fatal).
@@ -287,6 +315,8 @@ static int __init psci_probe(void)
psci_init_migrate();
+ psci_init_cpu_suspend();
+
return 0;
}
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 187b828d..0a9485f 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -58,6 +58,13 @@
#define PSCI_0_2_POWER_STATE_AFFL_MASK \
(0x3 << PSCI_0_2_POWER_STATE_AFFL_SHIFT)
+/* PSCI extended power state encoding for CPU_SUSPEND function */
+#define PSCI_1_0_EXT_POWER_STATE_ID_MASK 0xfffffff
+#define PSCI_1_0_EXT_POWER_STATE_ID_SHIFT 0
+#define PSCI_1_0_EXT_POWER_STATE_TYPE_SHIFT 30
+#define PSCI_1_0_EXT_POWER_STATE_TYPE_MASK \
+ (0x1 << PSCI_1_0_EXT_POWER_STATE_TYPE_SHIFT)
+
/* PSCI v0.2 affinity level state returned by AFFINITY_INFO */
#define PSCI_0_2_AFFINITY_LEVEL_ON 0
#define PSCI_0_2_AFFINITY_LEVEL_OFF 1
@@ -78,6 +85,11 @@
#define PSCI_VERSION_MINOR(ver) \
((ver) & PSCI_VERSION_MINOR_MASK)
+/* PSCI features decoding (>=1.0) */
+#define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT 1
+#define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK \
+ (0x1 << PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT)
+
/* PSCI return values (inclusive of all PSCI versions) */
#define PSCI_RET_SUCCESS 0
#define PSCI_RET_NOT_SUPPORTED -1
--
2.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 5/5] drivers: firmware: psci: add PSCI v1.0 DT bindings
2015-07-08 17:16 [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
` (3 preceding siblings ...)
2015-07-08 17:16 ` [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support Lorenzo Pieralisi
@ 2015-07-08 17:16 ` Lorenzo Pieralisi
2015-10-05 11:48 ` Andre Przywara
2015-09-14 13:35 ` [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
5 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-08 17:16 UTC (permalink / raw)
To: linux-arm-kernel
PSCI 1.0 is designed to be fully compliant to the PSCI 0.2
specification, with minor differences that are described in the
PSCI specification.
In particular, PSCI v1.0 augments the specification with a new
power_state format (extended stateid - probeable through the
PSCI_FEATURES call), changes some function return codes and
functions usage requirements wrt PSCI 0.2. These changes mean
that 1.0 vs 0.2 compliancy should be enforced through a DT
compatible string that allows firmware to specify 1.0 only
compliancy so that older kernels are prevented from using
PSCI 1.0 FW implementations in a non-compatible way (eg by
calling a 1.0 FW implementation and expecting 0.2 behaviour).
This patch adds PSCI 1.0 DT bindings and related compatible
string.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
Documentation/devicetree/bindings/arm/psci.txt | 6 ++++++
drivers/firmware/psci.c | 1 +
2 files changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt
index 5aa40ed..a9adab8 100644
--- a/Documentation/devicetree/bindings/arm/psci.txt
+++ b/Documentation/devicetree/bindings/arm/psci.txt
@@ -31,6 +31,10 @@ Main node required properties:
support, but are permitted to be present for compatibility with
existing software when "arm,psci" is later in the compatible list.
+ * "arm,psci-1.0" : for implementations complying to PSCI 1.0. PSCI 1.0 is
+ backward compatible with PSCI 0.2 with minor specification updates,
+ as defined in the PSCI specification[2].
+
- method : The method of calling the PSCI firmware. Permitted
values are:
@@ -100,3 +104,5 @@ Case 3: PSCI v0.2 and PSCI v0.1.
[1] Kernel documentation - ARM idle states bindings
Documentation/devicetree/bindings/arm/idle-states.txt
+[2] Power State Coordination Interface (PSCI) specification
+ http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index bd2ba5b..5b544d7 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -392,6 +392,7 @@ out_put_node:
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},
+ { .compatible = "arm,psci-1.0", .data = psci_0_2_init},
{},
};
--
2.2.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/5] drivers: firmware: psci: move power_state handling to generic code
2015-07-08 17:16 ` [PATCH v2 2/5] drivers: firmware: psci: move power_state handling to generic code Lorenzo Pieralisi
@ 2015-07-09 13:39 ` Catalin Marinas
0 siblings, 0 replies; 19+ messages in thread
From: Catalin Marinas @ 2015-07-09 13:39 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 08, 2015 at 06:16:48PM +0100, Lorenzo Pieralisi wrote:
> Functions implemented on arm64 to check if a power_state parameter
> is valid and if the power_state implies context loss are not
> arm64 specific and should be moved to generic code so that they
> can be reused on arm systems too.
>
> This patch moves the functions handling the power_state parameter
> to generic PSCI firmware layer code.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support
2015-07-08 17:16 [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
` (4 preceding siblings ...)
2015-07-08 17:16 ` [PATCH v2 5/5] drivers: firmware: psci: add PSCI v1.0 DT bindings Lorenzo Pieralisi
@ 2015-09-14 13:35 ` Lorenzo Pieralisi
2015-09-15 3:23 ` Jisheng Zhang
5 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-09-14 13:35 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 08, 2015 at 06:16:46PM +0100, Lorenzo Pieralisi wrote:
> This series is a v2 of a previous posting:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/347352.html
>
> v1 -> v2:
>
> - rebased against v4.2-rc1
> - added version to 1.0 specific macros
>
> The PSCI v1.0 specification[1] introduces a brand new set of features
> (ie OS initiated mode and system suspend being the most notable ones)
> and provides updates to the PSCI 0.2 specification, keeping backward
> compatibility.
>
> PSCI v1.0 applies minor changes to function return codes and function
> behaviour (ie AFFINITY_INFO requirements, INVALID_ADDRESS return value)
> and introduces a new power_state parameter format (extended stateid)
> that is probeable with the newly introduced PSCI_FEATURES call.
>
> This series upgrades the current kernel PSCI layer implementation with
> a set of patches that make the kernel PSCI v1.0 compliant.
>
> The series augments the PSCI firmware layer with a hook to retrieve
> the features for a specific PSCI function (ie based on the PSCI_FEATURES
> call) and uses it to detect the power_state parameter format, updating
> the power_state parameter handling functions accordingly.
>
> In order to prevent firmware interfaces mismatch, a new compatible
> string is added to the DT bindings to characterize a 1.0 compliant
> firmware interface.
>
> The series is built on top of M.Rutland's patch series:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/353795.html
>
> Tested on a Juno board.
Rebased against 4.3-rc1 (that includes the patch series above), if there
are no objections I will queue this series for v4.4.
Thanks,
Lorenzo
>
> [1] http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
>
> Lorenzo Pieralisi (5):
> drivers: firmware: psci: add INVALID_ADDRESS return value
> drivers: firmware: psci: move power_state handling to generic code
> drivers: firmware: psci: add PSCI_FEATURES call
> drivers: firmware: psci: add extended stateid power_state support
> drivers: firmware: psci: add PSCI v1.0 DT bindings
>
> Documentation/devicetree/bindings/arm/psci.txt | 6 +++
> arch/arm64/kernel/psci.c | 14 -------
> drivers/firmware/psci.c | 53 ++++++++++++++++++++++++++
> include/linux/psci.h | 2 +
> include/uapi/linux/psci.h | 15 ++++++++
> 5 files changed, 76 insertions(+), 14 deletions(-)
>
> --
> 2.2.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support
2015-09-14 13:35 ` [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
@ 2015-09-15 3:23 ` Jisheng Zhang
0 siblings, 0 replies; 19+ messages in thread
From: Jisheng Zhang @ 2015-09-15 3:23 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 14 Sep 2015 14:35:10 +0100
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> On Wed, Jul 08, 2015 at 06:16:46PM +0100, Lorenzo Pieralisi wrote:
> > This series is a v2 of a previous posting:
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/347352.html
> >
> > v1 -> v2:
> >
> > - rebased against v4.2-rc1
> > - added version to 1.0 specific macros
> >
> > The PSCI v1.0 specification[1] introduces a brand new set of features
> > (ie OS initiated mode and system suspend being the most notable ones)
> > and provides updates to the PSCI 0.2 specification, keeping backward
> > compatibility.
> >
> > PSCI v1.0 applies minor changes to function return codes and function
> > behaviour (ie AFFINITY_INFO requirements, INVALID_ADDRESS return value)
> > and introduces a new power_state parameter format (extended stateid)
> > that is probeable with the newly introduced PSCI_FEATURES call.
> >
> > This series upgrades the current kernel PSCI layer implementation with
> > a set of patches that make the kernel PSCI v1.0 compliant.
> >
> > The series augments the PSCI firmware layer with a hook to retrieve
> > the features for a specific PSCI function (ie based on the PSCI_FEATURES
> > call) and uses it to detect the power_state parameter format, updating
> > the power_state parameter handling functions accordingly.
> >
> > In order to prevent firmware interfaces mismatch, a new compatible
> > string is added to the DT bindings to characterize a 1.0 compliant
> > firmware interface.
> >
> > The series is built on top of M.Rutland's patch series:
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/353795.html
> >
> > Tested on a Juno board.
>
> Rebased against 4.3-rc1 (that includes the patch series above), if there
> are no objections I will queue this series for v4.4.
Tested on Marvell BG4CT DMP board. So feel free to add
Tested-by: Jisheng Zhang <jszhang@marvell.com>
Thanks
>
> Thanks,
> Lorenzo
>
> >
> > [1] http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
> >
> > Lorenzo Pieralisi (5):
> > drivers: firmware: psci: add INVALID_ADDRESS return value
> > drivers: firmware: psci: move power_state handling to generic code
> > drivers: firmware: psci: add PSCI_FEATURES call
> > drivers: firmware: psci: add extended stateid power_state support
> > drivers: firmware: psci: add PSCI v1.0 DT bindings
> >
> > Documentation/devicetree/bindings/arm/psci.txt | 6 +++
> > arch/arm64/kernel/psci.c | 14 -------
> > drivers/firmware/psci.c | 53 ++++++++++++++++++++++++++
> > include/linux/psci.h | 2 +
> > include/uapi/linux/psci.h | 15 ++++++++
> > 5 files changed, 76 insertions(+), 14 deletions(-)
> >
> > --
> > 2.2.1
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 5/5] drivers: firmware: psci: add PSCI v1.0 DT bindings
2015-07-08 17:16 ` [PATCH v2 5/5] drivers: firmware: psci: add PSCI v1.0 DT bindings Lorenzo Pieralisi
@ 2015-10-05 11:48 ` Andre Przywara
2015-10-05 12:06 ` Mark Rutland
2015-10-05 12:11 ` Lorenzo Pieralisi
0 siblings, 2 replies; 19+ messages in thread
From: Andre Przywara @ 2015-10-05 11:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lorenzo,
sorry for this late reply, but this came up recently during an IRC
discussion:
On 08/07/15 18:16, Lorenzo Pieralisi wrote:
> PSCI 1.0 is designed to be fully compliant to the PSCI 0.2
> specification, with minor differences that are described in the
> PSCI specification.
So if PSCI 1.0 is fully compliant to the 0.2 spec and PSCI 0.2 mandates
a version function, why do we need a new binding here?
IIRC device tree bindings are just for features that cannot be probed,
whereas the availability of PSCI 1.0 features can be safely probed by
issuing the PSCI_VERSION call and checking for bits [16:32] >= 1.
So can't we just skip this extra binding and keep the compatible string
to 0.2 for every upcoming PSCI implementation?
This should actually be the last binding we need, since availability of
specific functions can be checked as well with the PSCI_FEATURES call in
the future.
Cheers,
Andre
>
> In particular, PSCI v1.0 augments the specification with a new
> power_state format (extended stateid - probeable through the
> PSCI_FEATURES call), changes some function return codes and
> functions usage requirements wrt PSCI 0.2. These changes mean
> that 1.0 vs 0.2 compliancy should be enforced through a DT
> compatible string that allows firmware to specify 1.0 only
> compliancy so that older kernels are prevented from using
> PSCI 1.0 FW implementations in a non-compatible way (eg by
> calling a 1.0 FW implementation and expecting 0.2 behaviour).
>
> This patch adds PSCI 1.0 DT bindings and related compatible
> string.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
> Documentation/devicetree/bindings/arm/psci.txt | 6 ++++++
> drivers/firmware/psci.c | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt
> index 5aa40ed..a9adab8 100644
> --- a/Documentation/devicetree/bindings/arm/psci.txt
> +++ b/Documentation/devicetree/bindings/arm/psci.txt
> @@ -31,6 +31,10 @@ Main node required properties:
> support, but are permitted to be present for compatibility with
> existing software when "arm,psci" is later in the compatible list.
>
> + * "arm,psci-1.0" : for implementations complying to PSCI 1.0. PSCI 1.0 is
> + backward compatible with PSCI 0.2 with minor specification updates,
> + as defined in the PSCI specification[2].
> +
> - method : The method of calling the PSCI firmware. Permitted
> values are:
>
> @@ -100,3 +104,5 @@ Case 3: PSCI v0.2 and PSCI v0.1.
>
> [1] Kernel documentation - ARM idle states bindings
> Documentation/devicetree/bindings/arm/idle-states.txt
> +[2] Power State Coordination Interface (PSCI) specification
> + http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index bd2ba5b..5b544d7 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -392,6 +392,7 @@ out_put_node:
> 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},
> + { .compatible = "arm,psci-1.0", .data = psci_0_2_init},
> {},
> };
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 5/5] drivers: firmware: psci: add PSCI v1.0 DT bindings
2015-10-05 11:48 ` Andre Przywara
@ 2015-10-05 12:06 ` Mark Rutland
2015-10-05 12:11 ` Lorenzo Pieralisi
1 sibling, 0 replies; 19+ messages in thread
From: Mark Rutland @ 2015-10-05 12:06 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 05, 2015 at 12:48:09PM +0100, Andre Przywara wrote:
> Hi Lorenzo,
>
> sorry for this late reply, but this came up recently during an IRC
> discussion:
>
> On 08/07/15 18:16, Lorenzo Pieralisi wrote:
> > PSCI 1.0 is designed to be fully compliant to the PSCI 0.2
> > specification, with minor differences that are described in the
> > PSCI specification.
>
> So if PSCI 1.0 is fully compliant to the 0.2 spec and PSCI 0.2 mandates
> a version function, why do we need a new binding here?
Some possible PSCI 1.0 implementations are not PSCI 0.2 compliant (e.g.
if they implement the new state parameter format). They would list
"arm,psci-1.0" only so old OSs wouldn't explode unexpectedly trying to
use not-quite-compatible features.
> IIRC device tree bindings are just for features that cannot be probed,
> whereas the availability of PSCI 1.0 features can be safely probed by
> issuing the PSCI_VERSION call and checking for bits [16:32] >= 1.
> So can't we just skip this extra binding and keep the compatible string
> to 0.2 for every upcoming PSCI implementation?
If PSCI 0.2 is reported in the DT, we can and will probe PSCI_VERSION to
detect PSCI 1.0 support, but the existing string implies true PSCI 0.2
compatibility.
> This should actually be the last binding we need, since availability of
> specific functions can be checked as well with the PSCI_FEATURES call in
> the future.
So long as the spec doesn't break compatibility in this fashion again,
yes. I certainly hope we don't have this problem again.
Mark.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 5/5] drivers: firmware: psci: add PSCI v1.0 DT bindings
2015-10-05 11:48 ` Andre Przywara
2015-10-05 12:06 ` Mark Rutland
@ 2015-10-05 12:11 ` Lorenzo Pieralisi
1 sibling, 0 replies; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-10-05 12:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi Andre,
On Mon, Oct 05, 2015 at 12:48:09PM +0100, Andre Przywara wrote:
> Hi Lorenzo,
>
> sorry for this late reply, but this came up recently during an IRC
> discussion:
>
> On 08/07/15 18:16, Lorenzo Pieralisi wrote:
> > PSCI 1.0 is designed to be fully compliant to the PSCI 0.2
> > specification, with minor differences that are described in the
> > PSCI specification.
>
> So if PSCI 1.0 is fully compliant to the 0.2 spec and PSCI 0.2 mandates
> a version function, why do we need a new binding here?
> IIRC device tree bindings are just for features that cannot be probed,
> whereas the availability of PSCI 1.0 features can be safely probed by
> issuing the PSCI_VERSION call and checking for bits [16:32] >= 1.
> So can't we just skip this extra binding and keep the compatible string
> to 0.2 for every upcoming PSCI implementation?
> This should actually be the last binding we need, since availability of
> specific functions can be checked as well with the PSCI_FEATURES call in
> the future.
The reason is written below, and basically it is to prevent old
kernels using/matching PSCI 1.0 firmware compliant implementations,
owing to the minor 1.0 spec updates implemented in this patchset.
Thanks,
Lorenzo
> Cheers,
> Andre
>
> >
> > In particular, PSCI v1.0 augments the specification with a new
> > power_state format (extended stateid - probeable through the
> > PSCI_FEATURES call), changes some function return codes and
> > functions usage requirements wrt PSCI 0.2. These changes mean
> > that 1.0 vs 0.2 compliancy should be enforced through a DT
> > compatible string that allows firmware to specify 1.0 only
> > compliancy so that older kernels are prevented from using
> > PSCI 1.0 FW implementations in a non-compatible way (eg by
> > calling a 1.0 FW implementation and expecting 0.2 behaviour).
> >
> > This patch adds PSCI 1.0 DT bindings and related compatible
> > string.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Acked-by: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > ---
> > Documentation/devicetree/bindings/arm/psci.txt | 6 ++++++
> > drivers/firmware/psci.c | 1 +
> > 2 files changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt
> > index 5aa40ed..a9adab8 100644
> > --- a/Documentation/devicetree/bindings/arm/psci.txt
> > +++ b/Documentation/devicetree/bindings/arm/psci.txt
> > @@ -31,6 +31,10 @@ Main node required properties:
> > support, but are permitted to be present for compatibility with
> > existing software when "arm,psci" is later in the compatible list.
> >
> > + * "arm,psci-1.0" : for implementations complying to PSCI 1.0. PSCI 1.0 is
> > + backward compatible with PSCI 0.2 with minor specification updates,
> > + as defined in the PSCI specification[2].
> > +
> > - method : The method of calling the PSCI firmware. Permitted
> > values are:
> >
> > @@ -100,3 +104,5 @@ Case 3: PSCI v0.2 and PSCI v0.1.
> >
> > [1] Kernel documentation - ARM idle states bindings
> > Documentation/devicetree/bindings/arm/idle-states.txt
> > +[2] Power State Coordination Interface (PSCI) specification
> > + http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf
> > diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> > index bd2ba5b..5b544d7 100644
> > --- a/drivers/firmware/psci.c
> > +++ b/drivers/firmware/psci.c
> > @@ -392,6 +392,7 @@ out_put_node:
> > 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},
> > + { .compatible = "arm,psci-1.0", .data = psci_0_2_init},
> > {},
> > };
> >
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support
2015-07-08 17:16 ` [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support Lorenzo Pieralisi
@ 2015-10-22 22:07 ` Kevin Hilman
2015-10-23 10:23 ` Arnd Bergmann
0 siblings, 1 reply; 19+ messages in thread
From: Kevin Hilman @ 2015-10-22 22:07 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 8, 2015 at 10:16 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> PSCI v1.0 augmented the power_state parameter format specification
> (extended stateid) and introduced a way to probe it through the
> PSCI_FEATURES interface.
>
> This patch implements code that detects the power_state format at
> run-time through the PSCI_FEATURES interface, so that the power_state
> argument can be properly detected and validated in the kernel according
> to the information provided through firmware.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
kernelci.org started finding a new boot failures in the arm-soc tree
on arm64 qemu[1] and it was bisected down to this patch, which is in
arm-soc in the form of commit a5c00bb28da0 (drivers: firmware: psci:
add extended stateid power_state support)
The patch doesn't revert cleanly, so I didn't dig much further, but
this suggests that some more testing on qemu is needed (or does qemu
need to be upgraded?)
Kevin
[1] http://kernelci.org/boot/all/job/arm-soc/kernel/v4.3-rc5-557-g159ca7e43189/
> ---
> drivers/firmware/psci.c | 34 ++++++++++++++++++++++++++++++++--
> include/uapi/linux/psci.h | 12 ++++++++++++
> 2 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index 24ef3a8..bd2ba5b 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -75,14 +75,34 @@ static u32 psci_function_id[PSCI_FN_MAX];
> PSCI_0_2_POWER_STATE_TYPE_MASK | \
> PSCI_0_2_POWER_STATE_AFFL_MASK)
>
> +#define PSCI_1_0_EXT_POWER_STATE_MASK \
> + (PSCI_1_0_EXT_POWER_STATE_ID_MASK | \
> + PSCI_1_0_EXT_POWER_STATE_TYPE_MASK)
> +
> +static u32 psci_cpu_suspend_feature;
> +
> +static inline bool psci_has_ext_power_state(void)
> +{
> + return psci_cpu_suspend_feature &
> + PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK;
> +}
> +
> bool psci_power_state_loses_context(u32 state)
> {
> - return state & PSCI_0_2_POWER_STATE_TYPE_MASK;
> + const u32 mask = psci_has_ext_power_state() ?
> + PSCI_1_0_EXT_POWER_STATE_TYPE_MASK :
> + PSCI_0_2_POWER_STATE_TYPE_MASK;
> +
> + return state & mask;
> }
>
> bool psci_power_state_is_valid(u32 state)
> {
> - return !(state & ~PSCI_0_2_POWER_STATE_MASK);
> + const u32 valid_mask = psci_has_ext_power_state() ?
> + PSCI_1_0_EXT_POWER_STATE_MASK :
> + PSCI_0_2_POWER_STATE_MASK;
> +
> + return !(state & ~valid_mask);
> }
>
> static int psci_to_linux_errno(int errno)
> @@ -203,6 +223,14 @@ static int __init psci_features(u32 psci_func_id)
> psci_func_id, 0, 0);
> }
>
> +static void __init psci_init_cpu_suspend(void)
> +{
> + int feature = psci_features(psci_function_id[PSCI_FN_CPU_SUSPEND]);
> +
> + if (feature != PSCI_RET_NOT_SUPPORTED)
> + psci_cpu_suspend_feature = feature;
> +}
> +
> /*
> * Detect the presence of a resident Trusted OS which may cause CPU_OFF to
> * return DENIED (which would be fatal).
> @@ -287,6 +315,8 @@ static int __init psci_probe(void)
>
> psci_init_migrate();
>
> + psci_init_cpu_suspend();
> +
> return 0;
> }
>
> diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
> index 187b828d..0a9485f 100644
> --- a/include/uapi/linux/psci.h
> +++ b/include/uapi/linux/psci.h
> @@ -58,6 +58,13 @@
> #define PSCI_0_2_POWER_STATE_AFFL_MASK \
> (0x3 << PSCI_0_2_POWER_STATE_AFFL_SHIFT)
>
> +/* PSCI extended power state encoding for CPU_SUSPEND function */
> +#define PSCI_1_0_EXT_POWER_STATE_ID_MASK 0xfffffff
> +#define PSCI_1_0_EXT_POWER_STATE_ID_SHIFT 0
> +#define PSCI_1_0_EXT_POWER_STATE_TYPE_SHIFT 30
> +#define PSCI_1_0_EXT_POWER_STATE_TYPE_MASK \
> + (0x1 << PSCI_1_0_EXT_POWER_STATE_TYPE_SHIFT)
> +
> /* PSCI v0.2 affinity level state returned by AFFINITY_INFO */
> #define PSCI_0_2_AFFINITY_LEVEL_ON 0
> #define PSCI_0_2_AFFINITY_LEVEL_OFF 1
> @@ -78,6 +85,11 @@
> #define PSCI_VERSION_MINOR(ver) \
> ((ver) & PSCI_VERSION_MINOR_MASK)
>
> +/* PSCI features decoding (>=1.0) */
> +#define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT 1
> +#define PSCI_1_0_FEATURES_CPU_SUSPEND_PF_MASK \
> + (0x1 << PSCI_1_0_FEATURES_CPU_SUSPEND_PF_SHIFT)
> +
> /* PSCI return values (inclusive of all PSCI versions) */
> #define PSCI_RET_SUCCESS 0
> #define PSCI_RET_NOT_SUPPORTED -1
> --
> 2.2.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support
2015-10-22 22:07 ` Kevin Hilman
@ 2015-10-23 10:23 ` Arnd Bergmann
2015-10-23 10:44 ` Lorenzo Pieralisi
0 siblings, 1 reply; 19+ messages in thread
From: Arnd Bergmann @ 2015-10-23 10:23 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 22 October 2015 15:07:06 Kevin Hilman wrote:
> Spam Status: Spamassassin CRM114
> On Wed, Jul 8, 2015 at 10:16 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > PSCI v1.0 augmented the power_state parameter format specification
> > (extended stateid) and introduced a way to probe it through the
> > PSCI_FEATURES interface.
> >
> > This patch implements code that detects the power_state format at
> > run-time through the PSCI_FEATURES interface, so that the power_state
> > argument can be properly detected and validated in the kernel according
> > to the information provided through firmware.
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
>
> kernelci.org started finding a new boot failures in the arm-soc tree
> on arm64 qemu[1] and it was bisected down to this patch, which is in
> arm-soc in the form of commit a5c00bb28da0 (drivers: firmware: psci:
> add extended stateid power_state support)
>
> The patch doesn't revert cleanly, so I didn't dig much further, but
> this suggests that some more testing on qemu is needed (or does qemu
> need to be upgraded?)
>
> Kevin
>
> [1] http://kernelci.org/boot/all/job/arm-soc/kernel/v4.3-rc5-557-g159ca7e43189/
>
Could it be that qemu claims to support psci-1.0 but is actually not
compatible?
Arnd
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support
2015-10-23 10:23 ` Arnd Bergmann
@ 2015-10-23 10:44 ` Lorenzo Pieralisi
2015-10-23 10:55 ` Arnd Bergmann
0 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-10-23 10:44 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 23, 2015 at 12:23:53PM +0200, Arnd Bergmann wrote:
> On Thursday 22 October 2015 15:07:06 Kevin Hilman wrote:
> > On Wed, Jul 8, 2015 at 10:16 AM, Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> > > PSCI v1.0 augmented the power_state parameter format specification
> > > (extended stateid) and introduced a way to probe it through the
> > > PSCI_FEATURES interface.
> > >
> > > This patch implements code that detects the power_state format at
> > > run-time through the PSCI_FEATURES interface, so that the power_state
> > > argument can be properly detected and validated in the kernel according
> > > to the information provided through firmware.
> > >
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> >
> > kernelci.org started finding a new boot failures in the arm-soc tree
> > on arm64 qemu[1] and it was bisected down to this patch, which is in
> > arm-soc in the form of commit a5c00bb28da0 (drivers: firmware: psci:
> > add extended stateid power_state support)
> >
> > The patch doesn't revert cleanly, so I didn't dig much further, but
> > this suggests that some more testing on qemu is needed (or does qemu
> > need to be upgraded?)
> >
> > Kevin
> >
> > [1] http://kernelci.org/boot/all/job/arm-soc/kernel/v4.3-rc5-557-g159ca7e43189/
> >
>
> Could it be that qemu claims to support psci-1.0 but is actually not
> compatible?
Yes, the problem is, PSCI functions that are not implemented must return
NOT_SUPPORTED according to the SMC calling convention and the PSCI
spec. The KVM implementation was not compliant and we patched the
kernel already:
commit e2d997366dc5b ("ARM: kvm: psci: fix handling of unimplemented
functions")
So the KVM PSCI implementation is fine now.
Problem with Qemu emulation is that it does not emulate the PSCI 1.0
specs correctly (it does not even consider PSCI 1.0 functions proper PSCI
calls), I tested it and I think we should update Qemu as we
did with KVM kernel code instead of working around it by dodging the
problem in the PSCI implementation by adding code that checks the
PSCI version before issuing the PSCI calls through the respective
conduit.
Thoughts appreciated.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support
2015-10-23 10:44 ` Lorenzo Pieralisi
@ 2015-10-23 10:55 ` Arnd Bergmann
2015-10-23 11:36 ` Lorenzo Pieralisi
0 siblings, 1 reply; 19+ messages in thread
From: Arnd Bergmann @ 2015-10-23 10:55 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 23 October 2015 11:44:58 Lorenzo Pieralisi wrote:
>
> Problem with Qemu emulation is that it does not emulate the PSCI 1.0
> specs correctly (it does not even consider PSCI 1.0 functions proper PSCI
> calls), I tested it and I think we should update Qemu as we
> did with KVM kernel code instead of working around it by dodging the
> problem in the PSCI implementation by adding code that checks the
> PSCI version before issuing the PSCI calls through the respective
> conduit.
>
> Thoughts appreciated.
I think we really cannot break existing qemu installations, but need
a way to detect whether we are dealing with a broken qemu (and warn
about that) or with a working PSCI implementation.
Fixing qemu is of course a good idea, but it's not sufficient.
Arnd
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support
2015-10-23 10:55 ` Arnd Bergmann
@ 2015-10-23 11:36 ` Lorenzo Pieralisi
2015-10-23 15:10 ` Kevin Hilman
0 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-10-23 11:36 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 23, 2015 at 12:55:34PM +0200, Arnd Bergmann wrote:
> On Friday 23 October 2015 11:44:58 Lorenzo Pieralisi wrote:
> >
> > Problem with Qemu emulation is that it does not emulate the PSCI 1.0
> > specs correctly (it does not even consider PSCI 1.0 functions proper PSCI
> > calls), I tested it and I think we should update Qemu as we
> > did with KVM kernel code instead of working around it by dodging the
> > problem in the PSCI implementation by adding code that checks the
> > PSCI version before issuing the PSCI calls through the respective
> > conduit.
> >
> > Thoughts appreciated.
>
> I think we really cannot break existing qemu installations, but need
> a way to detect whether we are dealing with a broken qemu (and warn
> about that) or with a working PSCI implementation.
>
> Fixing qemu is of course a good idea, but it's not sufficient.
I will patch the PSCI back-end in the kernel to issue PSCI_FEATURES calls
only if the PSCI version detected is 1.0, therefore solving the issue, it
is not necessary according to the specs, but that's belt and braces
and sorts this niggle out.
I will send a patch shortly to be applied on top of arm-soc drivers/psci.
Thanks !
Lorenzo
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support
2015-10-23 11:36 ` Lorenzo Pieralisi
@ 2015-10-23 15:10 ` Kevin Hilman
2015-10-26 10:05 ` Lorenzo Pieralisi
0 siblings, 1 reply; 19+ messages in thread
From: Kevin Hilman @ 2015-10-23 15:10 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 23, 2015 at 4:36 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Fri, Oct 23, 2015 at 12:55:34PM +0200, Arnd Bergmann wrote:
>> On Friday 23 October 2015 11:44:58 Lorenzo Pieralisi wrote:
>> >
>> > Problem with Qemu emulation is that it does not emulate the PSCI 1.0
>> > specs correctly (it does not even consider PSCI 1.0 functions proper PSCI
>> > calls), I tested it and I think we should update Qemu as we
>> > did with KVM kernel code instead of working around it by dodging the
>> > problem in the PSCI implementation by adding code that checks the
>> > PSCI version before issuing the PSCI calls through the respective
>> > conduit.
>> >
>> > Thoughts appreciated.
>>
>> I think we really cannot break existing qemu installations, but need
>> a way to detect whether we are dealing with a broken qemu (and warn
>> about that) or with a working PSCI implementation.
>>
>> Fixing qemu is of course a good idea, but it's not sufficient.
>
> I will patch the PSCI back-end in the kernel to issue PSCI_FEATURES calls
> only if the PSCI version detected is 1.0, therefore solving the issue, it
> is not necessary according to the specs, but that's belt and braces
> and sorts this niggle out.
>
> I will send a patch shortly to be applied on top of arm-soc drivers/psci.
Thanks!
In parallel, since you understandn better what qemu is/isn't doing,
can you ping the qemu folks about this bug?
Thanks,
Kevin
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support
2015-10-23 15:10 ` Kevin Hilman
@ 2015-10-26 10:05 ` Lorenzo Pieralisi
0 siblings, 0 replies; 19+ messages in thread
From: Lorenzo Pieralisi @ 2015-10-26 10:05 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 23, 2015 at 08:10:30AM -0700, Kevin Hilman wrote:
> On Fri, Oct 23, 2015 at 4:36 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On Fri, Oct 23, 2015 at 12:55:34PM +0200, Arnd Bergmann wrote:
> >> On Friday 23 October 2015 11:44:58 Lorenzo Pieralisi wrote:
> >> >
> >> > Problem with Qemu emulation is that it does not emulate the PSCI 1.0
> >> > specs correctly (it does not even consider PSCI 1.0 functions proper PSCI
> >> > calls), I tested it and I think we should update Qemu as we
> >> > did with KVM kernel code instead of working around it by dodging the
> >> > problem in the PSCI implementation by adding code that checks the
> >> > PSCI version before issuing the PSCI calls through the respective
> >> > conduit.
> >> >
> >> > Thoughts appreciated.
> >>
> >> I think we really cannot break existing qemu installations, but need
> >> a way to detect whether we are dealing with a broken qemu (and warn
> >> about that) or with a working PSCI implementation.
> >>
> >> Fixing qemu is of course a good idea, but it's not sufficient.
> >
> > I will patch the PSCI back-end in the kernel to issue PSCI_FEATURES calls
> > only if the PSCI version detected is 1.0, therefore solving the issue, it
> > is not necessary according to the specs, but that's belt and braces
> > and sorts this niggle out.
> >
> > I will send a patch shortly to be applied on top of arm-soc drivers/psci.
>
> Thanks!
>
> In parallel, since you understandn better what qemu is/isn't doing,
> can you ping the qemu folks about this bug?
Yes, I will help them implement PSCI 1.0 support so that we can
upgrade smoothly.
Thanks Kevin !
Lorenzo
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-10-26 10:05 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-08 17:16 [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 1/5] drivers: firmware: psci: add INVALID_ADDRESS return value Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 2/5] drivers: firmware: psci: move power_state handling to generic code Lorenzo Pieralisi
2015-07-09 13:39 ` Catalin Marinas
2015-07-08 17:16 ` [PATCH v2 3/5] drivers: firmware: psci: add PSCI_FEATURES call Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 4/5] drivers: firmware: psci: add extended stateid power_state support Lorenzo Pieralisi
2015-10-22 22:07 ` Kevin Hilman
2015-10-23 10:23 ` Arnd Bergmann
2015-10-23 10:44 ` Lorenzo Pieralisi
2015-10-23 10:55 ` Arnd Bergmann
2015-10-23 11:36 ` Lorenzo Pieralisi
2015-10-23 15:10 ` Kevin Hilman
2015-10-26 10:05 ` Lorenzo Pieralisi
2015-07-08 17:16 ` [PATCH v2 5/5] drivers: firmware: psci: add PSCI v1.0 DT bindings Lorenzo Pieralisi
2015-10-05 11:48 ` Andre Przywara
2015-10-05 12:06 ` Mark Rutland
2015-10-05 12:11 ` Lorenzo Pieralisi
2015-09-14 13:35 ` [PATCH v2 0/5] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
2015-09-15 3:23 ` Jisheng Zhang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.