Re: [PATCH V3 13/23] perf/x86/intel/lbr: Factor out intel_pmu_store_lbr

[Liang, Kan]

On 7/3/2020 4:59 PM, Liang, Kan wrote:
> On 7/3/2020 3:50 PM, Peter Zijlstra wrote:
> > On Fri, Jul 03, 2020 at 05:49:19AM -0700, kan.liang@xxxxxxxxxxxxxxx 
> > wrote:
> > > +static void intel_pmu_store_lbr(struct cpu_hw_events *cpuc,
> > > +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct lbr_entry *entries)
> > > +{
> > > +ÂÂÂ struct perf_branch_entry *e;
> > > +ÂÂÂ struct lbr_entry *lbr;
> > > +ÂÂÂ u64 from, to, info;
> > > +ÂÂÂ int i;
> > > +
> > > +ÂÂÂ for (i = 0; i < x86_pmu.lbr_nr; i++) {
> > > +ÂÂÂÂÂÂÂ lbr = entries ? &entries[i] : NULL;
> > > +ÂÂÂÂÂÂÂ e = &cpuc->lbr_entries[i];
> > > +
> > > +ÂÂÂÂÂÂÂ from = rdlbr_from(i, lbr);
> > > +ÂÂÂÂÂÂÂ /*
> > > +ÂÂÂÂÂÂÂÂ * Read LBR entries until invalid entry (0s) is detected.
> > > +ÂÂÂÂÂÂÂÂ */
> > > +ÂÂÂÂÂÂÂ if (!from)
> > > +ÂÂÂÂÂÂÂÂÂÂÂ break;
> > > +
> > > +ÂÂÂÂÂÂÂ to = rdlbr_to(i, lbr);
> > > +ÂÂÂÂÂÂÂ info = rdlbr_info(i, lbr);
> > > +
> > > +ÂÂÂÂÂÂÂ e->fromÂÂÂÂÂÂÂ = from;
> > > +ÂÂÂÂÂÂÂ e->toÂÂÂÂÂÂÂ = to;
> > > +ÂÂÂÂÂÂÂ e->mispredÂÂÂ = !!(info & LBR_INFO_MISPRED);
> > > +ÂÂÂÂÂÂÂ e->predictedÂÂÂ = !(info & LBR_INFO_MISPRED);
> > > +ÂÂÂÂÂÂÂ e->in_txÂÂÂ = !!(info & LBR_INFO_IN_TX);
> > > +ÂÂÂÂÂÂÂ e->abortÂÂÂ = !!(info & LBR_INFO_ABORT);
> > > +ÂÂÂÂÂÂÂ e->cyclesÂÂÂ = info & LBR_INFO_CYCLES;
> > > +ÂÂÂÂÂÂÂ e->typeÂÂÂÂÂÂÂ = 0;
> > > +ÂÂÂÂÂÂÂ e->reservedÂÂÂ = 0;
> > > +ÂÂÂ }
> > > +
> > > +ÂÂÂ cpuc->lbr_stack.nr = i;
> > > +}
> >
> > If I'm not mistaken, this correctly deals with LBR_FORMAT_INFO, so can't
> > we also use the intel_pmu_arch_lbr_read() function for that case?

There is another more severe issue which prevents sharing the read of 
Arch LBR with LBR_FORMAT_INFO. Sorry I missed it.

For the legacy LBR, the youngest branch is stored in TOS MSR. The next 
youngest is in (TOS - 1)...

For Arch LBR and LBR PEBS, the youngest branch is always in entry 0. The 
next youngest is in entry 1...

The growth of the legacy LBR is in a reversed order of Arch LBR and LBR 
PEBS. The legacy LBR also relies on TOS. I'm afraid we cannot use the 
intel_pmu_arch_lbr_read() function for LBR_FORMAT_INFO.

I think I will only send a patch to support NO_{CYCLES,FLAGS} for all 
LBR formats.

Thanks,
Kan