* [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation
@ 2015-01-13 2:15 Zi Shen Lim
2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 1/4] psci: use MPIDR_INVALID instead of -1 Zi Shen Lim
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Zi Shen Lim @ 2015-01-13 2:15 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mark,
The first couple patches are cleanups.
The third patch switches to using v0.2 function ID values for the
existing cpu_{on,off} implementations. This intermediate step is
arguably unnecessary given the next patch...
The fourth patch provides a simple implementation of PSCI v0.2.
The deviations from spec (primarily cpu_suspend not supported)
is noted in code comment as well as in commit log.
Please consider merging.
Thanks,
z
Zi Shen Lim (4):
psci: use MPIDR_INVALID instead of -1
psci: remove sentinel from id_table
psci: use PSCI v0.2 function IDs
psci: implement PSCI v0.2
Makefile.am | 4 +-
psci.S | 170 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
2 files changed, 143 insertions(+), 31 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH boot-wrapper-aarch64 1/4] psci: use MPIDR_INVALID instead of -1 2015-01-13 2:15 [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Zi Shen Lim @ 2015-01-13 2:15 ` Zi Shen Lim 2015-01-14 11:03 ` Mark Rutland 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 2/4] psci: remove sentinel from id_table Zi Shen Lim ` (3 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Zi Shen Lim @ 2015-01-13 2:15 UTC (permalink / raw) To: linux-arm-kernel MPIDR_INVALID was already defined, so use it. Signed-off-by: Zi Shen Lim <zlim@broadcom.com> --- psci.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/psci.S b/psci.S index 856095b..234493d 100644 --- a/psci.S +++ b/psci.S @@ -126,7 +126,7 @@ psci_cpu_on: mov x0, x1 bl find_logical_id - cmp x0, #-1 + cmp x0, #MPIDR_INVALID b.eq 1f adr x3, branch_table @@ -172,7 +172,7 @@ __find_logical_index: b 1b 2: mov x0, x1 ret -3: mov x0, #-1 +3: mov x0, #MPIDR_INVALID ret setup_vector: @@ -207,7 +207,7 @@ spin: ldr x1, =MPIDR_ID_BITS and x0, x0, x1 bl find_logical_id - cmp x0, #-1 + cmp x0, #MPIDR_INVALID b.eq spin_dead adr x1, branch_table -- 2.1.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH boot-wrapper-aarch64 1/4] psci: use MPIDR_INVALID instead of -1 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 1/4] psci: use MPIDR_INVALID instead of -1 Zi Shen Lim @ 2015-01-14 11:03 ` Mark Rutland 2015-01-14 18:24 ` Z Lim 0 siblings, 1 reply; 10+ messages in thread From: Mark Rutland @ 2015-01-14 11:03 UTC (permalink / raw) To: linux-arm-kernel Hi, On Tue, Jan 13, 2015 at 02:15:01AM +0000, Zi Shen Lim wrote: > MPIDR_INVALID was already defined, so use it. MPIDR_INVALID is meant to be an invalid MPIDR value, rather than an error code in the case of an invalid MPIDR. __find_logical_index is meant to return a logical CPU ID, so returning MPIDR_INVALID isn't quite right. I'd be happier if we used a separate macro for the logical IDs (e.g. ID_INVALID). Thanks, Mark. > > Signed-off-by: Zi Shen Lim <zlim@broadcom.com> > --- > psci.S | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/psci.S b/psci.S > index 856095b..234493d 100644 > --- a/psci.S > +++ b/psci.S > @@ -126,7 +126,7 @@ psci_cpu_on: > mov x0, x1 > > bl find_logical_id > - cmp x0, #-1 > + cmp x0, #MPIDR_INVALID > b.eq 1f > > adr x3, branch_table > @@ -172,7 +172,7 @@ __find_logical_index: > b 1b > 2: mov x0, x1 > ret > -3: mov x0, #-1 > +3: mov x0, #MPIDR_INVALID > ret > > setup_vector: > @@ -207,7 +207,7 @@ spin: > ldr x1, =MPIDR_ID_BITS > and x0, x0, x1 > bl find_logical_id > - cmp x0, #-1 > + cmp x0, #MPIDR_INVALID > b.eq spin_dead > > adr x1, branch_table > -- > 2.1.0 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH boot-wrapper-aarch64 1/4] psci: use MPIDR_INVALID instead of -1 2015-01-14 11:03 ` Mark Rutland @ 2015-01-14 18:24 ` Z Lim 0 siblings, 0 replies; 10+ messages in thread From: Z Lim @ 2015-01-14 18:24 UTC (permalink / raw) To: linux-arm-kernel Hi Mark, On Wed, Jan 14, 2015 at 3:03 AM, Mark Rutland <mark.rutland@arm.com> wrote: > Hi, > > On Tue, Jan 13, 2015 at 02:15:01AM +0000, Zi Shen Lim wrote: >> MPIDR_INVALID was already defined, so use it. > > MPIDR_INVALID is meant to be an invalid MPIDR value, rather than an > error code in the case of an invalid MPIDR. > > __find_logical_index is meant to return a logical CPU ID, so returning > MPIDR_INVALID isn't quite right. > > I'd be happier if we used a separate macro for the logical IDs (e.g. > ID_INVALID). I follow your argument here. I guess I should've reordered patches 1 and 2. In patch 2, following removal of the sentinel, MPIDR_INVALID is no longer found in id_table itself. So MPIDR_INVALID is indeed the "ID_INVALID" you're suggesting. Are you comfortable with this repurposing of MPIDR_INVALID? Now, MPIDR_INVALID (-1) is consistently used as return value of find_logical_id for an invalid/unknown MPIDR. > > Thanks, > Mark. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH boot-wrapper-aarch64 2/4] psci: remove sentinel from id_table 2015-01-13 2:15 [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Zi Shen Lim 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 1/4] psci: use MPIDR_INVALID instead of -1 Zi Shen Lim @ 2015-01-13 2:15 ` Zi Shen Lim 2015-01-14 11:10 ` Mark Rutland 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 3/4] psci: use PSCI v0.2 function IDs Zi Shen Lim ` (2 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Zi Shen Lim @ 2015-01-13 2:15 UTC (permalink / raw) To: linux-arm-kernel Tweak the limit check in find_logical_id so we can do away with the sentinel from id_table. While at it, also remove unused label and fix up spacing. Signed-off-by: Zi Shen Lim <zlim@broadcom.com> --- psci.S | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/psci.S b/psci.S index 234493d..d045e56 100644 --- a/psci.S +++ b/psci.S @@ -69,7 +69,6 @@ vector: id_table: .quad CPU_IDS __id_end: - .quad MPIDR_INVALID .equ nr_cpus, ((__id_end - id_table) / 8) @@ -159,17 +158,16 @@ psci_cpu_on: * Clobbers x1, x2, x3 */ find_logical_id: -__find_logical_index: adr x2, id_table mov x1, xzr 1: mov x3, #nr_cpus // check we haven't walked off the end of the array cmp x1, x3 - b.gt 3f + b.ge 3f ldr x3, [x2, x1, lsl #3] cmp x3, x0 b.eq 2f add x1, x1, #1 - b 1b + b 1b 2: mov x0, x1 ret 3: mov x0, #MPIDR_INVALID -- 2.1.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH boot-wrapper-aarch64 2/4] psci: remove sentinel from id_table 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 2/4] psci: remove sentinel from id_table Zi Shen Lim @ 2015-01-14 11:10 ` Mark Rutland 0 siblings, 0 replies; 10+ messages in thread From: Mark Rutland @ 2015-01-14 11:10 UTC (permalink / raw) To: linux-arm-kernel Hi, On Tue, Jan 13, 2015 at 02:15:02AM +0000, Zi Shen Lim wrote: > Tweak the limit check in find_logical_id so we can > do away with the sentinel from id_table. > > While at it, also remove unused label and fix up spacing. > > Signed-off-by: Zi Shen Lim <zlim@broadcom.com> This looks like a nice cleanup to me. So long as this passes build and boot testing I'll apply this shortly. Thanks, Mark. > --- > psci.S | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/psci.S b/psci.S > index 234493d..d045e56 100644 > --- a/psci.S > +++ b/psci.S > @@ -69,7 +69,6 @@ vector: > id_table: > .quad CPU_IDS > __id_end: > - .quad MPIDR_INVALID > > .equ nr_cpus, ((__id_end - id_table) / 8) > > @@ -159,17 +158,16 @@ psci_cpu_on: > * Clobbers x1, x2, x3 > */ > find_logical_id: > -__find_logical_index: > adr x2, id_table > mov x1, xzr > 1: mov x3, #nr_cpus // check we haven't walked off the end of the array > cmp x1, x3 > - b.gt 3f > + b.ge 3f > ldr x3, [x2, x1, lsl #3] > cmp x3, x0 > b.eq 2f > add x1, x1, #1 > - b 1b > + b 1b > 2: mov x0, x1 > ret > 3: mov x0, #MPIDR_INVALID > -- > 2.1.0 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH boot-wrapper-aarch64 3/4] psci: use PSCI v0.2 function IDs 2015-01-13 2:15 [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Zi Shen Lim 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 1/4] psci: use MPIDR_INVALID instead of -1 Zi Shen Lim 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 2/4] psci: remove sentinel from id_table Zi Shen Lim @ 2015-01-13 2:15 ` Zi Shen Lim 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 4/4] psci: implement PSCI v0.2 Zi Shen Lim 2015-01-14 11:53 ` [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Mark Rutland 4 siblings, 0 replies; 10+ messages in thread From: Zi Shen Lim @ 2015-01-13 2:15 UTC (permalink / raw) To: linux-arm-kernel Update PSCI function IDs to match those defined in v0.2 of the specification. Signed-off-by: Zi Shen Lim <zlim@broadcom.com> --- Makefile.am | 4 ++-- psci.S | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index 0c174e6..4f5bfdd 100644 --- a/Makefile.am +++ b/Makefile.am @@ -25,8 +25,8 @@ BOOTMETHOD := psci.o PSCI_NODE := psci { \ compatible = \"arm,psci\"; \ method = \"smc\"; \ - cpu_on = <0x84000002>; \ - cpu_off = <0x84000001>; \ + cpu_on = <0xc4000003>; \ + cpu_off = <0x84000002>; \ }; CPU_NODES := $(shell $(top_srcdir)/gen-cpu-nodes.sh $(CPU_IDS)) CPUS_NODE := cpus { \ diff --git a/psci.S b/psci.S index d045e56..c51e125 100644 --- a/psci.S +++ b/psci.S @@ -8,8 +8,8 @@ */ #include "common.S" -#define PSCI_CPU_OFF 0x84000001 -#define PSCI_CPU_ON 0x84000002 +#define PSCI_CPU_OFF 0x84000002 +#define PSCI_CPU_ON 0xc4000003 #define PSCI_RET_SUCCESS 0 #define PSCI_RET_NOT_IMPL (-1) -- 2.1.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH boot-wrapper-aarch64 4/4] psci: implement PSCI v0.2 2015-01-13 2:15 [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Zi Shen Lim ` (2 preceding siblings ...) 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 3/4] psci: use PSCI v0.2 function IDs Zi Shen Lim @ 2015-01-13 2:15 ` Zi Shen Lim 2015-01-14 11:53 ` [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Mark Rutland 4 siblings, 0 replies; 10+ messages in thread From: Zi Shen Lim @ 2015-01-13 2:15 UTC (permalink / raw) To: linux-arm-kernel In this simple implementation: * cpu_suspend is not supported, and as such is not fully compliant with the spec. * The system_{off,reset} functions currently just spin on one CPU, when it should instead power {down,cycle} the system. Signed-off-by: Zi Shen Lim <zlim@broadcom.com> --- Makefile.am | 4 +- psci.S | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 138 insertions(+), 24 deletions(-) diff --git a/Makefile.am b/Makefile.am index 4f5bfdd..471462a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -23,10 +23,8 @@ BOOTLOADER := boot.S if PSCI BOOTMETHOD := psci.o PSCI_NODE := psci { \ - compatible = \"arm,psci\"; \ + compatible = \"arm,psci-0.2\"; \ method = \"smc\"; \ - cpu_on = <0xc4000003>; \ - cpu_off = <0x84000002>; \ }; CPU_NODES := $(shell $(top_srcdir)/gen-cpu-nodes.sh $(CPU_IDS)) CPUS_NODE := cpus { \ diff --git a/psci.S b/psci.S index c51e125..b8afb0e 100644 --- a/psci.S +++ b/psci.S @@ -8,13 +8,26 @@ */ #include "common.S" -#define PSCI_CPU_OFF 0x84000002 -#define PSCI_CPU_ON 0xc4000003 - -#define PSCI_RET_SUCCESS 0 -#define PSCI_RET_NOT_IMPL (-1) -#define PSCI_RET_INVALID (-2) -#define PSCI_RET_DENIED (-3) +#define PSCI_VERSION 0x84000000 +#define PSCI_CPU_SUSPEND 0xc4000001 +#define PSCI_CPU_OFF 0x84000002 +#define PSCI_CPU_ON 0xc4000003 +#define PSCI_AFFINITY_INFO 0xc4000004 +#define PSCI_MIGRATE 0xc4000005 +#define PSCI_MIGRATE_INFO_TYPE 0x84000006 +#define PSCI_MIGRATE_INFO_UP_CPU 0xc4000007 +#define PSCI_SYSTEM_OFF 0x84000008 +#define PSCI_SYSTEM_RESET 0x84000009 + +#define PSCI_RET_SUCCESS 0 +#define PSCI_RET_NOT_SUPPORTED (-1) +#define PSCI_RET_INVALID_PARAMETERS (-2) +#define PSCI_RET_DENIED (-3) +#define PSCI_RET_ALREADY_ON (-4) +#define PSCI_RET_ON_PENDING (-5) +#define PSCI_RET_INTERNAL_FAILURE (-6) +#define PSCI_RET_NOT_PRESENT (-7) +#define PSCI_RET_DISABLED (-8) #ifndef CPU_IDS #error No CPU MPIDRs provided. @@ -77,6 +90,16 @@ branch_table: .quad ADDR_INVALID .endr +#define AFFINFO_ON 0 +#define AFFINFO_OFF 1 +#define AFFINFO_ON_PENDING 2 +#define AFFINFO_DISABLED PSCI_RET_DISABLED + +affinfo_table: + .rept (nr_cpus) + .quad AFFINFO_DISABLED + .endr + .text .globl start_no_el3 @@ -86,10 +109,18 @@ err_exception: b err_exception psci_call32: - mov w0, PSCI_RET_NOT_IMPL + mov w0, PSCI_RET_NOT_SUPPORTED eret psci_call64: + ldr x7, =PSCI_VERSION + cmp x0, x7 + b.eq psci_version + + ldr x7, =PSCI_CPU_SUSPEND + cmp x0, x7 + b.eq psci_not_supported // XXX: not spec compliant + ldr x7, =PSCI_CPU_OFF cmp x0, x7 b.eq psci_cpu_off @@ -98,7 +129,36 @@ psci_call64: cmp x0, x7 b.eq psci_cpu_on - mov x0, PSCI_RET_NOT_IMPL + ldr x7, =PSCI_AFFINITY_INFO + cmp x0, x7 + b.eq psci_affinity_info + + ldr x7, =PSCI_MIGRATE + cmp x0, x7 + b.eq psci_not_supported + + ldr x7, =PSCI_MIGRATE_INFO_TYPE + cmp x0, x7 + b.eq psci_migrate_info_type + + ldr x7, =PSCI_MIGRATE_INFO_UP_CPU + cmp x0, x7 + b.eq psci_not_supported + + ldr x7, =PSCI_SYSTEM_OFF + cmp x0, x7 + b.eq psci_system_off + + ldr x7, =PSCI_SYSTEM_RESET + cmp x0, x7 + b.eq psci_system_reset + +psci_not_supported: + mov x0, PSCI_RET_NOT_SUPPORTED + eret + +psci_version: + mov x0, #((0 << 16) | (2 << 0)) eret /* @@ -109,6 +169,11 @@ psci_cpu_off: ldr x1, =MPIDR_ID_BITS and x0, x0, x1 bl find_logical_id + + adr x1, affinfo_table + mov x2, #AFFINFO_OFF + str x2, [x1, x0, lsl #3] + adr x1, branch_table mov x2, #ADDR_INVALID str x2, [x1, x0, lsl #3] @@ -118,39 +183,85 @@ psci_cpu_off: /* * x1 - target cpu * x2 - address + * x3 - context id (Note: currently ignored) */ psci_cpu_on: - mov x15, x30 mov x14, x2 - mov x0, x1 + mov x0, x1 + mov x15, x30 bl find_logical_id + mov x30, x15 cmp x0, #MPIDR_INVALID b.eq 1f + adr x1, affinfo_table + ldr x2, [x1, x0, lsl #3] + cmp x2, #AFFINFO_ON + b.eq 2f + cmp x2, #AFFINFO_ON_PENDING + b.eq 3f + cmp x2, #AFFINFO_DISABLED + b.eq 1f + + mov x2, #AFFINFO_ON_PENDING + str x2, [x1, x0, lsl #3] + adr x3, branch_table add x3, x3, x0, lsl #3 - ldr x4, =ADDR_INVALID - ldxr x5, [x3] - cmp x4, x5 - b.ne 1f + cmp x5, #ADDR_INVALID + b.ne 2f stxr w4, x14, [x3] - cbnz w4, 1f + cbnz w4, 2f dsb ishst sev mov x0, #PSCI_RET_SUCCESS - mov x30, x15 eret -1: mov x0, #PSCI_RET_DENIED +1: mov x0, #PSCI_RET_INVALID_PARAMETERS + eret + +2: mov x0, #PSCI_RET_ALREADY_ON + eret + +3: mov x0, #PSCI_RET_ON_PENDING + eret + +/* + * x1 - target affinity (same as target cpu) + * x2 - lowest affinity level (Note: only 0 is supported) + */ +psci_affinity_info: + cbnz x2, 1f + + mov x0, x1 + mov x15, x30 + bl find_logical_id mov x30, x15 + cmp x0, #MPIDR_INVALID + b.eq 1f + + adr x3, affinfo_table + ldr x0, [x3, x0, lsl #3] eret +1: mov x0, #PSCI_RET_INVALID_PARAMETERS + eret + +psci_migrate_info_type: + mov x0, #2 // Trusted OS not present, doesn't require migration. + eret + +psci_system_off: + b spin_dead // XXX: need to power down system + +psci_system_reset: + b spin_dead // XXX: need to power cycle system /* * Takes masked MPIDR in x0, returns logical id in x0 @@ -208,16 +319,21 @@ spin: cmp x0, #MPIDR_INVALID b.eq spin_dead - adr x1, branch_table - mov x3, #ADDR_INVALID + adr x15, affinfo_table + mov x14, #AFFINFO_OFF + str x14, [x15, x0, lsl #3] + adr x1, branch_table add x1, x1, x0, lsl #3 1: wfe ldr x2, [x1] - cmp x2, x3 + cmp x2, #ADDR_INVALID b.eq 1b + mov x14, #AFFINFO_ON + str x14, [x15, x0, lsl #3] + ldr x0, =SCTLR_EL2_RESET msr sctlr_el2, x0 -- 2.1.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation 2015-01-13 2:15 [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Zi Shen Lim ` (3 preceding siblings ...) 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 4/4] psci: implement PSCI v0.2 Zi Shen Lim @ 2015-01-14 11:53 ` Mark Rutland 2015-01-14 18:46 ` Z Lim 4 siblings, 1 reply; 10+ messages in thread From: Mark Rutland @ 2015-01-14 11:53 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jan 13, 2015 at 02:15:00AM +0000, Zi Shen Lim wrote: > Hi Mark, Hi, > The first couple patches are cleanups. > > The third patch switches to using v0.2 function ID values for the > existing cpu_{on,off} implementations. This intermediate step is > arguably unnecessary given the next patch... > > The fourth patch provides a simple implementation of PSCI v0.2. > The deviations from spec (primarily cpu_suspend not supported) > is noted in code comment as well as in commit log. While I'm not opposed to implementing PSCI 0.2, the two major reasons for not having implemented it so far were simplicity and spec-compliance. Unless implementing PSCI 0.2 gains us something, I don't see the point in moving from PSCI 0.1. I think not having any supported CPU_SUSPEND states is fine. For spec compliance I'm more worried by the behaviour of AFFINITY_INFO (as this implementation seems to ignore the lowest_affinity_level parameter), and the behaviour of SYSTEM_{OFF,RESET} -- I wonder if these could be implemented via semihosting. Given that SYSTEM_OFF and SYSTEM_RESET are effectively not implemented, the only thing that I see we gain from this PSCI 0.2 implementation is AFFINITY_INFO to close a race with CPU_OFF. Is PSCI 0.2 just a nice-to-have feature, or is there something it provides that you particularly want? I occasionally need to use a new boot-wrapper with old kernels which only handle PSCI 0.1, so we also need to maintain compatiblity in the way the DT binding doc describes: psci { compatible = "arm,psci-0.2", "arm,psci"; method = "smc"; cpu_on = < ... >; cpu_off = < ... >; }; Thanks, Mark. > > Please consider merging. > > Thanks, z > > Zi Shen Lim (4): psci: use MPIDR_INVALID instead of -1 psci: remove > sentinel from id_table psci: use PSCI v0.2 function IDs psci: > implement PSCI v0.2 > > Makefile.am | 4 +- psci.S | 170 > ++++++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files > changed, 143 insertions(+), 31 deletions(-) > > -- 2.1.0 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation 2015-01-14 11:53 ` [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Mark Rutland @ 2015-01-14 18:46 ` Z Lim 0 siblings, 0 replies; 10+ messages in thread From: Z Lim @ 2015-01-14 18:46 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jan 14, 2015 at 3:53 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Tue, Jan 13, 2015 at 02:15:00AM +0000, Zi Shen Lim wrote: >> Hi Mark, > > Hi, > >> The first couple patches are cleanups. >> >> The third patch switches to using v0.2 function ID values for the >> existing cpu_{on,off} implementations. This intermediate step is >> arguably unnecessary given the next patch... >> >> The fourth patch provides a simple implementation of PSCI v0.2. >> The deviations from spec (primarily cpu_suspend not supported) >> is noted in code comment as well as in commit log. > > While I'm not opposed to implementing PSCI 0.2, the two major reasons > for not having implemented it so far were simplicity and > spec-compliance. Unless implementing PSCI 0.2 gains us something, I > don't see the point in moving from PSCI 0.1. I'm in full agreement here. I don't think we'd want boot-wrapper to become overly complicated - at some point we might as well use ARM Trusted Firmware. > > I think not having any supported CPU_SUSPEND states is fine. For spec Okay :) > compliance I'm more worried by the behaviour of AFFINITY_INFO (as this > implementation seems to ignore the lowest_affinity_level parameter), and The simple implementation of psci_affinity_info only supports lowest_affinity_level==0. Motivation: the known user of this (arch/arm64/kernel/psci.c) only uses lowest_affinity_level=0. Do we need something more complex? > the behaviour of SYSTEM_{OFF,RESET} -- I wonder if these could be > implemented via semihosting. Good point. Haven't thought about that option. I also wonder if linux will use PSCI for these functions. > > Given that SYSTEM_OFF and SYSTEM_RESET are effectively not implemented, > the only thing that I see we gain from this PSCI 0.2 implementation is > AFFINITY_INFO to close a race with CPU_OFF. Is PSCI 0.2 just a > nice-to-have feature, or is there something it provides that you > particularly want? I'm currently using this as baseline for PSCI 0.2 related work. I have other (non-FVP) platform-specific changes as well, but figured at least the generic/common portions could be shared upstream. > > I occasionally need to use a new boot-wrapper with old kernels which > only handle PSCI 0.1, so we also need to maintain compatiblity in the > way the DT binding doc describes: > > psci { > compatible = "arm,psci-0.2", "arm,psci"; > method = "smc"; > cpu_on = < ... >; > cpu_off = < ... >; > }; Ok. In that case, along with the DT compatibility above, we'll also need patch 3. > > Thanks, > Mark. > >> >> Please consider merging. >> >> Thanks, z >> >> Zi Shen Lim (4): psci: use MPIDR_INVALID instead of -1 psci: remove >> sentinel from id_table psci: use PSCI v0.2 function IDs psci: >> implement PSCI v0.2 >> >> Makefile.am | 4 +- psci.S | 170 >> ++++++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files >> changed, 143 insertions(+), 31 deletions(-) >> >> -- 2.1.0 >> >> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-01-14 18:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-13 2:15 [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Zi Shen Lim 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 1/4] psci: use MPIDR_INVALID instead of -1 Zi Shen Lim 2015-01-14 11:03 ` Mark Rutland 2015-01-14 18:24 ` Z Lim 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 2/4] psci: remove sentinel from id_table Zi Shen Lim 2015-01-14 11:10 ` Mark Rutland 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 3/4] psci: use PSCI v0.2 function IDs Zi Shen Lim 2015-01-13 2:15 ` [PATCH boot-wrapper-aarch64 4/4] psci: implement PSCI v0.2 Zi Shen Lim 2015-01-14 11:53 ` [PATCH boot-wrapper-aarch64 0/4] psci cleanups + simple v0.2 implementation Mark Rutland 2015-01-14 18:46 ` Z Lim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).