From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
Michael Ellerman <mpe@ellerman.id.au>,
Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 2/2] powerpc: Enable OPTPROBES on PPC32
Date: Tue, 20 Apr 2021 12:21:39 +0530 [thread overview]
Message-ID: <1618900760.son2fwciv1.naveen@linux.ibm.com> (raw)
In-Reply-To: <28730d147adeaa6c2d0c98d0aa9e17e9e4bd043a.1617701875.git.christophe.leroy@csgroup.eu>
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
next prev parent reply other threads:[~2021-04-20 6:52 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2021-04-20 13:37 ` Christophe Leroy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1618900760.son2fwciv1.naveen@linux.ibm.com \
--to=naveen.n.rao@linux.vnet.ibm.com \
--cc=benh@kernel.crashing.org \
--cc=christophe.leroy@csgroup.eu \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).