[PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts

[Michael Ellerman]

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