All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] drivers: firmware: psci: add basic v1.0 support
@ 2015-05-29 12:16 ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12:16 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Lorenzo Pieralisi, Christoffer Dall, Will Deacon, Sudeep Holla,
	Catalin Marinas, Anup Patel, Mark Rutland, Marc Zyngier

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 branch:

git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git psci/unification

Tested on a Juno board.

[1] http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf

Lorenzo Pieralisi (6):
  ARM: kvm: psci: fix handling of unimplemented functions
  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/arm/kvm/psci.c                            |  6 +--
 arch/arm64/kernel/psci.c                       | 14 -------
 drivers/firmware/psci.c                        | 53 ++++++++++++++++++++++++++
 include/linux/psci.h                           |  2 +
 include/uapi/linux/psci.h                      | 15 ++++++++
 6 files changed, 79 insertions(+), 17 deletions(-)

-- 
2.2.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH 0/6] drivers: firmware: psci: add basic v1.0 support
@ 2015-05-29 12:16 ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12:16 UTC (permalink / raw)
  To: linux-arm-kernel

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 branch:

git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git psci/unification

Tested on a Juno board.

[1] http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_State_Coordination_Interface.pdf

Lorenzo Pieralisi (6):
  ARM: kvm: psci: fix handling of unimplemented functions
  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/arm/kvm/psci.c                            |  6 +--
 arch/arm64/kernel/psci.c                       | 14 -------
 drivers/firmware/psci.c                        | 53 ++++++++++++++++++++++++++
 include/linux/psci.h                           |  2 +
 include/uapi/linux/psci.h                      | 15 ++++++++
 6 files changed, 79 insertions(+), 17 deletions(-)

