<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On 21-Jul-2020, at 9:12 AM, Jordan Niethe <<a href="mailto:jniethe5@gmail.com" class="">jniethe5@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">On Sat, Jul 18, 2020 at 12:48 AM Athira Rajeev</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class=""><</span><a href="mailto:atrajeev@linux.vnet.ibm.com" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">atrajeev@linux.vnet.ibm.com</a><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">> wrote:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class="">core-book3s currently uses array to store the MMCR registers as part<br class="">of per-cpu `cpu_hw_events`. This patch does a clean up to use `struct`<br class="">to store mmcr regs instead of array. This will make code easier to read<br class="">and reduces chance of any subtle bug that may come in the future, say<br class="">when new registers are added. Patch updates all relevant code that was<br class="">using MMCR array ( cpuhw->mmcr[x]) to use newly introduced `struct`.<br class="">This includes the PMU driver code for supported platforms (power5<br class="">to power9) and ISA macros for counter support functions.<br class=""><br class="">Signed-off-by: Athira Rajeev <<a href="mailto:atrajeev@linux.vnet.ibm.com" class="">atrajeev@linux.vnet.ibm.com</a>><br class="">---<br class="">arch/powerpc/include/asm/perf_event_server.h | 10 ++++--<br class="">arch/powerpc/perf/core-book3s.c | 53 +++++++++++++---------------<br class="">arch/powerpc/perf/isa207-common.c | 20 +++++------<br class="">arch/powerpc/perf/isa207-common.h | 4 +--<br class="">arch/powerpc/perf/mpc7450-pmu.c | 21 +++++++----<br class="">arch/powerpc/perf/power5+-pmu.c | 17 ++++-----<br class="">arch/powerpc/perf/power5-pmu.c | 17 ++++-----<br class="">arch/powerpc/perf/power6-pmu.c | 16 ++++-----<br class="">arch/powerpc/perf/power7-pmu.c | 17 ++++-----<br class="">arch/powerpc/perf/ppc970-pmu.c | 24 ++++++-------<br class="">10 files changed, 105 insertions(+), 94 deletions(-)<br class=""><br class="">diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h<br class="">index 3e9703f..f9a3668 100644<br class="">--- a/arch/powerpc/include/asm/perf_event_server.h<br class="">+++ b/arch/powerpc/include/asm/perf_event_server.h<br class="">@@ -17,6 +17,12 @@<br class=""><br class="">struct perf_event;<br class=""><br class="">+struct mmcr_regs {<br class="">+ unsigned long mmcr0;<br class="">+ unsigned long mmcr1;<br class="">+ unsigned long mmcr2;<br class="">+ unsigned long mmcra;<br class="">+};<br class="">/*<br class=""> * This struct provides the constants and functions needed to<br class=""> * describe the PMU on a particular POWER-family CPU.<br class="">@@ -28,7 +34,7 @@ struct power_pmu {<br class=""> unsigned long add_fields;<br class=""> unsigned long test_adder;<br class=""> int (*compute_mmcr)(u64 events[], int n_ev,<br class="">- unsigned int hwc[], unsigned long mmcr[],<br class="">+ unsigned int hwc[], struct mmcr_regs *mmcr,<br class=""> struct perf_event *pevents[]);<br class=""> int (*get_constraint)(u64 event_id, unsigned long *mskp,<br class=""> unsigned long *valp);<br class="">@@ -41,7 +47,7 @@ struct power_pmu {<br class=""> unsigned long group_constraint_val;<br class=""> u64 (*bhrb_filter_map)(u64 branch_sample_type);<br class=""> void (*config_bhrb)(u64 pmu_bhrb_filter);<br class="">- void (*disable_pmc)(unsigned int pmc, unsigned long mmcr[]);<br class="">+ void (*disable_pmc)(unsigned int pmc, struct mmcr_regs *mmcr);<br class=""> int (*limited_pmc_event)(u64 event_id);<br class=""> u32 flags;<br class=""> const struct attribute_group **attr_groups;<br class="">diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c<br class="">index cd6a742..18b1b6a 100644<br class="">--- a/arch/powerpc/perf/core-book3s.c<br class="">+++ b/arch/powerpc/perf/core-book3s.c<br class="">@@ -37,12 +37,7 @@ struct cpu_hw_events {<br class=""> struct perf_event *event[MAX_HWEVENTS];<br class=""> u64 events[MAX_HWEVENTS];<br class=""> unsigned int flags[MAX_HWEVENTS];<br class="">- /*<br class="">- * The order of the MMCR array is:<br class="">- * - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2<br class="">- * - 32-bit, MMCR0, MMCR1, MMCR2<br class="">- */<br class="">- unsigned long mmcr[4];<br class="">+ struct mmcr_regs mmcr;<br class=""> struct perf_event *limited_counter[MAX_LIMITED_HWCOUNTERS];<br class=""> u8 limited_hwidx[MAX_LIMITED_HWCOUNTERS];<br class=""> u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];<br class="">@@ -121,7 +116,7 @@ static void ebb_event_add(struct perf_event *event) { }<br class="">static void ebb_switch_out(unsigned long mmcr0) { }<br class="">static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)<br class="">{<br class="">- return cpuhw->mmcr[0];<br class="">+ return cpuhw->mmcr.mmcr0;<br class="">}<br class=""><br class="">static inline void power_pmu_bhrb_enable(struct perf_event *event) {}<br class="">@@ -590,7 +585,7 @@ static void ebb_switch_out(unsigned long mmcr0)<br class=""><br class="">static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)<br class="">{<br class="">- unsigned long mmcr0 = cpuhw->mmcr[0];<br class="">+ unsigned long mmcr0 = cpuhw->mmcr.mmcr0;<br class=""><br class=""> if (!ebb)<br class=""> goto out;<br class="">@@ -624,7 +619,7 @@ static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)<br class=""> * unfreeze counters, it should not set exclude_xxx in its events and<br class=""> * instead manage the MMCR2 entirely by itself.<br class=""> */<br class="">- mtspr(SPRN_MMCR2, cpuhw->mmcr[3] | current->thread.mmcr2);<br class="">+ mtspr(SPRN_MMCR2, cpuhw->mmcr.mmcr2 | current->thread.mmcr2);<br class="">out:<br class=""> return mmcr0;<br class="">}<br class="">@@ -1232,9 +1227,9 @@ static void power_pmu_disable(struct pmu *pmu)<br class=""> /*<br class=""> * Disable instruction sampling if it was enabled<br class=""> */<br class="">- if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {<br class="">+ if (cpuhw->mmcr.mmcra & MMCRA_SAMPLE_ENABLE) {<br class=""> mtspr(SPRN_MMCRA,<br class="">- cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);<br class="">+ cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);<br class=""> mb();<br class=""> isync();<br class=""> }<br class="">@@ -1308,18 +1303,18 @@ static void power_pmu_enable(struct pmu *pmu)<br class=""> * (possibly updated for removal of events).<br class=""> */<br class=""> if (!cpuhw->n_added) {<br class="">- mtspr(SPRN_MMCRA, cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);<br class="">- mtspr(SPRN_MMCR1, cpuhw->mmcr[1]);<br class="">+ mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);<br class="">+ mtspr(SPRN_MMCR1, cpuhw->mmcr.mmcr1);<br class=""> goto out_enable;<br class=""> }<br class=""><br class=""> /*<br class=""> * Clear all MMCR settings and recompute them for the new set of events.<br class=""> */<br class="">- memset(cpuhw->mmcr, 0, sizeof(cpuhw->mmcr));<br class="">+ memset(&cpuhw->mmcr, 0, sizeof(cpuhw->mmcr));<br class=""><br class=""> if (ppmu->compute_mmcr(cpuhw->events, cpuhw->n_events, hwc_index,<br class="">- cpuhw->mmcr, cpuhw->event)) {<br class="">+ &cpuhw->mmcr, cpuhw->event)) {<br class=""> /* shouldn't ever get here */<br class=""> printk(KERN_ERR "oops compute_mmcr failed\n");<br class=""> goto out;<br class="">@@ -1333,11 +1328,11 @@ static void power_pmu_enable(struct pmu *pmu)<br class=""> */<br class=""> event = cpuhw->event[0];<br class=""> if (event->attr.exclude_user)<br class="">- cpuhw->mmcr[0] |= MMCR0_FCP;<br class="">+ cpuhw->mmcr.mmcr0 |= MMCR0_FCP;<br class=""> if (event->attr.exclude_kernel)<br class="">- cpuhw->mmcr[0] |= freeze_events_kernel;<br class="">+ cpuhw->mmcr.mmcr0 |= freeze_events_kernel;<br class=""> if (event->attr.exclude_hv)<br class="">- cpuhw->mmcr[0] |= MMCR0_FCHV;<br class="">+ cpuhw->mmcr.mmcr0 |= MMCR0_FCHV;<br class=""> }<br class=""><br class=""> /*<br class="">@@ -1346,12 +1341,12 @@ static void power_pmu_enable(struct pmu *pmu)<br class=""> * Then unfreeze the events.<br class=""> */<br class=""> ppc_set_pmu_inuse(1);<br class="">- mtspr(SPRN_MMCRA, cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);<br class="">- mtspr(SPRN_MMCR1, cpuhw->mmcr[1]);<br class="">- mtspr(SPRN_MMCR0, (cpuhw->mmcr[0] & ~(MMCR0_PMC1CE | MMCR0_PMCjCE))<br class="">+ mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);<br class="">+ mtspr(SPRN_MMCR1, cpuhw->mmcr.mmcr1);<br class="">+ mtspr(SPRN_MMCR0, (cpuhw->mmcr.mmcr0 & ~(MMCR0_PMC1CE | MMCR0_PMCjCE))<br class=""> | MMCR0_FC);<br class=""> if (ppmu->flags & PPMU_ARCH_207S)<br class="">- mtspr(SPRN_MMCR2, cpuhw->mmcr[3]);<br class="">+ mtspr(SPRN_MMCR2, cpuhw->mmcr.mmcr2);<br class=""><br class=""> /*<br class=""> * Read off any pre-existing events that need to move<br class="">@@ -1402,7 +1397,7 @@ static void power_pmu_enable(struct pmu *pmu)<br class=""> perf_event_update_userpage(event);<br class=""> }<br class=""> cpuhw->n_limited = n_lim;<br class="">- cpuhw->mmcr[0] |= MMCR0_PMXE | MMCR0_FCECE;<br class="">+ cpuhw->mmcr.mmcr0 |= MMCR0_PMXE | MMCR0_FCECE;<br class=""><br class=""> out_enable:<br class=""> pmao_restore_workaround(ebb);<br class="">@@ -1418,9 +1413,9 @@ static void power_pmu_enable(struct pmu *pmu)<br class=""> /*<br class=""> * Enable instruction sampling if necessary<br class=""> */<br class="">- if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {<br class="">+ if (cpuhw->mmcr.mmcra & MMCRA_SAMPLE_ENABLE) {<br class=""> mb();<br class="">- mtspr(SPRN_MMCRA, cpuhw->mmcr[2]);<br class="">+ mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra);<br class=""> }<br class=""><br class=""> out:<br class="">@@ -1550,7 +1545,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)<br class=""> cpuhw->flags[i-1] = cpuhw->flags[i];<br class=""> }<br class=""> --cpuhw->n_events;<br class="">- ppmu->disable_pmc(event->hw.idx - 1, cpuhw->mmcr);<br class="">+ ppmu->disable_pmc(event->hw.idx - 1, &cpuhw->mmcr);<br class=""> if (event->hw.idx) {<br class=""> write_pmc(event->hw.idx, 0);<br class=""> event->hw.idx = 0;<br class="">@@ -1571,7 +1566,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)<br class=""> }<br class=""> if (cpuhw->n_events == 0) {<br class=""> /* disable exceptions if no events are running */<br class="">- cpuhw->mmcr[0] &= ~(MMCR0_PMXE | MMCR0_FCECE);<br class="">+ cpuhw->mmcr.mmcr0 &= ~(MMCR0_PMXE | MMCR0_FCECE);<br class=""> }<br class=""><br class=""> if (has_branch_stack(event))<br class="">@@ -2240,7 +2235,7 @@ static void __perf_event_interrupt(struct pt_regs *regs)<br class=""> * XXX might want to use MSR.PM to keep the events frozen until<br class=""> * we get back out of this interrupt.<br class=""> */<br class="">- write_mmcr0(cpuhw, cpuhw->mmcr[0]);<br class="">+ write_mmcr0(cpuhw, cpuhw->mmcr.mmcr0);<br class=""><br class=""> if (nmi)<br class=""> nmi_exit();<br class="">@@ -2262,7 +2257,7 @@ static int power_pmu_prepare_cpu(unsigned int cpu)<br class=""><br class=""> if (ppmu) {<br class=""> memset(cpuhw, 0, sizeof(*cpuhw));<br class="">- cpuhw->mmcr[0] = MMCR0_FC;<br class="">+ cpuhw->mmcr.mmcr0 = MMCR0_FC;<br class=""> }<br class=""> return 0;<br class="">}<br class="">diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c<br class="">index 4c86da5..2fe63f2 100644<br class="">--- a/arch/powerpc/perf/isa207-common.c<br class="">+++ b/arch/powerpc/perf/isa207-common.c<br class="">@@ -363,7 +363,7 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp)<br class="">}<br class=""><br class="">int isa207_compute_mmcr(u64 event[], int n_ev,<br class="">- unsigned int hwc[], unsigned long mmcr[],<br class="">+ unsigned int hwc[], struct mmcr_regs *mmcr,<br class=""> struct perf_event *pevents[])<br class="">{<br class=""> unsigned long mmcra, mmcr1, mmcr2, unit, combine, psel, cache, val;<br class="">@@ -464,30 +464,30 @@ int isa207_compute_mmcr(u64 event[], int n_ev,<br class=""> }<br class=""><br class=""> /* Return MMCRx values */<br class="">- mmcr[0] = 0;<br class="">+ mmcr->mmcr0 = 0;<br class=""><br class=""> /* pmc_inuse is 1-based */<br class=""> if (pmc_inuse & 2)<br class="">- mmcr[0] = MMCR0_PMC1CE;<br class="">+ mmcr->mmcr0 = MMCR0_PMC1CE;<br class=""><br class=""> if (pmc_inuse & 0x7c)<br class="">- mmcr[0] |= MMCR0_PMCjCE;<br class="">+ mmcr->mmcr0 |= MMCR0_PMCjCE;<br class=""><br class=""> /* If we're not using PMC 5 or 6, freeze them */<br class=""> if (!(pmc_inuse & 0x60))<br class="">- mmcr[0] |= MMCR0_FC56;<br class="">+ mmcr->mmcr0 |= MMCR0_FC56;<br class=""><br class="">- mmcr[1] = mmcr1;<br class="">- mmcr[2] = mmcra;<br class="">- mmcr[3] = mmcr2;<br class="">+ mmcr->mmcr1 = mmcr1;<br class="">+ mmcr->mmcra = mmcra;<br class="">+ mmcr->mmcr2 = mmcr2;<br class=""><br class=""> return 0;<br class="">}<br class=""><br class="">-void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[])<br class="">+void isa207_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)<br class="">{<br class=""> if (pmc <= 3)<br class="">- mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));<br class="">+ mmcr->mmcr1 &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));<br class="">}<br class=""><br class="">static int find_alternative(u64 event, const unsigned int ev_alt[][MAX_ALT], int size)<br class="">diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h<br class="">index 63fd4f3..df968fd 100644<br class="">--- a/arch/powerpc/perf/isa207-common.h<br class="">+++ b/arch/powerpc/perf/isa207-common.h<br class="">@@ -217,9 +217,9 @@<br class=""><br class="">int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp);<br class="">int isa207_compute_mmcr(u64 event[], int n_ev,<br class="">- unsigned int hwc[], unsigned long mmcr[],<br class="">+ unsigned int hwc[], struct mmcr_regs *mmcr,<br class=""> struct perf_event *pevents[]);<br class="">-void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[]);<br class="">+void isa207_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr);<br class="">int isa207_get_alternatives(u64 event, u64 alt[], int size, unsigned int flags,<br class=""> const unsigned int ev_alt[][MAX_ALT]);<br class="">void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,<br class="">diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c<br class="">index 4d5ef92..826de25 100644<br class="">--- a/arch/powerpc/perf/mpc7450-pmu.c<br class="">+++ b/arch/powerpc/perf/mpc7450-pmu.c<br class="">@@ -257,7 +257,7 @@ static int mpc7450_get_alternatives(u64 event, unsigned int flags, u64 alt[])<br class=""> * Compute MMCR0/1/2 values for a set of events.<br class=""> */<br class="">static int mpc7450_compute_mmcr(u64 event[], int n_ev, unsigned int hwc[],<br class="">- unsigned long mmcr[],<br class="">+ struct mmcr_regs *mmcr,<br class=""> struct perf_event *pevents[])<br class="">{<br class=""> u8 event_index[N_CLASSES][N_COUNTER];<br class="">@@ -321,9 +321,16 @@ static int mpc7450_compute_mmcr(u64 event[], int n_ev, unsigned int hwc[],<br class=""> mmcr0 |= MMCR0_PMCnCE;<br class=""><br class=""> /* Return MMCRx values */<br class="">- mmcr[0] = mmcr0;<br class="">- mmcr[1] = mmcr1;<br class="">- mmcr[2] = mmcr2;<br class="">+ mmcr->mmcr0 = mmcr0;<br class="">+ mmcr->mmcr1 = mmcr1;<br class="">+ mmcr->mmcr2 = mmcr2;<br class=""></blockquote><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Will this mmcr->mmcr2 be used anywere, or will it always use mmcr->mmcra?</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div><br class=""></div>Hi Jordan</div><div><br class=""></div><div>We will be actually using mmcr->mmcra in the core-book3s for mpc7450-pmu.c</div><div>I have still assigned mmcr->mmcr2 so that it will work for any future changes if made in corebook</div><div><br class=""><blockquote type="cite" class=""><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">+ /*<br class="">+ * 32-bit doesn't have an MMCRA and uses SPRN_MMCR2 to define<br class="">+ * SPRN_MMCRA. So assign mmcra of cpu_hw_events with `mmcr2`<br class="">+ * value to ensure that any write to this SPRN_MMCRA will<br class="">+ * use mmcr2 value.<br class="">+ */<br class=""></blockquote><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Something like this comment would probably be useful to near the struct mmcr.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div><br class=""></div>Sure, I will add this information</div><div>Thanks for your suggestion</div><div><br class=""></div><div>Athira<br class=""><blockquote type="cite" class=""><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">+ mmcr->mmcra = mmcr2;<br class=""> return 0;<br class="">}<br class=""><br class="">@@ -331,12 +338,12 @@ static int mpc7450_compute_mmcr(u64 event[], int n_ev, unsigned int hwc[],<br class=""> * Disable counting by a PMC.<br class=""> * Note that the pmc argument is 0-based here, not 1-based.<br class=""> */<br class="">-static void mpc7450_disable_pmc(unsigned int pmc, unsigned long mmcr[])<br class="">+static void mpc7450_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)<br class="">{<br class=""> if (pmc <= 1)<br class="">- mmcr[0] &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);<br class="">+ mmcr->mmcr0 &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);<br class=""> else<br class="">- mmcr[1] &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);<br class="">+ mmcr->mmcr1 &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);<br class="">}<br class=""><br class="">static int mpc7450_generic_events[] = {<br class="">diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c<br class="">index f857454..5f0821e 100644<br class="">--- a/arch/powerpc/perf/power5+-pmu.c<br class="">+++ b/arch/powerpc/perf/power5+-pmu.c<br class="">@@ -448,7 +448,8 @@ static int power5p_marked_instr_event(u64 event)<br class="">}<br class=""><br class="">static int power5p_compute_mmcr(u64 event[], int n_ev,<br class="">- unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])<br class="">+ unsigned int hwc[], struct mmcr_regs *mmcr,<br class="">+ struct perf_event *pevents[])<br class="">{<br class=""> unsigned long mmcr1 = 0;<br class=""> unsigned long mmcra = 0;<br class="">@@ -586,20 +587,20 @@ static int power5p_compute_mmcr(u64 event[], int n_ev,<br class=""> }<br class=""><br class=""> /* Return MMCRx values */<br class="">- mmcr[0] = 0;<br class="">+ mmcr->mmcr0 = 0;<br class=""> if (pmc_inuse & 1)<br class="">- mmcr[0] = MMCR0_PMC1CE;<br class="">+ mmcr->mmcr0 = MMCR0_PMC1CE;<br class=""> if (pmc_inuse & 0x3e)<br class="">- mmcr[0] |= MMCR0_PMCjCE;<br class="">- mmcr[1] = mmcr1;<br class="">- mmcr[2] = mmcra;<br class="">+ mmcr->mmcr0 |= MMCR0_PMCjCE;<br class="">+ mmcr->mmcr1 = mmcr1;<br class="">+ mmcr->mmcra = mmcra;<br class=""> return 0;<br class="">}<br class=""><br class="">-static void power5p_disable_pmc(unsigned int pmc, unsigned long mmcr[])<br class="">+static void power5p_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)<br class="">{<br class=""> if (pmc <= 3)<br class="">- mmcr[1] &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));<br class="">+ mmcr->mmcr1 &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));<br class="">}<br class=""><br class="">static int power5p_generic_events[] = {<br class="">diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c<br class="">index da52eca..426021d 100644<br class="">--- a/arch/powerpc/perf/power5-pmu.c<br class="">+++ b/arch/powerpc/perf/power5-pmu.c<br class="">@@ -379,7 +379,8 @@ static int power5_marked_instr_event(u64 event)<br class="">}<br class=""><br class="">static int power5_compute_mmcr(u64 event[], int n_ev,<br class="">- unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])<br class="">+ unsigned int hwc[], struct mmcr_regs *mmcr,<br class="">+ struct perf_event *pevents[])<br class="">{<br class=""> unsigned long mmcr1 = 0;<br class=""> unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;<br class="">@@ -528,20 +529,20 @@ static int power5_compute_mmcr(u64 event[], int n_ev,<br class=""> }<br class=""><br class=""> /* Return MMCRx values */<br class="">- mmcr[0] = 0;<br class="">+ mmcr->mmcr0 = 0;<br class=""> if (pmc_inuse & 1)<br class="">- mmcr[0] = MMCR0_PMC1CE;<br class="">+ mmcr->mmcr0 = MMCR0_PMC1CE;<br class=""> if (pmc_inuse & 0x3e)<br class="">- mmcr[0] |= MMCR0_PMCjCE;<br class="">- mmcr[1] = mmcr1;<br class="">- mmcr[2] = mmcra;<br class="">+ mmcr->mmcr0 |= MMCR0_PMCjCE;<br class="">+ mmcr->mmcr1 = mmcr1;<br class="">+ mmcr->mmcra = mmcra;<br class=""> return 0;<br class="">}<br class=""><br class="">-static void power5_disable_pmc(unsigned int pmc, unsigned long mmcr[])<br class="">+static void power5_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)<br class="">{<br class=""> if (pmc <= 3)<br class="">- mmcr[1] &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));<br class="">+ mmcr->mmcr1 &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));<br class="">}<br class=""><br class="">static int power5_generic_events[] = {<br class="">diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c<br class="">index 3929cac..e343a51 100644<br class="">--- a/arch/powerpc/perf/power6-pmu.c<br class="">+++ b/arch/powerpc/perf/power6-pmu.c<br class="">@@ -171,7 +171,7 @@ static int power6_marked_instr_event(u64 event)<br class=""> * Assign PMC numbers and compute MMCR1 value for a set of events<br class=""> */<br class="">static int p6_compute_mmcr(u64 event[], int n_ev,<br class="">- unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])<br class="">+ unsigned int hwc[], struct mmcr_regs *mmcr, struct perf_event *pevents[])<br class="">{<br class=""> unsigned long mmcr1 = 0;<br class=""> unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;<br class="">@@ -243,13 +243,13 @@ static int p6_compute_mmcr(u64 event[], int n_ev,<br class=""> if (pmc < 4)<br class=""> mmcr1 |= (unsigned long)psel << MMCR1_PMCSEL_SH(pmc);<br class=""> }<br class="">- mmcr[0] = 0;<br class="">+ mmcr->mmcr0 = 0;<br class=""> if (pmc_inuse & 1)<br class="">- mmcr[0] = MMCR0_PMC1CE;<br class="">+ mmcr->mmcr0 = MMCR0_PMC1CE;<br class=""> if (pmc_inuse & 0xe)<br class="">- mmcr[0] |= MMCR0_PMCjCE;<br class="">- mmcr[1] = mmcr1;<br class="">- mmcr[2] = mmcra;<br class="">+ mmcr->mmcr0 |= MMCR0_PMCjCE;<br class="">+ mmcr->mmcr1 = mmcr1;<br class="">+ mmcr->mmcra = mmcra;<br class=""> return 0;<br class="">}<br class=""><br class="">@@ -457,11 +457,11 @@ static int p6_get_alternatives(u64 event, unsigned int flags, u64 alt[])<br class=""> return nalt;<br class="">}<br class=""><br class="">-static void p6_disable_pmc(unsigned int pmc, unsigned long mmcr[])<br class="">+static void p6_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)<br class="">{<br class=""> /* Set PMCxSEL to 0 to disable PMCx */<br class=""> if (pmc <= 3)<br class="">- mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));<br class="">+ mmcr->mmcr1 &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));<br class="">}<br class=""><br class="">static int power6_generic_events[] = {<br class="">diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c<br class="">index a137813..3152336 100644<br class="">--- a/arch/powerpc/perf/power7-pmu.c<br class="">+++ b/arch/powerpc/perf/power7-pmu.c<br class="">@@ -242,7 +242,8 @@ static int power7_marked_instr_event(u64 event)<br class="">}<br class=""><br class="">static int power7_compute_mmcr(u64 event[], int n_ev,<br class="">- unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])<br class="">+ unsigned int hwc[], struct mmcr_regs *mmcr,<br class="">+ struct perf_event *pevents[])<br class="">{<br class=""> unsigned long mmcr1 = 0;<br class=""> unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;<br class="">@@ -298,20 +299,20 @@ static int power7_compute_mmcr(u64 event[], int n_ev,<br class=""> }<br class=""><br class=""> /* Return MMCRx values */<br class="">- mmcr[0] = 0;<br class="">+ mmcr->mmcr0 = 0;<br class=""> if (pmc_inuse & 1)<br class="">- mmcr[0] = MMCR0_PMC1CE;<br class="">+ mmcr->mmcr0 = MMCR0_PMC1CE;<br class=""> if (pmc_inuse & 0x3e)<br class="">- mmcr[0] |= MMCR0_PMCjCE;<br class="">- mmcr[1] = mmcr1;<br class="">- mmcr[2] = mmcra;<br class="">+ mmcr->mmcr0 |= MMCR0_PMCjCE;<br class="">+ mmcr->mmcr1 = mmcr1;<br class="">+ mmcr->mmcra = mmcra;<br class=""> return 0;<br class="">}<br class=""><br class="">-static void power7_disable_pmc(unsigned int pmc, unsigned long mmcr[])<br class="">+static void power7_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)<br class="">{<br class=""> if (pmc <= 3)<br class="">- mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));<br class="">+ mmcr->mmcr1 &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));<br class="">}<br class=""><br class="">static int power7_generic_events[] = {<br class="">diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c<br class="">index 4035d93..89a90ab 100644<br class="">--- a/arch/powerpc/perf/ppc970-pmu.c<br class="">+++ b/arch/powerpc/perf/ppc970-pmu.c<br class="">@@ -253,7 +253,8 @@ static int p970_get_alternatives(u64 event, unsigned int flags, u64 alt[])<br class="">}<br class=""><br class="">static int p970_compute_mmcr(u64 event[], int n_ev,<br class="">- unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])<br class="">+ unsigned int hwc[], struct mmcr_regs *mmcr,<br class="">+ struct perf_event *pevents[])<br class="">{<br class=""> unsigned long mmcr0 = 0, mmcr1 = 0, mmcra = 0;<br class=""> unsigned int pmc, unit, byte, psel;<br class="">@@ -393,27 +394,26 @@ static int p970_compute_mmcr(u64 event[], int n_ev,<br class=""> mmcra |= 0x2000; /* mark only one IOP per PPC instruction */<br class=""><br class=""> /* Return MMCRx values */<br class="">- mmcr[0] = mmcr0;<br class="">- mmcr[1] = mmcr1;<br class="">- mmcr[2] = mmcra;<br class="">+ mmcr->mmcr0 = mmcr0;<br class="">+ mmcr->mmcr1 = mmcr1;<br class="">+ mmcr->mmcra = mmcra;<br class=""> return 0;<br class="">}<br class=""><br class="">-static void p970_disable_pmc(unsigned int pmc, unsigned long mmcr[])<br class="">+static void p970_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)<br class="">{<br class="">- int shift, i;<br class="">+ int shift;<br class=""><br class="">+ /*<br class="">+ * Setting the PMCxSEL field to 0x08 disables PMC x.<br class="">+ */<br class=""> if (pmc <= 1) {<br class=""> shift = MMCR0_PMC1SEL_SH - 7 * pmc;<br class="">- i = 0;<br class="">+ mmcr->mmcr0 = (mmcr->mmcr0 & ~(0x1fUL << shift)) | (0x08UL << shift);<br class=""> } else {<br class=""> shift = MMCR1_PMC3SEL_SH - 5 * (pmc - 2);<br class="">- i = 1;<br class="">+ mmcr->mmcr1 = (mmcr->mmcr1 & ~(0x1fUL << shift)) | (0x08UL << shift);<br class=""> }<br class="">- /*<br class="">- * Setting the PMCxSEL field to 0x08 disables PMC x.<br class="">- */<br class="">- mmcr[i] = (mmcr[i] & ~(0x1fUL << shift)) | (0x08UL << shift);<br class="">}<br class=""><br class="">static int ppc970_generic_events[] = {<br class="">--<br class="">1.8.3.1</blockquote></div></blockquote></div><br class=""></body></html>