LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v1 1/2] powerpc/inst: ppc_inst_as_u64() becomes ppc_inst_as_ulong()
@ 2021-04-06  9:38 Christophe Leroy
  2021-04-06  9:38 ` [PATCH v1 2/2] powerpc: Enable OPTPROBES on PPC32 Christophe Leroy
  0 siblings, 1 reply; 4+ messages in thread
From: Christophe Leroy @ 2021-04-06  9:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, naveen.n.rao
  Cc: linuxppc-dev, linux-kernel

In order to simplify use on PPC32, change ppc_inst_as_u64()
into ppc_inst_as_ulong() that returns the 32 bits instruction
on PPC32.

Will be used when porting OPTPROBES to PPC32.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/inst.h  | 13 +++++++------
 arch/powerpc/kernel/optprobes.c  |  2 +-
 arch/powerpc/lib/code-patching.c |  2 +-
 arch/powerpc/xmon/xmon.c         |  2 +-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index cc73c1267572..8ea0b503f32f 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -113,13 +113,14 @@ static inline struct ppc_inst *ppc_inst_next(void *location, struct ppc_inst *va
 	return location + ppc_inst_len(tmp);
 }
 
-static inline u64 ppc_inst_as_u64(struct ppc_inst x)
+static inline unsigned long ppc_inst_as_ulong(struct ppc_inst x)
 {
-#ifdef CONFIG_CPU_LITTLE_ENDIAN
-	return (u64)ppc_inst_suffix(x) << 32 | ppc_inst_val(x);
-#else
-	return (u64)ppc_inst_val(x) << 32 | ppc_inst_suffix(x);
-#endif
+	if (IS_ENABLED(CONFIG_PPC32))
+		return ppc_inst_val(x);
+	else if (IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN))
+		return (u64)ppc_inst_suffix(x) << 32 | ppc_inst_val(x);
+	else
+		return (u64)ppc_inst_val(x) << 32 | ppc_inst_suffix(x);
 }
 
 #define PPC_INST_STR_LEN sizeof("00000000 00000000")
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 7f7cdbeacd1a..58fdb9f66e0f 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -264,7 +264,7 @@ 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_as_u64(temp), 4, buff + TMPL_INSN_IDX);
+	patch_imm64_load_insns(ppc_inst_as_ulong(temp), 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 65aec4d6d9ba..870b30d9be2f 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -26,7 +26,7 @@ static int __patch_instruction(struct ppc_inst *exec_addr, struct ppc_inst instr
 
 		__put_kernel_nofault(patch_addr, &val, u32, failed);
 	} else {
-		u64 val = ppc_inst_as_u64(instr);
+		u64 val = ppc_inst_as_ulong(instr);
 
 		__put_kernel_nofault(patch_addr, &val, u64, failed);
 	}
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 3fe37495f63d..ff10a357d41d 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2980,7 +2980,7 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
 		if (!ppc_inst_prefixed(inst))
 			dump_func(ppc_inst_val(inst), adr);
 		else
-			dump_func(ppc_inst_as_u64(inst), adr);
+			dump_func(ppc_inst_as_ulong(inst), adr);
 		printf("\n");
 	}
 	return adr - first_adr;
-- 
2.25.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v1 2/2] powerpc: Enable OPTPROBES on PPC32
  2021-04-06  9:38 [PATCH v1 1/2] powerpc/inst: ppc_inst_as_u64() becomes ppc_inst_as_ulong() Christophe Leroy
@ 2021-04-06  9:38 ` Christophe Leroy
  2021-04-20  6:51   ` Naveen N. Rao
  0 siblings, 1 reply; 4+ messages in thread
From: Christophe Leroy @ 2021-04-06  9:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, naveen.n.rao
  Cc: linuxppc-dev, linux-kernel

For that, create a 32 bits version of patch_imm64_load_insns()
and create a patch_imm_load_insns() which calls
patch_imm32_load_insns() on PPC32 and patch_imm64_load_insns()
on PPC64.

Adapt optprobes_head.S for PPC32. Use PPC_LL/PPC_STL macros instead
of raw ld/std, opt out things linked to paca and use stmw/lmw to
save/restore registers.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/Kconfig                 |  2 +-
 arch/powerpc/kernel/optprobes.c      | 24 +++++++++++++--
 arch/powerpc/kernel/optprobes_head.S | 46 +++++++++++++++++++---------
 3 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c1344c05226c..49b538e54efb 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -227,7 +227,7 @@ config PPC
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI				if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
 	select HAVE_HARDLOCKUP_DETECTOR_ARCH	if PPC64 && PPC_BOOK3S && SMP
-	select HAVE_OPTPROBES			if PPC64
+	select HAVE_OPTPROBES
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_EVENTS_NMI		if PPC64
 	select HAVE_HARDLOCKUP_DETECTOR_PERF	if PERF_EVENTS && HAVE_PERF_EVENTS_NMI && !HAVE_HARDLOCKUP_DETECTOR_ARCH
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index 58fdb9f66e0f..cdf87086fa33 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -141,11 +141,21 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
 	}
 }
 
+static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
+{
+	patch_instruction((struct ppc_inst *)addr,
+			  ppc_inst(PPC_RAW_LIS(reg, IMM_H(val))));
+	addr++;
+
+	patch_instruction((struct ppc_inst *)addr,
+			  ppc_inst(PPC_RAW_ORI(reg, reg, IMM_L(val))));
+}
+
 /*
  * Generate instructions to load provided immediate 64-bit value
  * to register 'reg' and patch these instructions at 'addr'.
  */
-static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
+static void patch_imm64_load_insns(unsigned long long val, int reg, kprobe_opcode_t *addr)
 {
 	/* lis reg,(op)@highest */
 	patch_instruction((struct ppc_inst *)addr,
@@ -177,6 +187,14 @@ static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *
 				   ___PPC_RS(reg) | (val & 0xffff)));
 }
 
+static void patch_imm_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
+{
+	if (IS_ENABLED(CONFIG_PPC64))
+		patch_imm64_load_insns(val, reg, addr);
+	else
+		patch_imm32_load_insns(val, reg, addr);
+}
+
 int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 {
 	struct ppc_inst branch_op_callback, branch_emulate_step, temp;
@@ -230,7 +248,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, 3, buff + TMPL_OP_IDX);
+	patch_imm_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
 
 	/*
 	 * 2. branch to optimized_callback() and emulate_step()
@@ -264,7 +282,7 @@ 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_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
+	patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
 
 	/*
 	 * 4. branch back from trampoline
diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S
index ff8ba4d3824d..49f31e554573 100644
--- a/arch/powerpc/kernel/optprobes_head.S
+++ b/arch/powerpc/kernel/optprobes_head.S
@@ -30,39 +30,47 @@ optinsn_slot:
 	.global optprobe_template_entry
 optprobe_template_entry:
 	/* Create an in-memory pt_regs */
-	stdu	r1,-INT_FRAME_SIZE(r1)
+	PPC_STLU	r1,-INT_FRAME_SIZE(r1)
 	SAVE_GPR(0,r1)
 	/* Save the previous SP into stack */
 	addi	r0,r1,INT_FRAME_SIZE
-	std	r0,GPR1(r1)
+	PPC_STL	r0,GPR1(r1)
+#ifdef CONFIG_PPC64
 	SAVE_10GPRS(2,r1)
 	SAVE_10GPRS(12,r1)
 	SAVE_10GPRS(22,r1)
+#else
+	stmw	r2, GPR2(r1)
+#endif
 	/* Save SPRS */
 	mfmsr	r5
-	std	r5,_MSR(r1)
+	PPC_STL	r5,_MSR(r1)
 	li	r5,0x700
-	std	r5,_TRAP(r1)
+	PPC_STL	r5,_TRAP(r1)
 	li	r5,0
-	std	r5,ORIG_GPR3(r1)
-	std	r5,RESULT(r1)
+	PPC_STL	r5,ORIG_GPR3(r1)
+	PPC_STL	r5,RESULT(r1)
 	mfctr	r5
-	std	r5,_CTR(r1)
+	PPC_STL	r5,_CTR(r1)
 	mflr	r5
-	std	r5,_LINK(r1)
+	PPC_STL	r5,_LINK(r1)
 	mfspr	r5,SPRN_XER
-	std	r5,_XER(r1)
+	PPC_STL	r5,_XER(r1)
 	mfcr	r5
-	std	r5,_CCR(r1)
+	PPC_STL	r5,_CCR(r1)
+#ifdef CONFIG_PPC64
 	lbz     r5,PACAIRQSOFTMASK(r13)
 	std     r5,SOFTE(r1)
+#endif
 
 	/*
 	 * We may get here from a module, so load the kernel TOC in r2.
 	 * The original TOC gets restored when pt_regs is restored
 	 * further below.
 	 */
+#ifdef CONFIG_PPC64
 	ld	r2,PACATOC(r13)
+#endif
 
 	.global optprobe_template_op_address
 optprobe_template_op_address:
@@ -72,9 +80,11 @@ optprobe_template_op_address:
 	 */
 	nop
 	nop
+#ifdef CONFIG_PPC64
 	nop
 	nop
 	nop
+#endif
 	/* 2. pt_regs pointer in r4 */
 	addi	r4,r1,STACK_FRAME_OVERHEAD
 
@@ -94,9 +104,11 @@ optprobe_template_insn:
 	/* 2, Pass instruction to be emulated in r4 */
 	nop
 	nop
+#ifdef CONFIG_PPC64
 	nop
 	nop
 	nop
+#endif
 
 	.global optprobe_template_call_emulate
 optprobe_template_call_emulate:
@@ -107,20 +119,24 @@ optprobe_template_call_emulate:
 	 * All done.
 	 * Now, restore the registers...
 	 */
-	ld	r5,_MSR(r1)
+	PPC_LL	r5,_MSR(r1)
 	mtmsr	r5
-	ld	r5,_CTR(r1)
+	PPC_LL	r5,_CTR(r1)
 	mtctr	r5
-	ld	r5,_LINK(r1)
+	PPC_LL	r5,_LINK(r1)
 	mtlr	r5
-	ld	r5,_XER(r1)
+	PPC_LL	r5,_XER(r1)
 	mtxer	r5
-	ld	r5,_CCR(r1)
+	PPC_LL	r5,_CCR(r1)
 	mtcr	r5
 	REST_GPR(0,r1)
+#ifdef CONFIG_PPC64
 	REST_10GPRS(2,r1)
 	REST_10GPRS(12,r1)
 	REST_10GPRS(22,r1)
+#else
+	lmw	r2, GPR2(r1)
+#endif
 	/* Restore the previous SP */
 	addi	r1,r1,INT_FRAME_SIZE
 
-- 
2.25.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1 2/2] powerpc: Enable OPTPROBES on PPC32
  2021-04-06  9:38 ` [PATCH v1 2/2] powerpc: Enable OPTPROBES on PPC32 Christophe Leroy