-- 
2.2.1

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions
  2015-05-29 12:16 ` Lorenzo Pieralisi
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12:16 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Lorenzo Pieralisi, Christoffer Dall, Anup Patel, Marc Zyngier,
	Will Deacon, Sudeep Holla, Catalin Marinas, Mark Rutland

According to the PSCI specification and the SMC/HVC calling
convention, PSCI function_ids that are not implemented must
return NOT_SUPPORTED as return value.

Current KVM implementation takes an unhandled PSCI function_id
as an error and injects an undefined instruction into the guest
if PSCI implementation is called with a function_id that is not
handled by the resident PSCI version (ie it is not implemented),
which is not the behaviour expected by a guest when calling a
PSCI function_id that is not implemented.

This patch fixes this issue by returning NOT_SUPPORTED whenever
the kvm PSCI call is executed for a function_id that is not
implemented by the PSCI kvm layer.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
Reported-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
Cc: Christoffer Dall <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Anup Patel <anup.patel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm/kvm/psci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 7e9398c..ec5943b 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -273,7 +273,8 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 		ret = 0;
 		break;
 	default:
-		return -EINVAL;
+		val = PSCI_RET_NOT_SUPPORTED;
+		break;
 	}
 
 	*vcpu_reg(vcpu, 0) = val;
@@ -295,10 +296,9 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
 		break;
 	case KVM_PSCI_FN_CPU_SUSPEND:
 	case KVM_PSCI_FN_MIGRATE:
+	default:
 		val = PSCI_RET_NOT_SUPPORTED;
 		break;
-	default:
-		return -EINVAL;
 	}
 
 	*vcpu_reg(vcpu, 0) = val;
-- 
2.2.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12:16 UTC (permalink / raw)
  To: linux-arm-kernel

According to the PSCI specification and the SMC/HVC calling
convention, PSCI function_ids that are not implemented must
return NOT_SUPPORTED as return value.

Current KVM implementation takes an unhandled PSCI function_id
as an error and injects an undefined instruction into the guest
if PSCI implementation is called with a function_id that is not
handled by the resident PSCI version (ie it is not implemented),
which is not the behaviour expected by a guest when calling a
PSCI function_id that is not implemented.

This patch fixes this issue by returning NOT_SUPPORTED whenever
the kvm PSCI call is executed for a function_id that is not
implemented by the PSCI kvm layer.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reported-by: Sudeep Holla <sudeep.holla@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Anup Patel <anup.patel@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/psci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index 7e9398c..ec5943b 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -273,7 +273,8 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
 		ret = 0;
 		break;
 	default:
-		return -EINVAL;
+		val = PSCI_RET_NOT_SUPPORTED;
+		break;
 	}
 
 	*vcpu_reg(vcpu, 0) = val;
@@ -295,10 +296,9 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
 		break;
 	case KVM_PSCI_FN_CPU_SUSPEND:
 	case KVM_PSCI_FN_MIGRATE:
+	default:
 		val = PSCI_RET_NOT_SUPPORTED;
 		break;
-	default:
-		return -EINVAL;
 	}
 
 	*vcpu_reg(vcpu, 0) = val;
-- 
2.2.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 2/6] drivers: firmware: psci: add INVALID_ADDRESS return value
  2015-05-29 12:16 ` Lorenzo Pieralisi
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12:16 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Lorenzo Pieralisi, Christoffer Dall, Will Deacon, Sudeep Holla,
	Catalin Marinas, Anup Patel, Mark Rutland, Marc Zyngier

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-5wv7dgnIgG8@public.gmane.org>
Acked-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
Acked-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
---
 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 53c4ac8..afb955f0 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -77,6 +77,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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 2/6] drivers: firmware: psci: add INVALID_ADDRESS return value
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12: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 53c4ac8..afb955f0 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -77,6 +77,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] 26+ messages in thread
* [PATCH 3/6] drivers: firmware: psci: move power_state handling to generic code
  2015-05-29 12:16 ` Lorenzo Pieralisi
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12:16 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Lorenzo Pieralisi, Will Deacon, Catalin Marinas, Mark Rutland,
	Christoffer Dall, Sudeep Holla, Anup Patel, Marc Zyngier

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-5wv7dgnIgG8@public.gmane.org>
Acked-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
---
 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 8063891..a0fc99a 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 afb955f0..f1579a4 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -69,6 +69,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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 3/6] drivers: firmware: psci: move power_state handling to generic code
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12: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: Sudeep Holla <sudeep.holla@arm.com>
Cc: Will Deacon <will.deacon@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 8063891..a0fc99a 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 afb955f0..f1579a4 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -69,6 +69,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] 26+ messages in thread
* [PATCH 4/6] drivers: firmware: psci: add PSCI_FEATURES call
  2015-05-29 12:16 ` Lorenzo Pieralisi
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12:16 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Lorenzo Pieralisi, Mark Rutland, Christoffer Dall, Will Deacon,
	Sudeep Holla, Catalin Marinas, Anup Patel, Marc Zyngier

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-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
---
 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 f1579a4..2330099 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -196,6 +196,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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 4/6] drivers: firmware: psci: add PSCI_FEATURES call
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12: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 f1579a4..2330099 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -196,6 +196,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] 26+ messages in thread
* [PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support
  2015-05-29 12:16 ` Lorenzo Pieralisi
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12:16 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Lorenzo Pieralisi, Mark Rutland, Christoffer Dall, Will Deacon,
	Sudeep Holla, Catalin Marinas, Anup Patel, Marc Zyngier

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-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
---
 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 2330099..4063784 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -74,14 +74,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_EXT_POWER_STATE_MASK		\
+				(PSCI_EXT_POWER_STATE_ID_MASK | \
+				PSCI_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_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_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_EXT_POWER_STATE_MASK :
+			       PSCI_0_2_POWER_STATE_MASK;
+
+	return !(state & ~valid_mask);
 }
 
 static int psci_to_linux_errno(int errno)
@@ -202,6 +222,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).
@@ -280,6 +308,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..2598d7c 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_EXT_POWER_STATE_ID_MASK		0xfffffff
+#define PSCI_EXT_POWER_STATE_ID_SHIFT		0
+#define PSCI_EXT_POWER_STATE_TYPE_SHIFT		30
+#define PSCI_EXT_POWER_STATE_TYPE_MASK		\
+				(0x1 << PSCI_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_FEATURES_CPU_SUSPEND_PF_SHIFT	1
+#define PSCI_FEATURES_CPU_SUSPEND_PF_MASK	\
+			(0x1 << PSCI_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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12: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 2330099..4063784 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -74,14 +74,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_EXT_POWER_STATE_MASK		\
+				(PSCI_EXT_POWER_STATE_ID_MASK | \
+				PSCI_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_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_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_EXT_POWER_STATE_MASK :
+			       PSCI_0_2_POWER_STATE_MASK;
+
+	return !(state & ~valid_mask);
 }
 
 static int psci_to_linux_errno(int errno)
@@ -202,6 +222,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).
@@ -280,6 +308,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..2598d7c 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_EXT_POWER_STATE_ID_MASK		0xfffffff
+#define PSCI_EXT_POWER_STATE_ID_SHIFT		0
+#define PSCI_EXT_POWER_STATE_TYPE_SHIFT		30
+#define PSCI_EXT_POWER_STATE_TYPE_MASK		\
+				(0x1 << PSCI_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_FEATURES_CPU_SUSPEND_PF_SHIFT	1
+#define PSCI_FEATURES_CPU_SUSPEND_PF_MASK	\
+			(0x1 << PSCI_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] 26+ messages in thread
* [PATCH 6/6] drivers: firmware: psci: add PSCI v1.0 DT bindings
  2015-05-29 12:16 ` Lorenzo Pieralisi
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12:16 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Lorenzo Pieralisi, Mark Rutland, Christoffer Dall, Will Deacon,
	Sudeep Holla, Catalin Marinas, Anup Patel, Marc Zyngier

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-5wv7dgnIgG8@public.gmane.org>
Acked-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
---
 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 4063784..68d0c2f 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -385,6 +385,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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [PATCH 6/6] drivers: firmware: psci: add PSCI v1.0 DT bindings
@ 2015-05-29 12:16     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-05-29 12: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 4063784..68d0c2f 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -385,6 +385,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] 26+ messages in thread
* Re: [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions
  2015-05-29 12:16     ` Lorenzo Pieralisi
@ 2015-05-29 13:04         ` Sudeep Holla
  -1 siblings, 0 replies; 26+ messages in thread
From: Sudeep Holla @ 2015-05-29 13:04 UTC (permalink / raw)
  To: Lorenzo Pieralisi,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: Sudeep Holla, Christoffer Dall, Anup Patel, Marc Zyngier,
	Will Deacon, Catalin Marinas, Mark Rutland



On 29/05/15 13:16, Lorenzo Pieralisi wrote:
> According to the PSCI specification and the SMC/HVC calling
> convention, PSCI function_ids that are not implemented must
> return NOT_SUPPORTED as return value.
>
> Current KVM implementation takes an unhandled PSCI function_id
> as an error and injects an undefined instruction into the guest
> if PSCI implementation is called with a function_id that is not
> handled by the resident PSCI version (ie it is not implemented),
> which is not the behaviour expected by a guest when calling a
> PSCI function_id that is not implemented.
>
> This patch fixes this issue by returning NOT_SUPPORTED whenever
> the kvm PSCI call is executed for a function_id that is not
> implemented by the PSCI kvm layer.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> Reported-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> Cc: Christoffer Dall <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Anup Patel <anup.patel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
> ---
>   arch/arm/kvm/psci.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 7e9398c..ec5943b 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -273,7 +273,8 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>   		ret = 0;
>   		break;
>   	default:
> -		return -EINVAL;
> +		val = PSCI_RET_NOT_SUPPORTED;
> +		break;

IMO we can remove all the other optional non-implemented PSCI functions
(e.g. KVM_PSCI_FN_MIGRATE, KVM_PSCI_FN_CPU_SUSPEND, ..etc) returning
PSCI_RET_NOT_SUPPORTED here as they will be then automatically covered
by default case.

Otherwise looks good to me:
Acked-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions
@ 2015-05-29 13:04         ` Sudeep Holla
  0 siblings, 0 replies; 26+ messages in thread
From: Sudeep Holla @ 2015-05-29 13:04 UTC (permalink / raw)
  To: linux-arm-kernel



On 29/05/15 13:16, Lorenzo Pieralisi wrote:
> According to the PSCI specification and the SMC/HVC calling
> convention, PSCI function_ids that are not implemented must
> return NOT_SUPPORTED as return value.
>
> Current KVM implementation takes an unhandled PSCI function_id
> as an error and injects an undefined instruction into the guest
> if PSCI implementation is called with a function_id that is not
> handled by the resident PSCI version (ie it is not implemented),
> which is not the behaviour expected by a guest when calling a
> PSCI function_id that is not implemented.
>
> This patch fixes this issue by returning NOT_SUPPORTED whenever
> the kvm PSCI call is executed for a function_id that is not
> implemented by the PSCI kvm layer.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reported-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Anup Patel <anup.patel@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/arm/kvm/psci.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 7e9398c..ec5943b 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -273,7 +273,8 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>   		ret = 0;
>   		break;
>   	default:
> -		return -EINVAL;
> +		val = PSCI_RET_NOT_SUPPORTED;
> +		break;

IMO we can remove all the other optional non-implemented PSCI functions
(e.g. KVM_PSCI_FN_MIGRATE, KVM_PSCI_FN_CPU_SUSPEND, ..etc) returning
PSCI_RET_NOT_SUPPORTED here as they will be then automatically covered
by default case.

Otherwise looks good to me:
Acked-by: Sudeep Holla <sudeep.holla@arm.com>

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH 3/6] drivers: firmware: psci: move power_state handling to generic code
  2015-05-29 12:16     ` Lorenzo Pieralisi
@ 2015-06-01  9:28         ` Will Deacon
  -1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2015-06-01  9:28 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Catalin Marinas, Mark Rutland, Christoffer Dall, Sudeep Holla,
	Anup Patel, Marc Zyngier

On Fri, May 29, 2015 at 01:16:36PM +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-5wv7dgnIgG8@public.gmane.org>
> Acked-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> ---
>  arch/arm64/kernel/psci.c | 14 --------------
>  drivers/firmware/psci.c  | 15 +++++++++++++++
>  include/linux/psci.h     |  2 ++
>  3 files changed, 17 insertions(+), 14 deletions(-)

Looks fine to me:

  Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

Will
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH 3/6] drivers: firmware: psci: move power_state handling to generic code
@ 2015-06-01  9:28         ` Will Deacon
  0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2015-06-01  9:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 29, 2015 at 01:16:36PM +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: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Will Deacon <will.deacon@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(-)

Looks fine to me:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support
  2015-05-29 12:16     ` Lorenzo Pieralisi
@ 2015-06-05 14:16         ` Ashwin Chaugule
  -1 siblings, 0 replies; 26+ messages in thread
From: Ashwin Chaugule @ 2015-06-05 14:16 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Devicetree List, Mark Rutland, Anup Patel, Marc Zyngier,
	Catalin Marinas, Will Deacon, Sudeep Holla, Christoffer Dall

On 29 May 2015 at 08:16, Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> 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-5wv7dgnIgG8@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> ---
>  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 2330099..4063784 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -74,14 +74,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_EXT_POWER_STATE_MASK              \
> +                               (PSCI_EXT_POWER_STATE_ID_MASK | \
> +                               PSCI_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_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_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_EXT_POWER_STATE_MASK :
> +                              PSCI_0_2_POWER_STATE_MASK;
> +
> +       return !(state & ~valid_mask);
>  }
>
>  static int psci_to_linux_errno(int errno)
> @@ -202,6 +222,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).
> @@ -280,6 +308,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..2598d7c 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_EXT_POWER_STATE_ID_MASK           0xfffffff
> +#define PSCI_EXT_POWER_STATE_ID_SHIFT          0
> +#define PSCI_EXT_POWER_STATE_TYPE_SHIFT                30
> +#define PSCI_EXT_POWER_STATE_TYPE_MASK         \
> +                               (0x1 << PSCI_EXT_POWER_STATE_TYPE_SHIFT)
> +

