All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER
@ 2018-11-26 18:04 Madhavan Srinivasan
  2018-11-26 19:38 ` Arnaldo Carvalho de Melo
  2018-11-27  6:54 ` Christophe LEROY
  0 siblings, 2 replies; 7+ messages in thread
From: Madhavan Srinivasan @ 2018-11-26 18:04 UTC (permalink / raw)
  To: mpe, acme
  Cc: Madhavan Srinivasan, Alexander Shishkin, linuxppc-dev,
	Ravi Bangoria, Anju T Sudhakar, Namhyung Kim, Jiri Olsa

On each sample, Sample Instruction Event Register (SIER) content
is saved in pt_regs. SIER does not have a entry as-is in the pt_regs
but instead, SIER content is saved in the "dar" register of pt_regs.

Patch adds another entry to the perf_regs structure to include the "SIER"
printing which internally maps to the "dar" of pt_regs.

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 arch/powerpc/include/uapi/asm/perf_regs.h       | 1 +
 arch/powerpc/perf/perf_regs.c                   | 1 +
 tools/arch/powerpc/include/uapi/asm/perf_regs.h | 1 +
 tools/perf/arch/powerpc/include/perf_regs.h     | 3 ++-
 tools/perf/arch/powerpc/util/perf_regs.c        | 1 +
 5 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
index 9e52c86ccbd3..ff91192407d1 100644
--- a/arch/powerpc/include/uapi/asm/perf_regs.h
+++ b/arch/powerpc/include/uapi/asm/perf_regs.h
@@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
 	PERF_REG_POWERPC_TRAP,
 	PERF_REG_POWERPC_DAR,
 	PERF_REG_POWERPC_DSISR,
+	PERF_REG_POWERPC_SIER,
 	PERF_REG_POWERPC_MAX,
 };
 #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
index 09ceea6175ba..c262aea22ad9 100644
--- a/arch/powerpc/perf/perf_regs.c
+++ b/arch/powerpc/perf/perf_regs.c
@@ -69,6 +69,7 @@ static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
 	PT_REGS_OFFSET(PERF_REG_POWERPC_TRAP, trap),
 	PT_REGS_OFFSET(PERF_REG_POWERPC_DAR, dar),
 	PT_REGS_OFFSET(PERF_REG_POWERPC_DSISR, dsisr),
+	PT_REGS_OFFSET(PERF_REG_POWERPC_SIER, dar),
 };
 
 u64 perf_reg_value(struct pt_regs *regs, int idx)
diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
index 9e52c86ccbd3..ff91192407d1 100644
--- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
+++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
@@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
 	PERF_REG_POWERPC_TRAP,
 	PERF_REG_POWERPC_DAR,
 	PERF_REG_POWERPC_DSISR,
+	PERF_REG_POWERPC_SIER,
 	PERF_REG_POWERPC_MAX,
 };
 #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
index 00e37b106913..1076393e6f43 100644
--- a/tools/perf/arch/powerpc/include/perf_regs.h
+++ b/tools/perf/arch/powerpc/include/perf_regs.h
@@ -62,7 +62,8 @@ static const char *reg_names[] = {
 	[PERF_REG_POWERPC_SOFTE] = "softe",
 	[PERF_REG_POWERPC_TRAP] = "trap",
 	[PERF_REG_POWERPC_DAR] = "dar",
-	[PERF_REG_POWERPC_DSISR] = "dsisr"
+	[PERF_REG_POWERPC_DSISR] = "dsisr",
+	[PERF_REG_POWERPC_SIER] = "sier"
 };
 
 static inline const char *perf_reg_name(int id)
diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
index ec50939b0418..07fcd977d93e 100644
--- a/tools/perf/arch/powerpc/util/perf_regs.c
+++ b/tools/perf/arch/powerpc/util/perf_regs.c
@@ -52,6 +52,7 @@ const struct sample_reg sample_reg_masks[] = {
 	SMPL_REG(trap, PERF_REG_POWERPC_TRAP),
 	SMPL_REG(dar, PERF_REG_POWERPC_DAR),
 	SMPL_REG(dsisr, PERF_REG_POWERPC_DSISR),
+	SMPL_REG(sier, PERF_REG_POWERPC_SIER),
 	SMPL_REG_END
 };
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER
  2018-11-26 18:04 [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER Madhavan Srinivasan
@ 2018-11-26 19:38 ` Arnaldo Carvalho de Melo
  2018-11-28  3:34   ` Michael Ellerman
  2018-11-27  6:54 ` Christophe LEROY
  1 sibling, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-11-26 19:38 UTC (permalink / raw)
  To: Madhavan Srinivasan
  Cc: Alexander Shishkin, Thomas Richter, Hendrik Brueckner,
	linuxppc-dev, Ravi Bangoria, Anju T Sudhakar, Martin Schwidefsky,
	Namhyung Kim, Jiri Olsa

Em Mon, Nov 26, 2018 at 11:34:08PM +0530, Madhavan Srinivasan escreveu:
> On each sample, Sample Instruction Event Register (SIER) content
> is saved in pt_regs. SIER does not have a entry as-is in the pt_regs
> but instead, SIER content is saved in the "dar" register of pt_regs.
> 
> Patch adds another entry to the perf_regs structure to include the "SIER"
> printing which internally maps to the "dar" of pt_regs.

I think the patch is ok, when we talked in Vancouver I thought I saw
something like this before, i.e. adding more registers to a perf_regs.h
file, this was the cset:

  commit 0da0017f72554c005c1a04c3adc5da9eb64fa7e5
  Author: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
  Date:   Wed Nov 8 07:30:15 2017 +0100

      s390/perf: extend perf_regs support to include floating-point registers

That I came across because it broke the perf build, making me add this
cset:

  commit 10b9baa701d5023897f70a4acb3bf0235da3dc4f
  Author: Arnaldo Carvalho de Melo <acme@redhat.com>
  Date:   Tue Nov 28 11:08:41 2017 -0300

    tools arch s390: Do not include header files from the kernel sources

:-)

Michael? What about the ppc specific details?

- Arnaldo
 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Cc: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/uapi/asm/perf_regs.h       | 1 +
>  arch/powerpc/perf/perf_regs.c                   | 1 +
>  tools/arch/powerpc/include/uapi/asm/perf_regs.h | 1 +
>  tools/perf/arch/powerpc/include/perf_regs.h     | 3 ++-
>  tools/perf/arch/powerpc/util/perf_regs.c        | 1 +
>  5 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
> index 9e52c86ccbd3..ff91192407d1 100644
> --- a/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
>  	PERF_REG_POWERPC_TRAP,
>  	PERF_REG_POWERPC_DAR,
>  	PERF_REG_POWERPC_DSISR,
> +	PERF_REG_POWERPC_SIER,
>  	PERF_REG_POWERPC_MAX,
>  };
>  #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
> index 09ceea6175ba..c262aea22ad9 100644
> --- a/arch/powerpc/perf/perf_regs.c
> +++ b/arch/powerpc/perf/perf_regs.c
> @@ -69,6 +69,7 @@ static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
>  	PT_REGS_OFFSET(PERF_REG_POWERPC_TRAP, trap),
>  	PT_REGS_OFFSET(PERF_REG_POWERPC_DAR, dar),
>  	PT_REGS_OFFSET(PERF_REG_POWERPC_DSISR, dsisr),
> +	PT_REGS_OFFSET(PERF_REG_POWERPC_SIER, dar),
>  };
>  
>  u64 perf_reg_value(struct pt_regs *regs, int idx)
> diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> index 9e52c86ccbd3..ff91192407d1 100644
> --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
>  	PERF_REG_POWERPC_TRAP,
>  	PERF_REG_POWERPC_DAR,
>  	PERF_REG_POWERPC_DSISR,
> +	PERF_REG_POWERPC_SIER,
>  	PERF_REG_POWERPC_MAX,
>  };
>  #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
> index 00e37b106913..1076393e6f43 100644
> --- a/tools/perf/arch/powerpc/include/perf_regs.h
> +++ b/tools/perf/arch/powerpc/include/perf_regs.h
> @@ -62,7 +62,8 @@ static const char *reg_names[] = {
>  	[PERF_REG_POWERPC_SOFTE] = "softe",
>  	[PERF_REG_POWERPC_TRAP] = "trap",
>  	[PERF_REG_POWERPC_DAR] = "dar",
> -	[PERF_REG_POWERPC_DSISR] = "dsisr"
> +	[PERF_REG_POWERPC_DSISR] = "dsisr",
> +	[PERF_REG_POWERPC_SIER] = "sier"
>  };
>  
>  static inline const char *perf_reg_name(int id)
> diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
> index ec50939b0418..07fcd977d93e 100644
> --- a/tools/perf/arch/powerpc/util/perf_regs.c
> +++ b/tools/perf/arch/powerpc/util/perf_regs.c
> @@ -52,6 +52,7 @@ const struct sample_reg sample_reg_masks[] = {
>  	SMPL_REG(trap, PERF_REG_POWERPC_TRAP),
>  	SMPL_REG(dar, PERF_REG_POWERPC_DAR),
>  	SMPL_REG(dsisr, PERF_REG_POWERPC_DSISR),
> +	SMPL_REG(sier, PERF_REG_POWERPC_SIER),
>  	SMPL_REG_END
>  };
>  
> -- 
> 2.7.4

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER
  2018-11-26 18:04 [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER Madhavan Srinivasan
  2018-11-26 19:38 ` Arnaldo Carvalho de Melo
@ 2018-11-27  6:54 ` Christophe LEROY
  2018-11-27 10:55   ` Madhavan Srinivasan
  1 sibling, 1 reply; 7+ messages in thread
From: Christophe LEROY @ 2018-11-27  6:54 UTC (permalink / raw)
  To: Madhavan Srinivasan, mpe, acme
  Cc: Alexander Shishkin, linuxppc-dev, Ravi Bangoria, Anju T Sudhakar,
	Namhyung Kim, Jiri Olsa



Le 26/11/2018 à 19:04, Madhavan Srinivasan a écrit :
> On each sample, Sample Instruction Event Register (SIER) content
> is saved in pt_regs. SIER does not have a entry as-is in the pt_regs
> but instead, SIER content is saved in the "dar" register of pt_regs.
> 
> Patch adds another entry to the perf_regs structure to include the "SIER"
> printing which internally maps to the "dar" of pt_regs.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Cc: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---

I see this patch is already in patchwork 
https://patchwork.ozlabs.org/patch/941908/ and I can't spot any 
difference between the two. Please say here (under the ---) the reason 
for resending.

Christophe


>   arch/powerpc/include/uapi/asm/perf_regs.h       | 1 +
>   arch/powerpc/perf/perf_regs.c                   | 1 +
>   tools/arch/powerpc/include/uapi/asm/perf_regs.h | 1 +
>   tools/perf/arch/powerpc/include/perf_regs.h     | 3 ++-
>   tools/perf/arch/powerpc/util/perf_regs.c        | 1 +
>   5 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
> index 9e52c86ccbd3..ff91192407d1 100644
> --- a/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
>   	PERF_REG_POWERPC_TRAP,
>   	PERF_REG_POWERPC_DAR,
>   	PERF_REG_POWERPC_DSISR,
> +	PERF_REG_POWERPC_SIER,
>   	PERF_REG_POWERPC_MAX,
>   };
>   #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
> index 09ceea6175ba..c262aea22ad9 100644
> --- a/arch/powerpc/perf/perf_regs.c
> +++ b/arch/powerpc/perf/perf_regs.c
> @@ -69,6 +69,7 @@ static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
>   	PT_REGS_OFFSET(PERF_REG_POWERPC_TRAP, trap),
>   	PT_REGS_OFFSET(PERF_REG_POWERPC_DAR, dar),
>   	PT_REGS_OFFSET(PERF_REG_POWERPC_DSISR, dsisr),
> +	PT_REGS_OFFSET(PERF_REG_POWERPC_SIER, dar),
>   };
>   
>   u64 perf_reg_value(struct pt_regs *regs, int idx)
> diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> index 9e52c86ccbd3..ff91192407d1 100644
> --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
>   	PERF_REG_POWERPC_TRAP,
>   	PERF_REG_POWERPC_DAR,
>   	PERF_REG_POWERPC_DSISR,
> +	PERF_REG_POWERPC_SIER,
>   	PERF_REG_POWERPC_MAX,
>   };
>   #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
> diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
> index 00e37b106913..1076393e6f43 100644
> --- a/tools/perf/arch/powerpc/include/perf_regs.h
> +++ b/tools/perf/arch/powerpc/include/perf_regs.h
> @@ -62,7 +62,8 @@ static const char *reg_names[] = {
>   	[PERF_REG_POWERPC_SOFTE] = "softe",
>   	[PERF_REG_POWERPC_TRAP] = "trap",
>   	[PERF_REG_POWERPC_DAR] = "dar",
> -	[PERF_REG_POWERPC_DSISR] = "dsisr"
> +	[PERF_REG_POWERPC_DSISR] = "dsisr",
> +	[PERF_REG_POWERPC_SIER] = "sier"
>   };
>   
>   static inline const char *perf_reg_name(int id)
> diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
> index ec50939b0418..07fcd977d93e 100644
> --- a/tools/perf/arch/powerpc/util/perf_regs.c
> +++ b/tools/perf/arch/powerpc/util/perf_regs.c
> @@ -52,6 +52,7 @@ const struct sample_reg sample_reg_masks[] = {
>   	SMPL_REG(trap, PERF_REG_POWERPC_TRAP),
>   	SMPL_REG(dar, PERF_REG_POWERPC_DAR),
>   	SMPL_REG(dsisr, PERF_REG_POWERPC_DSISR),
> +	SMPL_REG(sier, PERF_REG_POWERPC_SIER),
>   	SMPL_REG_END
>   };
>   
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER
  2018-11-27  6:54 ` Christophe LEROY
@ 2018-11-27 10:55   ` Madhavan Srinivasan
  0 siblings, 0 replies; 7+ messages in thread