@ 2021-04-20  6:51   ` Naveen N. Rao
  2021-04-20 13:37     ` Christophe Leroy
  0 siblings, 1 reply; 4+ messages in thread
From: Naveen N. Rao @ 2021-04-20  6:51 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
	Paul Mackerras
  Cc: linuxppc-dev, linux-kernel

Christophe Leroy wrote:
> For that, create a 32 bits version of patch_imm64_load_insns()
> and create a patch_imm_load_insns() which calls
> patch_imm32_load_insns() on PPC32 and patch_imm64_load_insns()
> on PPC64.
> 
> Adapt optprobes_head.S for PPC32. Use PPC_LL/PPC_STL macros instead
> of raw ld/std, opt out things linked to paca and use stmw/lmw to
> save/restore registers.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/Kconfig                 |  2 +-
>  arch/powerpc/kernel/optprobes.c      | 24 +++++++++++++--
>  arch/powerpc/kernel/optprobes_head.S | 46 +++++++++++++++++++---------
>  3 files changed, 53 insertions(+), 19 deletions(-)

Thanks for adding support for ppc32. It is good to see that it works 
without too many changes.

> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index c1344c05226c..49b538e54efb 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -227,7 +227,7 @@ config PPC
>  	select HAVE_MOD_ARCH_SPECIFIC
>  	select HAVE_NMI				if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
>  	select HAVE_HARDLOCKUP_DETECTOR_ARCH	if PPC64 && PPC_BOOK3S && SMP
> -	select HAVE_OPTPROBES			if PPC64
> +	select HAVE_OPTPROBES
>  	select HAVE_PERF_EVENTS
>  	select HAVE_PERF_EVENTS_NMI		if PPC64
>  	select HAVE_HARDLOCKUP_DETECTOR_PERF	if PERF_EVENTS && HAVE_PERF_EVENTS_NMI && !HAVE_HARDLOCKUP_DETECTOR_ARCH
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index 58fdb9f66e0f..cdf87086fa33 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -141,11 +141,21 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
>  	}
>  }
> 
> +static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
> +{
> +	patch_instruction((struct ppc_inst *)addr,
> +			  ppc_inst(PPC_RAW_LIS(reg, IMM_H(val))));
> +	addr++;
> +
> +	patch_instruction((struct ppc_inst *)addr,
> +			  ppc_inst(PPC_RAW_ORI(reg, reg, IMM_L(val))));
> +}
> +
>  /*
>   * Generate instructions to load provided immediate 64-bit value
>   * to register 'reg' and patch these instructions at 'addr'.
>   */
> -static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
> +static void patch_imm64_load_insns(unsigned long long val, int reg, kprobe_opcode_t *addr)

