Re: [PATCH v6 2/2] drivers/perf: Add CCPI2 PMU support in ThunderX2 UNCORE driver.

[John Garry]

On 18/10/2019 05:21, Ganapatrao Kulkarni wrote:
> Hi Will,
>
> On Thu, Oct 17, 2019 at 9:17 PM Will Deacon <will@xxxxxxxxxx> wrote:
> > On Thu, Oct 17, 2019 at 12:38:51PM +0530, Ganapatrao Kulkarni wrote:
> > > On Wed, Oct 16, 2019 at 7:01 PM John Garry <john.garry@xxxxxxxxxx> wrote:
> > > > > +TX2_EVENT_ATTR(req_pktsent, CCPI2_EVENT_REQ_PKT_SENT);
> > > > > +TX2_EVENT_ATTR(snoop_pktsent, CCPI2_EVENT_SNOOP_PKT_SENT);
> > > > > +TX2_EVENT_ATTR(data_pktsent, CCPI2_EVENT_DATA_PKT_SENT);
> > > > > +TX2_EVENT_ATTR(gic_pktsent, CCPI2_EVENT_GIC_PKT_SENT);
> > > > > +
> > > > > +static struct attribute *ccpi2_pmu_events_attrs[] = {
> > > > > +     &tx2_pmu_event_attr_req_pktsent.attr.attr,
> > > > > +     &tx2_pmu_event_attr_snoop_pktsent.attr.attr,
> > > > > +     &tx2_pmu_event_attr_data_pktsent.attr.attr,
> > > > > +     &tx2_pmu_event_attr_gic_pktsent.attr.attr,
> > > > > +     NULL,
> > > > > +};
> > > >
> > > > Hi Ganapatrao,
> > > >
> > > > Have you considered adding these as uncore pmu-events in the perf tool?
> > > At the moment no, since the number of events exposed/listed are very few.
> >
> > Then sounds like a perfect time to nip it in the bud before the list grows
> > ;)
>
> I had internal discussion with architecture team, they have confirmed
> that, these are the only published events and no plan to add new.
> However, If any such request comes from HW team in future, i will add
> them to JSON files.

Don't you find perf list is swamped with all the uncore events?

For Huawei platform, I find this:
./perf list pmu | grep "Kernel PMU event" | grep hisi | wc -l
648

That's because we have so many instances of the same PMUs, not because 
there are many events per PMU.

TBH, I would like to delete all the events from the hisi uncore kernel 
drivers, now that they're supported in the perf tool, but I think that 
would constitute an ABI breakage.

Maybe there is a way to hide them, but I couldn't find it.

John

> I have incorporate all your previous comments, Can you please Ack and
> queue it to 5.5?
>
> > If you can manage with these things in userspace, then I agree with John
> > that it would be preferential to do it that way. It also offers more
> > flexibility if we get the metricgroup stuff working properly (I think it's
> > buggered for big/little atm).
> >
> > Will
>
> Thanks,
> Ganapat
>
> .