Re: [PATCH 1/3] ARM64: perf: add support for perf registers API

Hi Jean,

Thanks for the updated patches. One minor comment on this one.

On Mon, Dec 30, 2013 at 04:25:30PM +0000, Jean Pihet wrote:

From: Jean Pihet jean.pihet@newoldbits.com

This patch implements the functions required for the perf registers API, allowing the perf tool to interface kernel register dumps with libunwind in order to provide userspace backtracing. Compat mode is also supported.

[...]

+u64 perf_reg_value(struct pt_regs *regs, int idx) +{

• if (WARN_ON_ONCE((u32)idx >= PERF_REG_ARM64_MAX))

• return 0;
  

While this is probably fine, I'd feel more comfortable if you had separate limit checks for native and compat...

• /*

• * Compat (i.e. 32 bit) mode:
  

• * - PC has been set in the pt_regs struct in kernel_entry,
  

• * - Handle SP and LR here.
  

• */
  

• if (compat_user_mode(regs)) {

i.e. have a WARN_ON_ONCE here for the compat structure size.

• if ((u32)idx == PERF_REG_ARM64_SP)
  

• 	return regs->compat_sp;
  

• if ((u32)idx == PERF_REG_ARM64_LR)
  

• 	return regs->compat_lr;
  

• }

then stick an else here with the original check.

Make sense?

Will