From: Madhavan Srinivasan @ 2018-11-27 10:55 UTC (permalink / raw)
  To: Christophe LEROY, mpe, acme
  Cc: Alexander Shishkin, linuxppc-dev, Ravi Bangoria, Anju T Sudhakar,
	Namhyung Kim, Jiri Olsa


On 27/11/18 12:24 PM, Christophe LEROY wrote:
>
>
> Le 26/11/2018 à 19:04, Madhavan Srinivasan a écrit :
>> On each sample, Sample Instruction Event Register (SIER) content
>> is saved in pt_regs. SIER does not have a entry as-is in the pt_regs
>> but instead, SIER content is saved in the "dar" register of pt_regs.
>>
>> Patch adds another entry to the perf_regs structure to include the 
>> "SIER"
>> printing which internally maps to the "dar" of pt_regs.
>>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
>> Cc: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
>> Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>> ---
>
> I see this patch is already in patchwork 
> https://patchwork.ozlabs.org/patch/941908/ and I can't spot any 
> difference between the two. Please say here (under the ---) the reason 
> for resending.


Yes there is no change, justed added the Tested-by tag
and resent it for arnaldo's review/ack.
May be should have avoided resend tag. My bad.

Maddy

>
> Christophe
>
>
>> arch/powerpc/include/uapi/asm/perf_regs.h       | 1 +
>>   arch/powerpc/perf/perf_regs.c                   | 1 +
>>   tools/arch/powerpc/include/uapi/asm/perf_regs.h | 1 +
>>   tools/perf/arch/powerpc/include/perf_regs.h     | 3 ++-
>>   tools/perf/arch/powerpc/util/perf_regs.c        | 1 +
>>   5 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h 
>> b/arch/powerpc/include/uapi/asm/perf_regs.h
>> index 9e52c86ccbd3..ff91192407d1 100644
>> --- a/arch/powerpc/include/uapi/asm/perf_regs.h
>> +++ b/arch/powerpc/include/uapi/asm/perf_regs.h
>> @@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
>>       PERF_REG_POWERPC_TRAP,
>>       PERF_REG_POWERPC_DAR,
>>       PERF_REG_POWERPC_DSISR,
>> +    PERF_REG_POWERPC_SIER,
>>       PERF_REG_POWERPC_MAX,
>>   };
>>   #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
>> diff --git a/arch/powerpc/perf/perf_regs.c 
>> b/arch/powerpc/perf/perf_regs.c
>> index 09ceea6175ba..c262aea22ad9 100644
>> --- a/arch/powerpc/perf/perf_regs.c
>> +++ b/arch/powerpc/perf/perf_regs.c
>> @@ -69,6 +69,7 @@ static unsigned int 
>> pt_regs_offset[PERF_REG_POWERPC_MAX] = {
>>       PT_REGS_OFFSET(PERF_REG_POWERPC_TRAP, trap),
>>       PT_REGS_OFFSET(PERF_REG_POWERPC_DAR, dar),
>>       PT_REGS_OFFSET(PERF_REG_POWERPC_DSISR, dsisr),
>> +    PT_REGS_OFFSET(PERF_REG_POWERPC_SIER, dar),
>>   };
>>     u64 perf_reg_value(struct pt_regs *regs, int idx)
>> diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h 
>> b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
>> index 9e52c86ccbd3..ff91192407d1 100644
>> --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
>> +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
>> @@ -46,6 +46,7 @@ enum perf_event_powerpc_regs {
>>       PERF_REG_POWERPC_TRAP,
>>       PERF_REG_POWERPC_DAR,
>>       PERF_REG_POWERPC_DSISR,
>> +    PERF_REG_POWERPC_SIER,
>>       PERF_REG_POWERPC_MAX,
>>   };
>>   #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
>> diff --git a/tools/perf/arch/powerpc/include/perf_regs.h 
>> b/tools/perf/arch/powerpc/include/perf_regs.h
>> index 00e37b106913..1076393e6f43 100644
>> --- a/tools/perf/arch/powerpc/include/perf_regs.h
>> +++ b/tools/perf/arch/powerpc/include/perf_regs.h
>> @@ -62,7 +62,8 @@ static const char *reg_names[] = {
>>       [PERF_REG_POWERPC_SOFTE] = "softe",
>>       [PERF_REG_POWERPC_TRAP] = "trap",
>>       [PERF_REG_POWERPC_DAR] = "dar",
>> -    [PERF_REG_POWERPC_DSISR] = "dsisr"
>> +    [PERF_REG_POWERPC_DSISR] = "dsisr",
>> +    [PERF_REG_POWERPC_SIER] = "sier"
>>   };
>>     static inline const char *perf_reg_name(int id)
>> diff --git a/tools/perf/arch/powerpc/util/perf_regs.c 
>> b/tools/perf/arch/powerpc/util/perf_regs.c
>> index ec50939b0418..07fcd977d93e 100644
>> --- a/tools/perf/arch/powerpc/util/perf_regs.c
>> +++ b/tools/perf/arch/powerpc/util/perf_regs.c
>> @@ -52,6 +52,7 @@ const struct sample_reg sample_reg_masks[] = {
>>       SMPL_REG(trap, PERF_REG_POWERPC_TRAP),
>>       SMPL_REG(dar, PERF_REG_POWERPC_DAR),
>>       SMPL_REG(dsisr, PERF_REG_POWERPC_DSISR),
>> +    SMPL_REG(sier, PERF_REG_POWERPC_SIER),
>>       SMPL_REG_END
>>   };
>>
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER
  2018-11-26 19:38 ` Arnaldo Carvalho de Melo
@ 2018-11-28  3:34   ` Michael Ellerman
  2018-11-28  5:02     ` Madhavan Srinivasan
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Ellerman @ 2018-11-28  3:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Madhavan Srinivasan
  Cc: Alexander Shishkin, linuxppc-dev, Thomas Richter,
	Hendrik Brueckner, Ravi Bangoria, Anju T Sudhakar,
	Martin Schwidefsky, Namhyung Kim, Jiri Olsa

Arnaldo Carvalho de Melo <acme@kernel.org> writes:

> Em Mon, Nov 26, 2018 at 11:34:08PM +0530, Madhavan Srinivasan escreveu:
>> On each sample, Sample Instruction Event Register (SIER) content
>> is saved in pt_regs. SIER does not have a entry as-is in the pt_regs
>> but instead, SIER content is saved in the "dar" register of pt_regs.
>> 
>> Patch adds another entry to the perf_regs structure to include the "SIER"
>> printing which internally maps to the "dar" of pt_regs.
>
> I think the patch is ok, when we talked in Vancouver I thought I saw
> something like this before, i.e. adding more registers to a perf_regs.h
> file, this was the cset:
>
>   commit 0da0017f72554c005c1a04c3adc5da9eb64fa7e5
>   Author: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
>   Date:   Wed Nov 8 07:30:15 2017 +0100
>
>       s390/perf: extend perf_regs support to include floating-point registers
>
> That I came across because it broke the perf build, making me add this
> cset:
>
>   commit 10b9baa701d5023897f70a4acb3bf0235da3dc4f
>   Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>   Date:   Tue Nov 28 11:08:41 2017 -0300
>
>     tools arch s390: Do not include header files from the kernel sources
>
> :-)
>
> Michael? What about the ppc specific details?

The only possible objection is that not all CPUs have an SIER register,
so on CPUs without it you'll get the content of the DAR register rather
than the SIER (because we (ab)use the DAR slot of pt_regs for the SIER).

Perhaps we should make sure that we return 0 on CPUs that don't have the
register?

cheers

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER
  2018-11-28  3:34   ` Michael Ellerman
