| Message ID | 20220824020548.62625-12-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: Syscall wrapper and register clearing | expand |
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. 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
> 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 >
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)
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. 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(+)