[PATCH] arm64: perf: Report arm pc registers for compat perf

[PATCH] arm64: perf: Report arm pc registers for compat perf

[Seung-Woo Kim]

If perf is built as arm 32-bit, it only reads 15 registers as arm
32-bit register map and this breaks dwarf call-chain in compat
perf because pc register information is not filled.
Report arm pc registers for 32-bit compat perf.

Without this, arm 32-bit perf dwarf call-graph shows below
verbose message:
  unwind: reg 15, val 0
  unwind: reg 13, val ffbc6360
  unwind: no map for 0

Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
---
 arch/arm64/kernel/perf_regs.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c
index 0bbac61..d4172e7 100644
--- a/arch/arm64/kernel/perf_regs.c
+++ b/arch/arm64/kernel/perf_regs.c
@@ -24,6 +24,8 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
 			return regs->compat_sp;
 		if ((u32)idx == PERF_REG_ARM64_LR)
 			return regs->compat_lr;
+		if ((u32)idx == 15) /* PERF_REG_ARM_PC */
+			return regs->pc;
 	}
 
 	if ((u32)idx == PERF_REG_ARM64_SP)
-- 
1.7.4.1

Re: [PATCH] arm64: perf: Report arm pc registers for compat perf

[Mark Rutland]

On Tue, Nov 12, 2019 at 10:01:41AM +0900, Seung-Woo Kim wrote:
> If perf is built as arm 32-bit, it only reads 15 registers as arm
> 32-bit register map and this breaks dwarf call-chain in compat
> perf because pc register information is not filled.
> Report arm pc registers for 32-bit compat perf.
> 
> Without this, arm 32-bit perf dwarf call-graph shows below
> verbose message:
>   unwind: reg 15, val 0
>   unwind: reg 13, val ffbc6360
>   unwind: no map for 0
> 
> Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> ---
>  arch/arm64/kernel/perf_regs.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c
> index 0bbac61..d4172e7 100644
> --- a/arch/arm64/kernel/perf_regs.c
> +++ b/arch/arm64/kernel/perf_regs.c
> @@ -24,6 +24,8 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
>  			return regs->compat_sp;
>  		if ((u32)idx == PERF_REG_ARM64_LR)
>  			return regs->compat_lr;
> +		if ((u32)idx == 15) /* PERF_REG_ARM_PC */
> +			return regs->pc;
>  	}

This doesn't look quite right to me, since perf_regs_value() is
consuming the arm64 index for all other registers (e.g. the LR, in the
patch context).

i.e. this is designed for a native arm64 caller, and the fixup allows it
to view a compat task's registers as-if it were native.

How does this work for a native arm64 perf invocation with a compat
task? I assume it consumers regs->pc, and works as expected?

I suspect we need separate native and compat forms of this function, but
then it's not entirely clear how this should work -- how does this work
for a compat perf analysing a native arm64 binary?

Thanks,
Mark.

Re: [PATCH] arm64: perf: Report arm pc registers for compat perf

[Seung-Woo Kim]

Hi Mark Rutland,

On 2019년 11월 12일 18:40, Mark Rutland wrote:
> On Tue, Nov 12, 2019 at 10:01:41AM +0900, Seung-Woo Kim wrote:
>> If perf is built as arm 32-bit, it only reads 15 registers as arm
>> 32-bit register map and this breaks dwarf call-chain in compat
>> perf because pc register information is not filled.
>> Report arm pc registers for 32-bit compat perf.
>>
>> Without this, arm 32-bit perf dwarf call-graph shows below
>> verbose message:
>>   unwind: reg 15, val 0
>>   unwind: reg 13, val ffbc6360
>>   unwind: no map for 0
>>
>> Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
>> ---
>>  arch/arm64/kernel/perf_regs.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c
>> index 0bbac61..d4172e7 100644
>> --- a/arch/arm64/kernel/perf_regs.c
>> +++ b/arch/arm64/kernel/perf_regs.c
>> @@ -24,6 +24,8 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
>>  			return regs->compat_sp;
>>  		if ((u32)idx == PERF_REG_ARM64_LR)
>>  			return regs->compat_lr;
>> +		if ((u32)idx == 15) /* PERF_REG_ARM_PC */
>> +			return regs->pc;
>>  	}
> 
> This doesn't look quite right to me, since perf_regs_value() is
> consuming the arm64 index for all other registers (e.g. the LR, in the
> patch context).
> 
> i.e. this is designed for a native arm64 caller, and the fixup allows it
> to view a compat task's registers as-if it were native.
> 
> How does this work for a native arm64 perf invocation with a compat
> task? I assume it consumers regs->pc, and works as expected?

In native arm64 perf, compat task registers are set as arm64 register
map, and sp, lr, and pc are set properly. But compat_sp is from regs[13]
and compat_lr is from regs[14], so same register values are set for
regs[13]/egs->sp and regs[14]/regs->lr. With this change, it sets
regs[15] same with regs->pc, but the register is not use at least for
arm 32-bit compat binary callchain, so no issue as far as I understood
and tested.

> 
> I suspect we need separate native and compat forms of this function, but
> then it's not entirely clear how this should work -- how does this work
> for a compat perf analysing a native arm64 binary?

I didn't expect native arm64 binary callchain is possible to get from
arm 32-bit perf.

In my test with 32-bit compat perf, it sets perf
event->attr.sample_regs_user as 0xffff, which is matched with 32-bit
arm, but in arm64 perf part, it cannot be accessed. If there is way to
check it, it is possible to set difference register form. Anyway, in the
case, native arm64 register map is still not fully reported to 32-bit
compat perf.


Thanks,
- Seung-Woo Kim

> 
> Thanks,
> Mark.
> 
> 

-- 
Seung-Woo Kim
Samsung Research
--