All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm_pmu: Delete incorrect cache event mapping for some armv8_pmuv3 events.
Date: Fri, 5 Oct 2018 13:39:41 +0100	[thread overview]
Message-ID: <20181005123941.GD14398@arm.com> (raw)
In-Reply-To: <b04c43bd-5c90-cc06-e2cc-ce542981853a@huawei.com>

On Fri, Oct 05, 2018 at 01:27:08PM +0100, John Garry wrote:
> On 04/10/2018 13:22, Will Deacon wrote:
> >Hi Ganapat,
> >
> >On Thu, Oct 04, 2018 at 11:12:09AM +0530, Ganapatrao Kulkarni wrote:
> >>can you please pull this patch?
> >
> >I still don't like the idea of just removing events like this, especially
> >when other architectures (including some x86 and Power CPUs afaict) playa
> >similar games for generic events, and these events do actually appear in
> >user code.
> >
> >I also don't understand why you remove the TLB events. I think that logic
> >would imply we should remove all of the events, because we can't distinguish
> >prefetches from reads either. If we want to be consistent, then I think
> >we should just remove the OP_WRITE events for L1D and BPU -- would you be
> >ok with that instead?
> >
> >Also, looking at the code, I think our PMCEID parsing is broken for 8.1
> >parts, where the upper 32 bits of the register are offset by 0x4000 in the
> >event numbering space.
> >
> 
> Here's something I noticed:
> static ssize_t
> armv8pmu_events_sysfs_show(struct device *dev,
> 			   struct device_attribute *attr, char *page)
> {
> 	struct perf_pmu_events_attr *pmu_attr;
> 
> 	pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr);
> 
> 	return sprintf(page, "event=0x%03llx\n", pmu_attr->id);
> 
> Should this be min width now be 4, since event width is now 16 bits (even
> though I don't know why we need to specify this width at all)?

Yeah, that is pretty weird, but the 16-bit events won't be truncated, so
I think I'd rather just leave that string as-is since it's ABI. I agree
that we probably shouldn't have bothered with the width at all in hindsight.

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: John Garry <john.garry@huawei.com>
Cc: Ganapatrao Kulkarni <gklkml16@gmail.com>,
	Mark Rutland <mark.rutland@arm.com>,
	"Nair, Jayachandran" <Jayachandran.Nair@cavium.com>,
	Jan.Glauber@cavium.com, Peter Zijlstra <peterz@infradead.org>,
	catalin.marinas@arm.com, LKML <linux-kernel@vger.kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Robert Richter <Robert.Richter@cavium.com>,
	Ingo Molnar <mingo@redhat.com>,
	Vadim.Lomovtsev@cavium.com,
	Ganapatrao Kulkarni <Ganapatrao.Kulkarni@cavium.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm_pmu: Delete incorrect cache event mapping for some armv8_pmuv3 events.
Date: Fri, 5 Oct 2018 13:39:41 +0100	[thread overview]
Message-ID: <20181005123941.GD14398@arm.com> (raw)
In-Reply-To: <b04c43bd-5c90-cc06-e2cc-ce542981853a@huawei.com>

On Fri, Oct 05, 2018 at 01:27:08PM +0100, John Garry wrote:
> On 04/10/2018 13:22, Will Deacon wrote:
> >Hi Ganapat,
> >
> >On Thu, Oct 04, 2018 at 11:12:09AM +0530, Ganapatrao Kulkarni wrote:
> >>can you please pull this patch?
> >
> >I still don't like the idea of just removing events like this, especially
> >when other architectures (including some x86 and Power CPUs afaict) playa
> >similar games for generic events, and these events do actually appear in
> >user code.
> >
> >I also don't understand why you remove the TLB events. I think that logic
> >would imply we should remove all of the events, because we can't distinguish
> >prefetches from reads either. If we want to be consistent, then I think
> >we should just remove the OP_WRITE events for L1D and BPU -- would you be
> >ok with that instead?
> >
> >Also, looking at the code, I think our PMCEID parsing is broken for 8.1
> >parts, where the upper 32 bits of the register are offset by 0x4000 in the
> >event numbering space.
> >
> 
> Here's something I noticed:
> static ssize_t
> armv8pmu_events_sysfs_show(struct device *dev,
> 			   struct device_attribute *attr, char *page)
> {
> 	struct perf_pmu_events_attr *pmu_attr;
> 
> 	pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr);
> 
> 	return sprintf(page, "event=0x%03llx\n", pmu_attr->id);
> 
> Should this be min width now be 4, since event width is now 16 bits (even
> though I don't know why we need to specify this width at all)?

Yeah, that is pretty weird, but the 16-bit events won't be truncated, so
I think I'd rather just leave that string as-is since it's ABI. I agree
that we probably shouldn't have bothered with the width at all in hindsight.

Will

  reply	other threads:[~2018-10-05 12:39 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-01 10:07 [PATCH] arm_pmu: Delete incorrect cache event mapping for some armv8_pmuv3 events Kulkarni, Ganapatrao
2018-10-01 10:07 ` Kulkarni, Ganapatrao
2018-10-01 14:29 ` Will Deacon
2018-10-01 14:29   ` Will Deacon
2018-10-01 16:39   ` Ganapatrao Kulkarni
2018-10-01 16:39     ` Ganapatrao Kulkarni
2018-10-04  5:42     ` Ganapatrao Kulkarni
2018-10-04  5:42       ` Ganapatrao Kulkarni
2018-10-04 12:22       ` Will Deacon
2018-10-04 12:22         ` Will Deacon
2018-10-04 19:39         ` Ganapatrao Kulkarni
2018-10-04 19:39           ` Ganapatrao Kulkarni
2018-10-05 13:34           ` Will Deacon
2018-10-05 13:34             ` Will Deacon
2018-10-05 12:27         ` John Garry
2018-10-05 12:27           ` John Garry
2018-10-05 12:39           ` Will Deacon [this message]
2018-10-05 12:39             ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181005123941.GD14398@arm.com \
    --to=will.deacon@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.