For consistency, dont you need version numbers here, like we have for v0.2?

>  /* 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_FEATURES_CPU_SUSPEND_PF_SHIFT     1
> +#define PSCI_FEATURES_CPU_SUSPEND_PF_MASK      \
> +                       (0x1 << PSCI_FEATURES_CPU_SUSPEND_PF_SHIFT)
> +

Likewise.

>  /* PSCI return values (inclusive of all PSCI versions) */
>  #define PSCI_RET_SUCCESS                       0
>  #define PSCI_RET_NOT_SUPPORTED                 -1
> --
> 2.2.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support
@ 2015-06-05 14:16         ` Ashwin Chaugule
  0 siblings, 0 replies; 26+ messages in thread
From: Ashwin Chaugule @ 2015-06-05 14:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 29 May 2015 at 08:16, 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>
> ---
>  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 2330099..4063784 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -74,14 +74,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_EXT_POWER_STATE_MASK              \
> +                               (PSCI_EXT_POWER_STATE_ID_MASK | \
> +                               PSCI_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_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_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_EXT_POWER_STATE_MASK :
> +                              PSCI_0_2_POWER_STATE_MASK;
> +
> +       return !(state & ~valid_mask);
>  }
>
>  static int psci_to_linux_errno(int errno)
> @@ -202,6 +222,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).
> @@ -280,6 +308,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..2598d7c 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_EXT_POWER_STATE_ID_MASK           0xfffffff
> +#define PSCI_EXT_POWER_STATE_ID_SHIFT          0
> +#define PSCI_EXT_POWER_STATE_TYPE_SHIFT                30
> +#define PSCI_EXT_POWER_STATE_TYPE_MASK         \
> +                               (0x1 << PSCI_EXT_POWER_STATE_TYPE_SHIFT)
> +

For consistency, dont you need version numbers here, like we have for v0.2?

>  /* 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_FEATURES_CPU_SUSPEND_PF_SHIFT     1
> +#define PSCI_FEATURES_CPU_SUSPEND_PF_MASK      \
> +                       (0x1 << PSCI_FEATURES_CPU_SUSPEND_PF_SHIFT)
> +

Likewise.

>  /* PSCI return values (inclusive of all PSCI versions) */
>  #define PSCI_RET_SUCCESS                       0
>  #define PSCI_RET_NOT_SUPPORTED                 -1
> --
> 2.2.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support
  2015-06-05 14:16         ` Ashwin Chaugule
