On Wed, Oct 09, 2013 at 10:16:32AM +0530, Anshuman Khandual wrote: > On 10/09/2013 06:51 AM, Michael Ellerman wrote: > > On Tue, Oct 08, 2013 at 12:51:18PM +0530, Anshuman Khandual wrote: > >> On 10/08/2013 09:51 AM, Michael Ellerman wrote: > >>> On Mon, Oct 07, 2013 at 10:00:26AM +0530, Anshuman Khandual wrote: > >>>> Right now the `config_bhrb` PMU specific call happens after write_mmcr0 > >>>> which actually enables the PMU for event counting and interrupt. So > >>>> there is a small window of time where the PMU and BHRB runs without the > >>>> required HW branch filter (if any) enabled in BHRB. This can cause some > >>>> of the branch samples to be collected through BHRB without any filter > >>>> being applied and hence affecting the correctness of the results. This > >>>> patch moves the BHRB config function call before enabling the interrupts. > >>> > >>> Patch looks good. > >>> > >>> But it reminds me I have an item in my TODO list: > >>> - "Why can't config_bhrb() be done in compute_mmcr()" ? > >>> > >> > >> compute_mmcr() function deals with generic MMCR* configs for normal PMU > >> events. Even if BHRB config touches MMCRA register, it's configuration > >> does not interfere with the PMU config for general events. So its best > >> to keep them separate. > > > > I'm unconvinced. If they'd been together to begin with this bug never > > would have happened. > > This is an ordering of configuration problem. Putting them together in the > same function does not rule out the chances of this ordering problem. Could > you please kindly explain how this could have been avoided ?
The existing code already makes sure to write MMCRA before MMCR0. cheers _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
