Re: [PATCH v2] arm64: sleep: factor sleep_save_stash slot lookup into a macro

[Bradley Morgan]

On 4 August 2026 14:17:52 BST, Will Deacon <will at 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 at 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!