@ 2015-06-08 11:03             ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-06-08 11:03 UTC (permalink / raw)
  To: Ashwin Chaugule
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Devicetree List, Mark Rutland, Anup Patel, Marc Zyngier,
	Catalin Marinas, Will Deacon, Sudeep Holla, Christoffer Dall

On Fri, Jun 05, 2015 at 03:16:36PM +0100, Ashwin Chaugule wrote:
> On 29 May 2015 at 08:16, Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:

[...]

> > diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
> > index 187b828d..2598d7c 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_EXT_POWER_STATE_ID_MASK           0xfffffff
> > +#define PSCI_EXT_POWER_STATE_ID_SHIFT          0
> > +#define PSCI_EXT_POWER_STATE_TYPE_SHIFT                30
> > +#define PSCI_EXT_POWER_STATE_TYPE_MASK         \
> > +                               (0x1 << PSCI_EXT_POWER_STATE_TYPE_SHIFT)
> > +
> 
> For consistency, dont you need version numbers here, like we have for v0.2?

According to the docs (page 49, 5.12.2 Implementation responsibilities)
the choice is between "Original Format(PSCI0.2)" and "extended stateid"
parameter.

Do we really need to add a version number to the extended stateid defines ?
I could add it, I am not not fussed about this.

> >  /* 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_FEATURES_CPU_SUSPEND_PF_SHIFT     1
> > +#define PSCI_FEATURES_CPU_SUSPEND_PF_MASK      \
> > +                       (0x1 << PSCI_FEATURES_CPU_SUSPEND_PF_SHIFT)
> > +
> 
> Likewise.

I could do, yes.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support
@ 2015-06-08 11:03             ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-06-08 11:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jun 05, 2015 at 03:16:36PM +0100, Ashwin Chaugule wrote:
> On 29 May 2015 at 08:16, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:

[...]

> > diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
> > index 187b828d..2598d7c 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_EXT_POWER_STATE_ID_MASK           0xfffffff
> > +#define PSCI_EXT_POWER_STATE_ID_SHIFT          0
> > +#define PSCI_EXT_POWER_STATE_TYPE_SHIFT                30
> > +#define PSCI_EXT_POWER_STATE_TYPE_MASK         \
> > +                               (0x1 << PSCI_EXT_POWER_STATE_TYPE_SHIFT)
> > +
> 
> For consistency, dont you need version numbers here, like we have for v0.2?

According to the docs (page 49, 5.12.2 Implementation responsibilities)
the choice is between "Original Format(PSCI0.2)" and "extended stateid"
parameter.

Do we really need to add a version number to the extended stateid defines ?
I could add it, I am not not fussed about this.

> >  /* 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_FEATURES_CPU_SUSPEND_PF_SHIFT     1
> > +#define PSCI_FEATURES_CPU_SUSPEND_PF_MASK      \
> > +                       (0x1 << PSCI_FEATURES_CPU_SUSPEND_PF_SHIFT)
> > +
> 
> Likewise.

I could do, yes.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions
  2015-05-29 12:16     ` Lorenzo Pieralisi
@ 2015-06-09 17:18         ` Marc Zyngier
  -1 siblings, 0 replies; 26+ messages in thread
From: Marc Zyngier @ 2015-06-09 17:18 UTC (permalink / raw)
  To: Lorenzo Pieralisi,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: Christoffer Dall, Anup Patel, Will Deacon, Sudeep Holla,
	Catalin Marinas, Mark Rutland

Hi Lorenzo,

On 29/05/15 13:16, Lorenzo Pieralisi wrote:
> According to the PSCI specification and the SMC/HVC calling
> convention, PSCI function_ids that are not implemented must
> return NOT_SUPPORTED as return value.
> 
> Current KVM implementation takes an unhandled PSCI function_id
> as an error and injects an undefined instruction into the guest
> if PSCI implementation is called with a function_id that is not
> handled by the resident PSCI version (ie it is not implemented),
> which is not the behaviour expected by a guest when calling a
> PSCI function_id that is not implemented.
> 
> This patch fixes this issue by returning NOT_SUPPORTED whenever
> the kvm PSCI call is executed for a function_id that is not
> implemented by the PSCI kvm layer.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> Reported-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> Cc: Christoffer Dall <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Anup Patel <anup.patel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
> ---
>  arch/arm/kvm/psci.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 7e9398c..ec5943b 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -273,7 +273,8 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  		ret = 0;
>  		break;
>  	default:
> -		return -EINVAL;
> +		val = PSCI_RET_NOT_SUPPORTED;
> +		break;
>  	}
>  
>  	*vcpu_reg(vcpu, 0) = val;
> @@ -295,10 +296,9 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>  		break;
>  	case KVM_PSCI_FN_CPU_SUSPEND:
>  	case KVM_PSCI_FN_MIGRATE:
> +	default:
>  		val = PSCI_RET_NOT_SUPPORTED;
>  		break;
> -	default:
> -		return -EINVAL;
>  	}
>  
>  	*vcpu_reg(vcpu, 0) = val;
> 

Looks good to me. How do you want to proceed with this one? can I take
it independently from the rest of the series? Or would you prefer it
being kept as a whole?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions
@ 2015-06-09 17:18         ` Marc Zyngier
  0 siblings, 0 replies; 26+ messages in thread
From: Marc Zyngier @ 2015-06-09 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Lorenzo,

On 29/05/15 13:16, Lorenzo Pieralisi wrote:
> According to the PSCI specification and the SMC/HVC calling
> convention, PSCI function_ids that are not implemented must
> return NOT_SUPPORTED as return value.
> 
> Current KVM implementation takes an unhandled PSCI function_id
> as an error and injects an undefined instruction into the guest
> if PSCI implementation is called with a function_id that is not
> handled by the resident PSCI version (ie it is not implemented),
> which is not the behaviour expected by a guest when calling a
> PSCI function_id that is not implemented.
> 
> This patch fixes this issue by returning NOT_SUPPORTED whenever
> the kvm PSCI call is executed for a function_id that is not
> implemented by the PSCI kvm layer.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reported-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Anup Patel <anup.patel@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/kvm/psci.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> index 7e9398c..ec5943b 100644
> --- a/arch/arm/kvm/psci.c
> +++ b/arch/arm/kvm/psci.c
> @@ -273,7 +273,8 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
>  		ret = 0;
>  		break;
>  	default:
> -		return -EINVAL;
> +		val = PSCI_RET_NOT_SUPPORTED;
> +		break;
>  	}
>  
>  	*vcpu_reg(vcpu, 0) = val;
> @@ -295,10 +296,9 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
>  		break;
>  	case KVM_PSCI_FN_CPU_SUSPEND:
>  	case KVM_PSCI_FN_MIGRATE:
> +	default:
>  		val = PSCI_RET_NOT_SUPPORTED;
>  		break;
> -	default:
> -		return -EINVAL;
>  	}
>  
>  	*vcpu_reg(vcpu, 0) = val;
> 

Looks good to me. How do you want to proceed with this one? can I take
it independently from the rest of the series? Or would you prefer it
being kept as a whole?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions
  2015-06-09 17:18         ` Marc Zyngier
@ 2015-06-10  8:24             ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-06-10  8:24 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Christoffer Dall, Anup Patel, Will Deacon, Sudeep Holla,
	Catalin Marinas, Mark Rutland

Hi Marc,

On Tue, Jun 09, 2015 at 06:18:20PM +0100, Marc Zyngier wrote:
> Hi Lorenzo,
> 
> On 29/05/15 13:16, Lorenzo Pieralisi wrote:
> > According to the PSCI specification and the SMC/HVC calling
> > convention, PSCI function_ids that are not implemented must
> > return NOT_SUPPORTED as return value.
> > 
> > Current KVM implementation takes an unhandled PSCI function_id
> > as an error and injects an undefined instruction into the guest
> > if PSCI implementation is called with a function_id that is not
> > handled by the resident PSCI version (ie it is not implemented),
> > which is not the behaviour expected by a guest when calling a
> > PSCI function_id that is not implemented.
> > 
> > This patch fixes this issue by returning NOT_SUPPORTED whenever
> > the kvm PSCI call is executed for a function_id that is not
> > implemented by the PSCI kvm layer.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> > Reported-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
> > Cc: Christoffer Dall <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Anup Patel <anup.patel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
> > ---
> >  arch/arm/kvm/psci.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> > index 7e9398c..ec5943b 100644
> > --- a/arch/arm/kvm/psci.c
> > +++ b/arch/arm/kvm/psci.c
> > @@ -273,7 +273,8 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> >  		ret = 0;
> >  		break;
> >  	default:
> > -		return -EINVAL;
> > +		val = PSCI_RET_NOT_SUPPORTED;
> > +		break;
> >  	}
> >  
> >  	*vcpu_reg(vcpu, 0) = val;
> > @@ -295,10 +296,9 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
> >  		break;
> >  	case KVM_PSCI_FN_CPU_SUSPEND:
> >  	case KVM_PSCI_FN_MIGRATE:
> > +	default:
> >  		val = PSCI_RET_NOT_SUPPORTED;
> >  		break;
> > -	default:
> > -		return -EINVAL;
> >  	}
> >  
> >  	*vcpu_reg(vcpu, 0) = val;
> > 
> 
> Looks good to me. How do you want to proceed with this one? can I take
> it independently from the rest of the series? Or would you prefer it
> being kept as a whole?

I will prepare a v2 to take into account a comment from Sudeep and I
will add a stable tag too, yes it should be merged independently.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions
@ 2015-06-10  8:24             ` Lorenzo Pieralisi
  0 siblings, 0 replies; 26+ messages in thread
From: Lorenzo Pieralisi @ 2015-06-10  8:24 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marc,

On Tue, Jun 09, 2015 at 06:18:20PM +0100, Marc Zyngier wrote:
> Hi Lorenzo,
> 
> On 29/05/15 13:16, Lorenzo Pieralisi wrote:
> > According to the PSCI specification and the SMC/HVC calling
> > convention, PSCI function_ids that are not implemented must
> > return NOT_SUPPORTED as return value.
> > 
> > Current KVM implementation takes an unhandled PSCI function_id
> > as an error and injects an undefined instruction into the guest
> > if PSCI implementation is called with a function_id that is not
> > handled by the resident PSCI version (ie it is not implemented),
> > which is not the behaviour expected by a guest when calling a
> > PSCI function_id that is not implemented.
> > 
> > This patch fixes this issue by returning NOT_SUPPORTED whenever
> > the kvm PSCI call is executed for a function_id that is not
> > implemented by the PSCI kvm layer.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Reported-by: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Christoffer Dall <christoffer.dall@linaro.org>
> > Cc: Anup Patel <anup.patel@linaro.org>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm/kvm/psci.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
> > index 7e9398c..ec5943b 100644
> > --- a/arch/arm/kvm/psci.c
> > +++ b/arch/arm/kvm/psci.c
> > @@ -273,7 +273,8 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu)
> >  		ret = 0;
> >  		break;
> >  	default:
> > -		return -EINVAL;
> > +		val = PSCI_RET_NOT_SUPPORTED;
> > +		break;
> >  	}
> >  
> >  	*vcpu_reg(vcpu, 0) = val;
> > @@ -295,10 +296,9 @@ static int kvm_psci_0_1_call(struct kvm_vcpu *vcpu)
> >  		break;
> >  	case KVM_PSCI_FN_CPU_SUSPEND:
> >  	case KVM_PSCI_FN_MIGRATE:
> > +	default:
> >  		val = PSCI_RET_NOT_SUPPORTED;
> >  		break;
> > -	default:
> > -		return -EINVAL;
> >  	}
> >  
> >  	*vcpu_reg(vcpu, 0) = val;
> > 
> 
> Looks good to me. How do you want to proceed with this one? can I take
> it independently from the rest of the series? Or would you prefer it
> being kept as a whole?

I will prepare a v2 to take into account a comment from Sudeep and I
will add a stable tag too, yes it should be merged independently.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2015-06-10  8:24 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-29 12:16 [PATCH 0/6] drivers: firmware: psci: add basic v1.0 support Lorenzo Pieralisi
2015-05-29 12:16 ` Lorenzo Pieralisi
     [not found] ` <1432901799-18359-1-git-send-email-lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
2015-05-29 12:16   ` [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions Lorenzo Pieralisi
2015-05-29 12:16     ` Lorenzo Pieralisi
     [not found]     ` <1432901799-18359-2-git-send-email-lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
2015-05-29 13:04       ` Sudeep Holla
2015-05-29 13:04         ` Sudeep Holla
2015-06-09 17:18       ` Marc Zyngier
2015-06-09 17:18         ` Marc Zyngier
     [not found]         ` <55771FDC.3090800-5wv7dgnIgG8@public.gmane.org>
2015-06-10  8:24           ` Lorenzo Pieralisi
2015-06-10  8:24             ` Lorenzo Pieralisi
2015-05-29 12:16   ` [PATCH 2/6] drivers: firmware: psci: add INVALID_ADDRESS return value Lorenzo Pieralisi
2015-05-29 12:16     ` Lorenzo Pieralisi
2015-05-29 12:16   ` [PATCH 3/6] drivers: firmware: psci: move power_state handling to generic code Lorenzo Pieralisi
2015-05-29 12:16     ` Lorenzo Pieralisi
     [not found]     ` <1432901799-18359-4-git-send-email-lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
2015-06-01  9:28       ` Will Deacon
2015-06-01  9:28         ` Will Deacon
2015-05-29 12:16   ` [PATCH 4/6] drivers: firmware: psci: add PSCI_FEATURES call Lorenzo Pieralisi
2015-05-29 12:16     ` Lorenzo Pieralisi
2015-05-29 12:16   ` [PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support Lorenzo Pieralisi
2015-05-29 12:16     ` Lorenzo Pieralisi
     [not found]     ` <1432901799-18359-6-git-send-email-lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
2015-06-05 14:16       ` Ashwin Chaugule
2015-06-05 14:16         ` Ashwin Chaugule
     [not found]         ` <CAJ5Y-eYLAjOcTU1Md3Rmc0E49A5Dby1aoBzwtpKpD4Rg=XAYfA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-06-08 11:03           ` Lorenzo Pieralisi
2015-06-08 11:03             ` Lorenzo Pieralisi
2015-05-29 12:16   ` [PATCH 6/6] drivers: firmware: psci: add PSCI v1.0 DT bindings Lorenzo Pieralisi
2015-05-29 12:16     ` Lorenzo Pieralisi

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.