On Wed, May 27, 2020 at 09:33:00AM +0800, Jiping Ma wrote: > > > On 05/26/2020 06:26 PM, Mark Rutland wrote: > > On Mon, May 11, 2020 at 10:52:07AM +0800, Jiping Ma wrote: > > > Modified the patch subject and the change description. > > > > > > PC value is get from regs[15] in REGS_ABI_32 mode, but correct PC > > > is regs->pc(regs[PERF_REG_ARM64_PC]) in arm64 kernel, which caused > > > that perf can not parser the backtrace of app with dwarf mode in the > > > 32bit system and 64bit kernel. > > > > > > Signed-off-by: Jiping Ma <jiping....@windriver.com> > > Thanks for this. > > > > > > > --- > > > arch/arm64/kernel/perf_regs.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c > > > index 0bbac61..0ef2880 100644 > > > --- a/arch/arm64/kernel/perf_regs.c > > > +++ b/arch/arm64/kernel/perf_regs.c > > > @@ -32,6 +32,10 @@ u64 perf_reg_value(struct pt_regs *regs, int idx) > > > if ((u32)idx == PERF_REG_ARM64_PC) > > > return regs->pc; > > > + if (perf_reg_abi(current) == PERF_SAMPLE_REGS_ABI_32 > > > + && idx == 15) > > > + return regs->pc; > > I think there are some more issues here, and we may need a more > > substantial rework. For a compat thread, we always expose > > PERF_SAMPLE_REGS_ABI_32 via per_reg_abi(), but for some reason > > perf_reg_value() also munges the compat SP/LR into their ARM64 > > equivalents, which don't exist in the 32-bit sample ABI. We also don't > > zero the regs that don't exist in 32-bit (including the aliasing PC). > > > > I reckon what we should do is have seperate functions for the two ABIs, > > to ensure we don't conflate them, e.g. > > > > u64 perf_reg_value_abi32(struct pt_regs *regs, int idx) > > { > > if ((u32)idx > PERF_REG_ARM32_PC) > > return 0; > > if (idx == PERF_REG_ARM32_PC) > > return regs->pc; > > > > /* > > * Compat SP and LR already in-place > > */ > > return regs->regs[idx]; > > } > > > > u64 perf_reg_value_abi64(struct pt_regs *regs, int idx) > > { > > if ((u32)idx > PERF_REG_ARM64_MAX) > > return 0; > > if ((u32)idx == PERF_REG_ARM64_SP) > > return regs->sp; > > if ((u32)idx == PERF_REG_ARM64_PC) > > return regs->pc; > > > > reutrn regs->regs[idx]; > > } > > > > u64 perf_reg_value(struct pt_regs *regs, int idx) > > { > > if (compat_user_mode(regs)) > > return perf_reg_value_abi32(regs, idx); > > else > > return perf_reg_value_abi64(regs, idx); > > } > This modification can not fix our issue, we need > perf_reg_abi(current) == PERF_SAMPLE_REGS_ABI_32 to judge if it is 32-bit > task or not, > then return the correct PC value.
I must be missing something here. The core code perf_reg_abi(task) is called with the task being sampled, and the regs are from the task being sampled. For a userspace sample for a compat task, compat_user_mode(regs) should be equivalent to the is_compat_thread(task_thread_info(task)) check. What am I missing? Thanks, Mark.