* [PATCH v2] arm64: sleep: factor sleep_save_stash slot lookup into a macro
@ 2026-07-17 11:47 Bradley Morgan
2026-08-04 13:17 ` Will Deacon
0 siblings, 1 reply; 3+ messages in thread
From: Bradley Morgan @ 2026-07-17 11:47 UTC (permalink / raw)
To: catalin.marinas, will; +Cc: linux-arm-kernel, linux-kernel, Bradley Morgan
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. 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 just remove compute_mpidr_hash
altogether.
No functional change intended.
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
Changes since V1:
- Inline the macro, there only is one user. (Will)
- Also change the C psuedocode to be correct
arch/arm64/kernel/sleep.S | 103 +++++++++++++++-----------------------
1 file changed, 41 insertions(+), 62 deletions(-)
diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
index f093cdf71be1..dcf00a4d6233 100644
--- 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 @@ 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
-
+ /* 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 @@ 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]
+ /* 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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] arm64: sleep: factor sleep_save_stash slot lookup into a macro
2026-07-17 11:47 [PATCH v2] arm64: sleep: factor sleep_save_stash slot lookup into a macro Bradley Morgan
@ 2026-08-04 13:17 ` Will Deacon
2026-08-04 14:04 ` Bradley Morgan
0 siblings, 1 reply; 3+ messages in thread
From: Will Deacon @ 2026-08-04 13:17 UTC (permalink / raw)
To: Bradley Morgan; +Cc: catalin.marinas, linux-arm-kernel, linux-kernel
On Fri, Jul 17, 2026 at 11:47:46AM +0000, Bradley Morgan wrote:
> 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. 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 just remove compute_mpidr_hash
> altogether.
>
> No functional change intended.
>
> Signed-off-by: Bradley Morgan <include@grrlz.net>
> ---
> Changes since V1:
> - Inline the macro, there only is one user. (Will)
> - Also change the C psuedocode to be correct
>
> arch/arm64/kernel/sleep.S | 103 +++++++++++++++-----------------------
> 1 file changed, 41 insertions(+), 62 deletions(-)
>
> diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
> index f093cdf71be1..dcf00a4d6233 100644
> --- 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]
nit: In the old code, rs0-rs3 were arguments to the macro. Now that you've
removed those, the comments are stale.
Will
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] arm64: sleep: factor sleep_save_stash slot lookup into a macro
2026-08-04 13:17 ` Will Deacon
@ 2026-08-04 14:04 ` Bradley Morgan
0 siblings, 0 replies; 3+ messages in thread
From: Bradley Morgan @ 2026-08-04 14:04 UTC (permalink / raw)
To: Will Deacon; +Cc: catalin.marinas, linux-arm-kernel, linux-kernel
On 4 August 2026 14:17:52 BST, Will Deacon <will@kernel.org> wrote:
>On Fri, Jul 17, 2026 at 11:47:46AM +0000, Bradley Morgan wrote:
>> 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. 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 just remove compute_mpidr_hash
>> altogether.
>>
>> No functional change intended.
>>
>> Signed-off-by: Bradley Morgan <include@grrlz.net>
>> ---
>> Changes since V1:
>> - Inline the macro, there only is one user. (Will)
>> - Also change the C psuedocode to be correct
>>
>> arch/arm64/kernel/sleep.S | 103 +++++++++++++++-----------------------
>> 1 file changed, 41 insertions(+), 62 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
>> index f093cdf71be1..dcf00a4d6233 100644
>> --- 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]
>
>nit: In the old code, rs0-rs3 were arguments to the macro. Now that you've
>removed those, the comments are stale.
>
>Will
>
fair enough, gimme 5 or 10 and I'll fix that real quick because I have
time.. thanks for the review!
Thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-04 14:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 11:47 [PATCH v2] arm64: sleep: factor sleep_save_stash slot lookup into a macro Bradley Morgan
2026-08-04 13:17 ` Will Deacon
2026-08-04 14:04 ` Bradley Morgan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox