Messages in this thread | | | Date | Mon, 6 Jan 2014 18:30:01 +0000 | From | Will Deacon <> | Subject | 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
| |