Re: [PATCH][V3] arm64: perf: Get the wrong PC value in REGS_ABI_32 mode

From: Will Deacon <will@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Jiping Ma <Jiping.Ma2@windriver.com>,
	zhe.he@windriver.com, bruce.ashfield@gmail.com,
	yue.tao@windriver.com, will.deacon@arm.com,
	linux-kernel@vger.kernel.org, paul.gortmaker@windriver.com,
	catalin.marinas@arm.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH][V3] arm64: perf: Get the wrong PC value in REGS_ABI_32 mode
Date: Tue, 23 Jun 2020 18:44:56 +0100	[thread overview]
Message-ID: <20200623174456.GA5087@willie-the-truck> (raw)
In-Reply-To: <20200623171909.GC4819@willie-the-truck>

On Tue, Jun 23, 2020 at 06:19:10PM +0100, Will Deacon wrote:
> So, I think we should take this patch (which puts the PC where you'd expect
> to find it for compat tasks) and then we could consider removing the current
> lr/sp fudging as a separate patch, which we could revert if it causes a
> problem. However, I'm not sure I want to open that up.

Patch below...

Will

--->8

From 7452148b87ed8c82826474366dbe536fd960d3a7 Mon Sep 17 00:00:00 2001
From: Jiping Ma <jiping.ma2@windriver.com>
Date: Mon, 11 May 2020 10:52:07 +0800
Subject: [PATCH] arm64: perf: Report the PC value in REGS_ABI_32 mode

A 32-bit perf querying the registers of a compat task using REGS_ABI_32
will receive zeroes from w15, when it expects to find the PC.

Return the PC value for register dwarf register 15 when returning register
values for a compat task to perf.

Signed-off-by: Jiping Ma <jiping.ma2@windriver.com>
Link: https://lore.kernel.org/r/1589165527-188401-1-git-send-email-jiping.ma2@windriver.com
[will: Shuffled code and added a comment]
Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/perf_regs.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c
index 0bbac612146e..952b26a05d0f 100644
--- a/arch/arm64/kernel/perf_regs.c
+++ b/arch/arm64/kernel/perf_regs.c
@@ -15,15 +15,25 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
 		return 0;
 
 	/*
-	 * Compat (i.e. 32 bit) mode:
-	 * - PC has been set in the pt_regs struct in kernel_entry,
-	 * - Handle SP and LR here.
+	 * Our handling of compat tasks (PERF_SAMPLE_REGS_ABI_32) is weird. For
+	 * a 32-bit perf inspecting a 32-bit task, then it will look at the
+	 * first 16 registers. These correspond directly to the registers saved
+	 * in our pt_regs structure, with the exception of the PC, so we copy
+	 * that down (x15 corresponds to SP_hyp in the architecture). So far, so
+	 * good. The oddity arises when a 64-bit perf looks at a 32-bit task and
+	 * asks for registers beyond PERF_REG_ARM_MAX. In this case, we return
+	 * SP_usr, LR_usr and PC in the positions where the AArch64 registers
+	 * would normally live. The initial idea was to allow a 64-bit unwinder
+	 * to unwinder a 32-bit task and, although it's not clear how well that
+	 * works in practice, we're kind of stuck with this interface now.
 	 */
 	if (compat_user_mode(regs)) {
 		if ((u32)idx == PERF_REG_ARM64_SP)
 			return regs->compat_sp;
 		if ((u32)idx == PERF_REG_ARM64_LR)
 			return regs->compat_lr;
+		if (idx == 15)
+			return regs->pc;
 	}
 
 	if ((u32)idx == PERF_REG_ARM64_SP)
-- 
2.27.0.111.gc72c7da667-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
