Re: [PATCH bpf-next] selftests/bpf: Fix test_attach_probe for powerpc uprobes

On Thu, Mar 04, 2021 at 11:46:27AM +1100, Michael Ellerman wrote:

"Naveen N. Rao" [off-list ref] writes:

quoted

On 2021/03/02 11:35AM, Jiri Olsa wrote:

quoted

On Mon, Mar 01, 2021 at 02:58:53PM -0800, Yonghong Song wrote:

quoted

On 3/1/21 11:04 AM, Jiri Olsa wrote:

quoted

When testing uprobes we the test gets GEP (Global Entry Point)
address from kallsyms, but then the function is called locally
so the uprobe is not triggered.

Fixing this by adjusting the address to LEP (Local Entry Point)
for powerpc arch.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
  .../selftests/bpf/prog_tests/attach_probe.c    | 18 +++++++++++++++++-
  1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index a0ee87c8e1ea..c3cfb48d3ed0 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c

@@ -2,6 +2,22 @@
  #include <test_progs.h>
  #include "test_attach_probe.skel.h"
+#if defined(__powerpc64__)

This needs to be specific to ELF v2 ABI, so you'll need to check 
_CALL_ELF. See commit d5c2e2c17ae1d6 ("perf probe ppc64le: Prefer symbol 
table lookup over DWARF") for an example.

quoted

quoted

quoted

+/*
+ * We get the GEP (Global Entry Point) address from kallsyms,
+ * but then the function is called locally, so we need to adjust
+ * the address to get LEP (Local Entry Point).

Any documentation in the kernel about this behavior? This will
help to validate the change without trying with powerpc64 qemu...

I don't think we have documented this in the kernel anywhere, but this 
is specific to the ELF v2 ABI and is described there:
- 2.3.2.1.  Function Prologue: 
  http://cdn.openpowerfoundation.org/wp-content/uploads/resources/leabi/content/dbdoclet.50655240___RefHeading___Toc377640597.html
- 3.4.1.  Symbol Values:
   http://cdn.openpowerfoundation.org/wp-content/uploads/resources/leabi/content/dbdoclet.50655241_95185.html

There's a comment in ppc_function_entry(), but I don't think we have any
actual "documentation".

static inline unsigned long ppc_function_entry(void *func)
{
#ifdef PPC64_ELF_ABI_v2
	u32 *insn = func;

	/*
	 * A PPC64 ABIv2 function may have a local and a global entry
	 * point. We need to use the local entry point when patching
	 * functions, so identify and step over the global entry point
	 * sequence.

hm, so I need to do the instructions check below as well

	 *
	 * The global entry point sequence is always of the form:
	 *
	 * addis r2,r12,XXXX
	 * addi  r2,r2,XXXX
	 *
	 * A linker optimisation may convert the addis to lis:
	 *
	 * lis   r2,XXXX
	 * addi  r2,r2,XXXX
	 */
	if ((((*insn & OP_RT_RA_MASK) == ADDIS_R2_R12) ||
	     ((*insn & OP_RT_RA_MASK) == LIS_R2)) &&
	    ((*(insn+1) & OP_RT_RA_MASK) == ADDI_R2_R2))

is this check/instructions specific to kernel code?

In the test prog I see following instructions:

Dump of assembler code for function get_base_addr:
   0x0000000010034cb0 <+0>:     lis     r2,4256
   0x0000000010034cb4 <+4>:     addi    r2,r2,31488
   ...

but first instruction does not match the check in kernel code above:

	1.insn value:	0x3c4010a0
	2.insn value:	0x38427b00

the used defines are:
	#define OP_RT_RA_MASK   0xffff0000UL
	#define LIS_R2          0x3c020000UL
	#define ADDIS_R2_R12    0x3c4c0000UL
	#define ADDI_R2_R2      0x38420000UL


maybe we could skip the check, and run the test twice: first on
kallsym address and if the uprobe is not hit we will run it again
on address + 8

thanks,
jirka

		return (unsigned long)(insn + 2);
	else
		return (unsigned long)func;


cheers