2014-12-08 22:09:46

by Liang, Kan

Subject: [PATCH 1/1] perf, x86: bug fix for cycles:p and cycles:pp on SLM

From: Kan Liang <[email protected]>

cycles:p and cycles:pp don't work on SLM since 86a0446

This regression issue is because UOPS_RETIRED.ALL is not a PEBS event on SLM.

According to pebs_aliases, INST_RETIRED.ANY_P (a PEBS capable event) can be
used as an alternative way to count cycles. Roll back the pebs_constraints to
use INST_RETIRED.ANY_P.

Signed-off-by: Kan Liang <[email protected]>
---
arch/x86/kernel/cpu/perf_event_intel_ds.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c
index 3c895d4..ac8f941 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -568,8 +568,8 @@ struct event_constraint intel_atom_pebs_event_constraints[] = {
};

struct event_constraint intel_slm_pebs_event_constraints[] = {
- /* UOPS_RETIRED.ALL, inv=1, cmask=16 (cycles:p). */
- INTEL_FLAGS_EVENT_CONSTRAINT(0x108001c2, 0xf),
+ /* INST_RETIRED.ANY_P */
+ INTEL_UEVENT_CONSTRAINT(0x00c0, 0x1),
/* Allow all events as PEBS with no flags */
INTEL_ALL_EVENT_CONSTRAINT(0, 0x1),
EVENT_CONSTRAINT_END
--
1.8.3.1

2014-12-15 15:18:29

by Peter Zijlstra

Subject: Re: [PATCH 1/1] perf, x86: bug fix for cycles:p and cycles:pp on SLM

On Mon, Dec 08, 2014 at 06:27:43AM -0800, [email protected] wrote:
> From: Kan Liang <[email protected]>
>
> cycles:p and cycles:pp don't work on SLM since 86a0446

The canonical format for referencing commits is:
86a04461a99f ("perf/x86: Revamp PEBS event selection")

> This regression issue is because UOPS_RETIRED.ALL is not a PEBS event on SLM.

Nor is it actually the alias used, afaict SLM uses
intel_pebs_aliases_core2().

> According to pebs_aliases, INST_RETIRED.ANY_P (a PEBS capable event) can be
> used as an alternative way to count cycles. Roll back the pebs_constraints to
> use INST_RETIRED.ANY_P.

Furthermore the counter mask of 0xf seems wrong too, and you've indeed
corrected that too (to 0x01, seeing how the SLM only has PEBS on cnt0).

Fixes: 86a04461a99f ("perf/x86: Revamp PEBS event selection")
> Signed-off-by: Kan Liang <[email protected]>

2014-12-15 15:22:07

by Peter Zijlstra

Subject: Re: [PATCH 1/1] perf, x86: bug fix for cycles:p and cycles:pp on SLM

On Mon, Dec 08, 2014 at 06:27:43AM -0800, [email protected] wrote:
> +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> @@ -568,8 +568,8 @@ struct event_constraint intel_atom_pebs_event_constraints[] = {
> };
>
> struct event_constraint intel_slm_pebs_event_constraints[] = {
> - /* UOPS_RETIRED.ALL, inv=1, cmask=16 (cycles:p). */
> - INTEL_FLAGS_EVENT_CONSTRAINT(0x108001c2, 0xf),
> + /* INST_RETIRED.ANY_P */
> + INTEL_UEVENT_CONSTRAINT(0x00c0, 0x1),

Should that not be:
INTEL_FLAGS_EVENT_CONSTRAINT(0x108000c0, 0x1),

instead?

> /* Allow all events as PEBS with no flags */
> INTEL_ALL_EVENT_CONSTRAINT(0, 0x1),
> EVENT_CONSTRAINT_END
> --
> 1.8.3.1
>

2014-12-22 13:34:59

by Liang, Kan

Subject: RE: [PATCH 1/1] perf, x86: bug fix for cycles:p and cycles:pp on SLM

>
> On Mon, Dec 08, 2014 at 06:27:43AM -0800, [email protected] wrote:
> > +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> > @@ -568,8 +568,8 @@ struct event_constraint
> > intel_atom_pebs_event_constraints[] = { };
> >
> > struct event_constraint intel_slm_pebs_event_constraints[] = {
> > - /* UOPS_RETIRED.ALL, inv=1, cmask=16 (cycles:p). */
> > - INTEL_FLAGS_EVENT_CONSTRAINT(0x108001c2, 0xf),
> > + /* INST_RETIRED.ANY_P */
> > + INTEL_UEVENT_CONSTRAINT(0x00c0, 0x1),
>
> Should that not be:
> INTEL_FLAGS_EVENT_CONSTRAINT(0x108000c0, 0x1),
>
> instead?

No, please refer to Table 19-20 in 19.10 Vol. 3 of SDM.
PEBS is only supported with PMC0.
There is no requirement to set inv and cmask.

Thanks,
Kan

>
> > /* Allow all events as PEBS with no flags */
> > INTEL_ALL_EVENT_CONSTRAINT(0, 0x1),
> > EVENT_CONSTRAINT_END
> > --
> > 1.8.3.1
> >