@ 2018-11-28  5:02     ` Madhavan Srinivasan
  2018-11-28 22:14       ` Michael Ellerman
  0 siblings, 1 reply; 7+ messages in thread
From: Madhavan Srinivasan @ 2018-11-28  5:02 UTC (permalink / raw)
  To: Michael Ellerman, Arnaldo Carvalho de Melo
  Cc: Alexander Shishkin, linuxppc-dev, Thomas Richter,
	Hendrik Brueckner, Ravi Bangoria, Anju T Sudhakar,
	Martin Schwidefsky, Namhyung Kim, Jiri Olsa


On 28/11/18 9:04 AM, Michael Ellerman wrote:
> Arnaldo Carvalho de Melo <acme@kernel.org> writes:
>
>> Em Mon, Nov 26, 2018 at 11:34:08PM +0530, Madhavan Srinivasan escreveu:
>>> On each sample, Sample Instruction Event Register (SIER) content
>>> is saved in pt_regs. SIER does not have a entry as-is in the pt_regs
>>> but instead, SIER content is saved in the "dar" register of pt_regs.
>>>
>>> Patch adds another entry to the perf_regs structure to include the "SIER"
>>> printing which internally maps to the "dar" of pt_regs.
>> I think the patch is ok, when we talked in Vancouver I thought I saw
>> something like this before, i.e. adding more registers to a perf_regs.h
>> file, this was the cset:
>>
>>    commit 0da0017f72554c005c1a04c3adc5da9eb64fa7e5
>>    Author: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
>>    Date:   Wed Nov 8 07:30:15 2017 +0100
>>
>>        s390/perf: extend perf_regs support to include floating-point registers
>>
>> That I came across because it broke the perf build, making me add this
>> cset:
>>
>>    commit 10b9baa701d5023897f70a4acb3bf0235da3dc4f
>>    Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>>    Date:   Tue Nov 28 11:08:41 2017 -0300
>>
>>      tools arch s390: Do not include header files from the kernel sources
>>
>> :-)
>>
>> Michael? What about the ppc specific details?
> The only possible objection is that not all CPUs have an SIER register,
> so on CPUs without it you'll get the content of the DAR register rather
> than the SIER (because we (ab)use the DAR slot of pt_regs for the SIER).
>
> Perhaps we should make sure that we return 0 on CPUs that don't have the
> register?


