Re: [PATCH v2 06/10] powerpc/perf: power10 Performance Monitoring support

2020-07-08 Thread Athira Rajeev



> On 07-Jul-2020, at 12:20 PM, Michael Neuling  wrote:
> 
> 
>> @@ -480,6 +520,7 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
>>  mmcr[1] = mmcr1;
>>  mmcr[2] = mmcra;
>>  mmcr[3] = mmcr2;
>> +mmcr[4] = mmcr3;
> 
> This is fragile like the kvm vcpu case I commented on before but it gets 
> passed
> in via a function parameter?! Can you create a struct to store these in rather
> than this odd ball numbering?

Mikey,
Yes, it gets passed as cpuhw->mmcr array 
I will check on these cleanup changes for the kvm vcpu case as well as 
cpu_hw_events mmcr array

Thanks
Athira
> 
> The cleanup should start in patch 1/10 here:
> 
>/*
> * The order of the MMCR array is:
> -*  - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2
> +*  - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2, MMCR3
> *  - 32-bit, MMCR0, MMCR1, MMCR2
> */
> -   unsigned long mmcr[4];
> +   unsigned long mmcr[5];
> 
> 
> 
> mikey



Re: [PATCH v2 06/10] powerpc/perf: power10 Performance Monitoring support

2020-07-07 Thread Michael Neuling


> @@ -480,6 +520,7 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
>   mmcr[1] = mmcr1;
>   mmcr[2] = mmcra;
>   mmcr[3] = mmcr2;
> + mmcr[4] = mmcr3;

This is fragile like the kvm vcpu case I commented on before but it gets passed
in via a function parameter?! Can you create a struct to store these in rather
than this odd ball numbering?

The cleanup should start in patch 1/10 here:

/*
 * The order of the MMCR array is:
-*  - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2
+*  - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2, MMCR3
 *  - 32-bit, MMCR0, MMCR1, MMCR2
 */
-   unsigned long mmcr[4];
+   unsigned long mmcr[5];



mikey


Re: [PATCH v2 06/10] powerpc/perf: power10 Performance Monitoring support

2020-07-02 Thread kernel test robot
Hi Athira,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on tip/perf/core v5.8-rc3 next-20200702]
[cannot apply to kvm-ppc/kvm-ppc-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Athira-Rajeev/powerpc-perf-Add-support-for-power10-PMU-Hardware/20200701-181147
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross 
ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):

>> arch/powerpc/perf/power10-pmu.c:393:5: warning: no previous prototype for 
>> 'init_power10_pmu' [-Wmissing-prototypes]
 393 | int init_power10_pmu(void)
 | ^~~~

vim +/init_power10_pmu +393 arch/powerpc/perf/power10-pmu.c

   392  
 > 393  int init_power10_pmu(void)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip


[PATCH v2 06/10] powerpc/perf: power10 Performance Monitoring support

2020-07-01 Thread Athira Rajeev
Base enablement patch to register performance monitoring
hardware support for power10. Patch introduce the raw event
encoding format, defines the supported list of events, config
fields for the event attributes and their corresponding bit values
which are exported via sysfs.

Patch also enhances the support function in isa207_common.c to
include power10 pmu hardware.

[Enablement of base PMU driver code]
Signed-off-by: Madhavan Srinivasan 
[Addition of ISA macros for counter support functions]
Signed-off-by: Athira Rajeev 
---
 arch/powerpc/perf/Makefile  |   2 +-
 arch/powerpc/perf/core-book3s.c |   2 +
 arch/powerpc/perf/internal.h|   1 +
 arch/powerpc/perf/isa207-common.c   |  59 -
 arch/powerpc/perf/isa207-common.h   |  33 ++-
 arch/powerpc/perf/power10-events-list.h |  70 ++
 arch/powerpc/perf/power10-pmu.c | 410 
 7 files changed, 566 insertions(+), 11 deletions(-)
 create mode 100644 arch/powerpc/perf/power10-events-list.h
 create mode 100644 arch/powerpc/perf/power10-pmu.c

diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
index 53d614e..c02854d 100644
--- a/arch/powerpc/perf/Makefile
+++ b/arch/powerpc/perf/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_PPC_PERF_CTRS) += core-book3s.o bhrb.o
 obj64-$(CONFIG_PPC_PERF_CTRS)  += ppc970-pmu.o power5-pmu.o \
   power5+-pmu.o power6-pmu.o power7-pmu.o \
   isa207-common.o power8-pmu.o power9-pmu.o \
-  generic-compat-pmu.o
+  generic-compat-pmu.o power10-pmu.o
 obj32-$(CONFIG_PPC_PERF_CTRS)  += mpc7450-pmu.o
 
 obj-$(CONFIG_PPC_POWERNV)  += imc-pmu.o
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 58bfb9a..fad5159 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2333,6 +2333,8 @@ static int __init init_ppc64_pmu(void)
return 0;
else if (!init_power9_pmu())
return 0;
+   else if (!init_power10_pmu())
+   return 0;
else if (!init_ppc970_pmu())
return 0;
else
diff --git a/arch/powerpc/perf/internal.h b/arch/powerpc/perf/internal.h
index f755c64..80bbf72 100644
--- a/arch/powerpc/perf/internal.h
+++ b/arch/powerpc/perf/internal.h
@@ -9,4 +9,5 @@
 extern int init_power7_pmu(void);
 extern int init_power8_pmu(void);
 extern int init_power9_pmu(void);
+extern int init_power10_pmu(void);
 extern int init_generic_compat_pmu(void);
diff --git a/arch/powerpc/perf/isa207-common.c 
b/arch/powerpc/perf/isa207-common.c
index 4c86da5..7d4839e 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -55,7 +55,9 @@ static bool is_event_valid(u64 event)
 {
u64 valid_mask = EVENT_VALID_MASK;
 
-   if (cpu_has_feature(CPU_FTR_ARCH_300))
+   if (cpu_has_feature(CPU_FTR_ARCH_31))
+   valid_mask = p10_EVENT_VALID_MASK;
+   else if (cpu_has_feature(CPU_FTR_ARCH_300))
valid_mask = p9_EVENT_VALID_MASK;
 
return !(event & ~valid_mask);
@@ -69,6 +71,14 @@ static inline bool is_event_marked(u64 event)
return false;
 }
 
+static unsigned long sdar_mod_val(u64 event)
+{
+   if (cpu_has_feature(CPU_FTR_ARCH_31))
+   return p10_SDAR_MODE(event);
+
+   return p9_SDAR_MODE(event);
+}
+
 static void mmcra_sdar_mode(u64 event, unsigned long *mmcra)
 {
/*
@@ -79,7 +89,7 @@ static void mmcra_sdar_mode(u64 event, unsigned long *mmcra)
 * MMCRA[SDAR_MODE] will be programmed as "0b01" for continous sampling
 * mode and will be un-changed when setting MMCRA[63] (Marked events).
 *
-* Incase of Power9:
+* Incase of Power9/power10:
 * Marked event: MMCRA[SDAR_MODE] will be set to 0b00 ('No Updates'),
 *   or if group already have any marked events.
 * For rest
@@ -90,8 +100,8 @@ static void mmcra_sdar_mode(u64 event, unsigned long *mmcra)
if (cpu_has_feature(CPU_FTR_ARCH_300)) {
if (is_event_marked(event) || (*mmcra & MMCRA_SAMPLE_ENABLE))
*mmcra &= MMCRA_SDAR_MODE_NO_UPDATES;
-   else if (p9_SDAR_MODE(event))
-   *mmcra |=  p9_SDAR_MODE(event) << MMCRA_SDAR_MODE_SHIFT;
+   else if (sdar_mod_val(event))
+   *mmcra |= sdar_mod_val(event) << MMCRA_SDAR_MODE_SHIFT;
else
*mmcra |= MMCRA_SDAR_MODE_DCACHE;
} else
@@ -134,7 +144,11 @@ static bool is_thresh_cmp_valid(u64 event)
/*
 * Check the mantissa upper two bits are not zero, unless the
 * exponent is also zero. See the THRESH_CMP_MANTISSA doc.
+* Power10: thresh_cmp is replaced by l2_l3 event select.
 */
+   if (cpu_has_feature(CPU_FTR_ARCH_31))
+