Re: [PATCH 8/8] powerpc/perf: Add power8 EBB support

2013-07-05 Thread Adhemerval Zanella 
On 04-07-2013 23:54, Michael Ellerman wrote:
 On Thu, Jul 04, 2013 at 03:58:01PM -0300, Adhemerval Zanella wrote:
 Hi Michael,

 I believe you forgot to add the cpu_user_features2 bit to announce the EBB 
 support
 for P8, patch following:
 Hi Adhemerval,

 You're right, I haven't added it. I was wondering how best to do it.

 It's possible to configure the kernel so that it doesn't have PMU
 support, and in that case EBB is unsupported. It's also possible that 
 something
 goes wrong with the PMU registration (kernel bug or OOM), and again EBB is 
 then
 unsupported.

 So I think it might be better if we add PPC_FEATURE2_EBB at runtime in
 init_power8_pmu().

 What do you think?
Indeed your approach seems better (I wasn't aware you could configure
kernel with perf subsystem).


 Something like:

 diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
 index c7f8ccc..fd9ed89 100644
 --- a/arch/powerpc/perf/power8-pmu.c
 +++ b/arch/powerpc/perf/power8-pmu.c
 @@ -620,10 +682,19 @@ static struct power_pmu power8_pmu = {

  static int __init init_power8_pmu(void)
  {
 +   int rc;
 +
 if (!cur_cpu_spec-oprofile_cpu_type ||
 strcmp(cur_cpu_spec-oprofile_cpu_type, ppc64/power8))
 return -ENODEV;

 -   return register_power_pmu(power8_pmu);
 +   rc = register_power_pmu(power8_pmu);
 +   if (rc)
 +   return rc;
 +
 +   /* Tell userspace that EBB is supported */
 +   cur_cpu_spec-cpu_user_features2 |= PPC_FEATURE2_EBB;
 +
 +   return 0;
  }
  early_initcall(init_power8_pmu);


 cheers


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 8/8] powerpc/perf: Add power8 EBB support

2013-07-04 Thread Adhemerval Zanella 
Hi Michael,

I believe you forgot to add the cpu_user_features2 bit to announce the EBB 
support
for P8, patch following:

Signed-off-by: Adhemerval Zanella azane...@linux.vnet.ibm.com
---
 arch/powerpc/kernel/cputable.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 2a45d0f..5f0c80a 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -105,6 +105,7 @@ extern void __restore_cpu_e6500(void);
 PPC_FEATURE_PSERIES_PERFMON_COMPAT)
 #define COMMON_USER2_POWER8(PPC_FEATURE2_ARCH_2_07 | \
 PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_DSCR | \
+PPC_FEATURE2_EBB | \
 PPC_FEATURE2_ISEL | PPC_FEATURE2_TAR)
 #define COMMON_USER_PA6T   (COMMON_USER_PPC64 | PPC_FEATURE_PA6T |\
 PPC_FEATURE_TRUE_LE | \


On 28-06-2013 01:15, Anshuman Khandual wrote:
 On 06/27/2013 05:22 PM, Michael Ellerman wrote:
 On Wed, 2013-06-26 at 15:28 +0530, Anshuman Khandual wrote:
 @@ -117,6 +117,7 @@
 (EVENT_UNIT_MASK   EVENT_UNIT_SHIFT) |   \
 (EVENT_COMBINE_MASKEVENT_COMBINE_SHIFT)  |   \
 (EVENT_MARKED_MASK EVENT_MARKED_SHIFT)   |   \
 +   (1ull  EVENT_CONFIG_EBB_SHIFT)   |   \
 We should define this macro like EVENT_MARKED_MASK

 #define EVENT_EBB_MASK   0x1

 Numeric value of 1ull stands out odd in the scheme.
 Yeah I guess.

 Would you like it in blue? :)

 :) No, I meant probably a macro definition would be cool.

 ___
 Linuxppc-dev mailing list
 Linuxppc-dev@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/linuxppc-dev


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 8/8] powerpc/perf: Add power8 EBB support

2013-07-04 Thread Michael Ellerman 
On Thu, Jul 04, 2013 at 03:58:01PM -0300, Adhemerval Zanella wrote:
 Hi Michael,
 
 I believe you forgot to add the cpu_user_features2 bit to announce the EBB 
 support
 for P8, patch following:

Hi Adhemerval,

You're right, I haven't added it. I was wondering how best to do it.

It's possible to configure the kernel so that it doesn't have PMU
support, and in that case EBB is unsupported. It's also possible that something
goes wrong with the PMU registration (kernel bug or OOM), and again EBB is then
unsupported.