Do you really need this?

>  {
>  	/* lis reg,(op)@highest */
>  	patch_instruction((struct ppc_inst *)addr,
> @@ -177,6 +187,14 @@ static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *
>  				   ___PPC_RS(reg) | (val & 0xffff)));
>  }
> 
> +static void patch_imm_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
> +{
> +	if (IS_ENABLED(CONFIG_PPC64))
> +		patch_imm64_load_insns(val, reg, addr);
> +	else
> +		patch_imm32_load_insns(val, reg, addr);
> +}
> +
>  int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
>  {
>  	struct ppc_inst branch_op_callback, branch_emulate_step, temp;
> @@ -230,7 +248,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, 3, buff + TMPL_OP_IDX);
> +	patch_imm_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
> 
>  	/*
>  	 * 2. branch to optimized_callback() and emulate_step()
> @@ -264,7 +282,7 @@ 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_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
> +	patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
> 
>  	/*
>  	 * 4. branch back from trampoline
> diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S
> index ff8ba4d3824d..49f31e554573 100644
> --- a/arch/powerpc/kernel/optprobes_head.S
> +++ b/arch/powerpc/kernel/optprobes_head.S
> @@ -30,39 +30,47 @@ optinsn_slot:
>  	.global optprobe_template_entry
>  optprobe_template_entry:
>  	/* Create an in-memory pt_regs */
> -	stdu	r1,-INT_FRAME_SIZE(r1)
> +	PPC_STLU	r1,-INT_FRAME_SIZE(r1)
>  	SAVE_GPR(0,r1)
>  	/* Save the previous SP into stack */
>  	addi	r0,r1,INT_FRAME_SIZE
> -	std	r0,GPR1(r1)
> +	PPC_STL	r0,GPR1(r1)
> +#ifdef CONFIG_PPC64
>  	SAVE_10GPRS(2,r1)
>  	SAVE_10GPRS(12,r1)
>  	SAVE_10GPRS(22,r1)
> +#else
> +	stmw	r2, GPR2(r1)
> +#endif
>  	/* Save SPRS */
>  	mfmsr	r5
> -	std	r5,_MSR(r1)
> +	PPC_STL	r5,_MSR(r1)
>  	li	r5,0x700
> -	std	r5,_TRAP(r1)
> +	PPC_STL	r5,_TRAP(r1)
>  	li	r5,0
> -	std	r5,ORIG_GPR3(r1)
> -	std	r5,RESULT(r1)
> +	PPC_STL	r5,ORIG_GPR3(r1)
> +	PPC_STL	r5,RESULT(r1)
>  	mfctr	r5
> -	std	r5,_CTR(r1)
> +	PPC_STL	r5,_CTR(r1)
>  	mflr	r5
> -	std	r5,_LINK(r1)
> +	PPC_STL	r5,_LINK(r1)
>  	mfspr	r5,SPRN_XER
> -	std	r5,_XER(r1)
> +	PPC_STL	r5,_XER(r1)
>  	mfcr	r5
> -	std	r5,_CCR(r1)
> +	PPC_STL	r5,_CCR(r1)
> +#ifdef CONFIG_PPC64
>  	lbz     r5,PACAIRQSOFTMASK(r13)
>  	std     r5,SOFTE(r1)
> +#endif
> 
>  	/*
>  	 * We may get here from a module, so load the kernel TOC in r2.
>  	 * The original TOC gets restored when pt_regs is restored
>  	 * further below.
>  	 */
> +#ifdef CONFIG_PPC64
>  	ld	r2,PACATOC(r13)
> +#endif

