<html><head></head><body dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="ApplePlainTextBody"><div class="ApplePlainTextBody"><br><br><blockquote type="cite">On 12-Apr-2021, at 8:38 AM, Nicholas Piggin <npiggin@gmail.com> wrote:<br><br>Excerpts from Athira Rajeev's message of April 9, 2021 10:53 pm:<br><blockquote type="cite"><br><br><blockquote type="cite">On 09-Apr-2021, at 6:38 AM, Nicholas Piggin <npiggin@gmail.com> wrote:<br><br></blockquote>Hi Nick,<br><br>Thanks for checking the patch and sharing review comments.<br><br><blockquote type="cite">I was going to nitpick "overflown" here as something birds do, but some<br>sources says overflown is okay for past tense.<br><br>You could use "overflowed" for that, but I understand the issue with the <br>word: you are talking about counters that are currently in an "overflow" <br>state, but the overflow occurred in the past and is not still happening<br>so you "overflowing" doesn't exactly fit either.<br><br>overflown kind of works for some reason you can kind of use it for<br>present tense!<br></blockquote><br>Ok sure, Yes counter is currently in an “overflow” state.<br><br><blockquote type="cite"><br>Excerpts from Athira Rajeev's message of April 7, 2021 12:47 am:<br><blockquote type="cite">Running perf fuzzer showed below in dmesg logs:<br>"Can't find PMC that caused IRQ"<br><br>This means a PMU exception happened, but none of the PMC's (Performance<br>Monitor Counter) were found to be overflown. There are some corner cases<br>that clears the PMCs after PMI gets masked. In such cases, the perf<br>interrupt handler will not find the active PMC values that had caused<br>the overflow and thus leads to this message while replaying.<br><br>Case 1: PMU Interrupt happens during replay of other interrupts and<br>counter values gets cleared by PMU callbacks before replay:<br><br>During replay of interrupts like timer, __do_irq and doorbell exception, we<br>conditionally enable interrupts via may_hard_irq_enable(). This could<br>potentially create a window to generate a PMI. Since irq soft mask is set<br>to ALL_DISABLED, the PMI will get masked here.<br></blockquote><br>I wonder if may_hard_irq_enable shouldn't enable if PMI is soft<br>disabled. And also maybe replay should not set ALL_DISABLED if<br>there are no PMI interrupts pending.<br><br>Still, I think those are a bit more tricky and might take a while<br>to get right or just not be worth while, so I think your patch is<br>fine.<br></blockquote><br>Ok Nick.<br><blockquote type="cite"><br><blockquote type="cite">We could get IPIs run before<br>perf interrupt is replayed and the PMU events could deleted or stopped.<br>This will change the PMU SPR values and resets the counters. Snippet of<br>ftrace log showing PMU callbacks invoked in "__do_irq":<br><br><idle>-0 [051] dns. 132025441306354: __do_irq <-call_do_irq<br><idle>-0 [051] dns. 132025441306430: irq_enter <-__do_irq<br><idle>-0 [051] dns. 132025441306503: irq_enter_rcu <-__do_irq<br><idle>-0 [051] dnH. 132025441306599: xive_get_irq <-__do_irq<br><<>><br><idle>-0 [051] dnH. 132025441307770: generic_smp_call_function_single_interrupt <-smp_ipi_demux_relaxed<br><idle>-0 [051] dnH. 132025441307839: flush_smp_call_function_queue <-smp_ipi_demux_relaxed<br><idle>-0 [051] dnH. 132025441308057: _raw_spin_lock <-event_function<br><idle>-0 [051] dnH. 132025441308206: power_pmu_disable <-perf_pmu_disable<br><idle>-0 [051] dnH. 132025441308337: power_pmu_del <-event_sched_out<br><idle>-0 [051] dnH. 132025441308407: power_pmu_read <-power_pmu_del<br><idle>-0 [051] dnH. 132025441308477: read_pmc <-power_pmu_read<br><idle>-0 [051] dnH. 132025441308590: isa207_disable_pmc <-power_pmu_del<br><idle>-0 [051] dnH. 132025441308663: write_pmc <-power_pmu_del<br><idle>-0 [051] dnH. 132025441308787: power_pmu_event_idx <-perf_event_update_userpage<br><idle>-0 [051] dnH. 132025441308859: rcu_read_unlock_strict <-perf_event_update_userpage<br><idle>-0 [051] dnH. 132025441308975: power_pmu_enable <-perf_pmu_enable<br><<>><br><idle>-0 [051] dnH. 132025441311108: irq_exit <-__do_irq<br><idle>-0 [051] dns. 132025441311319: performance_monitor_exception <-replay_soft_interrupts<br><br>Case 2: PMI's masked during local_* operations, example local_add.<br>If the local_add operation happens within a local_irq_save, replay of<br>PMI will be during local_irq_restore. Similar to case 1, this could<br>also create a window before replay where PMU events gets deleted or<br>stopped.<br></blockquote><br>Here as well perhaps PMIs should be replayed if they are unmasked<br>even if other interrupts are still masked. Again that might be more<br>complexity than it's worth.<br></blockquote>Ok..<br><br><blockquote type="cite"><br><blockquote type="cite"><br>Patch adds a fix to update the PMU callback functions (del,stop,enable) to<br>check for pending perf interrupt. If there is an overflown PMC and pending<br>perf interrupt indicated in Paca, clear the PMI bit in paca to drop that<br>sample. In case of power_pmu_del, also clear the MMCR0 PMAO bit which<br>otherwise could lead to spurious interrupts in some corner cases. Example,<br>a timer after power_pmu_del which will re-enable interrupts since PMI is<br>cleared and triggers a PMI again since PMAO bit is still set.<br><br>We can't just replay PMI any time. Hence this approach is preferred rather<br>than replaying PMI before resetting overflown PMC. Patch also documents<br>core-book3s on a race condition which can trigger these PMC messages during<br>idle path in PowerNV.<br><br>Fixes: f442d004806e ("powerpc/64s: Add support to mask perf interrupts and replay them")<br>Reported-by: Nageswara R Sastry <nasastry@in.ibm.com><br>Suggested-by: Nicholas Piggin <npiggin@gmail.com><br>Suggested-by: Madhavan Srinivasan <maddy@linux.ibm.com><br>Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com><br>---<br>arch/powerpc/include/asm/pmc.h  | 11 +++++++++<br>arch/powerpc/perf/core-book3s.c | 55 +++++++++++++++++++++++++++++++++++++++++<br>2 files changed, 66 insertions(+)<br><br>diff --git a/arch/powerpc/include/asm/pmc.h b/arch/powerpc/include/asm/pmc.h<br>index c6bbe9778d3c..97b4bd8de25b 100644<br>--- a/arch/powerpc/include/asm/pmc.h<br>+++ b/arch/powerpc/include/asm/pmc.h<br>@@ -34,11 +34,22 @@ static inline void ppc_set_pmu_inuse(int inuse)<br>#endif<br>}<br><br>+static inline int clear_paca_irq_pmi(void)<br>+{<br>+<span class="Apple-tab-span" style="white-space:pre">   </span>if (get_paca()->irq_happened & PACA_IRQ_PMI) {<br>+<span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span>WARN_ON_ONCE(mfmsr() & MSR_EE);<br>+<span class="Apple-tab-span" style="white-space:pre">      </span><span class="Apple-tab-span" style="white-space:pre">    </span>get_paca()->irq_happened &= ~PACA_IRQ_PMI;<br>+<span class="Apple-tab-span" style="white-space:pre">        </span><span class="Apple-tab-span" style="white-space:pre">    </span>return 1;<br>+<span class="Apple-tab-span" style="white-space:pre">        </span>}<br>+<span class="Apple-tab-span" style="white-space:pre">        </span>return 0;<br>+}<br></blockquote><br>Could you put this in arch/powerpc/include/asm/hw_irq.h and<br>rather than paca_irq, call it irq_pending perhaps<br><br>clear_pmi_irq_pending()<br><br>get_clear_pmi_irq_pending() if you're also testing it.<br></blockquote><br>Sure,  I will use “get_clear_pmi_irq_pending()” and try with moving this to arch/powerpc/include/asm/hw_irq.h<br><br><blockquote type="cite"><br>Could you add a little comment about the corner cases above it too?<br>The root cause seem to be interrupt replay while a masked PMI is<br>pending can result in other interrupts arriving which clear the PMU<br>overflow so the pending PMI must be cleared.<br></blockquote><br>Ok, I will add comment and fix this in next version.<br><br><blockquote type="cite"><br><blockquote type="cite">+<br>extern void power4_enable_pmcs(void);<br><br>#else /* CONFIG_PPC64 */<br><br>static inline void ppc_set_pmu_inuse(int inuse) { }<br>+static inline int clear_paca_irq_pmi(void) { return 0; }<br><br>#endif<br><br>diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c<br>index 766f064f00fb..18ca3c90f866 100644<br>--- a/arch/powerpc/perf/core-book3s.c<br>+++ b/arch/powerpc/perf/core-book3s.c<br>@@ -847,6 +847,20 @@ static void write_pmc(int idx, unsigned long val)<br><span class="Apple-tab-span" style="white-space:pre">      </span>}<br>}<br><br>+static int pmc_overflown(int idx)<br>+{<br>+<span class="Apple-tab-span" style="white-space:pre">   </span>unsigned long val[8];<br>+<span class="Apple-tab-span" style="white-space:pre">    </span>int i;<br>+<br>+<span class="Apple-tab-span" style="white-space:pre">        </span>for (i = 0; i < ppmu->n_counter; i++)<br>+<span class="Apple-tab-span" style="white-space:pre">      </span><span class="Apple-tab-span" style="white-space:pre">    </span>val[i] = read_pmc(i + 1);<br>+<br>+<span class="Apple-tab-span" style="white-space:pre">     </span>if ((int)val[idx-1] < 0)<br>+<span class="Apple-tab-span" style="white-space:pre">      </span><span class="Apple-tab-span" style="white-space:pre">    </span>return 1;<br>+<br>+<span class="Apple-tab-span" style="white-space:pre">     </span>return 0;<br>+}<br>+<br>/* Called from sysrq_handle_showregs() */<br>void perf_event_print_debug(void)<br>{<br>@@ -1438,6 +1452,15 @@ static void power_pmu_enable(struct pmu *pmu)<br><span class="Apple-tab-span" style="white-space:pre">   </span><span class="Apple-tab-span" style="white-space:pre">    </span>event = cpuhw->event[i];<br><span class="Apple-tab-span" style="white-space:pre">       </span><span class="Apple-tab-span" style="white-space:pre">    </span>if (event->hw.idx && event->hw.idx != hwc_index[i] + 1) {<br><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>power_pmu_read(event);<br>+<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>/*<br>+<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> * if the PMC corresponding to event->hw.idx is<br>+<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> * overflown, check if there is any pending perf<br>+<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> * interrupt set in paca. If so, disable the interrupt<br>+<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> * by clearing the paca bit for PMI since we are going<br>+<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> * to reset the PMC.<br>+<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> */<br>+<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>if (pmc_overflown(event->hw.idx))<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span>clear_paca_irq_pmi();<br></blockquote><br>If the pmc is not overflown, could there still be a PMI pending?<br></blockquote><br>I didn’t hit that scenario where PMI is pending without an overflown PMC.<br>Also I believe if such a case happens, we will need an investigation there. It could be a different case to be handled.<br></blockquote><br>Okay, so a PMI will not occur without an overflown PMC, and the <br>overflown PMC will only be cleared in places where you also clear a <br>possible pending PMI?<br></blockquote><br>Hi Nick,<br><br>Yes, I have added this PMI check in possible places we clear PMC’s.<br><br><blockquote type="cite"><br><blockquote type="cite"><br>I actually considered below two points for adding this PMC check instead of just clearing the PMI.<br><br>1. Make sure we are not masking any bug here by just clearing PACA_IRQ_PMI.<br>Ideally if PMI is set in irq_happened, it means there was a counter overflow.<br>2. If there is more than one PMU event, say two events. Make sure we are clearing PMI only for the<br>event whose counter is overflown.<br></blockquote><br>Those are good points. Would you consider also adding a warning for the <br>case of no PMCs overflown but PMI is pending? That way you might have more <br>information about such a problem if it ever happens.<br><br>We try to add a good deal of warnings around the soft-mask code because <br>it's very tricky to change without causing more bugs, so even for future<br>changes to the code this would probably be useful.<br></blockquote><br>Sure, I will check to add a warning.<br><br><blockquote type="cite"><br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">@@ -1636,6 +1664,22 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)<br><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>--cpuhw->n_events;<br><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>ppmu->disable_pmc(event->hw.idx - 1, &cpuhw->mmcr);<br><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>if (event->hw.idx) {<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span>/*<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * if the PMC corresponding to event->hw.idx is<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * overflown, check if there is any pending perf<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * interrupt set in paca. If so, disable the interrupt<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * and clear the MMCR0 PMAO bit since we are going<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * to reset the PMC and delete the event.<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> */<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span>if (pmc_overflown(event->hw.idx)) {<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span>if (clear_paca_irq_pmi()) {<br>+<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><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>val_mmcr0 = mfspr(SPRN_MMCR0);<br>+<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><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>val_mmcr0 &= ~MMCR0_PMAO;<br>+<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><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>write_mmcr0(cpuhw, val_mmcr0);<br>+<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><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>mb();<br>+<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><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>isync();<br></blockquote><br>I don't know the perf subsystem, but just out of curiosity why does<br>MMCR0 need to be cleared only in this case?<br></blockquote><br>I got a corner case in power_pmu_del, with only clearing PACA_IRQ_PMI and without resetting MMCR0 PMAO bit.<br>Here is the flow:<br><br>1. We clear the PMI bit Paca, but MMCR0 has the PMAO bit still set. PMAO bit indicates a PMI has occurred.<br>2. A timer interrupt is replayed after power_pmu_del which does a “may_hard_irq_enable”.<br>This will re-enable interrupts and triggers a PMI again since PMAO bit is still set.<br><br>So clear PMAO bit to avoid such spurious interrupts.<br>Ftrace logs showing the same with some debug trace_printks :<br><br>    <idle>-0    [134] d.h. 327287888478: power_pmu_del <-event_sched_out.isra.126<br>    <<>>    Here we cleared the PMI<br>    <idle>-0    [134] d.h. 327287889272: write_pmc <-power_pmu_del<br>    <idle>-0    [134] d.h. 327287889346: rcu_read_unlock_strict <-perf_event_update_userpage<br>    <idle>-0    [134] d.h. 327287889711: power_pmu_del: In power_pmu_del MMCR0 is 82004090, local_paca->irq_happened is 9<br>    <idle>-0    [134] d.h. 327287889811: power_pmu_enable <-perf_pmu_enable<br>    <idle>-0    [134] d.h. 327287889982: irq_exit <-doorbell_exception<br>    <idle>-0    [134] d... 327287890053: idle_cpu <-irq_exit<br>    <idle>-0    [134] d... 327287890158: tick_nohz_irq_exit <-irq_exit<br>    <idle>-0    [134] d... 327287890219: ktime_get <-tick_nohz_irq_exit<br>    <idle>-0    [134] d... 327287890328: replay_soft_interrupts <-interrupt_exit_kernel_prepare<br>    <idle>-0    [134] d... 327287890399: irq_enter <-timer_interrupt<br>    <<>><br>    <idle>-0    [134] d.h. 327287891163: timer_interrupt: Before may_hard_irq_enable MMCR0 is 82004090, local_paca->irq_happened is 1<br>    <<>><br>    <idle>-0    [134] d.h. 327287894310: timer_interrupt: After may_hard_irq_enable MMCR0 is 82004090, local_paca->irq_happened is 21<br><br>In case of other callbacks like pmu enable, we are programming MMCR0. But in case of event getting deleted, there is no<br>way we clear PMAO unless an event gets scheduled again in that cpu. Hence added this check only in pmu_del callback.<br><br><br><blockquote type="cite">What if we disabled MSR[EE]<br>right before a perf interrupt came in, so we don't get a pending PMI<br>but the condition is still close to the same.<br></blockquote><br>Nick, I didn’t get this question exactly. Can you please help explain a bit ?<br>From my understanding, consider that we disabled MSR[EE] before perf interrupt came in.<br>So once the interrupts are re-enabled:<br><br>1. If soft mask is set to IRQS_DISABLED, perf interrupt will be triggered as NMI.<br>2. In case of ALL_DISABLED, it will be masked for replay. If PMU callbacks are invoked before replay,<br>our present patch will take care of clearing PMI in corner cases.<br></blockquote><br>Well I'm wondering about the same PMAO bug. Above you said:<br><br> 1. We clear the PMI bit Paca, but MMCR0 has the PMAO bit still set. PMAO bit indicates a PMI has occurred.<br> 2. A timer interrupt is replayed after power_pmu_del which does a “may_hard_irq_enable”.<br> This will re-enable interrupts and triggers a PMI again since PMAO bit is still set.<br><br>So in this situation, what if we had disabled interrupts and that had <br>caused MSR[EE] to be cleared (let's say due to a PCI interrupt <br>arriving), and then a PMC overflows and causes PMAO to be set.<br><br>Then you run this code:<br><br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span>/*<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * if the PMC corresponding to event->hw.idx is<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * overflown, check if there is any pending perf<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * interrupt set in paca. If so, disable the interrupt<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * and clear the MMCR0 PMAO bit since we are going<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> * to reset the PMC and delete the event.<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span> */<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span>if (pmc_overflown(event->hw.idx)) {<br>+<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><span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span>if (clear_paca_irq_pmi()) {<br>+<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><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>val_mmcr0 = mfspr(SPRN_MMCR0);<br>+<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><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>val_mmcr0 &= ~MMCR0_PMAO;<br>+<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><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>write_mmcr0(cpuhw, val_mmcr0);<br>+<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><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>mb();<br>+<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><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>isync();<br><br>And this does not clear PMAO because we had no pending PMI, but we still <br>have the pending PMAO exception.<br><br>The only difference was that MSR[EE] happened to be disabled when the <br>PMC overflowed so no pending PMI was recorded, but otherwise everything <br>is the same so I wonder why it's not subject to the same problem?<br></blockquote><br>Ok, thanks for explaining Nick, I got the scenario now :<br><br>1. MSR[EE] is set to zero<br>2. PMC gets overflown and PMAO bit gets set. But since MSR[EE] is set to zero, interrupt won’t be triggered<br>    and hence Paca won’t mark the pending PMI.<br>3. Next power_pmu callbacks were called which clears the PMC.<br>    Here though PMC is an overflown value, we won’t be clearing PMAO since my patch checks for only Paca PMI bit.<br><br>To address this issue, I will try with the below change:<br><br>If we find a PMC is overflown before clearing, do two checks:<br>1. If a PMI is pending in paca, clear the paca pmi bit and also clear PMAO bit<br>2. Else if a PMI is not pending in paca, check for PMAO bit and clear if it is set.<br>    This will disable the PMI coming in later.<br><br><br>Thanks<br>Athira<br><br><blockquote type="cite"><br>Thanks,<br>Nick<br></blockquote><br></div></body></html>