[Patch v4 5/5] pef/x86/intel: Always snapshot live hardware LBR state for BPF path

[Dapeng Mi]

__intel_pmu_snapshot_branch_stack() is meant to snapshot the current
runtime LBR state from hardware for the BPF path. However, stale cached
PEBS LBR data can be snapshotted instead when a PEBS LBR user is active.

Fix this by always snapshotting the live hardware LBR state, regardless
of whether a PEBS LBR user is active. Reading the runtime LBR registers
can overwrite the cached PEBS LBR stack, so save and restore the cached
PEBS LBR state around the snapshot. This preserves the PEBS LBR data for
later output to userspace in the normal PEBS record path.

Fixes: c22ac2a3d4bd ("perf: Enable branch record for software events")
Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
---
 arch/x86/events/intel/core.c | 43 ++++++++++++++++++++++++++++++++++--
 arch/x86/events/intel/lbr.c  |  9 --------
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 5dfb9e9156e8..07d1a8a7845b 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2815,16 +2815,50 @@ static void intel_pmu_enable_all(int added)
 	__intel_pmu_enable_all(added, false);
 }
 
+struct lbr_snapshot {
+	struct perf_branch_stack stack;
+	struct perf_branch_entry entries[MAX_LBR_ENTRIES];
+	u64 counters[MAX_LBR_ENTRIES];
+};
+static DEFINE_PER_CPU(struct lbr_snapshot, lbr_snapshot);
+
 static noinline int
 __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
 				  unsigned int cnt, unsigned long flags)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+	struct lbr_snapshot *snapshot = this_cpu_ptr(&lbr_snapshot);
+
+	/*
+	 * This may be called from bpf_event_handler() in a PMI context. If
+	 * a PEBS LBR user is active, the PEBS sample record has already updated
+	 * cpuc->lbr_stack. Reading the LBR registers here would overwrite that
+	 * cached PEBS LBR stack, so save it first and restore it afterward. This
+	 * keeps the cached LBR data available for later output to userspace in
+	 * the normal PEBS record path.
+	 */
+	if (cpuc->lbr_pebs_users) {
+		memcpy(&snapshot->stack, &cpuc->lbr_stack,
+		       sizeof(struct perf_branch_stack));
+		memcpy(snapshot->entries, cpuc->lbr_entries,
+		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
+		memcpy(snapshot->counters, cpuc->lbr_counters,
+		       sizeof(u64) * cpuc->lbr_stack.nr);
+	}
 
 	intel_pmu_lbr_read();
 	cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
-
 	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
+
+	if (cpuc->lbr_pebs_users) {
+		memcpy(&cpuc->lbr_stack, &snapshot->stack,
+		       sizeof(struct perf_branch_stack));
+		memcpy(cpuc->lbr_entries, snapshot->entries,
+		       sizeof(struct perf_branch_entry) * cpuc->lbr_stack.nr);
+		memcpy(cpuc->lbr_counters, snapshot->counters,
+		       sizeof(u64) * cpuc->lbr_stack.nr);
+	}
+
 	intel_pmu_enable_all(0);
 	local_irq_restore(flags);
 	return cnt;
@@ -4022,7 +4056,12 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
 
 	loops = 0;
 again:
-	intel_pmu_lbr_read();
+	/*
+	 * No need to read the LBR stack when all LBR users are PEBS events,
+	 * since the PEBS handler updates it.
+	 */
+	if (cpuc->lbr_users != cpuc->lbr_pebs_users)
+		intel_pmu_lbr_read();
 	intel_pmu_ack_status(status);
 	if (++loops > 100) {
 		static bool warned;
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index ac1759a71288..07b32255e48d 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1030,15 +1030,6 @@ void intel_pmu_lbr_read(void)
 		return;
 	}
 
-	/*
-	 * Don't read when all LBRs users are using adaptive PEBS.
-	 *
-	 * This could be smarter and actually check the event,
-	 * but this simple approach seems to work for now.
-	 */
-	if (cpuc->lbr_users == cpuc->lbr_pebs_users)
-		return;
-
 	x86_pmu.lbr_read(cpuc);
 
 	intel_pmu_lbr_filter(cpuc);
-- 
2.34.1