Are the ISA and ABI documents for ppc32 available publicly? I would have 
thought that the TOC issues apply to ppc32 as well, but want to 
understand why this isn't a problem there.

> 
>  	.global optprobe_template_op_address
>  optprobe_template_op_address:
> @@ -72,9 +80,11 @@ optprobe_template_op_address:
>  	 */
>  	nop
>  	nop
> +#ifdef CONFIG_PPC64
>  	nop
>  	nop
>  	nop
> +#endif
>  	/* 2. pt_regs pointer in r4 */
>  	addi	r4,r1,STACK_FRAME_OVERHEAD
> 
> @@ -94,9 +104,11 @@ optprobe_template_insn:
>  	/* 2, Pass instruction to be emulated in r4 */
>  	nop
>  	nop
> +#ifdef CONFIG_PPC64
>  	nop
>  	nop
>  	nop
> +#endif

It would be nice to put these behind a macro so as to avoid these #ifdef 
blocks here, as well as with the register save/restore sequence.

- Naveen


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1 2/2] powerpc: Enable OPTPROBES on PPC32
  2021-04-20  6:51   ` Naveen N. Rao
@ 2021-04-20 13:37     ` Christophe Leroy
  0 siblings, 0 replies; 4+ messages in thread
From: Christophe Leroy @ 2021-04-20 13:37 UTC (permalink / raw)
  To: Naveen N. Rao, Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras
  Cc: linuxppc-dev, linux-kernel



