Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
Hey mpe, fixes for the issues highlighted by Christophe, except KUAP
as discussed. Will make the optprobe change as a preceding patch.
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -11,9 +11,9 @@
struct ppc_inst {
u32 val;
-#ifdef __powerpc64__
+#ifdef CONFIG_PPC64
u32 suffix;
-#endif /* __powerpc64__ */
+#endif /* CONFIG_PPC64 */
} __packed;
static inline u32 ppc_inst_val(struct ppc_inst x)
@@ -26,7 +26,7 @@ static inline int ppc_inst_primary_opcode(struct ppc_inst x)
return get_op(ppc_inst_val(x));
}
-#ifdef __powerpc64__
+#ifdef CONFIG_PPC64
#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix = (y) })
@@ -52,7 +52,7 @@ static inline struct ppc_inst ppc_inst_read(const
struct ppc_inst *ptr)
u32 val, suffix;
val = *(u32 *)ptr;
-if ((val >> 26) == 1) {
+if ((get_op(val)) == OP_PREFIX) {
suffix = *((u32 *)ptr + 1);
return ppc_inst_prefix(val, suffix);
} else {
@@ -94,7 +94,7 @@ static inline bool ppc_inst_equal(struct ppc_inst x,
struct ppc_inst y)
return ppc_inst_val(x) == ppc_inst_val(y);
}
-#endif /* __powerpc64__ */
+#endif /* CONFIG_PPC64 */
static inline int ppc_inst_len(struct ppc_inst x)
{
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
index e9027b3c641a..ac36a82321d4 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -105,7 +105,7 @@ static inline int __access_ok(unsigned long addr,
unsigned long size,
#define __put_user_inatomic(x, ptr) \
__put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
-#ifdef __powerpc64__
+#ifdef CONFIG_PPC64
#define __get_user_instr(x, ptr)\
({\
long __gui_ret = 0;\
@@ -113,7 +113,7 @@ static inline int __access_ok(unsigned long addr,
unsigned long size,
struct ppc_inst __gui_inst;\
unsigned int prefix, suffix;\
__gui_ret = __get_user(prefix, (unsigned int __user *)__gui_ptr);\
-if (!__gui_ret && (prefix >> 26) == OP_PREFIX) {\
+if (!__gui_ret && (get_op(prefix)) == OP_PREFIX) {\
__gui_ret = __get_user(suffix,\
(unsigned int __user *)__gui_ptr + 1);\
__gui_inst = ppc_inst_prefix(prefix, suffix);\
@@ -131,7 +131,7 @@ static inline int __access_ok(unsigned long addr,
unsigned long size,
struct ppc_inst __gui_inst;\
unsigned int prefix, suffix;\
__gui_ret = __get_user_inatomic(prefix, (unsigned int __user
*)__gui_ptr);\
-if (!__gui_ret && (prefix >> 26) == OP_PREFIX) {\
+if (!__gui_ret && (get_op(prefix)) == OP_PREFIX) {\
__gui_ret = __get_user_inatomic(suffix,\
(unsigned int __user *)__gui_ptr + 1);\
__gui_inst = ppc_inst_prefix(prefix, suffix);\
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index a8e66603d12b..3ac105e7faae 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -283,10 +283,8 @@ int arch_prepare_optimized_kprobe(struct
optimized_kprobe *op, struct kprobe *p)
* 3. load instruction to be emulated into relevant register, and
*/
temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
-patch_imm64_load_insns(ppc_inst_val(temp) |
- ((u64)ppc_inst_suffix(temp) << 32),
- 4,
- buff + TMPL_INSN_IDX);
+patch_imm64_load_insns(ppc_inst_val(temp) |
((u64)ppc_inst_suffix(temp) << 32),
+ 4, buff + TMPL_INSN_IDX);
/*
* 4. branch back from trampoline
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 58b67b62d5d3..bfd4e1dae0fb 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -26,8 +26,6 @@ static int __patch_instruction(struct ppc_inst
*exec_addr, struct ppc_inst instr
if (!ppc_inst_prefixed(instr)) {
__put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
-if (err)
-return err;
} else {
#ifdef CONFIG_CPU_LITTLE_ENDIAN
__put_user_asm((u64)ppc_inst_suffix(instr) << 32 |
@@ -36,12 +34,13 @@ static int __patch_instruction(struct ppc_inst
*exec_addr, struct ppc_inst instr
__put_user_asm((u64)ppc_inst_val(instr) << 32 |
ppc_inst_suffix(instr), patch_addr, err, "std");
#endif /* CONFIG_CPU_LITTLE_ENDIAN */
-if (err)
-return err;
}
+if (err)
+return err;
asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
"r" (exec_addr));
+
return 0;
}
diff --git a/arch/powerpc/lib/inst.c
Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
Christophe Leroy writes:
> Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
>> For powerpc64, redefine the ppc_inst type so both word and prefixed
>> instructions can be represented. On powerpc32 the type will remain the
>> same. Update places which had assumed instructions to be 4 bytes long.
...
>> diff --git a/arch/powerpc/include/asm/uaccess.h
>> b/arch/powerpc/include/asm/uaccess.h
>> index c0a35e4586a5..217897927926 100644
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -105,11 +105,49 @@ static inline int __access_ok(unsigned long addr,
>> unsigned long size,
>> #define __put_user_inatomic(x, ptr) \
>> __put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
>>
>> +#ifdef __powerpc64__
>
> Replace by CONFIG_PPC64
>
>> +#define __get_user_instr(x, ptr)\
>> +({ \
>> +long __gui_ret = 0; \
>> +unsigned long __gui_ptr = (unsigned long)ptr; \
>> +struct ppc_inst __gui_inst; \
>> +unsigned int prefix, suffix;\
>> +__gui_ret = __get_user(prefix, (unsigned int __user *)__gui_ptr);
>> \
>
> __get_user() can be costly especially with KUAP. I think you should
> perform a 64 bits read and fallback on a 32 bits read only if the 64
> bits read failed.
I worry that might break something.
It _should_ be safe, but I'm paranoid.
If we think the KUAP overhead is a problem then I think we'd be better
off pulling the KUAP disable/enable into this macro.
>> diff --git a/arch/powerpc/lib/feature-fixups.c
>> b/arch/powerpc/lib/feature-fixups.c
>> index 2bd2b752de4f..a8238eff3a31 100644
>> --- a/arch/powerpc/lib/feature-fixups.c
>> +++ b/arch/powerpc/lib/feature-fixups.c
>> @@ -84,12 +84,13 @@ static int patch_feature_section(unsigned long value,
>> struct fixup_entry *fcur)
>> src = alt_start;
>> dest = start;
>>
>> -for (; src < alt_end; src++, dest++) {
>> +for (; src < alt_end; src = (void *)src +
>> ppc_inst_len(ppc_inst_read(src)),
>> + (dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest {
>
> Can we do this outside the for() for readability ?
I have an idea for cleaning these up, will post it as a follow-up to the series.
>> diff --git a/arch/powerpc/lib/inst.c b/arch/powerpc/lib/inst.c
>> index 08dedd927268..eb6f9ee28ac6 100644
>> --- a/arch/powerpc/lib/inst.c
>> +++ b/arch/powerpc/lib/inst.c
>> @@ -3,9 +3,49 @@
>>* Copyright 2020, IBM Corporation.
>>*/
>>
>> +#include
>> #include
>> #include
>>
>> +#ifdef __powerpc64__
>> +int probe_user_read_inst(struct ppc_inst *inst,
>> + struct ppc_inst *nip)
>> +{
>> +unsigned int val, suffix;
>> +int err;
>> +
>> +err = probe_user_read(, nip, sizeof(val));
>
> A user read is costly with KUAP. Can we do a 64 bits read and perform a
> 32 bits read only when 64 bits read fails ?
Same comment as above.
cheers
Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
Le 14/05/2020 à 14:06, Alistair Popple a écrit : On Thursday, 14 May 2020 4:11:43 PM AEST Christophe Leroy wrote: @@ -249,7 +249,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p) * Fixup the template with instructions to: * 1. load the address of the actual probepoint */ - patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX); + patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX); /* * 2. branch to optimized_callback() and emulate_step() @@ -282,7 +282,11 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p) /* * 3. load instruction to be emulated into relevant register, and */ - patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX); + temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn); + patch_imm64_load_insns(ppc_inst_val(temp) | + ((u64)ppc_inst_suffix(temp) << 32), + 4, So now we are also using r4 ? Any explanation somewhere on the way it works ? This change seems unrelated to this patch, nothing in the description about it. Can we suddenly use a new register without problem ? Unless I missed something there is no change in register usage here that I could see. patch_imm32_load_insns() was/is hardcoded to use register r4. Ah ... Euh ... Ok I missed the change from patch_imm32_load_insns() to patch_imm64_load_insns(), I'll check again.
Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
On Thu, May 14, 2020 at 10:06 PM Alistair Popple wrote: > > On Thursday, 14 May 2020 4:11:43 PM AEST Christophe Leroy wrote: > > @@ -249,7 +249,7 @@ int arch_prepare_optimized_kprobe(struct > > optimized_kprobe *op, struct kprobe *p) > > > * Fixup the template with instructions to: > > > * 1. load the address of the actual probepoint > > > */ > > > - patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX); > > > + patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX); > > > > > > /* > > > * 2. branch to optimized_callback() and emulate_step() > > > @@ -282,7 +282,11 @@ int arch_prepare_optimized_kprobe(struct > > > optimized_kprobe *op, struct kprobe *p) /* > > > * 3. load instruction to be emulated into relevant register, and > > > */ > > > - patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX); > > > + temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn); > > > + patch_imm64_load_insns(ppc_inst_val(temp) | > > > + ((u64)ppc_inst_suffix(temp) << 32), > > > + 4, > > > > So now we are also using r4 ? Any explanation somewhere on the way it > > works ? This change seems unrelated to this patch, nothing in the > > description about it. Can we suddenly use a new register without problem ? > > Unless I missed something there is no change in register usage here that I > could see. patch_imm32_load_insns() was/is hardcoded to use register r4. Yes, that is right. > > - Alistair > >
Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
On Thu, May 14, 2020 at 4:12 PM Christophe Leroy
wrote:
>
>
>
> Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> > For powerpc64, redefine the ppc_inst type so both word and prefixed
> > instructions can be represented. On powerpc32 the type will remain the
> > same. Update places which had assumed instructions to be 4 bytes long.
> >
> > Reviewed-by: Alistair Popple
> > Signed-off-by: Jordan Niethe
> > ---
> > v4: New to series
> > v5: - Distinguish normal instructions from prefixed instructions with a
> > 0xff marker for the suffix.
> > - __patch_instruction() using std for prefixed instructions
> > v6: - Return false instead of 0 in ppc_inst_prefixed()
> > - Fix up types for ppc32 so it compiles
> > - remove ppc_inst_write()
> > - __patching_instruction(): move flush out of condition
> > v8: - style
> > - Define and use OP_PREFIX instead of '1' (back from v3)
> > - __patch_instruction() fix for big endian
> > ---
> > arch/powerpc/include/asm/inst.h | 69 ---
> > arch/powerpc/include/asm/kprobes.h| 2 +-
> > arch/powerpc/include/asm/ppc-opcode.h | 3 ++
> > arch/powerpc/include/asm/uaccess.h| 40 +++-
> > arch/powerpc/include/asm/uprobes.h| 2 +-
> > arch/powerpc/kernel/crash_dump.c | 2 +-
> > arch/powerpc/kernel/optprobes.c | 42
> > arch/powerpc/kernel/optprobes_head.S | 3 ++
> > arch/powerpc/lib/code-patching.c | 19 ++--
> > arch/powerpc/lib/feature-fixups.c | 5 +-
> > arch/powerpc/lib/inst.c | 41
> > arch/powerpc/lib/sstep.c | 4 +-
> > arch/powerpc/xmon/xmon.c | 4 +-
> > arch/powerpc/xmon/xmon_bpts.S | 2 +
> > 14 files changed, 200 insertions(+), 38 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/inst.h
> > b/arch/powerpc/include/asm/inst.h
> > index 2f3c9d5bcf7c..7868b80b610e 100644
> > --- a/arch/powerpc/include/asm/inst.h
> > +++ b/arch/powerpc/include/asm/inst.h
> > @@ -2,29 +2,79 @@
> > #ifndef _ASM_INST_H
> > #define _ASM_INST_H
> >
> > +#include
> > /*
> >* Instruction data type for POWER
> >*/
> >
> > struct ppc_inst {
> > u32 val;
> > +#ifdef __powerpc64__
>
> CONFIG_PPC64 should be used instead. This is also reported by checkpatch.
Sure will use that instead.
>
> > + u32 suffix;
> > +#endif /* __powerpc64__ */
> > } __packed;
> >
> > -#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > -
> > static inline u32 ppc_inst_val(struct ppc_inst x)
> > {
> > return x.val;
> > }
> >
> > -static inline int ppc_inst_len(struct ppc_inst x)
> > +static inline int ppc_inst_primary_opcode(struct ppc_inst x)
> > {
> > - return sizeof(struct ppc_inst);
> > + return ppc_inst_val(x) >> 26;
>
> What about using get_op() from asm/disassemble.h instead of hardcodiing ?
Okay will use it here and the other places you point out.
>
> > }
> >
> > -static inline int ppc_inst_primary_opcode(struct ppc_inst x)
> > +#ifdef __powerpc64__
>
> Use CONFIG_PPC64
>
> > +#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
> > +
> > +#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix =
> > (y) })
> > +
> > +static inline u32 ppc_inst_suffix(struct ppc_inst x)
> > {
> > - return ppc_inst_val(x) >> 26;
> > + return x.suffix;
> > +}
> > +
> > +static inline bool ppc_inst_prefixed(struct ppc_inst x)
> > +{
> > + return (ppc_inst_primary_opcode(x) == 1) && ppc_inst_suffix(x) !=
> > 0xff;
> > +}
> > +
> > +static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > +{
> > + return ppc_inst_prefix(swab32(ppc_inst_val(x)),
> > +swab32(ppc_inst_suffix(x)));
> > +}
> > +
> > +static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> > +{
> > + u32 val, suffix;
> > +
> > + val = *(u32 *)ptr;
> > + if ((val >> 26) == 1) {
>
> Don't hardcode, use ppc_inst_primary_opcode() and compare it to OP_PREFIX
> Or use get_op() from asm/disassemble.h
>
>
> > + suffix = *((u32 *)ptr + 1);
> > + return ppc_inst_prefix(val, suffix);
> > + } else {
> > + return ppc_inst(val);
> > + }
> > +}
> > +
> > +static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> > +{
> > + return *(u64 *) == *(u64 *)
> > +}
> > +
> > +#else
> > +
> > +#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > +
> > +static inline bool ppc_inst_prefixed(struct ppc_inst x)
> > +{
> > + return false;
> > +}
> > +
> > +static inline u32 ppc_inst_suffix(struct ppc_inst x)
> > +{
> > + return 0;
> > }
> >
> > static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > @@ -42,6 +92,13 @@ static inline bool ppc_inst_equal(struct ppc_inst x,
> > struct ppc_inst y)
> > return ppc_inst_val(x) == ppc_inst_val(y);
> > }
> >
> > +#endif /* __powerpc64__ */
> > +
> > +static
Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
On Thursday, 14 May 2020 4:11:43 PM AEST Christophe Leroy wrote: > @@ -249,7 +249,7 @@ int arch_prepare_optimized_kprobe(struct > optimized_kprobe *op, struct kprobe *p) > > * Fixup the template with instructions to: > > * 1. load the address of the actual probepoint > > */ > > - patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX); > > + patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX); > > > > /* > > * 2. branch to optimized_callback() and emulate_step() > > @@ -282,7 +282,11 @@ int arch_prepare_optimized_kprobe(struct > > optimized_kprobe *op, struct kprobe *p) /* > > * 3. load instruction to be emulated into relevant register, and > > */ > > - patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX); > > + temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn); > > + patch_imm64_load_insns(ppc_inst_val(temp) | > > + ((u64)ppc_inst_suffix(temp) << 32), > > + 4, > > So now we are also using r4 ? Any explanation somewhere on the way it > works ? This change seems unrelated to this patch, nothing in the > description about it. Can we suddenly use a new register without problem ? Unless I missed something there is no change in register usage here that I could see. patch_imm32_load_insns() was/is hardcoded to use register r4. - Alistair
Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
For powerpc64, redefine the ppc_inst type so both word and prefixed
instructions can be represented. On powerpc32 the type will remain the
same. Update places which had assumed instructions to be 4 bytes long.
Reviewed-by: Alistair Popple
Signed-off-by: Jordan Niethe
---
v4: New to series
v5: - Distinguish normal instructions from prefixed instructions with a
0xff marker for the suffix.
- __patch_instruction() using std for prefixed instructions
v6: - Return false instead of 0 in ppc_inst_prefixed()
- Fix up types for ppc32 so it compiles
- remove ppc_inst_write()
- __patching_instruction(): move flush out of condition
v8: - style
- Define and use OP_PREFIX instead of '1' (back from v3)
- __patch_instruction() fix for big endian
---
arch/powerpc/include/asm/inst.h | 69 ---
arch/powerpc/include/asm/kprobes.h| 2 +-
arch/powerpc/include/asm/ppc-opcode.h | 3 ++
arch/powerpc/include/asm/uaccess.h| 40 +++-
arch/powerpc/include/asm/uprobes.h| 2 +-
arch/powerpc/kernel/crash_dump.c | 2 +-
arch/powerpc/kernel/optprobes.c | 42
arch/powerpc/kernel/optprobes_head.S | 3 ++
arch/powerpc/lib/code-patching.c | 19 ++--
arch/powerpc/lib/feature-fixups.c | 5 +-
arch/powerpc/lib/inst.c | 41
arch/powerpc/lib/sstep.c | 4 +-
arch/powerpc/xmon/xmon.c | 4 +-
arch/powerpc/xmon/xmon_bpts.S | 2 +
14 files changed, 200 insertions(+), 38 deletions(-)
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 2f3c9d5bcf7c..7868b80b610e 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -2,29 +2,79 @@
#ifndef _ASM_INST_H
#define _ASM_INST_H
+#include
/*
* Instruction data type for POWER
*/
struct ppc_inst {
u32 val;
+#ifdef __powerpc64__
CONFIG_PPC64 should be used instead. This is also reported by checkpatch.
+ u32 suffix;
+#endif /* __powerpc64__ */
} __packed;
-#define ppc_inst(x) ((struct ppc_inst){ .val = x })
-
static inline u32 ppc_inst_val(struct ppc_inst x)
{
return x.val;
}
-static inline int ppc_inst_len(struct ppc_inst x)
+static inline int ppc_inst_primary_opcode(struct ppc_inst x)
{
- return sizeof(struct ppc_inst);
+ return ppc_inst_val(x) >> 26;
What about using get_op() from asm/disassemble.h instead of hardcodiing ?
}
-static inline int ppc_inst_primary_opcode(struct ppc_inst x)
+#ifdef __powerpc64__
Use CONFIG_PPC64
+#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
+
+#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix = (y) })
+
+static inline u32 ppc_inst_suffix(struct ppc_inst x)
{
- return ppc_inst_val(x) >> 26;
+ return x.suffix;
+}
+
+static inline bool ppc_inst_prefixed(struct ppc_inst x)
+{
+ return (ppc_inst_primary_opcode(x) == 1) && ppc_inst_suffix(x) != 0xff;
+}
+
+static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
+{
+ return ppc_inst_prefix(swab32(ppc_inst_val(x)),
+ swab32(ppc_inst_suffix(x)));
+}
+
+static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
+{
+ u32 val, suffix;
+
+ val = *(u32 *)ptr;
+ if ((val >> 26) == 1) {
Don't hardcode, use ppc_inst_primary_opcode() and compare it to OP_PREFIX
Or use get_op() from asm/disassemble.h
+ suffix = *((u32 *)ptr + 1);
+ return ppc_inst_prefix(val, suffix);
+ } else {
+ return ppc_inst(val);
+ }
+}
+
+static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
+{
+ return *(u64 *) == *(u64 *)
+}
+
+#else
+
+#define ppc_inst(x) ((struct ppc_inst){ .val = x })
+
+static inline bool ppc_inst_prefixed(struct ppc_inst x)
+{
+ return false;
+}
+
+static inline u32 ppc_inst_suffix(struct ppc_inst x)
+{
+ return 0;
}
static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
@@ -42,6 +92,13 @@ static inline bool ppc_inst_equal(struct ppc_inst x, struct
ppc_inst y)
return ppc_inst_val(x) == ppc_inst_val(y);
}
+#endif /* __powerpc64__ */
+
+static inline int ppc_inst_len(struct ppc_inst x)
+{
+ return (ppc_inst_prefixed(x)) ? 8 : 4;
+}
+
int probe_user_read_inst(struct ppc_inst *inst,
struct ppc_inst *nip);
int probe_kernel_read_inst(struct ppc_inst *inst,
diff --git a/arch/powerpc/include/asm/kprobes.h
b/arch/powerpc/include/asm/kprobes.h
index 66b3f2983b22..4fc0e15e23a5 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -43,7 +43,7 @@ extern kprobe_opcode_t optprobe_template_ret[];
extern kprobe_opcode_t optprobe_template_end[];
/* Fixed instruction size for powerpc */
-#define
Re: [PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
Hi mpe,
Relating to your message on [PATCH v8 16/30] powerpc: Define and use
__get_user_instr{,inatomic}() - could you please take this.
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -106,6 +106,24 @@ static inline int __access_ok(unsigned long addr,
unsigned long size,
__put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
#ifdef __powerpc64__
+#define get_user_instr(x, ptr) \
+({\
+long __gui_ret = 0;\
+unsigned long __gui_ptr = (unsigned long)ptr;\
+struct ppc_inst __gui_inst;\
+unsigned int prefix, suffix;\
+__gui_ret = get_user(prefix, (unsigned int __user *)__gui_ptr);\
+if (!__gui_ret && (prefix >> 26) == OP_PREFIX) {\
+__gui_ret = get_user(suffix,\
+ (unsigned int __user *)__gui_ptr + 1);\
+__gui_inst = ppc_inst_prefix(prefix, suffix);\
+} else {\
+__gui_inst = ppc_inst(prefix);\
+}\
+(x) = __gui_inst;\
+__gui_ret;\
+})
+
#define __get_user_instr(x, ptr)\
({\
long __gui_ret = 0;\
@@ -142,6 +160,8 @@ static inline int __access_ok(unsigned long addr,
unsigned long size,
__gui_ret;\
})
#else
+#define get_user_instr(x, ptr) \
+get_user((x).val, (u32 *)(ptr))
#define __get_user_instr(x, ptr) \
__get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
#define __get_user_instr_inatomic(x, ptr) \
[PATCH v8 23/30] powerpc: Add prefixed instructions to instruction data type
For powerpc64, redefine the ppc_inst type so both word and prefixed
instructions can be represented. On powerpc32 the type will remain the
same. Update places which had assumed instructions to be 4 bytes long.
Reviewed-by: Alistair Popple
Signed-off-by: Jordan Niethe
---
v4: New to series
v5: - Distinguish normal instructions from prefixed instructions with a
0xff marker for the suffix.
- __patch_instruction() using std for prefixed instructions
v6: - Return false instead of 0 in ppc_inst_prefixed()
- Fix up types for ppc32 so it compiles
- remove ppc_inst_write()
- __patching_instruction(): move flush out of condition
v8: - style
- Define and use OP_PREFIX instead of '1' (back from v3)
- __patch_instruction() fix for big endian
---
arch/powerpc/include/asm/inst.h | 69 ---
arch/powerpc/include/asm/kprobes.h| 2 +-
arch/powerpc/include/asm/ppc-opcode.h | 3 ++
arch/powerpc/include/asm/uaccess.h| 40 +++-
arch/powerpc/include/asm/uprobes.h| 2 +-
arch/powerpc/kernel/crash_dump.c | 2 +-
arch/powerpc/kernel/optprobes.c | 42
arch/powerpc/kernel/optprobes_head.S | 3 ++
arch/powerpc/lib/code-patching.c | 19 ++--
arch/powerpc/lib/feature-fixups.c | 5 +-
arch/powerpc/lib/inst.c | 41
arch/powerpc/lib/sstep.c | 4 +-
arch/powerpc/xmon/xmon.c | 4 +-
arch/powerpc/xmon/xmon_bpts.S | 2 +
14 files changed, 200 insertions(+), 38 deletions(-)
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 2f3c9d5bcf7c..7868b80b610e 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -2,29 +2,79 @@
#ifndef _ASM_INST_H
#define _ASM_INST_H
+#include
/*
* Instruction data type for POWER
*/
struct ppc_inst {
u32 val;
+#ifdef __powerpc64__
+ u32 suffix;
+#endif /* __powerpc64__ */
} __packed;
-#define ppc_inst(x) ((struct ppc_inst){ .val = x })
-
static inline u32 ppc_inst_val(struct ppc_inst x)
{
return x.val;
}
-static inline int ppc_inst_len(struct ppc_inst x)
+static inline int ppc_inst_primary_opcode(struct ppc_inst x)
{
- return sizeof(struct ppc_inst);
+ return ppc_inst_val(x) >> 26;
}
-static inline int ppc_inst_primary_opcode(struct ppc_inst x)
+#ifdef __powerpc64__
+#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
+
+#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix = (y) })
+
+static inline u32 ppc_inst_suffix(struct ppc_inst x)
{
- return ppc_inst_val(x) >> 26;
+ return x.suffix;
+}
+
+static inline bool ppc_inst_prefixed(struct ppc_inst x)
+{
+ return (ppc_inst_primary_opcode(x) == 1) && ppc_inst_suffix(x) != 0xff;
+}
+
+static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
+{
+ return ppc_inst_prefix(swab32(ppc_inst_val(x)),
+ swab32(ppc_inst_suffix(x)));
+}
+
+static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
+{
+ u32 val, suffix;
+
+ val = *(u32 *)ptr;
+ if ((val >> 26) == 1) {
+ suffix = *((u32 *)ptr + 1);
+ return ppc_inst_prefix(val, suffix);
+ } else {
+ return ppc_inst(val);
+ }
+}
+
+static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
+{
+ return *(u64 *) == *(u64 *)
+}
+
+#else
+
+#define ppc_inst(x) ((struct ppc_inst){ .val = x })
+
+static inline bool ppc_inst_prefixed(struct ppc_inst x)
+{
+ return false;
+}
+
+static inline u32 ppc_inst_suffix(struct ppc_inst x)
+{
+ return 0;
}
static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
@@ -42,6 +92,13 @@ static inline bool ppc_inst_equal(struct ppc_inst x, struct
ppc_inst y)
return ppc_inst_val(x) == ppc_inst_val(y);
}
+#endif /* __powerpc64__ */
+
+static inline int ppc_inst_len(struct ppc_inst x)
+{
+ return (ppc_inst_prefixed(x)) ? 8 : 4;
+}
+
int probe_user_read_inst(struct ppc_inst *inst,
struct ppc_inst *nip);
int probe_kernel_read_inst(struct ppc_inst *inst,
diff --git a/arch/powerpc/include/asm/kprobes.h
b/arch/powerpc/include/asm/kprobes.h
index 66b3f2983b22..4fc0e15e23a5 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -43,7 +43,7 @@ extern kprobe_opcode_t optprobe_template_ret[];
extern kprobe_opcode_t optprobe_template_end[];
/* Fixed instruction size for powerpc */
-#define MAX_INSN_SIZE 1
+#define MAX_INSN_SIZE 2
#define MAX_OPTIMIZED_LENGTH sizeof(kprobe_opcode_t) /* 4 bytes */
#define MAX_OPTINSN_SIZE (optprobe_template_end -
optprobe_template_entry)
#define RELATIVEJUMP_SIZE sizeof(kprobe_opcode_t) /* 4 bytes */
diff --git a/arch/powerpc/include/asm/ppc-opcode.h
b/arch/powerpc/include/asm/ppc-opcode.h
index c1df75edde44..2a39c716c343
