| Message ID | 2c6e83581b4fa434aa7cf2fa7714c41e98f57007.1615398265.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc: Cleanup of uaccess.h and adding asm goto for get_user() | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (91966823812efbd175f904599e5cf2a854b39809) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 1 checks, 80 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Hi Christophe, > Those helpers use get_user helpers but they don't participate > in their implementation, so they do not belong to asm/uaccess.h > > Move them in asm/inst.h Hmm, is asm/inst.h the right place for this? asm/inst.h seems to be entirely concerned with the ppc_inst type: converting things to and from ppc_inst, print ppc_inst as a string, dealing with prefixed instructs, etc., etc. The only things currently that look at memory are the probe_user_read_inst and probe_kernel_read_inst prototypes... Having said that, I'm not sure quite where else to put it, and none of the other places in arch/powerpc/include that currently reference ppc_inst seem any better... If we do use asm/inst.h, I think maybe it makes sense to put the code towards the end rather than at the start, as uses structs and calls macros that are defined later on in the function. Kind regards, Daniel > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/include/asm/inst.h | 34 ++++++++++++++++++++++++++++++ > arch/powerpc/include/asm/uaccess.h | 34 ------------------------------ > 2 files changed, 34 insertions(+), 34 deletions(-) > > diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h > index cc73c1267572..19e18af2fac9 100644 > --- a/arch/powerpc/include/asm/inst.h > +++ b/arch/powerpc/include/asm/inst.h > @@ -4,6 +4,40 @@ > > #include <asm/ppc-opcode.h> > > +#ifdef CONFIG_PPC64 > + > +#define ___get_user_instr(gu_op, dest, ptr) \ > +({ \ > + long __gui_ret = 0; \ > + unsigned long __gui_ptr = (unsigned long)ptr; \ > + struct ppc_inst __gui_inst; \ > + unsigned int __prefix, __suffix; \ > + __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \ > + if (__gui_ret == 0) { \ > + if ((__prefix >> 26) == OP_PREFIX) { \ > + __gui_ret = gu_op(__suffix, \ > + (unsigned int __user *)__gui_ptr + 1); \ > + __gui_inst = ppc_inst_prefix(__prefix, \ > + __suffix); \ > + } else { \ > + __gui_inst = ppc_inst(__prefix); \ > + } \ > + if (__gui_ret == 0) \ > + (dest) = __gui_inst; \ > + } \ > + __gui_ret; \ > +}) > +#else /* !CONFIG_PPC64 */ > +#define ___get_user_instr(gu_op, dest, ptr) \ > + gu_op((dest).val, (u32 __user *)(ptr)) > +#endif /* CONFIG_PPC64 */ > + > +#define get_user_instr(x, ptr) \ > + ___get_user_instr(get_user, x, ptr) > + > +#define __get_user_instr(x, ptr) \ > + ___get_user_instr(__get_user, x, ptr) > + > /* > * Instruction data type for POWER > */ > diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h > index 01aea0df4dd0..eaa828a6a419 100644 > --- a/arch/powerpc/include/asm/uaccess.h > +++ b/arch/powerpc/include/asm/uaccess.h > @@ -53,40 +53,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size) > #define __put_user(x, ptr) \ > __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) > > -#ifdef CONFIG_PPC64 > - > -#define ___get_user_instr(gu_op, dest, ptr) \ > -({ \ > - long __gui_ret = 0; \ > - unsigned long __gui_ptr = (unsigned long)ptr; \ > - struct ppc_inst __gui_inst; \ > - unsigned int __prefix, __suffix; \ > - __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \ > - if (__gui_ret == 0) { \ > - if ((__prefix >> 26) == OP_PREFIX) { \ > - __gui_ret = gu_op(__suffix, \ > - (unsigned int __user *)__gui_ptr + 1); \ > - __gui_inst = ppc_inst_prefix(__prefix, \ > - __suffix); \ > - } else { \ > - __gui_inst = ppc_inst(__prefix); \ > - } \ > - if (__gui_ret == 0) \ > - (dest) = __gui_inst; \ > - } \ > - __gui_ret; \ > -}) > -#else /* !CONFIG_PPC64 */ > -#define ___get_user_instr(gu_op, dest, ptr) \ > - gu_op((dest).val, (u32 __user *)(ptr)) > -#endif /* CONFIG_PPC64 */ > - > -#define get_user_instr(x, ptr) \ > - ___get_user_instr(get_user, x, ptr) > - > -#define __get_user_instr(x, ptr) \ > - ___get_user_instr(__get_user, x, ptr) > - > extern long __put_user_bad(void); > > #define __put_user_size(x, ptr, size, retval) \ > -- > 2.25.0
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h index cc73c1267572..19e18af2fac9 100644 --- a/arch/powerpc/include/asm/inst.h +++ b/arch/powerpc/include/asm/inst.h @@ -4,6 +4,40 @@ #include <asm/ppc-opcode.h> +#ifdef CONFIG_PPC64 + +#define ___get_user_instr(gu_op, dest, ptr) \ +({ \ + long __gui_ret = 0; \ + unsigned long __gui_ptr = (unsigned long)ptr; \ + struct ppc_inst __gui_inst; \ + unsigned int __prefix, __suffix; \ + __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \ + if (__gui_ret == 0) { \ + if ((__prefix >> 26) == OP_PREFIX) { \ + __gui_ret = gu_op(__suffix, \ + (unsigned int __user *)__gui_ptr + 1); \ + __gui_inst = ppc_inst_prefix(__prefix, \ + __suffix); \ + } else { \ + __gui_inst = ppc_inst(__prefix); \ + } \ + if (__gui_ret == 0) \ + (dest) = __gui_inst; \ + } \ + __gui_ret; \ +}) +#else /* !CONFIG_PPC64 */ +#define ___get_user_instr(gu_op, dest, ptr) \ + gu_op((dest).val, (u32 __user *)(ptr)) +#endif /* CONFIG_PPC64 */ + +#define get_user_instr(x, ptr) \ + ___get_user_instr(get_user, x, ptr) + +#define __get_user_instr(x, ptr) \ + ___get_user_instr(__get_user, x, ptr) + /* * Instruction data type for POWER */ diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h index 01aea0df4dd0..eaa828a6a419 100644 --- a/arch/powerpc/include/asm/uaccess.h +++ b/arch/powerpc/include/asm/uaccess.h @@ -53,40 +53,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size) #define __put_user(x, ptr) \ __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) -#ifdef CONFIG_PPC64 - -#define ___get_user_instr(gu_op, dest, ptr) \ -({ \ - long __gui_ret = 0; \ - unsigned long __gui_ptr = (unsigned long)ptr; \ - struct ppc_inst __gui_inst; \ - unsigned int __prefix, __suffix; \ - __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \ - if (__gui_ret == 0) { \ - if ((__prefix >> 26) == OP_PREFIX) { \ - __gui_ret = gu_op(__suffix, \ - (unsigned int __user *)__gui_ptr + 1); \ - __gui_inst = ppc_inst_prefix(__prefix, \ - __suffix); \ - } else { \ - __gui_inst = ppc_inst(__prefix); \ - } \ - if (__gui_ret == 0) \ - (dest) = __gui_inst; \ - } \ - __gui_ret; \ -}) -#else /* !CONFIG_PPC64 */ -#define ___get_user_instr(gu_op, dest, ptr) \ - gu_op((dest).val, (u32 __user *)(ptr)) -#endif /* CONFIG_PPC64 */ - -#define get_user_instr(x, ptr) \ - ___get_user_instr(get_user, x, ptr) - -#define __get_user_instr(x, ptr) \ - ___get_user_instr(__get_user, x, ptr) - extern long __put_user_bad(void); #define __put_user_size(x, ptr, size, retval) \
Those helpers use get_user helpers but they don't participate in their implementation, so they do not belong to asm/uaccess.h Move them in asm/inst.h Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- arch/powerpc/include/asm/inst.h | 34 ++++++++++++++++++++++++++++++ arch/powerpc/include/asm/uaccess.h | 34 ------------------------------ 2 files changed, 34 insertions(+), 34 deletions(-)