Le 20/04/2021 à 08:51, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>> For that, create a 32 bits version of patch_imm64_load_insns()
>> and create a patch_imm_load_insns() which calls
>> patch_imm32_load_insns() on PPC32 and patch_imm64_load_insns()
>> on PPC64.
>>
>> Adapt optprobes_head.S for PPC32. Use PPC_LL/PPC_STL macros instead
>> of raw ld/std, opt out things linked to paca and use stmw/lmw to
>> save/restore registers.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>>  arch/powerpc/Kconfig                 |  2 +-
>>  arch/powerpc/kernel/optprobes.c      | 24 +++++++++++++--
>>  arch/powerpc/kernel/optprobes_head.S | 46 +++++++++++++++++++---------
>>  3 files changed, 53 insertions(+), 19 deletions(-)
> 
> Thanks for adding support for ppc32. It is good to see that it works without too many changes.
> 
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index c1344c05226c..49b538e54efb 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -227,7 +227,7 @@ config PPC
>>      select HAVE_MOD_ARCH_SPECIFIC
>>      select HAVE_NMI                if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
>>      select HAVE_HARDLOCKUP_DETECTOR_ARCH    if PPC64 && PPC_BOOK3S && SMP
>> -    select HAVE_OPTPROBES            if PPC64
>> +    select HAVE_OPTPROBES
>>      select HAVE_PERF_EVENTS
>>      select HAVE_PERF_EVENTS_NMI        if PPC64
>>      select HAVE_HARDLOCKUP_DETECTOR_PERF    if PERF_EVENTS && HAVE_PERF_EVENTS_NMI && 
>> !HAVE_HARDLOCKUP_DETECTOR_ARCH
>> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
>> index 58fdb9f66e0f..cdf87086fa33 100644
>> --- a/arch/powerpc/kernel/optprobes.c
>> +++ b/arch/powerpc/kernel/optprobes.c
>> @@ -141,11 +141,21 @@ void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
>>      }
>>  }
>>
>> +static void patch_imm32_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
>> +{
>> +    patch_instruction((struct ppc_inst *)addr,
>> +              ppc_inst(PPC_RAW_LIS(reg, IMM_H(val))));
>> +    addr++;
>> +
>> +    patch_instruction((struct ppc_inst *)addr,
>> +              ppc_inst(PPC_RAW_ORI(reg, reg, IMM_L(val))));
>> +}
>> +
>>  /*
>>   * Generate instructions to load provided immediate 64-bit value
>>   * to register 'reg' and patch these instructions at 'addr'.
>>   */
>> -static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
>> +static void patch_imm64_load_insns(unsigned long long val, int reg, kprobe_opcode_t *addr)
> 
> Do you really need this?

