[PATCH 0/6] drivers: firmware: psci: add basic v1.0 support

[PATCH 0/6] drivers: firmware: psci: add basic v1.0 support

[Lorenzo Pieralisi]

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

[PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions

[Lorenzo Pieralisi]

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

Re: [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions

[Sudeep Holla]

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

Re: [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions

[Marc Zyngier]

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

Re: [PATCH 1/6] ARM: kvm: psci: fix handling of unimplemented functions

[Lorenzo Pieralisi]

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

[PATCH 2/6] drivers: firmware: psci: add INVALID_ADDRESS return value

[Lorenzo Pieralisi]

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

[PATCH 3/6] drivers: firmware: psci: move power_state handling to generic code

[Lorenzo Pieralisi]

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

Re: [PATCH 3/6] drivers: firmware: psci: move power_state handling to generic code

[Will Deacon]

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

[PATCH 4/6] drivers: firmware: psci: add PSCI_FEATURES call

[Lorenzo Pieralisi]

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

[PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support

[Lorenzo Pieralisi]

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

Re: [PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support

[Ashwin Chaugule]

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

Re: [PATCH 5/6] drivers: firmware: psci: add extended stateid power_state support

[Lorenzo Pieralisi]

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

[PATCH 6/6] drivers: firmware: psci: add PSCI v1.0 DT bindings

[Lorenzo Pieralisi]

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