[PATCH] perf/amd/ibs: Don't set exclude_guest by default

From: Ravi Bangoria
Date: Tue Nov 02 2021 - 01:31:12 EST


Perf tool sets exclude_guest by default while calling perf_event_open().
Because IBS does not have filtering capability, it always gets rejected
by IBS PMU driver and thus perf falls back to non-precise sampling. Fix
it by not setting exclude_guest by default on AMD.

Before:
$ sudo ./perf record -C 0 -vvv true |& grep precise
precise_ip 3
decreasing precise_ip by one (2)
precise_ip 2
decreasing precise_ip by one (1)
precise_ip 1
decreasing precise_ip by one (0)

After:
$ sudo ./perf record -C 0 -vvv true |& grep precise
precise_ip 3
decreasing precise_ip by one (2)
precise_ip 2

Reported-by: Kim Phillips <kim.phillips@xxxxxxx>
Signed-off-by: Ravi Bangoria <ravi.bangoria@xxxxxxx>
---
tools/perf/arch/x86/util/evsel.c | 23 +++++++++++++++++++++++
tools/perf/util/evsel.c | 12 +++++++-----
tools/perf/util/evsel.h | 1 +
3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/evsel.c
index 2f733cdc8dbb..7841a49ce856 100644
--- a/tools/perf/arch/x86/util/evsel.c
+++ b/tools/perf/arch/x86/util/evsel.c
@@ -1,8 +1,31 @@
// SPDX-License-Identifier: GPL-2.0
#include <stdio.h>
+#include <stdlib.h>
#include "util/evsel.h"
+#include "util/env.h"
+#include "linux/string.h"

void arch_evsel__set_sample_weight(struct evsel *evsel)
{
evsel__set_sample_bit(evsel, WEIGHT_STRUCT);
}
+
+void arch_evsel__fixup_new_cycles(struct perf_event_attr *attr)
+{
+ struct perf_env env = {0};
+
+ if (!perf_env__cpuid(&env))
+ return;
+
+ /*
+ * On AMD, precise cycles event sampling internally uses IBS pmu.
+ * But IBS does not have filtering capabilities and perf by default
+ * sets exclude_guest = 1. This makes IBS pmu event init fail and
+ * thus perf ends up doing non-precise sampling. Avoid it by clearing
+ * exclude_guest.
+ */
+ if (env.cpuid && strstarts(env.cpuid, "AuthenticAMD"))
+ attr->exclude_guest = 0;
+
+ free(env.cpuid);
+}
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index dbfeceb2546c..0b4267d4bb38 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -294,7 +294,7 @@ static bool perf_event_can_profile_kernel(void)
return perf_event_paranoid_check(1);
}

-struct evsel *evsel__new_cycles(bool precise, __u32 type, __u64 config)
+struct evsel *evsel__new_cycles(bool precise __maybe_unused, __u32 type, __u64 config)
{
struct perf_event_attr attr = {
.type = type,
@@ -305,18 +305,16 @@ struct evsel *evsel__new_cycles(bool precise, __u32 type, __u64 config)

event_attr_init(&attr);

- if (!precise)
- goto new_event;
-
/*
* Now let the usual logic to set up the perf_event_attr defaults
* to kick in when we return and before perf_evsel__open() is called.
*/
-new_event:
evsel = evsel__new(&attr);
if (evsel == NULL)
goto out;

+ arch_evsel__fixup_new_cycles(&evsel->core.attr);
+
evsel->precise_max = true;

/* use asprintf() because free(evsel) assumes name is allocated */
@@ -1047,6 +1045,10 @@ void __weak arch_evsel__set_sample_weight(struct evsel *evsel)
evsel__set_sample_bit(evsel, WEIGHT);
}

+void __weak arch_evsel__fixup_new_cycles(struct perf_event_attr *attr __maybe_unused)
+{
+}
+
/*
* The enable_on_exec/disabled value strategy:
*
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 1f7edfa8568a..764e54c6a23d 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -277,6 +277,7 @@ void __evsel__reset_sample_bit(struct evsel *evsel, enum perf_event_sample_forma
void evsel__set_sample_id(struct evsel *evsel, bool use_sample_identifier);

void arch_evsel__set_sample_weight(struct evsel *evsel);
+void arch_evsel__fixup_new_cycles(struct perf_event_attr *attr);

int evsel__set_filter(struct evsel *evsel, const char *filter);
int evsel__append_tp_filter(struct evsel *evsel, const char *filter);
--
2.27.0