Without it I get:

                  from arch/powerpc/kernel/optprobes.c:8:
arch/powerpc/kernel/optprobes.c: In function 'patch_imm64_load_insns':
arch/powerpc/kernel/optprobes.c:163:14: error: right shift count >= width of type 
[-Werror=shift-count-overflow]
   163 |        ((val >> 48) & 0xffff)));
       |              ^~
./arch/powerpc/include/asm/inst.h:69:48: note: in definition of macro 'ppc_inst'
    69 | #define ppc_inst(x) ((struct ppc_inst){ .val = x })
       |                                                ^
arch/powerpc/kernel/optprobes.c:169:31: error: right shift count >= width of type 
[-Werror=shift-count-overflow]
   169 |        ___PPC_RS(reg) | ((val >> 32) & 0xffff)));
       |                               ^~
./arch/powerpc/include/asm/inst.h:69:48: note: in definition of macro 'ppc_inst'
    69 | #define ppc_inst(x) ((struct ppc_inst){ .val = x })
       |                                                ^


> 
>>  {
>>      /* lis reg,(op)@highest */
>>      patch_instruction((struct ppc_inst *)addr,
>> @@ -177,6 +187,14 @@ static void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t *
>>                     ___PPC_RS(reg) | (val & 0xffff)));
>>  }
>>
>> +static void patch_imm_load_insns(unsigned long val, int reg, kprobe_opcode_t *addr)
>> +{
>> +    if (IS_ENABLED(CONFIG_PPC64))
>> +        patch_imm64_load_insns(val, reg, addr);
>> +    else
>> +        patch_imm32_load_insns(val, reg, addr);
>> +}
>> +
>>  int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
>>  {
>>      struct ppc_inst branch_op_callback, branch_emulate_step, temp;
>> @@ -230,7 +248,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, 3, buff + TMPL_OP_IDX);
>> +    patch_imm_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
>>
>>      /*
>>       * 2. branch to optimized_callback() and emulate_step()
>> @@ -264,7 +282,7 @@ 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_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
>> +    patch_imm_load_insns(ppc_inst_as_ulong(temp), 4, buff + TMPL_INSN_IDX);
>>
>>      /*
>>       * 4. branch back from trampoline
>> diff --git a/arch/powerpc/kernel/optprobes_head.S b/arch/powerpc/kernel/optprobes_head.S
>> index ff8ba4d3824d..49f31e554573 100644
>> --- a/arch/powerpc/kernel/optprobes_head.S
>> +++ b/arch/powerpc/kernel/optprobes_head.S
>> @@ -30,39 +30,47 @@ optinsn_slot:
>>      .global optprobe_template_entry
>>  optprobe_template_entry:
>>      /* Create an in-memory pt_regs */
>> -    stdu    r1,-INT_FRAME_SIZE(r1)
>> +    PPC_STLU    r1,-INT_FRAME_SIZE(r1)
>>      SAVE_GPR(0,r1)
>>      /* Save the previous SP into stack */
>>      addi    r0,r1,INT_FRAME_SIZE
>> -    std    r0,GPR1(r1)
>> +    PPC_STL    r0,GPR1(r1)
>> +#ifdef CONFIG_PPC64
>>      SAVE_10GPRS(2,r1)
>>      SAVE_10GPRS(12,r1)
>>      SAVE_10GPRS(22,r1)
>> +#else
>> +    stmw    r2, GPR2(r1)
>> +#endif
>>      /* Save SPRS */
>>      mfmsr    r5
>> -    std    r5,_MSR(r1)
>> +    PPC_STL    r5,_MSR(r1)
>>      li    r5,0x700
>> -    std    r5,_TRAP(r1)
>> +    PPC_STL    r5,_TRAP(r1)
>>      li    r5,0
>> -    std    r5,ORIG_GPR3(r1)
>> -    std    r5,RESULT(r1)
>> +    PPC_STL    r5,ORIG_GPR3(r1)
>> +    PPC_STL    r5,RESULT(r1)
>>      mfctr    r5
>> -    std    r5,_CTR(r1)
>> +    PPC_STL    r5,_CTR(r1)
>>      mflr    r5
>> -    std    r5,_LINK(r1)
>> +    PPC_STL    r5,_LINK(r1)
>>      mfspr    r5,SPRN_XER
>> -    std    r5,_XER(r1)
>> +    PPC_STL    r5,_XER(r1)
>>      mfcr    r5
>> -    std    r5,_CCR(r1)
>> +    PPC_STL    r5,_CCR(r1)
>> +#ifdef CONFIG_PPC64
>>      lbz     r5,PACAIRQSOFTMASK(r13)
>>      std     r5,SOFTE(r1)
>> +#endif
>>
>>      /*
>>       * We may get here from a module, so load the kernel TOC in r2.
>>       * The original TOC gets restored when pt_regs is restored
>>       * further below.
>>       */
>> +#ifdef CONFIG_PPC64
>>      ld    r2,PACATOC(r13)
>> +#endif
> 
> Are the ISA and ABI documents for ppc32 available publicly? 