Yes this make sense. We should make it zero instead of having the DAR
value. I will respin the patch with that change.

Thanks for the review comments.

Maddy

>
> cheers
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER
  2018-11-28  5:02     ` Madhavan Srinivasan
@ 2018-11-28 22:14       ` Michael Ellerman
  0 siblings, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2018-11-28 22:14 UTC (permalink / raw)
  To: Madhavan Srinivasan, Arnaldo Carvalho de Melo
  Cc: Alexander Shishkin, linuxppc-dev, Thomas Richter,
	Hendrik Brueckner, Ravi Bangoria, Anju T Sudhakar,
	Martin Schwidefsky, Namhyung Kim, Jiri Olsa

Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> On 28/11/18 9:04 AM, Michael Ellerman wrote:
>> Arnaldo Carvalho de Melo <acme@kernel.org> writes:
>>
>>> Em Mon, Nov 26, 2018 at 11:34:08PM +0530, Madhavan Srinivasan escreveu:
>>>> On each sample, Sample Instruction Event Register (SIER) content
>>>> is saved in pt_regs. SIER does not have a entry as-is in the pt_regs
>>>> but instead, SIER content is saved in the "dar" register of pt_regs.
>>>>
>>>> Patch adds another entry to the perf_regs structure to include the "SIER"
>>>> printing which internally maps to the "dar" of pt_regs.
>>> I think the patch is ok, when we talked in Vancouver I thought I saw
>>> something like this before, i.e. adding more registers to a perf_regs.h
>>> file, this was the cset:
>>>
>>>    commit 0da0017f72554c005c1a04c3adc5da9eb64fa7e5
>>>    Author: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
>>>    Date:   Wed Nov 8 07:30:15 2017 +0100
>>>
>>>        s390/perf: extend perf_regs support to include floating-point registers
>>>
>>> That I came across because it broke the perf build, making me add this
>>> cset:
>>>
>>>    commit 10b9baa701d5023897f70a4acb3bf0235da3dc4f
>>>    Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>>>    Date:   Tue Nov 28 11:08:41 2017 -0300
>>>
>>>      tools arch s390: Do not include header files from the kernel sources
>>>
>>> :-)
>>>
>>> Michael? What about the ppc specific details?
>> The only possible objection is that not all CPUs have an SIER register,
>> so on CPUs without it you'll get the content of the DAR register rather
>> than the SIER (because we (ab)use the DAR slot of pt_regs for the SIER).
>>
>> Perhaps we should make sure that we return 0 on CPUs that don't have the
>> register?
>
>
> Yes this make sense. We should make it zero instead of having the DAR
> value. I will respin the patch with that change.

Thanks.

There's three cases we need to cover.

The first is when we're not using core-book3s.c at all, ie. when we're
using the Freescale PMU code. You can probably handle that at build time
using CONFIG_FSL_EMB_PERF_EVENT?

Then there's the 32-bit case in core-book3s.c, currently that version of
perf_read_regs() never sets a value in dar.

And then there's the 64-bit case but PPMU_HAS_SIER is not set.

cheers

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-11-28 22:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-26 18:04 [PATCH RESEND] powerpc/perf: Update perf_regs structure to include SIER Madhavan Srinivasan
2018-11-26 19:38 ` Arnaldo Carvalho de Melo
2018-11-28  3:34   ` Michael Ellerman
2018-11-28  5:02     ` Madhavan Srinivasan
2018-11-28 22:14       ` Michael Ellerman
2018-11-27  6:54 ` Christophe LEROY
2018-11-27 10:55   ` Madhavan Srinivasan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.