* [PATCH 0/2] arm64: sleep: two small compute_mpidr_hash cleanups @ 2026-07-05 19:23 Bradley Morgan 2026-07-05 19:23 ` [PATCH 1/2] arm64: sleep: assert compute_mpidr_hash registers are distinct Bradley Morgan 2026-07-05 19:23 ` [PATCH 2/2] arm64: sleep: dedup the sleep_save_stash slot lookup Bradley Morgan 0 siblings, 2 replies; 6+ messages in thread From: Bradley Morgan @ 2026-07-05 19:23 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Bradley Morgan Cc: linux-arm-kernel, linux-kernel compute_mpidr_hash silently produces a wrong hash if its register arguments overlap. Patch 1 turns that documented constraint into a build time error. Patch 2 factors the MPIDR hash slot lookup, open coded twice in the suspend and resume paths, into a shared macro so both callers stay in sync. No functional change intended. Bradley Morgan (2): arm64: sleep: assert compute_mpidr_hash registers are distinct arm64: sleep: dedup the sleep_save_stash slot lookup arch/arm64/kernel/sleep.S | 69 +++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 25 deletions(-) -- 2.53.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] arm64: sleep: assert compute_mpidr_hash registers are distinct 2026-07-05 19:23 [PATCH 0/2] arm64: sleep: two small compute_mpidr_hash cleanups Bradley Morgan @ 2026-07-05 19:23 ` Bradley Morgan 2026-07-05 19:23 ` [PATCH 2/2] arm64: sleep: dedup the sleep_save_stash slot lookup Bradley Morgan 1 sibling, 0 replies; 6+ messages in thread From: Bradley Morgan @ 2026-07-05 19:23 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Bradley Morgan Cc: linux-arm-kernel, linux-kernel Turn the documented register distinctness requirement into an error at build time instead of silent corruption. Signed-off-by: Bradley Morgan <include@grrlz.net> --- arch/arm64/kernel/sleep.S | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S index f093cdf71be1..e112b8537f10 100644 --- a/arch/arm64/kernel/sleep.S +++ b/arch/arm64/kernel/sleep.S @@ -35,8 +35,22 @@ * Output register: dst * Note: input and output registers must be disjoint register sets (eg: a macro instance with mpidr = x1 and dst = x1 is invalid) + This is enforced at build time by the assertions below. */ + .macro mpidr_hash_assert_distinct reg, regs:vararg + .irp r, \regs + .ifc \reg, \r + .error "compute_mpidr_hash: register arguments must be distinct" + .endif + .endr + .endm + .macro compute_mpidr_hash dst, rs0, rs1, rs2, rs3, mpidr, mask + /* \dst is written before any input is consumed */ + mpidr_hash_assert_distinct \dst, \rs0, \rs1, \rs2, \rs3, \mpidr, \mask + /* \mpidr and \mask are clobbered while other inputs are still live */ + mpidr_hash_assert_distinct \mpidr, \rs0, \rs1, \rs2, \rs3, \mask + mpidr_hash_assert_distinct \mask, \rs1, \rs2, \rs3 and \mpidr, \mpidr, \mask // mask out MPIDR bits and \dst, \mpidr, #0xff // mask=aff0 lsr \dst ,\dst, \rs0 // dst=aff0>>rs0 -- 2.53.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] arm64: sleep: dedup the sleep_save_stash slot lookup 2026-07-05 19:23 [PATCH 0/2] arm64: sleep: two small compute_mpidr_hash cleanups Bradley Morgan 2026-07-05 19:23 ` [PATCH 1/2] arm64: sleep: assert compute_mpidr_hash registers are distinct Bradley Morgan @ 2026-07-05 19:23 ` Bradley Morgan 2026-07-16 14:35 ` Will Deacon 1 sibling, 1 reply; 6+ messages in thread From: Bradley Morgan @ 2026-07-05 19:23 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Bradley Morgan Cc: linux-arm-kernel, linux-kernel Both __cpu_suspend_enter() and _cpu_resume() open code the same MPIDR hash lookup. Factor it into a get_sleep_stash_slot macro. Signed-off-by: Bradley Morgan <include@grrlz.net> --- arch/arm64/kernel/sleep.S | 55 +++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S index e112b8537f10..8820a131aef7 100644 --- a/arch/arm64/kernel/sleep.S +++ b/arch/arm64/kernel/sleep.S @@ -64,6 +64,31 @@ lsr \mask ,\mask, \rs3 orr \dst, \dst, \mask // dst|=(aff3>>rs3) .endm + +/* + * Compute the address of the current CPU's entry in sleep_save_stash, + * i.e. &sleep_save_stash[hash(MPIDR_EL1)]. + * + * @slot: output register + * + * Clobbers: x2 - x8 + */ + .macro get_sleep_stash_slot slot + mpidr_hash_assert_distinct \slot, x2 + mrs x3, mpidr_el1 + adr_l x2, mpidr_hash + ldr x8, [x2, #MPIDR_HASH_MASK] + /* + * Following code relies on the struct mpidr_hash + * members size. + */ + ldp w4, w5, [x2, #MPIDR_HASH_SHIFTS] + ldp w6, w7, [x2, #(MPIDR_HASH_SHIFTS + 8)] + compute_mpidr_hash x2, x4, x5, x6, x7, x3, x8 + ldr_l \slot, sleep_save_stash + add \slot, \slot, x2, lsl #3 + .endm + /* * Save CPU state in the provided sleep_stack_data area, and publish its * location for cpu_resume()'s use in sleep_save_stash. @@ -88,19 +113,8 @@ SYM_FUNC_START(__cpu_suspend_enter) mov x2, sp str x2, [x0, #SLEEP_STACK_DATA_SYSTEM_REGS + CPU_CTX_SP] - /* find the mpidr_hash */ - ldr_l x1, sleep_save_stash - mrs x7, mpidr_el1 - adr_l x9, mpidr_hash - ldr x10, [x9, #MPIDR_HASH_MASK] - /* - * Following code relies on the struct mpidr_hash - * members size. - */ - ldp w3, w4, [x9, #MPIDR_HASH_SHIFTS] - ldp w5, w6, [x9, #(MPIDR_HASH_SHIFTS + 8)] - compute_mpidr_hash x8, x3, x4, x5, x6, x7, x10 - add x1, x1, x8, lsl #3 + /* find this CPU's slot in sleep_save_stash */ + get_sleep_stash_slot x1 str x0, [x1] add x0, x0, #SLEEP_STACK_DATA_SYSTEM_REGS @@ -131,18 +145,9 @@ SYM_FUNC_START(_cpu_resume) mov x0, x19 bl finalise_el2 - mrs x1, mpidr_el1 - adr_l x8, mpidr_hash // x8 = struct mpidr_hash virt address - - /* retrieve mpidr_hash members to compute the hash */ - ldr x2, [x8, #MPIDR_HASH_MASK] - ldp w3, w4, [x8, #MPIDR_HASH_SHIFTS] - ldp w5, w6, [x8, #(MPIDR_HASH_SHIFTS + 8)] - compute_mpidr_hash x7, x3, x4, x5, x6, x1, x2 - - /* x7 contains hash index, let's use it to grab context pointer */ - ldr_l x0, sleep_save_stash - ldr x0, [x0, x7, lsl #3] + /* grab this CPU's context pointer from sleep_save_stash */ + get_sleep_stash_slot x0 + ldr x0, [x0] add x29, x0, #SLEEP_STACK_DATA_CALLEE_REGS add x0, x0, #SLEEP_STACK_DATA_SYSTEM_REGS /* load sp from context */ -- 2.53.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] arm64: sleep: dedup the sleep_save_stash slot lookup 2026-07-05 19:23 ` [PATCH 2/2] arm64: sleep: dedup the sleep_save_stash slot lookup Bradley Morgan @ 2026-07-16 14:35 ` Will Deacon 2026-07-16 16:55 ` Bradley Morgan 0 siblings, 1 reply; 6+ messages in thread From: Will Deacon @ 2026-07-16 14:35 UTC (permalink / raw) To: Bradley Morgan; +Cc: Catalin Marinas, linux-arm-kernel, linux-kernel On Sun, Jul 05, 2026 at 07:23:31PM +0000, Bradley Morgan wrote: > Both __cpu_suspend_enter() and _cpu_resume() open code the same MPIDR > hash lookup. Factor it into a get_sleep_stash_slot macro. > > Signed-off-by: Bradley Morgan <include@grrlz.net> > --- > arch/arm64/kernel/sleep.S | 55 +++++++++++++++++++++------------------ > 1 file changed, 30 insertions(+), 25 deletions(-) > > diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S > index e112b8537f10..8820a131aef7 100644 > --- a/arch/arm64/kernel/sleep.S > +++ b/arch/arm64/kernel/sleep.S > @@ -64,6 +64,31 @@ > lsr \mask ,\mask, \rs3 > orr \dst, \dst, \mask // dst|=(aff3>>rs3) > .endm > + > +/* > + * Compute the address of the current CPU's entry in sleep_save_stash, > + * i.e. &sleep_save_stash[hash(MPIDR_EL1)]. > + * > + * @slot: output register > + * > + * Clobbers: x2 - x8 > + */ > + .macro get_sleep_stash_slot slot > + mpidr_hash_assert_distinct \slot, x2 > + mrs x3, mpidr_el1 > + adr_l x2, mpidr_hash > + ldr x8, [x2, #MPIDR_HASH_MASK] > + /* > + * Following code relies on the struct mpidr_hash > + * members size. > + */ > + ldp w4, w5, [x2, #MPIDR_HASH_SHIFTS] > + ldp w6, w7, [x2, #(MPIDR_HASH_SHIFTS + 8)] > + compute_mpidr_hash x2, x4, x5, x6, x7, x3, x8 > + ldr_l \slot, sleep_save_stash > + add \slot, \slot, x2, lsl #3 > + .endm I think this would now be the only user of compute_mpidr_hash, so it's probably better to inline that macro here and then we don't need to bother with mpidr_hash_assert_distinct at all (i.e. your first patch isn't needed). Will ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] arm64: sleep: dedup the sleep_save_stash slot lookup 2026-07-16 14:35 ` Will Deacon @ 2026-07-16 16:55 ` Bradley Morgan 2026-07-17 11:31 ` Will Deacon 0 siblings, 1 reply; 6+ messages in thread From: Bradley Morgan @ 2026-07-16 16:55 UTC (permalink / raw) To: Will Deacon; +Cc: Catalin Marinas, linux-arm-kernel, linux-kernel On July 16, 2026 3:35:48 PM GMT+01:00, Will Deacon <will@kernel.org> wrote: >On Sun, Jul 05, 2026 at 07:23:31PM +0000, Bradley Morgan wrote: >> Both __cpu_suspend_enter() and _cpu_resume() open code the same MPIDR >> hash lookup. Factor it into a get_sleep_stash_slot macro. >> >> Signed-off-by: Bradley Morgan <include@grrlz.net> >> --- >> arch/arm64/kernel/sleep.S | 55 +++++++++++++++++++++------------------ >> 1 file changed, 30 insertions(+), 25 deletions(-) >> >> diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S >> index e112b8537f10..8820a131aef7 100644 >> --- a/arch/arm64/kernel/sleep.S >> +++ b/arch/arm64/kernel/sleep.S >> @@ -64,6 +64,31 @@ >> lsr \mask ,\mask, \rs3 >> orr \dst, \dst, \mask // dst|=(aff3>>rs3) >> .endm >> + >> +/* >> + * Compute the address of the current CPU's entry in sleep_save_stash, >> + * i.e. &sleep_save_stash[hash(MPIDR_EL1)]. >> + * >> + * @slot: output register >> + * >> + * Clobbers: x2 - x8 >> + */ >> + .macro get_sleep_stash_slot slot >> + mpidr_hash_assert_distinct \slot, x2 >> + mrs x3, mpidr_el1 >> + adr_l x2, mpidr_hash >> + ldr x8, [x2, #MPIDR_HASH_MASK] >> + /* >> + * Following code relies on the struct mpidr_hash >> + * members size. >> + */ >> + ldp w4, w5, [x2, #MPIDR_HASH_SHIFTS] >> + ldp w6, w7, [x2, #(MPIDR_HASH_SHIFTS + 8)] >> + compute_mpidr_hash x2, x4, x5, x6, x7, x3, x8 >> + ldr_l \slot, sleep_save_stash >> + add \slot, \slot, x2, lsl #3 >> + .endm > >I think this would now be the only user of compute_mpidr_hash, so it's >probably better to inline that macro here and then we don't need to >bother with mpidr_hash_assert_distinct at all (i.e. your first patch >isn't needed). > >Will > Will! What do you think of this? Subject: [PATCH v2] arm64: suspend: factor sleep_save_stash slot lookup into a macro Both __cpu_suspend_enter() and _cpu_resume() open code the same MPIDR_EL1 hash lookup to find the current CPU's slot in sleep_save_stash. Let's just factor it into a get_sleep_stash_slot macro. Since that macro would be the only remaining user of compute_mpidr_hash, inline the hash computation into get_sleep_stash_slot and remove compute_mpidr_hash altogether. No functional change intended. Signed-off-by: Bradley Morgan <include@grrlz.net> --- Changes in v2 (suggested by Will): - inline compute_mpidr_hash into get_sleep_stash_slot and drop it - drop former patch 1, it's not needed. arch/arm64/kernel/sleep.S | 62 +++++++++++++-------------------- 1 file changed, 41 insertions(+), 62 deletions(-) --- a/arch/arm64/kernel/sleep.S +++ b/arch/arm64/kernel/sleep.S @@ -7,48 +7,48 @@ .text /* - * Implementation of MPIDR_EL1 hash algorithm through shifting + * Compute the address of the current CPU's entry in sleep_save_stash, + * i.e. &sleep_save_stash[hash(MPIDR_EL1)], where the hash is an + * implementation of the MPIDR_EL1 hash algorithm through shifting * and OR'ing. * - * @dst: register containing hash result - * @rs0: register containing affinity level 0 bit shift - * @rs1: register containing affinity level 1 bit shift - * @rs2: register containing affinity level 2 bit shift - * @rs3: register containing affinity level 3 bit shift - * @mpidr: register containing MPIDR_EL1 value - * @mask: register containing MPIDR mask - * * Pseudo C-code: * - *u32 dst; + * u64 mpidr = MPIDR_EL1 & mpidr_hash.mask; + * u32 hash = ((mpidr & 0xff) >> mpidr_hash.shift_aff[0]) | + * ((mpidr & 0xff00) >> mpidr_hash.shift_aff[1]) | + * ((mpidr & 0xff0000) >> mpidr_hash.shift_aff[2]) | + * ((mpidr & 0xff00000000) >> mpidr_hash.shift_aff[3]); + * slot = &sleep_save_stash[hash]; + * + * @slot: output register * - *compute_mpidr_hash(u32 rs0, u32 rs1, u32 rs2, u32 rs3, u64 mpidr, u64 mask) { - * u32 aff0, aff1, aff2, aff3; - * u64 mpidr_masked = mpidr & mask; - * aff0 = mpidr_masked & 0xff; - * aff1 = mpidr_masked & 0xff00; - * aff2 = mpidr_masked & 0xff0000; - * aff3 = mpidr_masked & 0xff00000000; - * dst = (aff0 >> rs0 | aff1 >> rs1 | aff2 >> rs2 | aff3 >> rs3); - *} - * Input registers: rs0, rs1, rs2, rs3, mpidr, mask - * Output register: dst - * Note: input and output registers must be disjoint register sets - (eg: a macro instance with mpidr = x1 and dst = x1 is invalid) + * Clobbers: x2 - x8 */ - .macro compute_mpidr_hash dst, rs0, rs1, rs2, rs3, mpidr, mask - and \mpidr, \mpidr, \mask // mask out MPIDR bits - and \dst, \mpidr, #0xff // mask=aff0 - lsr \dst ,\dst, \rs0 // dst=aff0>>rs0 - and \mask, \mpidr, #0xff00 // mask = aff1 - lsr \mask ,\mask, \rs1 - orr \dst, \dst, \mask // dst|=(aff1>>rs1) - and \mask, \mpidr, #0xff0000 // mask = aff2 - lsr \mask ,\mask, \rs2 - orr \dst, \dst, \mask // dst|=(aff2>>rs2) - and \mask, \mpidr, #0xff00000000 // mask = aff3 - lsr \mask ,\mask, \rs3 - orr \dst, \dst, \mask // dst|=(aff3>>rs3) + .macro get_sleep_stash_slot slot + mrs x3, mpidr_el1 + adr_l x2, mpidr_hash + ldr x8, [x2, #MPIDR_HASH_MASK] + /* + * Following code relies on the struct mpidr_hash + * members size. + */ + ldp w4, w5, [x2, #MPIDR_HASH_SHIFTS] + ldp w6, w7, [x2, #(MPIDR_HASH_SHIFTS + 8)] + and x3, x3, x8 // mask out MPIDR bits + and x2, x3, #0xff // aff0 + lsr x2, x2, x4 // hash = aff0 >> rs0 + and x8, x3, #0xff00 // aff1 + lsr x8, x8, x5 + orr x2, x2, x8 // hash |= aff1 >> rs1 + and x8, x3, #0xff0000 // aff2 + lsr x8, x8, x6 + orr x2, x2, x8 // hash |= aff2 >> rs2 + and x8, x3, #0xff00000000 // aff3 + lsr x8, x8, x7 + orr x2, x2, x8 // hash |= aff3 >> rs3 + ldr_l \slot, sleep_save_stash + add \slot, \slot, x2, lsl #3 // slot = &sleep_save_stash[hash] .endm /* * Save CPU state in the provided sleep_stack_data area, and publish its @@ -74,20 +74,8 @@ mov x2, sp str x2, [x0, #SLEEP_STACK_DATA_SYSTEM_REGS + CPU_CTX_SP] - /* find the mpidr_hash */ - ldr_l x1, sleep_save_stash - mrs x7, mpidr_el1 - adr_l x9, mpidr_hash - ldr x10, [x9, #MPIDR_HASH_MASK] - /* - * Following code relies on the struct mpidr_hash - * members size. - */ - ldp w3, w4, [x9, #MPIDR_HASH_SHIFTS] - ldp w5, w6, [x9, #(MPIDR_HASH_SHIFTS + 8)] - compute_mpidr_hash x8, x3, x4, x5, x6, x7, x10 - add x1, x1, x8, lsl #3 - + /* publish this CPU's sleep_stack_data area in its stash slot */ + get_sleep_stash_slot x1 str x0, [x1] add x0, x0, #SLEEP_STACK_DATA_SYSTEM_REGS stp x29, lr, [sp, #-16]! @@ -117,18 +105,9 @@ mov x0, x19 bl finalise_el2 - mrs x1, mpidr_el1 - adr_l x8, mpidr_hash // x8 = struct mpidr_hash virt address - - /* retrieve mpidr_hash members to compute the hash */ - ldr x2, [x8, #MPIDR_HASH_MASK] - ldp w3, w4, [x8, #MPIDR_HASH_SHIFTS] - ldp w5, w6, [x8, #(MPIDR_HASH_SHIFTS + 8)] - compute_mpidr_hash x7, x3, x4, x5, x6, x1, x2 - - /* x7 contains hash index, let's use it to grab context pointer */ - ldr_l x0, sleep_save_stash - ldr x0, [x0, x7, lsl #3] + /* retrieve this CPU's context pointer from its stash slot */ + get_sleep_stash_slot x1 + ldr x0, [x1] add x29, x0, #SLEEP_STACK_DATA_CALLEE_REGS add x0, x0, #SLEEP_STACK_DATA_SYSTEM_REGS /* load sp from context */ -- 2.53.0 Thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] arm64: sleep: dedup the sleep_save_stash slot lookup 2026-07-16 16:55 ` Bradley Morgan @ 2026-07-17 11:31 ` Will Deacon 0 siblings, 0 replies; 6+ messages in thread From: Will Deacon @ 2026-07-17 11:31 UTC (permalink / raw) To: Bradley Morgan; +Cc: Catalin Marinas, linux-arm-kernel, linux-kernel On Thu, Jul 16, 2026 at 05:55:38PM +0100, Bradley Morgan wrote: > On July 16, 2026 3:35:48 PM GMT+01:00, Will Deacon <will@kernel.org> wrote: > >On Sun, Jul 05, 2026 at 07:23:31PM +0000, Bradley Morgan wrote: > >> Both __cpu_suspend_enter() and _cpu_resume() open code the same MPIDR > >> hash lookup. Factor it into a get_sleep_stash_slot macro. > >> > >> Signed-off-by: Bradley Morgan <include@grrlz.net> > >> --- > >> arch/arm64/kernel/sleep.S | 55 +++++++++++++++++++++------------------ > >> 1 file changed, 30 insertions(+), 25 deletions(-) > >> > >> diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S > >> index e112b8537f10..8820a131aef7 100644 > >> --- a/arch/arm64/kernel/sleep.S > >> +++ b/arch/arm64/kernel/sleep.S > >> @@ -64,6 +64,31 @@ > >> lsr \mask ,\mask, \rs3 > >> orr \dst, \dst, \mask // dst|=(aff3>>rs3) > >> .endm > >> + > >> +/* > >> + * Compute the address of the current CPU's entry in sleep_save_stash, > >> + * i.e. &sleep_save_stash[hash(MPIDR_EL1)]. > >> + * > >> + * @slot: output register > >> + * > >> + * Clobbers: x2 - x8 > >> + */ > >> + .macro get_sleep_stash_slot slot > >> + mpidr_hash_assert_distinct \slot, x2 > >> + mrs x3, mpidr_el1 > >> + adr_l x2, mpidr_hash > >> + ldr x8, [x2, #MPIDR_HASH_MASK] > >> + /* > >> + * Following code relies on the struct mpidr_hash > >> + * members size. > >> + */ > >> + ldp w4, w5, [x2, #MPIDR_HASH_SHIFTS] > >> + ldp w6, w7, [x2, #(MPIDR_HASH_SHIFTS + 8)] > >> + compute_mpidr_hash x2, x4, x5, x6, x7, x3, x8 > >> + ldr_l \slot, sleep_save_stash > >> + add \slot, \slot, x2, lsl #3 > >> + .endm > > > >I think this would now be the only user of compute_mpidr_hash, so it's > >probably better to inline that macro here and then we don't need to > >bother with mpidr_hash_assert_distinct at all (i.e. your first patch > >isn't needed). > > > >Will > > > > > Will! What do you think of this? I think it's been mangled by your mail client. Please just send a v2 and I'll get to it on the next pass through my inbox (which is an unholy mess right now). Will ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-17 11:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-07-05 19:23 [PATCH 0/2] arm64: sleep: two small compute_mpidr_hash cleanups Bradley Morgan 2026-07-05 19:23 ` [PATCH 1/2] arm64: sleep: assert compute_mpidr_hash registers are distinct Bradley Morgan 2026-07-05 19:23 ` [PATCH 2/2] arm64: sleep: dedup the sleep_save_stash slot lookup Bradley Morgan 2026-07-16 14:35 ` Will Deacon 2026-07-16 16:55 ` Bradley Morgan 2026-07-17 11:31 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox