Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
> Filters? You mean the inv,cmask etc? Well, we have the Intel provided > 'cycle' events that use them, so exposing them makes sense and allows > such experimentation when we need another such alias. The SDM explicitely discourages it. The 'cycle' event is now handled explicitely, but for others it is still not ok. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Mon, Jun 23, 2014 at 09:35:00AM +0200, Peter Zijlstra wrote: > On Thu, Jun 19, 2014 at 01:40:41PM -0700, Andi Kleen wrote: > > On Thu, Jun 19, 2014 at 10:31:29PM +0200, Stephane Eranian wrote: > > > On Thu, Jun 19, 2014 at 10:18 PM, Andi Kleen wrote: > > > >> I don't quite understand that. > > > >> You need to know which events support PEBS. You need a table > > > > > > > > We're talking about the kernel allowing things here. > > > > Yes the user still needs to know what supports PEBS, but > > > > that doesn't concern the kernel. > > > > > > > Just need to make sure you don't return bogus information. > > > > GIGO. We only need to prevent security issues. > > > Yes if the user specifies a bogus raw event it will not count. > > That's fine. The important part is just that nothing ever crashes. > > Right. But IIRC you were previously arguing that we can in fact crash > the machine with raw PEBS events, as illustrated with the SNB PEBS > cycles 'event'. The potential problem could only happen for a recognized PEBS event/umask, but with unsupported flag combinations. That is what the SDM warns about in 18.8.4. If the event is not recognized as PEBS it will just effectively disable the event. > Which is where my strict_pebs patch came from; by default only allow the > sanitized known-safe list of events, but allow the system administrator > to disable that test and allow any PEBS event. I don't think we need to enforce the list of events (except for the few with special limited counters) -Andi -- a...@linux.intel.com -- Speaking for myself only -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Mon, Jun 23, 2014 at 1:37 PM, Peter Zijlstra wrote:
> On Mon, Jun 23, 2014 at 10:06:43AM +0200, Stephane Eranian wrote:
>> > @@ -1736,8 +1742,17 @@ static int intel_pmu_hw_config(struct perf_event
>> > *event)
>> > if (ret)
>> > return ret;
>> >
>> > - if (event->attr.precise_ip && x86_pmu.pebs_aliases)
>> > - x86_pmu.pebs_aliases(event);
>> > + if (event->attr.precise_ip) {
>> > + if ((event->attr.config & INTEL_ARCH_EVENT_MASK) == 0x)
>> > + return -EINVAL;
>> > +
>> > + if ((event->attr.config & ARCH_PERFMON_STRICT_PEBS) &&
>> > + (x86_pmu.attr_strict_pebs || !capable(CAP_SYS_ADMIN)))
>> > + return -EINVAL;
>> > +
>> I don't think filters work with any PEBS events. The event captured
>> does not qualify
>> for any of the filters (root or non-root).
>
> Filters? You mean the inv,cmask etc? Well, we have the Intel provided
> 'cycle' events that use them, so exposing them makes sense and allows
> such experimentation when we need another such alias.
>
Yes, inv, cmask, edge, any. They are ignored for the sampled instruction.
They are used to get to the PEBS trigger, then after it is the plain event/umask
for the next qualifying instruction.
I believe that the reason while cycles:pp somewhat works is because you are
after stalls. cycles:pp = uops_retired:any:cmask=1:inv. The sampled instruction
matches uops_retired:any (filters ignored). Given you are stalled,
nothing retires.
The next uops to retire (forward progress) is captured.
>> > + if (x86_pmu.pebs_aliases)
>> > + x86_pmu.pebs_aliases(event);
>> > + }
>> >
>> > if (intel_pmu_needs_lbr_smpl(event)) {
>> > ret = intel_pmu_setup_lbr_filter(event);
>> > diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c
>> > b/arch/x86/kernel/cpu/perf_event_intel_ds.c
>> > index ae96cfa5eddd..36b1f2afa61c 100644
>> > --- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
>> > +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
>> > @@ -540,6 +540,7 @@ struct event_constraint
>> > intel_core2_pebs_event_constraints[] = {
>> > INTEL_UEVENT_CONSTRAINT(0x00c5, 0x1), /* BR_INST_RETIRED.MISPRED */
>> > INTEL_UEVENT_CONSTRAINT(0x1fc7, 0x1), /* SIMD_INST_RETURED.ANY */
>> > INTEL_EVENT_CONSTRAINT(0xcb, 0x1),/* MEM_LOAD_RETIRED.* */
>> > + INTEL_UEVENT_CONSTRAINT(0x, 0x1), /* generic PEBS mask */
>> > EVENT_CONSTRAINT_END
>> > };
>> >
>> You probably need to explain that 0x MUST be the last event in
>> each table, i.e., catch all
>> event.
>
> Yeah, probably ;-) Alternatively we could make PEBS_CONSTRAINT_END that
> includes it or so.
You could do that, though it would serve dual purpose: end marker and catch all.
>
> Like said (possibly in another email) this patch was a very quick draft
> and I don't think I've even ran it, it was on the todo pile..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Mon, Jun 23, 2014 at 10:06:43AM +0200, Stephane Eranian wrote:
> > @@ -1736,8 +1742,17 @@ static int intel_pmu_hw_config(struct perf_event
> > *event)
> > if (ret)
> > return ret;
> >
> > - if (event->attr.precise_ip && x86_pmu.pebs_aliases)
> > - x86_pmu.pebs_aliases(event);
> > + if (event->attr.precise_ip) {
> > + if ((event->attr.config & INTEL_ARCH_EVENT_MASK) == 0x)
> > + return -EINVAL;
> > +
> > + if ((event->attr.config & ARCH_PERFMON_STRICT_PEBS) &&
> > + (x86_pmu.attr_strict_pebs || !capable(CAP_SYS_ADMIN)))
> > + return -EINVAL;
> > +
> I don't think filters work with any PEBS events. The event captured
> does not qualify
> for any of the filters (root or non-root).
Filters? You mean the inv,cmask etc? Well, we have the Intel provided
'cycle' events that use them, so exposing them makes sense and allows
such experimentation when we need another such alias.
> > + if (x86_pmu.pebs_aliases)
> > + x86_pmu.pebs_aliases(event);
> > + }
> >
> > if (intel_pmu_needs_lbr_smpl(event)) {
> > ret = intel_pmu_setup_lbr_filter(event);
> > diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c
> > b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> > index ae96cfa5eddd..36b1f2afa61c 100644
> > --- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
> > +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> > @@ -540,6 +540,7 @@ struct event_constraint
> > intel_core2_pebs_event_constraints[] = {
> > INTEL_UEVENT_CONSTRAINT(0x00c5, 0x1), /* BR_INST_RETIRED.MISPRED */
> > INTEL_UEVENT_CONSTRAINT(0x1fc7, 0x1), /* SIMD_INST_RETURED.ANY */
> > INTEL_EVENT_CONSTRAINT(0xcb, 0x1),/* MEM_LOAD_RETIRED.* */
> > + INTEL_UEVENT_CONSTRAINT(0x, 0x1), /* generic PEBS mask */
> > EVENT_CONSTRAINT_END
> > };
> >
> You probably need to explain that 0x MUST be the last event in
> each table, i.e., catch all
> event.
Yeah, probably ;-) Alternatively we could make PEBS_CONSTRAINT_END that
includes it or so.
Like said (possibly in another email) this patch was a very quick draft
and I don't think I've even ran it, it was on the todo pile..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Mon, Jun 23, 2014 at 9:14 AM, Peter Zijlstra wrote:
> On Thu, Jun 19, 2014 at 11:00:28AM -0700, Andi Kleen wrote:
>> However these days I'm actually thinking of just getting
>> rid of the detailed table except for PREC_DIST. All the PEBS
>> controls should be noops if the event does not support PEBS
>
> I had something like the below stuck on the 'look more at later' list
> and later never really ever happened.
>
> ---
> arch/x86/kernel/cpu/perf_event.c | 3 +++
> arch/x86/kernel/cpu/perf_event.h | 1 +
> arch/x86/kernel/cpu/perf_event_intel.c| 19 ++--
> arch/x86/kernel/cpu/perf_event_intel_ds.c | 38
> +--
> 4 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event.c
> b/arch/x86/kernel/cpu/perf_event.c
> index ae407f7226c8..f42405c9868b 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -1541,6 +1541,7 @@ static int __init init_hw_perf_events(void)
> pr_cont("%s PMU driver.\n", x86_pmu.name);
>
> x86_pmu.attr_rdpmc = 1; /* enable userspace RDPMC usage by default */
> + x86_pmu.attr_strict_pebs = 1;
>
> for (quirk = x86_pmu.quirks; quirk; quirk = quirk->next)
> quirk->func();
> @@ -1855,9 +1856,11 @@ static ssize_t set_attr_rdpmc(struct device *cdev,
> }
>
> static DEVICE_ATTR(rdpmc, S_IRUSR | S_IWUSR, get_attr_rdpmc, set_attr_rdpmc);
> +static DEVICE_BOOL_ATTR(strict_pebs, S_IRUSR | S_IWUSR,
> x86_pmu.attr_strict_pebs);
>
> static struct attribute *x86_pmu_attrs[] = {
> &dev_attr_rdpmc.attr,
> + &dev_attr_strict_pebs.attr.attr,
> NULL,
> };
>
> diff --git a/arch/x86/kernel/cpu/perf_event.h
> b/arch/x86/kernel/cpu/perf_event.h
> index 3b2f9bdd974b..a11eeab9b611 100644
> --- a/arch/x86/kernel/cpu/perf_event.h
> +++ b/arch/x86/kernel/cpu/perf_event.h
> @@ -413,6 +413,7 @@ struct x86_pmu {
> */
> int attr_rdpmc_broken;
> int attr_rdpmc;
> + boolattr_strict_pebs;
> struct attribute **format_attrs;
> struct attribute **event_attrs;
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel.c
> b/arch/x86/kernel/cpu/perf_event_intel.c
> index aa333d966886..6e68f3dc9a30 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel.c
> @@ -1729,6 +1729,12 @@ static void intel_pebs_aliases_snb(struct perf_event
> *event)
> }
> }
>
> +#define ARCH_PERFMON_STRICT_PEBS \
> + (ARCH_PERFMON_EVENTSEL_ANY | \
> +ARCH_PERFMON_EVENTSEL_CMASK | \
> +ARCH_PERFMON_EVENTSEL_EDGE | \
> +ARCH_PERFMON_EVENTSEL_INV)
> +
> static int intel_pmu_hw_config(struct perf_event *event)
> {
> int ret = x86_pmu_hw_config(event);
> @@ -1736,8 +1742,17 @@ static int intel_pmu_hw_config(struct perf_event
> *event)
> if (ret)
> return ret;
>
> - if (event->attr.precise_ip && x86_pmu.pebs_aliases)
> - x86_pmu.pebs_aliases(event);
> + if (event->attr.precise_ip) {
> + if ((event->attr.config & INTEL_ARCH_EVENT_MASK) == 0x)
> + return -EINVAL;
> +
> + if ((event->attr.config & ARCH_PERFMON_STRICT_PEBS) &&
> + (x86_pmu.attr_strict_pebs || !capable(CAP_SYS_ADMIN)))
> + return -EINVAL;
> +
I don't think filters work with any PEBS events. The event captured
does not qualify
for any of the filters (root or non-root).
> + if (x86_pmu.pebs_aliases)
> + x86_pmu.pebs_aliases(event);
> + }
>
> if (intel_pmu_needs_lbr_smpl(event)) {
> ret = intel_pmu_setup_lbr_filter(event);
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c
> b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> index ae96cfa5eddd..36b1f2afa61c 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> @@ -540,6 +540,7 @@ struct event_constraint
> intel_core2_pebs_event_constraints[] = {
> INTEL_UEVENT_CONSTRAINT(0x00c5, 0x1), /* BR_INST_RETIRED.MISPRED */
> INTEL_UEVENT_CONSTRAINT(0x1fc7, 0x1), /* SIMD_INST_RETURED.ANY */
> INTEL_EVENT_CONSTRAINT(0xcb, 0x1),/* MEM_LOAD_RETIRED.* */
> + INTEL_UEVENT_CONSTRAINT(0x, 0x1), /* generic PEBS mask */
> EVENT_CONSTRAINT_END
> };
>
You probably need to explain that 0x MUST be the last event in
each table, i.e., catch all
event.
> @@ -547,6 +548,7 @@ struct event_constraint
> intel_atom_pebs_event_constraints[] = {
> INTEL_UEVENT_CONSTRAINT(0x00c0, 0x1), /* INST_RETIRED.ANY */
> INTEL_UEVENT_CONSTRAINT(0x00c5, 0x1), /* MISPREDICTED_BRANCH_RETIRED
> */
> INTEL_EVENT_CONSTRAINT(0xcb, 0x1),/* MEM_LOAD_RETIRED.* */
> + INTEL_UEVENT_CONSTRAINT(0x, 0x1), /* generic PEBS mask */
> EVENT_CONSTRAINT_END
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Thu, Jun 19, 2014 at 01:40:41PM -0700, Andi Kleen wrote: > On Thu, Jun 19, 2014 at 10:31:29PM +0200, Stephane Eranian wrote: > > On Thu, Jun 19, 2014 at 10:18 PM, Andi Kleen wrote: > > >> I don't quite understand that. > > >> You need to know which events support PEBS. You need a table > > > > > > We're talking about the kernel allowing things here. > > > Yes the user still needs to know what supports PEBS, but > > > that doesn't concern the kernel. > > > > > Just need to make sure you don't return bogus information. > > GIGO. We only need to prevent security issues. > Yes if the user specifies a bogus raw event it will not count. > That's fine. The important part is just that nothing ever crashes. Right. But IIRC you were previously arguing that we can in fact crash the machine with raw PEBS events, as illustrated with the SNB PEBS cycles 'event'. Which is where my strict_pebs patch came from; by default only allow the sanitized known-safe list of events, but allow the system administrator to disable that test and allow any PEBS event. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Thu, Jun 19, 2014 at 11:00:28AM -0700, Andi Kleen wrote:
> However these days I'm actually thinking of just getting
> rid of the detailed table except for PREC_DIST. All the PEBS
> controls should be noops if the event does not support PEBS
I had something like the below stuck on the 'look more at later' list
and later never really ever happened.
---
arch/x86/kernel/cpu/perf_event.c | 3 +++
arch/x86/kernel/cpu/perf_event.h | 1 +
arch/x86/kernel/cpu/perf_event_intel.c| 19 ++--
arch/x86/kernel/cpu/perf_event_intel_ds.c | 38 +--
4 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index ae407f7226c8..f42405c9868b 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1541,6 +1541,7 @@ static int __init init_hw_perf_events(void)
pr_cont("%s PMU driver.\n", x86_pmu.name);
x86_pmu.attr_rdpmc = 1; /* enable userspace RDPMC usage by default */
+ x86_pmu.attr_strict_pebs = 1;
for (quirk = x86_pmu.quirks; quirk; quirk = quirk->next)
quirk->func();
@@ -1855,9 +1856,11 @@ static ssize_t set_attr_rdpmc(struct device *cdev,
}
static DEVICE_ATTR(rdpmc, S_IRUSR | S_IWUSR, get_attr_rdpmc, set_attr_rdpmc);
+static DEVICE_BOOL_ATTR(strict_pebs, S_IRUSR | S_IWUSR,
x86_pmu.attr_strict_pebs);
static struct attribute *x86_pmu_attrs[] = {
&dev_attr_rdpmc.attr,
+ &dev_attr_strict_pebs.attr.attr,
NULL,
};
diff --git a/arch/x86/kernel/cpu/perf_event.h b/arch/x86/kernel/cpu/perf_event.h
index 3b2f9bdd974b..a11eeab9b611 100644
--- a/arch/x86/kernel/cpu/perf_event.h
+++ b/arch/x86/kernel/cpu/perf_event.h
@@ -413,6 +413,7 @@ struct x86_pmu {
*/
int attr_rdpmc_broken;
int attr_rdpmc;
+ boolattr_strict_pebs;
struct attribute **format_attrs;
struct attribute **event_attrs;
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c
b/arch/x86/kernel/cpu/perf_event_intel.c
index aa333d966886..6e68f3dc9a30 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -1729,6 +1729,12 @@ static void intel_pebs_aliases_snb(struct perf_event
*event)
}
}
+#define ARCH_PERFMON_STRICT_PEBS \
+ (ARCH_PERFMON_EVENTSEL_ANY | \
+ARCH_PERFMON_EVENTSEL_CMASK | \
+ARCH_PERFMON_EVENTSEL_EDGE | \
+ARCH_PERFMON_EVENTSEL_INV)
+
static int intel_pmu_hw_config(struct perf_event *event)
{
int ret = x86_pmu_hw_config(event);
@@ -1736,8 +1742,17 @@ static int intel_pmu_hw_config(struct perf_event *event)
if (ret)
return ret;
- if (event->attr.precise_ip && x86_pmu.pebs_aliases)
- x86_pmu.pebs_aliases(event);
+ if (event->attr.precise_ip) {
+ if ((event->attr.config & INTEL_ARCH_EVENT_MASK) == 0x)
+ return -EINVAL;
+
+ if ((event->attr.config & ARCH_PERFMON_STRICT_PEBS) &&
+ (x86_pmu.attr_strict_pebs || !capable(CAP_SYS_ADMIN)))
+ return -EINVAL;
+
+ if (x86_pmu.pebs_aliases)
+ x86_pmu.pebs_aliases(event);
+ }
if (intel_pmu_needs_lbr_smpl(event)) {
ret = intel_pmu_setup_lbr_filter(event);
diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c
b/arch/x86/kernel/cpu/perf_event_intel_ds.c
index ae96cfa5eddd..36b1f2afa61c 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -540,6 +540,7 @@ struct event_constraint
intel_core2_pebs_event_constraints[] = {
INTEL_UEVENT_CONSTRAINT(0x00c5, 0x1), /* BR_INST_RETIRED.MISPRED */
INTEL_UEVENT_CONSTRAINT(0x1fc7, 0x1), /* SIMD_INST_RETURED.ANY */
INTEL_EVENT_CONSTRAINT(0xcb, 0x1),/* MEM_LOAD_RETIRED.* */
+ INTEL_UEVENT_CONSTRAINT(0x, 0x1), /* generic PEBS mask */
EVENT_CONSTRAINT_END
};
@@ -547,6 +548,7 @@ struct event_constraint intel_atom_pebs_event_constraints[]
= {
INTEL_UEVENT_CONSTRAINT(0x00c0, 0x1), /* INST_RETIRED.ANY */
INTEL_UEVENT_CONSTRAINT(0x00c5, 0x1), /* MISPREDICTED_BRANCH_RETIRED */
INTEL_EVENT_CONSTRAINT(0xcb, 0x1),/* MEM_LOAD_RETIRED.* */
+ INTEL_UEVENT_CONSTRAINT(0x, 0x1), /* generic PEBS mask */
EVENT_CONSTRAINT_END
};
@@ -573,6 +575,7 @@ struct event_constraint intel_slm_pebs_event_constraints[]
= {
INTEL_UEVENT_CONSTRAINT(0xf7c5, 0x1), /* BR_INST_MISP_RETIRED.RETURN_PS
*/
INTEL_UEVENT_CONSTRAINT(0xfbc5, 0x1), /*
BR_INST_MISP_RETIRED.IND_CALL_PS */
INTEL_UEVENT_CONSTRAINT(0xfec5, 0x1), /*
BR_INST_MISP_RETIRED.TAKEN_JCC_PS */
+ INTEL_UEVENT_CONSTRAINT(0x, 0x1), /* generic PEBS mask */
EVENT_CONSTRAINT_END
};
@@ -588,6 +591,7 @@ struct event_constraint
inte
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Thu, Jun 19, 2014 at 05:58:28PM +0200, Stephane Eranian wrote: > + INTEL_EVENT_CONSTRAINT(0xd1, 0xf), /* MEM_LOAD_UOPS_RETIRED.* */ > + /* MEM_LOAD_UOPS_LLC_HIT_RETIRED.* */ > + INTEL_EVENT_CONSTRAINT(0xd2, 0xf), > + /* MEM_LOAD_UOPS_LLC_MISS_RETIRED.* */ > + INTEL_EVENT_CONSTRAINT(0xd3, 0xf), > INTEL_UEVENT_CONSTRAINT(0x04c8, 0xf), /* HLE_RETIRED.Abort */ > INTEL_UEVENT_CONSTRAINT(0x04c9, 0xf), /* RTM_RETIRED.Abort */ Please keep the event and comment on the same line, screw 80 chars, this interleaves stuff is impossible to read. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Thu, Jun 19, 2014 at 10:45 PM, Stephane Eranian wrote: > On Thu, Jun 19, 2014 at 10:40 PM, Andi Kleen wrote: >> On Thu, Jun 19, 2014 at 10:31:29PM +0200, Stephane Eranian wrote: >>> On Thu, Jun 19, 2014 at 10:18 PM, Andi Kleen wrote: >>> >> I don't quite understand that. >>> >> You need to know which events support PEBS. You need a table >>> > >>> > We're talking about the kernel allowing things here. >>> > Yes the user still needs to know what supports PEBS, but >>> > that doesn't concern the kernel. >>> > >>> Just need to make sure you don't return bogus information. >> >> GIGO. We only need to prevent security issues. >> >>> > You can just allow it for all, it's a nop if the event doesn't >>> > support it. And also the fields like DataLA are simply 0 when >>> > not supported. >>> > >>> >>> Let's take a example. If I do resource_stalls:pp, the kernel >>> will let it go through and clear the PMI bit on the config as >>> is required for PEBS mode. The counter will count normally >>> and never fire an interrupt, even when it overflows. It would >>> never execute the PMI handler and thus never look at the >>> PEBS content. You'd never get any samples. >> >> Yes if the user specifies a bogus raw event it will not count. >> That's fine. The important part is just that nothing ever crashes. >> > That would certainly avoid the problem of missing events in pebs table. > I had a problem with that just today. It also speed up scheduling > as well by avoid the table lookups. > I can take of writing the patch to do this, if you want. > Note that I will soon post a patch to speed up scheduling for all x86 > processors. I'll put that one in too. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Thu, Jun 19, 2014 at 10:40 PM, Andi Kleen wrote: > On Thu, Jun 19, 2014 at 10:31:29PM +0200, Stephane Eranian wrote: >> On Thu, Jun 19, 2014 at 10:18 PM, Andi Kleen wrote: >> >> I don't quite understand that. >> >> You need to know which events support PEBS. You need a table >> > >> > We're talking about the kernel allowing things here. >> > Yes the user still needs to know what supports PEBS, but >> > that doesn't concern the kernel. >> > >> Just need to make sure you don't return bogus information. > > GIGO. We only need to prevent security issues. > >> > You can just allow it for all, it's a nop if the event doesn't >> > support it. And also the fields like DataLA are simply 0 when >> > not supported. >> > >> >> Let's take a example. If I do resource_stalls:pp, the kernel >> will let it go through and clear the PMI bit on the config as >> is required for PEBS mode. The counter will count normally >> and never fire an interrupt, even when it overflows. It would >> never execute the PMI handler and thus never look at the >> PEBS content. You'd never get any samples. > > Yes if the user specifies a bogus raw event it will not count. > That's fine. The important part is just that nothing ever crashes. > That would certainly avoid the problem of missing events in pebs table. I had a problem with that just today. It also speed up scheduling as well by avoid the table lookups. Note that I will soon post a patch to speed up scheduling for all x86 processors. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Thu, Jun 19, 2014 at 10:31:29PM +0200, Stephane Eranian wrote: > On Thu, Jun 19, 2014 at 10:18 PM, Andi Kleen wrote: > >> I don't quite understand that. > >> You need to know which events support PEBS. You need a table > > > > We're talking about the kernel allowing things here. > > Yes the user still needs to know what supports PEBS, but > > that doesn't concern the kernel. > > > Just need to make sure you don't return bogus information. GIGO. We only need to prevent security issues. > > You can just allow it for all, it's a nop if the event doesn't > > support it. And also the fields like DataLA are simply 0 when > > not supported. > > > > Let's take a example. If I do resource_stalls:pp, the kernel > will let it go through and clear the PMI bit on the config as > is required for PEBS mode. The counter will count normally > and never fire an interrupt, even when it overflows. It would > never execute the PMI handler and thus never look at the > PEBS content. You'd never get any samples. Yes if the user specifies a bogus raw event it will not count. That's fine. The important part is just that nothing ever crashes. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Thu, Jun 19, 2014 at 10:18 PM, Andi Kleen wrote: >> I don't quite understand that. >> You need to know which events support PEBS. You need a table > > We're talking about the kernel allowing things here. > Yes the user still needs to know what supports PEBS, but > that doesn't concern the kernel. > Just need to make sure you don't return bogus information. > You can just allow it for all, it's a nop if the event doesn't > support it. And also the fields like DataLA are simply 0 when > not supported. > Let's take a example. If I do resource_stalls:pp, the kernel will let it go through and clear the PMI bit on the config as is required for PEBS mode. The counter will count normally and never fire an interrupt, even when it overflows. It would never execute the PMI handler and thus never look at the PEBS content. You'd never get any samples. > The only thing you need is a rule to limit to 4 counters. > > Then only cases that are special (PREC_DIST, extra registers) > would need to be handled explicitely. > extra registers do no impose counter constraints. That's the approach used by Intel tools to simplify scheduling. As I said in my patch, in Linux we do this differently. So yes, you'd need this for PREC_DIST and precise store on SNB, IVB. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
> I don't quite understand that. > You need to know which events support PEBS. You need a table We're talking about the kernel allowing things here. Yes the user still needs to know what supports PEBS, but that doesn't concern the kernel. You can just allow it for all, it's a nop if the event doesn't support it. And also the fields like DataLA are simply 0 when not supported. The only thing you need is a rule to limit to 4 counters. Then only cases that are special (PREC_DIST, extra registers) would need to be handled explicitely. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Thu, Jun 19, 2014 at 8:00 PM, Andi Kleen wrote: > > On Thu, Jun 19, 2014 at 05:58:28PM +0200, Stephane Eranian wrote: > > The following events support PEBS for all umasks, > > thus use INTEL_EVENT_CONSTRAINT() instead of > > INTEL_UEVENT_CONSTRAINT(): > > > > 0xd1 MEM_LOAD_UOPS_RETIRED > > 0xd2 MEM_LOAD_UOPS_LLC_HIT_RETIRED > > 0xd3 MEM_LOAD_UOPS_LLC_MISS_RETIRED > > > > For event 0xd0 (MEM_UOPS_RETIRED), the same is true, except > > we need to distinguish precise store (umask 0x82) from load > > latency events, thus we keep the breakdown per umask. But all > > umasks do support PEBS. > > I sent a similar patch some time ago > > http://lkml.iu.edu/hypermail/linux/kernel/1404.2/01509.html > > However these days I'm actually thinking of just getting > rid of the detailed table except for PREC_DIST. All the PEBS > controls should be noops if the event does not support PEBS > I don't quite understand that. You need to know which events support PEBS. You need a table for that. then I think you could drop the counter mask because with HT on, no mask is needed. When HT is off you need a mask, but it can be hardcoded for all events, well at least on HSW. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] perf/x86: update Haswell PEBS event constraints
On Thu, Jun 19, 2014 at 05:58:28PM +0200, Stephane Eranian wrote: > The following events support PEBS for all umasks, > thus use INTEL_EVENT_CONSTRAINT() instead of > INTEL_UEVENT_CONSTRAINT(): > > 0xd1 MEM_LOAD_UOPS_RETIRED > 0xd2 MEM_LOAD_UOPS_LLC_HIT_RETIRED > 0xd3 MEM_LOAD_UOPS_LLC_MISS_RETIRED > > For event 0xd0 (MEM_UOPS_RETIRED), the same is true, except > we need to distinguish precise store (umask 0x82) from load > latency events, thus we keep the breakdown per umask. But all > umasks do support PEBS. I sent a similar patch some time ago http://lkml.iu.edu/hypermail/linux/kernel/1404.2/01509.html However these days I'm actually thinking of just getting rid of the detailed table except for PREC_DIST. All the PEBS controls should be noops if the event does not support PEBS -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
