LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT
@ 2019-05-02 10:50 Alexander Shishkin
  2019-05-02 10:50 ` [PATCH 1/2] perf: Allow normal events to be sources of AUX data Alexander Shishkin
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Alexander Shishkin @ 2019-05-02 10:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel, jolsa,
	adrian.hunter, Alexander Shishkin

Hi Peter,

New PEBS feature: output to Intel PT stream instead of the DS area. It's
theoretically useful in virtualized environments, where DS area can't be
used. It's also good for those who are interested in instruction trace for
context of the PEBS events. As PEBS goes, it can provide LBR context with
all the branch-related information that PT doesn't provide at the moment.

PEBS records are packetized in the PT stream, so instead of extracting
them in the PMI, we leave it to the perf tool, because real time PT
decoding is not practical. Tooling patches are not included, but can be
found here [1].

Added is an attribute bit 'aux_source' to mean that an event is a source of
AUX data. This bit enables PEBS output to PT.

[1] http://git.infradead.org/users/ahunter/linux-perf.git

Alexander Shishkin (2):
  perf: Allow normal events to be sources of AUX data
  perf/x86/intel: Support PEBS output to PT

 arch/x86/events/intel/core.c     | 13 +++++++
 arch/x86/events/intel/ds.c       | 59 +++++++++++++++++++++++++++++++-
 arch/x86/events/perf_event.h     |  9 +++++
 arch/x86/include/asm/msr-index.h |  3 ++
 include/uapi/linux/perf_event.h  |  3 +-
 kernel/events/core.c             |  9 +++++
 6 files changed, 94 insertions(+), 2 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] perf: Allow normal events to be sources of AUX data
  2019-05-02 10:50 [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT Alexander Shishkin
@ 2019-05-02 10:50 ` Alexander Shishkin
  2019-05-02 10:50 ` [PATCH 2/2] perf/x86/intel: Support PEBS output to PT Alexander Shishkin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Alexander Shishkin @ 2019-05-02 10:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel, jolsa,
	adrian.hunter, Alexander Shishkin

In some cases, ordinary (non-AUX) events can generate data for AUX events.
For example, PEBS events can come out as records in the Intel PT stream
instead of their usual DS records, if configured to do so.

Add an attribute bit for such events to be configured to generate AUX data.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 include/uapi/linux/perf_event.h | 3 ++-
 kernel/events/core.c            | 9 +++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 7198ddd0c6b1..213cae95e713 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -374,7 +374,8 @@ struct perf_event_attr {
 				namespaces     :  1, /* include namespaces data */
 				ksymbol        :  1, /* include ksymbol events */
 				bpf_event      :  1, /* include bpf events */
-				__reserved_1   : 33;
+				aux_source     :  1, /* generate AUX records instead of events */
+				__reserved_1   : 32;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index abbd4b3b96c2..ea9eff6fcf6a 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10382,6 +10382,15 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 		goto err_ns;
 	}
 
+	/*
+	 * If it's set at this point, the PMU didn't handle it, so
+	 * it doesn't do it.
+	 */
+	if (event->attr.aux_source) {
+		err = -EOPNOTSUPP;
+		goto err_pmu;
+	}
+
 	err = exclusive_event_init(event);
 	if (err)
 		goto err_pmu;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] perf/x86/intel: Support PEBS output to PT
  2019-05-02 10:50 [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT Alexander Shishkin
  2019-05-02 10:50 ` [PATCH 1/2] perf: Allow normal events to be sources of AUX data Alexander Shishkin