ABI: https://wiki.raptorcs.com/w/images/0/03/Power-Arch-32-bit-ABI-supp-1.0-Unified.pdf

ISA: https://www.nxp.com/files-static/product/doc/MPCFPE32B.pdf


> I would have thought that the TOC issues 
> apply to ppc32 as well, but want to understand why this isn't a problem there.

r2 is the pointer to 'current' on PPC32.

No TOC.


> 
>>
>>      .global optprobe_template_op_address
>>  optprobe_template_op_address:
>> @@ -72,9 +80,11 @@ optprobe_template_op_address:
>>       */
>>      nop
>>      nop
>> +#ifdef CONFIG_PPC64
>>      nop
>>      nop
>>      nop
>> +#endif
>>      /* 2. pt_regs pointer in r4 */
>>      addi    r4,r1,STACK_FRAME_OVERHEAD
>>
>> @@ -94,9 +104,11 @@ optprobe_template_insn:
>>      /* 2, Pass instruction to be emulated in r4 */
>>      nop
>>      nop
>> +#ifdef CONFIG_PPC64
>>      nop
>>      nop
>>      nop
>> +#endif
> 
> It would be nice to put these behind a macro so as to avoid these #ifdef blocks here, as well as 
> with the register save/restore sequence.
> 

Will see what I can do

Christophe

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-06  9:38 [PATCH v1 1/2] powerpc/inst: ppc_inst_as_u64() becomes ppc_inst_as_ulong() Christophe Leroy
2021-04-06  9:38 ` [PATCH v1 2/2] powerpc: Enable OPTPROBES on PPC32 Christophe Leroy
2021-04-20  6:51   ` Naveen N. Rao
2021-04-20 13:37     ` Christophe Leroy

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git