Re: [PATCH v4 11/20] powerpc: Add ZEROIZE_GPRS macros for register clears

[Rohan McLure <rmclure@linux.ibm.com>]

> On 12 Sep 2022, at 9:09 pm, Nicholas Piggin <npiggin@gmail.com> wrote:
> 
> On Wed Aug 24, 2022 at 12:05 PM AEST, Rohan McLure wrote:
>> Macros for restoring and saving registers to and from the stack exist.
>> Provide macros with the same interface for clearing a range of gprs by
>> setting each register's value in that range to zero.
> 
> Can I bikeshed this a bit more and ask if you would change the order?
> 
> Saving and restoring macros are incidental, and are neither the change
> nor the reson for it. I think the changelog reads better if you state
> the change (or the problem) up front.
> 
> Provid register zeroing macros ... follow the same convention as
> existing register stack save/restore macros ... will be used in later
> change to zero registers.

Thanks for the suggestion, that should read better.

> 
> Or not, if you disagree.
> 
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> 
>> The resulting macros are called ZEROIZE_GPRS and ZEROIZE_NVGPRS, keeping
>> with the naming of the accompanying restore and save macros, and usage
>> of zeroize to describe this operation elsewhere in the kernel.
>> 
>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>> ---
>> V1 -> V2: Change 'ZERO' usage in naming to 'NULLIFY', a more obvious verb
>> V2 -> V3: Change 'NULLIFY' usage in naming to 'ZEROIZE', which has
>> precedent in kernel and explicitly specifies that we are zeroing.
>> V3 -> V4: Update commit message to use zeroize.
>> ---
>> arch/powerpc/include/asm/ppc_asm.h | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>> 
>> diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
>> index 83c02f5a7f2a..b95689ada59c 100644
>> --- a/arch/powerpc/include/asm/ppc_asm.h
>> +++ b/arch/powerpc/include/asm/ppc_asm.h
>> @@ -33,6 +33,20 @@
>> 	.endr
>> .endm
>> 
>> +/*
>> + * This expands to a sequence of register clears for regs start to end
>> + * inclusive, of the form:
>> + *
>> + *   li rN, 0
>> + */
>> +.macro ZEROIZE_REGS start, end
>> +	.Lreg=\start
>> +	.rept (\end - \start + 1)
>> +	li	.Lreg, 0
>> +	.Lreg=.Lreg+1
>> +	.endr
>> +.endm
>> +
>> /*
>>  * Macros for storing registers into and loading registers from
>>  * exception frames.
>> @@ -49,6 +63,14 @@
>> #define REST_NVGPRS(base)		REST_GPRS(13, 31, base)
>> #endif
>> 
>> +#define	ZEROIZE_GPRS(start, end)	ZEROIZE_REGS start, end
>> +#ifdef __powerpc64__
>> +#define	ZEROIZE_NVGPRS()		ZEROIZE_GPRS(14, 31)
>> +#else
>> +#define	ZEROIZE_NVGPRS()		ZEROIZE_GPRS(13, 31)
>> +#endif
>> +#define	ZEROIZE_GPR(n)			ZEROIZE_GPRS(n, n)
>> +
>> #define SAVE_GPR(n, base)		SAVE_GPRS(n, n, base)
>> #define REST_GPR(n, base)		REST_GPRS(n, n, base)
>> 
>> -- 
>> 2.34.1
>