So I think it might be better if we add PPC_FEATURE2_EBB at runtime in
init_power8_pmu().

What do you think?

Something like:

diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index c7f8ccc..fd9ed89 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -620,10 +682,19 @@ static struct power_pmu power8_pmu = {
 
 static int __init init_power8_pmu(void)
 {
+   int rc;
+
if (!cur_cpu_spec-oprofile_cpu_type ||
strcmp(cur_cpu_spec-oprofile_cpu_type, ppc64/power8))
return -ENODEV;
 
-   return register_power_pmu(power8_pmu);
+   rc = register_power_pmu(power8_pmu);
+   if (rc)
+   return rc;
+
+   /* Tell userspace that EBB is supported */
+   cur_cpu_spec-cpu_user_features2 |= PPC_FEATURE2_EBB;
+
+   return 0;
 }
 early_initcall(init_power8_pmu);


cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 8/8] powerpc/perf: Add power8 EBB support

2013-06-27 Thread Michael Ellerman 

On Wed, 2013-06-26 at 15:28 +0530, Anshuman Khandual wrote:
  @@ -117,6 +117,7 @@
   (EVENT_UNIT_MASK   EVENT_UNIT_SHIFT) |   \
   (EVENT_COMBINE_MASKEVENT_COMBINE_SHIFT)  |   \
   (EVENT_MARKED_MASK EVENT_MARKED_SHIFT)   |   \
  +(1ull  EVENT_CONFIG_EBB_SHIFT)   |   \
 
 We should define this macro like EVENT_MARKED_MASK
 
 #define EVENT_EBB_MASK   0x1
 
 Numeric value of 1ull stands out odd in the scheme.

Yeah I guess.

Would you like it in blue? :)

cheers


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 8/8] powerpc/perf: Add power8 EBB support

2013-06-27 Thread Anshuman Khandual 
On 06/27/2013 05:22 PM, Michael Ellerman wrote:
 
 On Wed, 2013-06-26 at 15:28 +0530, Anshuman Khandual wrote:
 @@ -117,6 +117,7 @@
  (EVENT_UNIT_MASK   EVENT_UNIT_SHIFT) |   \
  (EVENT_COMBINE_MASKEVENT_COMBINE_SHIFT)  |   \
  (EVENT_MARKED_MASK EVENT_MARKED_SHIFT)   |   \
 +(1ull  EVENT_CONFIG_EBB_SHIFT)   |   \

 We should define this macro like EVENT_MARKED_MASK

 #define EVENT_EBB_MASK   0x1

 Numeric value of 1ull stands out odd in the scheme.
 
 Yeah I guess.
 
 Would you like it in blue? :)
 

:) No, I meant probably a macro definition would be cool.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 8/8] powerpc/perf: Add power8 EBB support

2013-06-26 Thread Anshuman Khandual 
 @@ -117,6 +117,7 @@
(EVENT_UNIT_MASK   EVENT_UNIT_SHIFT) |   \
(EVENT_COMBINE_MASKEVENT_COMBINE_SHIFT)  |   \
(EVENT_MARKED_MASK EVENT_MARKED_SHIFT)   |   \
 +  (1ull  EVENT_CONFIG_EBB_SHIFT)   |   \

We should define this macro like EVENT_MARKED_MASK

#define EVENT_EBB_MASK   0x1

Numeric value of 1ull stands out odd in the scheme.


 EVENT_PSEL_MASK)
 
 + *  EBB -*| |
 + *| |  Count of events for 
 each PMC.
 + *  L1 I/D qualifier -* |p1, p2, p3, p4, p5, 
 p6.
   * nc - number of counters -*
   *
   * The PMC fields P1..P6, and NC, are adder fields. As we accumulate 
 constraints
 @@ -159,6 +160,9 @@
  #define CNST_THRESH_VAL(v)   (((v)  EVENT_THRESH_MASK)  32)
  #define CNST_THRESH_MASK CNST_THRESH_VAL(EVENT_THRESH_MASK)
 
 +#define CNST_EBB_VAL(v)  (((v)  1)  24)

EVENT_EBB_MASK can be used here as well.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 8/8] powerpc/perf: Add power8 EBB support

2013-06-24 Thread Michael Ellerman 
Add logic to the power8 PMU code to support EBB. Future processors would
also be expected to implement similar constraints. At that time we could
possibly factor these out into common code.

Finally mark the power8 PMU as supporting EBB, which is the actual
enable switch which allows EBBs to be configured.

Signed-off-by: Michael Ellerman mich...@ellerman.id.au
---
 arch/powerpc/perf/power8-pmu.c |   44 +---
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index d59f5b2..c7f8ccc 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -31,9 +31,9 @@
  *
  *60565248444036   
 32
  * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - 
