<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 13-Jul-2020, at 6:20 PM, Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" class="">mpe@ellerman.id.au</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="">Athira Rajeev <</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="">> writes:</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=""><blockquote type="cite" class="">On 08-Jul-2020, at 4:32 PM, Michael Ellerman <<a href="mailto:mpe@ellerman.id.au" class="">mpe@ellerman.id.au</a>> wrote:<br class=""><br class="">Athira Rajeev <<a href="mailto:atrajeev@linux.vnet.ibm.com" class="">atrajeev@linux.vnet.ibm.com</a>> writes:<br class="">...<br class=""><blockquote type="cite" class="">diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c<br class="">index cd6a742..5c64bd3 100644<br class="">--- a/arch/powerpc/perf/core-book3s.c<br class="">+++ b/arch/powerpc/perf/core-book3s.c<br class="">@@ -39,10 +39,10 @@ struct cpu_hw_events {<br class=""><span class="Apple-tab-span" style="white-space: pre;">       </span>unsigned int flags[MAX_HWEVENTS];<br class=""><span class="Apple-tab-span" style="white-space: pre;">    </span>/*<br class=""><span class="Apple-tab-span" style="white-space: pre;">   </span><span class="Apple-converted-space"> </span>* The order of the MMCR array is:<br class="">-<span class="Apple-tab-span" style="white-space: pre;">    </span><span class="Apple-converted-space"> </span>*  - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span>*  - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2, MMCR3<br class=""><span class="Apple-tab-span" style="white-space: pre;">   </span><span class="Apple-converted-space"> </span>*  - 32-bit, MMCR0, MMCR1, MMCR2<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span>*/<br class="">-<span class="Apple-tab-span" style="white-space: pre;">   </span>unsigned long mmcr[4];<br class="">+<span class="Apple-tab-span" style="white-space: pre;">      </span>unsigned long mmcr[5];<br class=""><span class="Apple-tab-span" style="white-space: pre;">       </span>struct perf_event *limited_counter[MAX_LIMITED_HWCOUNTERS];<br class=""><span class="Apple-tab-span" style="white-space: pre;">  </span>u8  limited_hwidx[MAX_LIMITED_HWCOUNTERS];<br class=""><span class="Apple-tab-span" style="white-space: pre;">      </span>u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];<br class=""></blockquote>...<br class=""><blockquote type="cite" class="">@@ -1310,6 +1326,10 @@ static void power_pmu_enable(struct pmu *pmu)<br class=""><span class="Apple-tab-span" style="white-space: pre;">       </span>if (!cpuhw->n_added) {<br class=""><span class="Apple-tab-span" style="white-space: pre;">    </span><span class="Apple-tab-span" style="white-space: pre;">  </span>mtspr(SPRN_MMCRA, cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);<br class=""><span class="Apple-tab-span" style="white-space: pre;">     </span><span class="Apple-tab-span" style="white-space: pre;">  </span>mtspr(SPRN_MMCR1, cpuhw->mmcr[1]);<br class="">+#ifdef CONFIG_PPC64<br class="">+<span class="Apple-tab-span" style="white-space: pre;">      </span><span class="Apple-tab-span" style="white-space: pre;">  </span>if (ppmu->flags & PPMU_ARCH_310S)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">    </span><span class="Apple-tab-span" style="white-space: pre;">  </span><span class="Apple-tab-span" style="white-space: pre;">  </span>mtspr(SPRN_MMCR3, cpuhw->mmcr[4]);<br class="">+#endif /* CONFIG_PPC64 */<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;">  </span>goto out_enable;<br class=""><span class="Apple-tab-span" style="white-space: pre;">     </span>}<br class=""><br class="">@@ -1353,6 +1373,11 @@ static void power_pmu_enable(struct pmu *pmu)<br class=""><span class="Apple-tab-span" style="white-space: pre;">      </span>if (ppmu->flags & PPMU_ARCH_207S)<br class=""><span class="Apple-tab-span" style="white-space: pre;">     </span><span class="Apple-tab-span" style="white-space: pre;">  </span>mtspr(SPRN_MMCR2, cpuhw->mmcr[3]);<br class=""><br class="">+#ifdef CONFIG_PPC64<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>if (ppmu->flags & PPMU_ARCH_310S)<br class="">+<span class="Apple-tab-span" style="white-space: pre;">    </span><span class="Apple-tab-span" style="white-space: pre;">  </span>mtspr(SPRN_MMCR3, cpuhw->mmcr[4]);<br class="">+#endif /* CONFIG_PPC64 */<br class=""></blockquote><br class="">I don't think you need the #ifdef CONFIG_PPC64?<br class=""></blockquote><br class="">Hi Michael<br class=""><br class="">Thanks for reviewing this series.<br class=""><br class="">SPRN_MMCR3 is not defined for PPC32 and we hit build failure for pmac32_defconfig.<br class="">The #ifdef CONFIG_PPC64 is to address this.<br class=""></blockquote><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="">We like to avoid #ifdefs in the body of the code like that.</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=""><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="">There's a bunch of existing #defines near the top of the file to make</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="">32-bit work, I think you should just add another for this, so eg:</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=""><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="">#ifdef CONFIG_PPC32</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><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="">#define SPRN_MMCR3</span><span class="Apple-tab-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: pre; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">        </span><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="">0</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>Ok Ok. Found that currently we do same way as you mentioned for MMCRA which is not supported for 32-bit</div><div>I will work on this change</div><div><br class=""></div><div>Thanks</div><div>Athira <br class=""><blockquote type="cite" class=""><div class=""><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="">cheers</span></div></blockquote></div><br class=""></body></html>