@ 2019-05-02 10:50 ` Alexander Shishkin
  2019-05-08  9:12   ` Peter Zijlstra
                     ` (2 more replies)
  2019-05-03  8:57 ` [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT Alexander Shishkin
  2019-05-08 10:39 ` Peter Zijlstra
  3 siblings, 3 replies; 8+ messages in thread
From: Alexander Shishkin @ 2019-05-02 10:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel, jolsa,
	adrian.hunter, Alexander Shishkin

If PEBS declares ability to output its data to Intel PT stream, use the
aux_source attribute bit to enable PEBS data output to PT. This requires
a PT event to be present and scheduled in the same context. Unlike the
DS area, the kernel does not extract PEBS records from the PT stream to
generate corresponding records in the perf stream, because that would
require real time in-kernel PT decoding, which is not feasible. The PMI,
however, can still be used.

The output setting is per-CPU, so all PEBS events must be either writing
to PT or to the DS area, so in order to not mess up the event scheduling,
we fall back to the latter in case both types of events are scheduled in.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/events/intel/core.c     | 13 +++++++
 arch/x86/events/intel/ds.c       | 59 +++++++++++++++++++++++++++++++-
 arch/x86/events/perf_event.h     |  9 +++++
 arch/x86/include/asm/msr-index.h |  3 ++
 4 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 4b4dac089635..949a589fd9b1 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3291,6 +3291,19 @@ static int intel_pmu_hw_config(struct perf_event *event)
 		}
 	}
 
+	if (event->attr.aux_source) {
+		if (!event->attr.precise_ip)
+			return -EINVAL;
+
+		if (!x86_pmu.intel_cap.pebs_output_pt_available)
+			return -EOPNOTSUPP;
+
+		event->hw.flags |= PERF_X86_EVENT_PEBS_VIA_PT;
+
+		/* Signal to the core that we handled it */
+		event->attr.aux_source = 0;
+	}
+
 	if (event->attr.type != PERF_TYPE_RAW)
 		return 0;
 
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 7a9f5dac5abe..c38defc7c386 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -902,6 +902,9 @@ struct event_constraint *intel_pebs_constraints(struct perf_event *event)
  */
 static inline bool pebs_needs_sched_cb(struct cpu_hw_events *cpuc)
 {
+	if (cpuc->n_pebs == cpuc->n_pebs_via_pt)
+		return false;
+
 	return cpuc->n_pebs && (cpuc->n_pebs == cpuc->n_large_pebs);
 }
 
@@ -919,6 +922,9 @@ static inline void pebs_update_threshold(struct cpu_hw_events *cpuc)
 	u64 threshold;
 	int reserved;
 
+	if (cpuc->n_pebs_via_pt)
+		return;
+
 	if (x86_pmu.flags & PMU_FL_PEBS_ALL)
 		reserved = x86_pmu.max_pebs_events + x86_pmu.num_counters_fixed;
 	else
@@ -1059,10 +1065,50 @@ void intel_pmu_pebs_add(struct perf_event *event)
 	cpuc->n_pebs++;
 	if (hwc->flags & PERF_X86_EVENT_LARGE_PEBS)
 		cpuc->n_large_pebs++;
+	if (hwc->flags & PERF_X86_EVENT_PEBS_VIA_PT)
+		cpuc->n_pebs_via_pt++;
 
 	pebs_update_state(needed_cb, cpuc, event, true);
 }
 
+static void intel_pmu_pebs_via_pt_disable(struct perf_event *event)
+{
+	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+
+	if (!(event->hw.flags & PERF_X86_EVENT_PEBS_VIA_PT))
+		return;
+
+	if (!(cpuc->pebs_enabled & ~PEBS_VIA_PT_MASK))
+		cpuc->pebs_enabled &= ~PEBS_VIA_PT_MASK;
+}
+
+static void intel_pmu_pebs_via_pt_enable(struct perf_event *event)
+{
+	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+	struct hw_perf_event *hwc = &event->hw;
+	struct debug_store *ds = cpuc->ds;
+
+	if (!(event->hw.flags & PERF_X86_EVENT_PEBS_VIA_PT))
+		return;
+
+	/*
+	 * In case there's a mix of PEBS->PT and PEBS->DS, fall back
+	 * to DS.
+	 */
+	if (cpuc->n_pebs != cpuc->n_pebs_via_pt) {
+		/* PEBS-to-DS events present, fall back to DS */
+		intel_pmu_pebs_via_pt_disable(event);
+		return;
+	}
+
+	if (!(event->hw.flags & PERF_X86_EVENT_LARGE_PEBS))
+		cpuc->pebs_enabled |= PEBS_PMI_AFTER_EACH_RECORD;
+
+	cpuc->pebs_enabled |= PEBS_OUTPUT_PT;
+
+	wrmsrl(MSR_RELOAD_PMC0 + hwc->idx, ds->pebs_event_reset[hwc->idx]);
+}
+
 void intel_pmu_pebs_enable(struct perf_event *event)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
@@ -1100,6 +1146,8 @@ void intel_pmu_pebs_enable(struct perf_event *event)
 	} else {
 		ds->pebs_event_reset[hwc->idx] = 0;
 	}
+
+	intel_pmu_pebs_via_pt_enable(event);
 }
 
 void intel_pmu_pebs_del(struct perf_event *event)
@@ -1111,6 +1159,8 @@ void intel_pmu_pebs_del(struct perf_event *event)
 	cpuc->n_pebs--;
 	if (hwc->flags & PERF_X86_EVENT_LARGE_PEBS)
 		cpuc->n_large_pebs--;
+	if (hwc->flags & PERF_X86_EVENT_PEBS_VIA_PT)
+		cpuc->n_pebs_via_pt--;
 
 	pebs_update_state(needed_cb, cpuc, event, false);
 }
@@ -1120,7 +1170,8 @@ void intel_pmu_pebs_disable(struct perf_event *event)
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 	struct hw_perf_event *hwc = &event->hw;
 
-	if (cpuc->n_pebs == cpuc->n_large_pebs)
+	if (cpuc->n_pebs == cpuc->n_large_pebs &&
+	    cpuc->n_pebs != cpuc->n_pebs_via_pt)
 		intel_pmu_drain_pebs_buffer();
 
 	cpuc->pebs_enabled &= ~(1ULL << hwc->idx);
@@ -1131,6 +1182,8 @@ void intel_pmu_pebs_disable(struct perf_event *event)
 	else if (event->hw.flags & PERF_X86_EVENT_PEBS_ST)
 		cpuc->pebs_enabled &= ~(1ULL << 63);
 
+	intel_pmu_pebs_via_pt_disable(event);
+
 	if (cpuc->enabled)
 		wrmsrl(MSR_IA32_PEBS_ENABLE, cpuc->pebs_enabled);
 
@@ -2032,6 +2085,10 @@ void __init intel_ds_init(void)
 					  PERF_SAMPLE_REGS_INTR);
 			}
 			pr_cont("PEBS fmt4%c%s, ", pebs_type, pebs_qual);
+
+			if (x86_pmu.intel_cap.pebs_output_pt_available)
+				pr_cont("PEBS-via-PT, ");
+
 			break;
 
 		default:
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 07fc84bb85c1..39d8c1b70866 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -76,6 +76,7 @@ static inline bool constraint_match(struct event_constraint *c, u64 ecode)
 #define PERF_X86_EVENT_EXCL_ACCT	0x0100 /* accounted EXCL event */
 #define PERF_X86_EVENT_AUTO_RELOAD	0x0200 /* use PEBS auto-reload */
 #define PERF_X86_EVENT_LARGE_PEBS	0x0400 /* use large PEBS */
+#define PERF_X86_EVENT_PEBS_VIA_PT	0x0800 /* use PT buffer for PEBS */
 
 struct amd_nb {
 	int nb_id;  /* NorthBridge id */
@@ -85,6 +86,11 @@ struct amd_nb {
 };
 
 #define PEBS_COUNTER_MASK	((1ULL << MAX_PEBS_EVENTS) - 1)
+#define PEBS_PMI_AFTER_EACH_RECORD BIT_ULL(60)
+#define PEBS_OUTPUT_OFFSET	61
+#define PEBS_OUTPUT_MASK	(3ull << PEBS_OUTPUT_OFFSET)
+#define PEBS_OUTPUT_PT		(1ull << PEBS_OUTPUT_OFFSET)
+#define PEBS_VIA_PT_MASK	(PEBS_OUTPUT_PT | PEBS_PMI_AFTER_EACH_RECORD)
 
 /*
  * Flags PEBS can handle without an PMI.
@@ -229,6 +235,7 @@ struct cpu_hw_events {
 	u64			pebs_enabled;
 	int			n_pebs;
 	int			n_large_pebs;
+	int			n_pebs_via_pt;
 
 	/* Current super set of events hardware configuration */
 	u64			pebs_data_cfg;
@@ -528,6 +535,8 @@ union perf_capabilities {
 		 */
 		u64	full_width_write:1;
 		u64     pebs_baseline:1;
+		u64	pebs_metrics_available:1;
+		u64	pebs_output_pt_available:1;
 	};
 	u64	capabilities;
 };
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 1378518cf63f..c7ef38693654 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -782,6 +782,9 @@
 #define MSR_CORE_PERF_GLOBAL_CTRL	0x0000038f
 #define MSR_CORE_PERF_GLOBAL_OVF_CTRL	0x00000390
 
+#define MSR_RELOAD_PMC0			0x000014c1
+#define MSR_RELOAD_FIXED_CTR0		0x00001309
+
 /* Geode defined MSRs */
 #define MSR_GEODE_BUSCONT_CONF0		0x00001900
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT
  2019-05-02 10:50 [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT Alexander Shishkin
  2019-05-02 10:50 ` [PATCH 1/2] perf: Allow normal events to be sources of AUX data Alexander Shishkin
  2019-05-02 10:50 ` [PATCH 2/2] perf/x86/intel: Support PEBS output to PT Alexander Shishkin
@ 2019-05-03  8:57 ` Alexander Shishkin
  2019-05-08 10:39 ` Peter Zijlstra
  3 siblings, 0 replies; 8+ messages in thread
From: Alexander Shishkin @ 2019-05-03  8:57 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel, jolsa,
	adrian.hunter, kan.liang, alexander.shishkin

Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:

> Hi Peter,
>
> New PEBS feature: output to Intel PT stream instead of the DS area. It's
> theoretically useful in virtualized environments, where DS area can't be
> used. It's also good for those who are interested in instruction trace for
> context of the PEBS events. As PEBS goes, it can provide LBR context with
> all the branch-related information that PT doesn't provide at the moment.
>
> PEBS records are packetized in the PT stream, so instead of extracting
> them in the PMI, we leave it to the perf tool, because real time PT
> decoding is not practical. Tooling patches are not included, but can be
> found here [1].
>
> Added is an attribute bit 'aux_source' to mean that an event is a source of
> AUX data. This bit enables PEBS output to PT.

Forgot to CC Kan.

Regards,
--
Alex

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] perf/x86/intel: Support PEBS output to PT
  2019-05-02 10:50 ` [PATCH 2/2] perf/x86/intel: Support PEBS output to PT Alexander Shishkin
@ 2019-05-08  9:12   ` Peter Zijlstra
  2019-05-08  9:18   ` Peter Zijlstra
  2019-05-08  9:34   ` Peter Zijlstra
  2 siblings, 0 replies; 8+ messages in thread
From: Peter Zijlstra @ 2019-05-08  9:12 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel, jolsa,
	adrian.hunter

On Thu, May 02, 2019 at 01:50:22PM +0300, Alexander Shishkin wrote:
> If PEBS declares ability to output its data to Intel PT stream, use the
> aux_source attribute bit to enable PEBS data output to PT. This requires
> a PT event to be present and scheduled in the same context. Unlike the
> DS area, the kernel does not extract PEBS records from the PT stream to
> generate corresponding records in the perf stream, because that would
> require real time in-kernel PT decoding, which is not feasible. The PMI,
> however, can still be used.
> 
> The output setting is per-CPU, so all PEBS events must be either writing
> to PT or to the DS area, so in order to not mess up the event scheduling,
> we fall back to the latter in case both types of events are scheduled in.
> 
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> ---
>  arch/x86/events/intel/core.c     | 13 +++++++
>  arch/x86/events/intel/ds.c       | 59 +++++++++++++++++++++++++++++++-
>  arch/x86/events/perf_event.h     |  9 +++++
>  arch/x86/include/asm/msr-index.h |  3 ++
>  4 files changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 4b4dac089635..949a589fd9b1 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -3291,6 +3291,19 @@ static int intel_pmu_hw_config(struct perf_event *event)
>  		}
>  	}
>  
> +	if (event->attr.aux_source) {
> +		if (!event->attr.precise_ip)
> +			return -EINVAL;
> +
> +		if (!x86_pmu.intel_cap.pebs_output_pt_available)
> +			return -EOPNOTSUPP;
> +
> +		event->hw.flags |= PERF_X86_EVENT_PEBS_VIA_PT;
> +
> +		/* Signal to the core that we handled it */
> +		event->attr.aux_source = 0;

That's a bit yuck, ideally we'd not modify attrs.

Can't we change that test in perf_event_alloc() to something like:

	if (event->attr.aux_source &&
	    !(pmu->capabilities & PERF_PMU_CAP_AUX_SOURCE))
		// error

?


> +	}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] perf/x86/intel: Support PEBS output to PT
  2019-05-02 10:50 ` [PATCH 2/2] perf/x86/intel: Support PEBS output to PT Alexander Shishkin
  2019-05-08  9:12   ` Peter Zijlstra
@ 2019-05-08  9:18   ` Peter Zijlstra
  2019-05-08  9:34   ` Peter Zijlstra
  2 siblings, 0 replies; 8+ messages in thread
From: Peter Zijlstra @ 2019-05-08  9:18 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel, jolsa,
	adrian.hunter

On Thu, May 02, 2019 at 01:50:22PM +0300, Alexander Shishkin wrote:
> The output setting is per-CPU, so all PEBS events must be either writing
> to PT or to the DS area, so in order to not mess up the event scheduling,
> we fall back to the latter in case both types of events are scheduled in.

Urgh, that blows... I really don't like that.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] perf/x86/intel: Support PEBS output to PT
  2019-05-02 10:50 ` [PATCH 2/2] perf/x86/intel: Support PEBS output to PT Alexander Shishkin
  2019-05-08  9:12   ` Peter Zijlstra
  2019-05-08  9:18   ` Peter Zijlstra
@ 2019-05-08  9:34   ` Peter Zijlstra
  2 siblings, 0 replies; 8+ messages in thread
From: Peter Zijlstra @ 2019-05-08  9:34 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel, jolsa,
	adrian.hunter

On Thu, May 02, 2019 at 01:50:22PM +0300, Alexander Shishkin wrote:

> The output setting is per-CPU, so all PEBS events must be either writing
> to PT or to the DS area, so in order to not mess up the event scheduling,
> we fall back to the latter in case both types of events are scheduled in.

> +static void intel_pmu_pebs_via_pt_disable(struct perf_event *event)
> +{
> +	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> +
> +	if (!(event->hw.flags & PERF_X86_EVENT_PEBS_VIA_PT))
> +		return;
> +
> +	if (!(cpuc->pebs_enabled & ~PEBS_VIA_PT_MASK))
> +		cpuc->pebs_enabled &= ~PEBS_VIA_PT_MASK;
> +}
> +
> +static void intel_pmu_pebs_via_pt_enable(struct perf_event *event)
> +{
> +	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct debug_store *ds = cpuc->ds;
> +
> +	if (!(event->hw.flags & PERF_X86_EVENT_PEBS_VIA_PT))
> +		return;
> +
> +	/*
> +	 * In case there's a mix of PEBS->PT and PEBS->DS, fall back
> +	 * to DS.
> +	 */
> +	if (cpuc->n_pebs != cpuc->n_pebs_via_pt) {
> +		/* PEBS-to-DS events present, fall back to DS */
> +		intel_pmu_pebs_via_pt_disable(event);
> +		return;
> +	}
> +
> +	if (!(event->hw.flags & PERF_X86_EVENT_LARGE_PEBS))
> +		cpuc->pebs_enabled |= PEBS_PMI_AFTER_EACH_RECORD;
> +
> +	cpuc->pebs_enabled |= PEBS_OUTPUT_PT;
> +
> +	wrmsrl(MSR_RELOAD_PMC0 + hwc->idx, ds->pebs_event_reset[hwc->idx]);
> +}
> +
>  void intel_pmu_pebs_enable(struct perf_event *event)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> @@ -1100,6 +1146,8 @@ void intel_pmu_pebs_enable(struct perf_event *event)
>  	} else {
>  		ds->pebs_event_reset[hwc->idx] = 0;
>  	}
> +
> +	intel_pmu_pebs_via_pt_enable(event);
>  }

I think that doesn't even do what it says on the tin. Suppose you first
schedule that PEBS-via-PT event and then the normal one, nothing then
cancels the PT link.

Like I wrote in that prevoius email; I really don't like this. I think
silently falling back to another output method is wrong.

Ideally we create schedulig conflicts and cause the PT and DS events to
round robin.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT
  2019-05-02 10:50 [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT Alexander Shishkin
                   ` (2 preceding siblings ...)
  2019-05-03  8:57 ` [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT Alexander Shishkin
@ 2019-05-08 10:39 ` Peter Zijlstra
  3 siblings, 0 replies; 8+ messages in thread
From: Peter Zijlstra @ 2019-05-08 10:39 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel, jolsa,
	adrian.hunter

On Thu, May 02, 2019 at 01:50:20PM +0300, Alexander Shishkin wrote:
> Hi Peter,
> 
> New PEBS feature: output to Intel PT stream instead of the DS area. It's
> theoretically useful in virtualized environments, where DS area can't be
> used. It's also good for those who are interested in instruction trace for
> context of the PEBS events. As PEBS goes, it can provide LBR context with
> all the branch-related information that PT doesn't provide at the moment.
> 
> PEBS records are packetized in the PT stream, so instead of extracting
> them in the PMI, we leave it to the perf tool, because real time PT
> decoding is not practical. Tooling patches are not included, but can be
> found here [1].
> 
> Added is an attribute bit 'aux_source' to mean that an event is a source of
> AUX data. This bit enables PEBS output to PT.

There is a distinct lack of permission checks in this. When creating
this construct we should verify the creator has access to PT. And we
should verify we're not (accidentally or otherwise) writing into someone
else's PT buffers.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-05-08 10:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-02 10:50 [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT Alexander Shishkin
2019-05-02 10:50 ` [PATCH 1/2] perf: Allow normal events to be sources of AUX data Alexander Shishkin
2019-05-02 10:50 ` [PATCH 2/2] perf/x86/intel: Support PEBS output to PT Alexander Shishkin
2019-05-08  9:12   ` Peter Zijlstra
2019-05-08  9:18   ` Peter Zijlstra
2019-05-08  9:34   ` Peter Zijlstra
2019-05-03  8:57 ` [PATCH 0/2] perf, intel: Add support for PEBS output to Intel PT Alexander Shishkin
2019-05-08 10:39 ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).