- - |
- * [  thresh_cmp ]   [  thresh_ctl 
  ]
- *   |
- *   thresh start/stop OR FAB match -*
+ *   | [  thresh_cmp ]   [  thresh_ctl 
  ]
+ *   |   |
+ *   *- EBB (Linux)  thresh start/stop OR FAB match -*
  *
  *2824201612 8 4   
  0
  * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - 
- - |
@@ -117,6 +117,7 @@
 (EVENT_UNIT_MASK   EVENT_UNIT_SHIFT) |   \
 (EVENT_COMBINE_MASKEVENT_COMBINE_SHIFT)  |   \
 (EVENT_MARKED_MASK EVENT_MARKED_SHIFT)   |   \
+(1ull  EVENT_CONFIG_EBB_SHIFT)   |   \
  EVENT_PSEL_MASK)
 
 /* MMCRA IFM bits - POWER8 */
@@ -140,10 +141,10 @@
  *
  *2824201612 8 4   
  0
  * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - 
- - |
- *   [ ]   [  sample ]   [ ]   [6] [5]   [4] [3]   [2] 
[1]
- *| |
- *  L1 I/D qualifier -* |  Count of events for 
each PMC.
- *  |p1, p2, p3, p4, p5, 
p6.
+ *   |   [ ]   [  sample ]   [ ]   [6] [5]   [4] [3]   [2] 
[1]
+ *  EBB -*| |
+ *| |  Count of events for 
each PMC.
+ *  L1 I/D qualifier -* |p1, p2, p3, p4, p5, 
p6.
  * nc - number of counters -*
  *
  * The PMC fields P1..P6, and NC, are adder fields. As we accumulate 
constraints
@@ -159,6 +160,9 @@
 #define CNST_THRESH_VAL(v) (((v)  EVENT_THRESH_MASK)  32)
 #define CNST_THRESH_MASK   CNST_THRESH_VAL(EVENT_THRESH_MASK)
 
+#define CNST_EBB_VAL(v)(((v)  1)  24)
+#define CNST_EBB_MASK  CNST_EBB_VAL(1)
+
 #define CNST_L1_QUAL_VAL(v)(((v)  3)  22)
 #define CNST_L1_QUAL_MASK  CNST_L1_QUAL_VAL(3)
 
@@ -217,7 +221,7 @@ static inline bool event_is_fab_match(u64 event)
 
 static int power8_get_constraint(u64 event, unsigned long *maskp, unsigned 
long *valp)
 {
-   unsigned int unit, pmc, cache;
+   unsigned int unit, pmc, cache, ebb;
unsigned long mask, value;
 
mask = value = 0;
@@ -225,9 +229,13 @@ static int power8_get_constraint(u64 event, unsigned long 
*maskp, unsigned long
if (event  ~EVENT_VALID_MASK)
return -1;
 
-   pmc   = (event  EVENT_PMC_SHIFT)EVENT_PMC_MASK;
-   unit  = (event  EVENT_UNIT_SHIFT)   EVENT_UNIT_MASK;
-   cache = (event  EVENT_CACHE_SEL_SHIFT)  EVENT_CACHE_SEL_MASK;
+   pmc   = (event  EVENT_PMC_SHIFT) EVENT_PMC_MASK;
+   unit  = (event  EVENT_UNIT_SHIFT)EVENT_UNIT_MASK;
+   cache = (event  EVENT_CACHE_SEL_SHIFT)   EVENT_CACHE_SEL_MASK;
+   ebb   = (event  EVENT_CONFIG_EBB_SHIFT)  1;
+
+   /* Clear the EBB bit in the event, so event checks work below */
+   event = ~(1ull  EVENT_CONFIG_EBB_SHIFT);
 
if (pmc) {
if (pmc  6)
@@ -297,6 +305,18 @@ static int power8_get_constraint(u64 event, unsigned long 
*maskp, unsigned long
value |= CNST_THRESH_VAL(event  EVENT_THRESH_SHIFT);
}
 
+   if (!pmc  ebb)
+   /* EBB events must specify the PMC */
+   return -1;
+
+   /*
+* All events must agree on EBB, either all request it or none.
+* EBB events are pinned  exclusive, so this should never actually
+* hit, but we leave it as a fallback in case.
+*/
+   mask  |= CNST_EBB_VAL(ebb);
+   value |= CNST_EBB_MASK;
+
*maskp = mask;
*valp = value;
 
@@ -591,7 +611,7 @@ static struct power_pmu power8_pmu = {