* [PATCH 0/2] perf/ibs: Fix interface via core pmu events @ 2023-03-02 9:21 Ravi Bangoria 2023-03-02 9:21 ` [PATCH 1/2] " Ravi Bangoria 2023-03-02 9:21 ` [PATCH 2/2] perf test: Add selftest to test IBS invocation " Ravi Bangoria 0 siblings, 2 replies; 9+ messages in thread From: Ravi Bangoria @ 2023-03-02 9:21 UTC (permalink / raw) To: peterz Cc: ravi.bangoria, eranian, acme, mark.rutland, jolsa, namhyung, irogers, bp, x86, linux-perf-users, linux-kernel, sandipan.das, ananth.narayan, santosh.shukla IBS used to allow event creation via core pmu events with precise_ip attribute set, but it's broken since late 2019. 1st patch fixes the issue and 2nd patch adds simple perf test for the same. Ravi Bangoria (2): perf/ibs: Fix interface via core pmu events perf test: Add selftest to test IBS invocation via core pmu events arch/x86/events/amd/core.c | 5 +++ arch/x86/events/core.c | 2 + arch/x86/events/perf_event.h | 3 ++ include/linux/perf_event.h | 1 + kernel/events/core.c | 11 +++-- tools/perf/tests/Build | 1 + tools/perf/tests/builtin-test.c | 1 + tools/perf/tests/ibs-via-core-pmu.c | 70 +++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + 9 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 tools/perf/tests/ibs-via-core-pmu.c -- 2.39.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] perf/ibs: Fix interface via core pmu events 2023-03-02 9:21 [PATCH 0/2] perf/ibs: Fix interface via core pmu events Ravi Bangoria @ 2023-03-02 9:21 ` Ravi Bangoria 2023-03-02 22:10 ` Namhyung Kim 2023-03-02 9:21 ` [PATCH 2/2] perf test: Add selftest to test IBS invocation " Ravi Bangoria 1 sibling, 1 reply; 9+ messages in thread From: Ravi Bangoria @ 2023-03-02 9:21 UTC (permalink / raw) To: peterz Cc: ravi.bangoria, eranian, acme, mark.rutland, jolsa, namhyung, irogers, bp, x86, linux-perf-users, linux-kernel, sandipan.das, ananth.narayan, santosh.shukla Although, IBS pmu can be invoked via it's own interface, indirect IBS invocation via core pmu event is also supported with fixed set of events: cpu-cycles:p, r076:p (same as cpu-cycles:p) and r0C1:p (micro-ops) for user convenience. This indirect IBS invocation is broken since commit 66d258c5b048 ("perf/core: Optimize perf_init_event()"), which added RAW pmu under pmu_idr list and thus if event_init() fails with RAW pmu, it started returning error instead of trying other pmus. Fix it by introducing new pmu capability PERF_PMU_CAP_FORWARD_EVENT. Kernel will try to open event on other pmus if user requested pmu, having this capability, fails to open event. Without patch: $ sudo ./perf record -C 0 -e r076:p -- sleep 1 Error: The r076:p event is not supported. With patch: $ sudo ./perf record -C 0 -e r076:p -- sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.341 MB perf.data (37 samples) ] This new capability does not have a notion of forward pmu mapping. i.e. it doesn't know which pmu(or set of pmus) the event should be forwarded to. As of now, only AMD core pmu forwards a set of events to IBS pmu when precise_ip attribute is set and thus trying with all pmus works. But if more pmus start using this capability, some sort of forward pmu mapping needs to be introduced through which the event can directly get forwarded to only mapped pmus. Otherwise, trying all pmus can inadvertently open event on wrong pmu. Fixes: 66d258c5b048 ("perf/core: Optimize perf_init_event()") Reported-by: Stephane Eranian <eranian@google.com> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com> --- arch/x86/events/amd/core.c | 5 +++++ arch/x86/events/core.c | 2 ++ arch/x86/events/perf_event.h | 3 +++ include/linux/perf_event.h | 1 + kernel/events/core.c | 11 ++++++++--- 5 files changed, 19 insertions(+), 3 deletions(-) diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c index 8c45b198b62f..f4c67362cfde 100644 --- a/arch/x86/events/amd/core.c +++ b/arch/x86/events/amd/core.c @@ -1264,6 +1264,11 @@ static __initconst const struct x86_pmu amd_pmu = { .cpu_dead = amd_pmu_cpu_dead, .amd_nb_constraints = 1, + /* + * Raw events with precise attribute set needs to be + * forwarded to IBS pmu. + */ + .capabilities = PERF_PMU_CAP_FORWARD_EVENT, }; static ssize_t branches_show(struct device *cdev, diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index d096b04bf80e..3f27b44f337a 100644 --- a/arch/x86/events/core.c +++ b/arch/x86/events/core.c @@ -2156,6 +2156,8 @@ static int __init init_hw_perf_events(void) if (err) goto out1; + pmu.capabilities |= x86_pmu.capabilities; + if (!is_hybrid()) { err = perf_pmu_register(&pmu, "cpu", PERF_TYPE_RAW); if (err) diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h index d6de4487348c..41e792bb442d 100644 --- a/arch/x86/events/perf_event.h +++ b/arch/x86/events/perf_event.h @@ -941,6 +941,9 @@ struct x86_pmu { int num_hybrid_pmus; struct x86_hybrid_pmu *hybrid_pmu; u8 (*get_hybrid_cpu_type) (void); + + /* Capabilities that needs to be forwarded to pmu->capabilities */ + int capabilities; }; struct x86_perf_task_context_opt { diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index d5628a7b5eaa..4459e0918e28 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -292,6 +292,7 @@ struct perf_event_pmu_context; #define PERF_PMU_CAP_NO_EXCLUDE 0x0080 #define PERF_PMU_CAP_AUX_OUTPUT 0x0100 #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200 +#define PERF_PMU_CAP_FORWARD_EVENT 0x0400 struct perf_output_handle; diff --git a/kernel/events/core.c b/kernel/events/core.c index a5a51dfdd622..c3f59d937280 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -11633,9 +11633,13 @@ static struct pmu *perf_init_event(struct perf_event *event) goto fail; ret = perf_try_init_event(pmu, event); - if (ret == -ENOENT && event->attr.type != type && !extended_type) { - type = event->attr.type; - goto again; + if (ret == -ENOENT) { + if (event->attr.type != type && !extended_type) { + type = event->attr.type; + goto again; + } + if (pmu->capabilities & PERF_PMU_CAP_FORWARD_EVENT) + goto try_all; } if (ret) @@ -11644,6 +11648,7 @@ static struct pmu *perf_init_event(struct perf_event *event) goto unlock; } +try_all: list_for_each_entry_rcu(pmu, &pmus, entry, lockdep_is_held(&pmus_srcu)) { ret = perf_try_init_event(pmu, event); if (!ret) -- 2.39.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] perf/ibs: Fix interface via core pmu events 2023-03-02 9:21 ` [PATCH 1/2] " Ravi Bangoria @ 2023-03-02 22:10 ` Namhyung Kim 2023-03-03 5:54 ` Ravi Bangoria 0 siblings, 1 reply; 9+ messages in thread From: Namhyung Kim @ 2023-03-02 22:10 UTC (permalink / raw) To: Ravi Bangoria Cc: peterz, eranian, acme, mark.rutland, jolsa, irogers, bp, x86, linux-perf-users, linux-kernel, sandipan.das, ananth.narayan, santosh.shukla Hi Ravi, On Thu, Mar 2, 2023 at 1:22 AM Ravi Bangoria <ravi.bangoria@amd.com> wrote: > > Although, IBS pmu can be invoked via it's own interface, indirect > IBS invocation via core pmu event is also supported with fixed set > of events: cpu-cycles:p, r076:p (same as cpu-cycles:p) and r0C1:p > (micro-ops) for user convenience. > > This indirect IBS invocation is broken since commit 66d258c5b048 > ("perf/core: Optimize perf_init_event()"), which added RAW pmu > under pmu_idr list and thus if event_init() fails with RAW pmu, > it started returning error instead of trying other pmus. > > Fix it by introducing new pmu capability PERF_PMU_CAP_FORWARD_EVENT. > Kernel will try to open event on other pmus if user requested pmu, > having this capability, fails to open event. > > Without patch: > $ sudo ./perf record -C 0 -e r076:p -- sleep 1 > Error: > The r076:p event is not supported. > > With patch: > $ sudo ./perf record -C 0 -e r076:p -- sleep 1 > [ perf record: Woken up 1 times to write data ] > [ perf record: Captured and wrote 0.341 MB perf.data (37 samples) ] > > This new capability does not have a notion of forward pmu mapping. > i.e. it doesn't know which pmu(or set of pmus) the event should be > forwarded to. As of now, only AMD core pmu forwards a set of events > to IBS pmu when precise_ip attribute is set and thus trying with all > pmus works. But if more pmus start using this capability, some sort > of forward pmu mapping needs to be introduced through which the event > can directly get forwarded to only mapped pmus. Otherwise, trying all > pmus can inadvertently open event on wrong pmu. > > Fixes: 66d258c5b048 ("perf/core: Optimize perf_init_event()") > Reported-by: Stephane Eranian <eranian@google.com> > Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com> > --- > arch/x86/events/amd/core.c | 5 +++++ > arch/x86/events/core.c | 2 ++ > arch/x86/events/perf_event.h | 3 +++ > include/linux/perf_event.h | 1 + > kernel/events/core.c | 11 ++++++++--- > 5 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c > index 8c45b198b62f..f4c67362cfde 100644 > --- a/arch/x86/events/amd/core.c > +++ b/arch/x86/events/amd/core.c > @@ -1264,6 +1264,11 @@ static __initconst const struct x86_pmu amd_pmu = { > .cpu_dead = amd_pmu_cpu_dead, > > .amd_nb_constraints = 1, > + /* > + * Raw events with precise attribute set needs to be > + * forwarded to IBS pmu. > + */ > + .capabilities = PERF_PMU_CAP_FORWARD_EVENT, > }; > > static ssize_t branches_show(struct device *cdev, > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c > index d096b04bf80e..3f27b44f337a 100644 > --- a/arch/x86/events/core.c > +++ b/arch/x86/events/core.c > @@ -2156,6 +2156,8 @@ static int __init init_hw_perf_events(void) > if (err) > goto out1; > > + pmu.capabilities |= x86_pmu.capabilities; > + > if (!is_hybrid()) { > err = perf_pmu_register(&pmu, "cpu", PERF_TYPE_RAW); > if (err) > diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h > index d6de4487348c..41e792bb442d 100644 > --- a/arch/x86/events/perf_event.h > +++ b/arch/x86/events/perf_event.h > @@ -941,6 +941,9 @@ struct x86_pmu { > int num_hybrid_pmus; > struct x86_hybrid_pmu *hybrid_pmu; > u8 (*get_hybrid_cpu_type) (void); > + > + /* Capabilities that needs to be forwarded to pmu->capabilities */ > + int capabilities; > }; > > struct x86_perf_task_context_opt { > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index d5628a7b5eaa..4459e0918e28 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -292,6 +292,7 @@ struct perf_event_pmu_context; > #define PERF_PMU_CAP_NO_EXCLUDE 0x0080 > #define PERF_PMU_CAP_AUX_OUTPUT 0x0100 > #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200 > +#define PERF_PMU_CAP_FORWARD_EVENT 0x0400 > > struct perf_output_handle; > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index a5a51dfdd622..c3f59d937280 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -11633,9 +11633,13 @@ static struct pmu *perf_init_event(struct perf_event *event) > goto fail; > > ret = perf_try_init_event(pmu, event); > - if (ret == -ENOENT && event->attr.type != type && !extended_type) { > - type = event->attr.type; > - goto again; > + if (ret == -ENOENT) { > + if (event->attr.type != type && !extended_type) { > + type = event->attr.type; > + goto again; > + } > + if (pmu->capabilities & PERF_PMU_CAP_FORWARD_EVENT) > + goto try_all; Wouldn't it be better to use a different error code to indicate it's about precise_ip (or forwarding in general)? Otherwise other invalid config might cause the forwarding unnecessarily.. Thanks, Namhyung > } > > if (ret) > @@ -11644,6 +11648,7 @@ static struct pmu *perf_init_event(struct perf_event *event) > goto unlock; > } > > +try_all: > list_for_each_entry_rcu(pmu, &pmus, entry, lockdep_is_held(&pmus_srcu)) { > ret = perf_try_init_event(pmu, event); > if (!ret) > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] perf/ibs: Fix interface via core pmu events 2023-03-02 22:10 ` Namhyung Kim @ 2023-03-03 5:54 ` Ravi Bangoria 2023-03-06 22:29 ` Namhyung Kim 0 siblings, 1 reply; 9+ messages in thread From: Ravi Bangoria @ 2023-03-03 5:54 UTC (permalink / raw) To: Namhyung Kim Cc: peterz, eranian, acme, mark.rutland, jolsa, irogers, bp, x86, linux-perf-users, linux-kernel, sandipan.das, ananth.narayan, santosh.shukla, Ravi Bangoria Hi Namhyung, >> diff --git a/kernel/events/core.c b/kernel/events/core.c >> index a5a51dfdd622..c3f59d937280 100644 >> --- a/kernel/events/core.c >> +++ b/kernel/events/core.c >> @@ -11633,9 +11633,13 @@ static struct pmu *perf_init_event(struct perf_event *event) >> goto fail; >> >> ret = perf_try_init_event(pmu, event); >> - if (ret == -ENOENT && event->attr.type != type && !extended_type) { >> - type = event->attr.type; >> - goto again; >> + if (ret == -ENOENT) { >> + if (event->attr.type != type && !extended_type) { >> + type = event->attr.type; >> + goto again; >> + } >> + if (pmu->capabilities & PERF_PMU_CAP_FORWARD_EVENT) >> + goto try_all; > > Wouldn't it be better to use a different error code to indicate > it's about precise_ip (or forwarding in general)? Otherwise > other invalid config might cause the forwarding unnecessarily.. That would make things easier and we might not need this new capability. Most appropriate error codes seems ENOENT, EOPNOTSUPP and EINVAL but all are already used for other purposes. Any other suggestions? Thanks, Ravi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] perf/ibs: Fix interface via core pmu events 2023-03-03 5:54 ` Ravi Bangoria @ 2023-03-06 22:29 ` Namhyung Kim 2023-03-07 3:19 ` Ravi Bangoria 0 siblings, 1 reply; 9+ messages in thread From: Namhyung Kim @ 2023-03-06 22:29 UTC (permalink / raw) To: Ravi Bangoria Cc: peterz, eranian, acme, mark.rutland, jolsa, irogers, bp, x86, linux-perf-users, linux-kernel, sandipan.das, ananth.narayan, santosh.shukla On Thu, Mar 2, 2023 at 9:54 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote: > > Hi Namhyung, > > >> diff --git a/kernel/events/core.c b/kernel/events/core.c > >> index a5a51dfdd622..c3f59d937280 100644 > >> --- a/kernel/events/core.c > >> +++ b/kernel/events/core.c > >> @@ -11633,9 +11633,13 @@ static struct pmu *perf_init_event(struct perf_event *event) > >> goto fail; > >> > >> ret = perf_try_init_event(pmu, event); > >> - if (ret == -ENOENT && event->attr.type != type && !extended_type) { > >> - type = event->attr.type; > >> - goto again; > >> + if (ret == -ENOENT) { > >> + if (event->attr.type != type && !extended_type) { > >> + type = event->attr.type; > >> + goto again; > >> + } > >> + if (pmu->capabilities & PERF_PMU_CAP_FORWARD_EVENT) > >> + goto try_all; > > > > Wouldn't it be better to use a different error code to indicate > > it's about precise_ip (or forwarding in general)? Otherwise > > other invalid config might cause the forwarding unnecessarily.. > > That would make things easier and we might not need this new capability. > Most appropriate error codes seems ENOENT, EOPNOTSUPP and EINVAL but all > are already used for other purposes. Any other suggestions? Maybe we can have more liberty for the error code since it's not returned to the user. How about ESRCH, EIO or ENXIO? Thanks, Namhyung ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] perf/ibs: Fix interface via core pmu events 2023-03-06 22:29 ` Namhyung Kim @ 2023-03-07 3:19 ` Ravi Bangoria 0 siblings, 0 replies; 9+ messages in thread From: Ravi Bangoria @ 2023-03-07 3:19 UTC (permalink / raw) To: Namhyung Kim Cc: peterz, eranian, acme, mark.rutland, jolsa, irogers, bp, x86, linux-perf-users, linux-kernel, sandipan.das, ananth.narayan, santosh.shukla, Ravi Bangoria On 07-Mar-23 3:59 AM, Namhyung Kim wrote: > On Thu, Mar 2, 2023 at 9:54 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote: >> >> Hi Namhyung, >> >>>> diff --git a/kernel/events/core.c b/kernel/events/core.c >>>> index a5a51dfdd622..c3f59d937280 100644 >>>> --- a/kernel/events/core.c >>>> +++ b/kernel/events/core.c >>>> @@ -11633,9 +11633,13 @@ static struct pmu *perf_init_event(struct perf_event *event) >>>> goto fail; >>>> >>>> ret = perf_try_init_event(pmu, event); >>>> - if (ret == -ENOENT && event->attr.type != type && !extended_type) { >>>> - type = event->attr.type; >>>> - goto again; >>>> + if (ret == -ENOENT) { >>>> + if (event->attr.type != type && !extended_type) { >>>> + type = event->attr.type; >>>> + goto again; >>>> + } >>>> + if (pmu->capabilities & PERF_PMU_CAP_FORWARD_EVENT) >>>> + goto try_all; >>> >>> Wouldn't it be better to use a different error code to indicate >>> it's about precise_ip (or forwarding in general)? Otherwise >>> other invalid config might cause the forwarding unnecessarily.. >> >> That would make things easier and we might not need this new capability. >> Most appropriate error codes seems ENOENT, EOPNOTSUPP and EINVAL but all >> are already used for other purposes. Any other suggestions? > > Maybe we can have more liberty for the error code since > it's not returned to the user. How about ESRCH, EIO or ENXIO? Ok. We can probably use ESRCH, although it's meaning is little different: $ errno -l | grep ESRCH ESRCH 3 No such process Thanks, Ravi ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] perf test: Add selftest to test IBS invocation via core pmu events 2023-03-02 9:21 [PATCH 0/2] perf/ibs: Fix interface via core pmu events Ravi Bangoria 2023-03-02 9:21 ` [PATCH 1/2] " Ravi Bangoria @ 2023-03-02 9:21 ` Ravi Bangoria 2023-03-02 22:12 ` Namhyung Kim 1 sibling, 1 reply; 9+ messages in thread From: Ravi Bangoria @ 2023-03-02 9:21 UTC (permalink / raw) To: peterz Cc: ravi.bangoria, eranian, acme, mark.rutland, jolsa, namhyung, irogers, bp, x86, linux-perf-users, linux-kernel, sandipan.das, ananth.narayan, santosh.shukla IBS pmu can be invoked via fixed set of core pmu events. Add simple event open test for all these events. Without kernel fix: $ sudo ./perf test -vv 76 76: IBS via core pmu : --- start --- test child forked, pid 6553 Using CPUID AuthenticAMD-25-1-1 type: 0x0, config: 0x0, fd: 3 - Pass type: 0x0, config: 0x1, fd: -1 - Pass type: 0x4, config: 0x76, fd: -1 - Fail type: 0x4, config: 0xc1, fd: -1 - Fail type: 0x4, config: 0x12, fd: -1 - Pass test child finished with -1 ---- end ---- IBS via core pmu: FAILED! With kernel fix: $ sudo ./perf test -vv 76 76: IBS via core pmu : --- start --- test child forked, pid 7526 Using CPUID AuthenticAMD-25-1-1 type: 0x0, config: 0x0, fd: 3 - Pass type: 0x0, config: 0x1, fd: -1 - Pass type: 0x4, config: 0x76, fd: 3 - Pass type: 0x4, config: 0xc1, fd: 3 - Pass type: 0x4, config: 0x12, fd: -1 - Pass test child finished with 0 ---- end ---- IBS via core pmu: Ok Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com> --- tools/perf/tests/Build | 1 + tools/perf/tests/builtin-test.c | 1 + tools/perf/tests/ibs-via-core-pmu.c | 70 +++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + 4 files changed, 73 insertions(+) create mode 100644 tools/perf/tests/ibs-via-core-pmu.c diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build index fb9ac5dc4079..1a232cf13c33 100644 --- a/tools/perf/tests/Build +++ b/tools/perf/tests/Build @@ -69,6 +69,7 @@ perf-y += dlfilter-test.o perf-y += sigtrap.o perf-y += event_groups.o perf-y += symbols.o +perf-y += ibs-via-core-pmu.o $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build $(call rule_mkdir) diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 35cc3807cc9e..aed887234500 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -119,6 +119,7 @@ static struct test_suite *generic_tests[] = { &suite__sigtrap, &suite__event_groups, &suite__symbols, + &suite__ibs_via_core_pmu, NULL, }; diff --git a/tools/perf/tests/ibs-via-core-pmu.c b/tools/perf/tests/ibs-via-core-pmu.c new file mode 100644 index 000000000000..6ac539509791 --- /dev/null +++ b/tools/perf/tests/ibs-via-core-pmu.c @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "linux/perf_event.h" +#include "tests.h" +#include "pmu.h" +#include "pmus.h" +#include "../perf-sys.h" +#include "debug.h" + +#define NR_SUB_TESTS 5 + +static struct sub_tests { + int type; + unsigned long config; + bool valid; +} sub_tests[NR_SUB_TESTS] = { + { PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES, true }, + { PERF_TYPE_HARDWARE, PERF_COUNT_HW_INSTRUCTIONS, false }, + { PERF_TYPE_RAW, 0x076, true }, + { PERF_TYPE_RAW, 0x0C1, true }, + { PERF_TYPE_RAW, 0x012, false }, +}; + +static int event_open(int type, unsigned long config) +{ + struct perf_event_attr attr; + + memset(&attr, 0, sizeof(struct perf_event_attr)); + attr.type = type; + attr.size = sizeof(struct perf_event_attr); + attr.config = config; + attr.disabled = 1; + attr.precise_ip = 1; + attr.sample_type = PERF_SAMPLE_IP | PERF_SAMPLE_TID; + attr.sample_period = 100000; + + return sys_perf_event_open(&attr, -1, 0, -1, 0); +} + +static int test__ibs_via_core_pmu(struct test_suite *text __maybe_unused, + int subtest __maybe_unused) +{ + struct perf_pmu *ibs_pmu; + int ret = TEST_OK; + int fd, i; + + if (list_empty(&pmus)) + perf_pmu__scan(NULL); + + ibs_pmu = perf_pmu__find("ibs_op"); + if (!ibs_pmu) + return TEST_SKIP; + + for (i = 0; i < NR_SUB_TESTS; i++) { + fd = event_open(sub_tests[i].type, sub_tests[i].config); + pr_debug("type: 0x%x, config: 0x%lx, fd: %d - ", sub_tests[i].type, + sub_tests[i].config, fd); + if ((sub_tests[i].valid && fd == -1) || + (!sub_tests[i].valid && fd > 0)) { + pr_debug("Fail\n"); + ret = TEST_FAIL; + } else { + pr_debug("Pass\n"); + close(fd); + } + } + + return ret; +} + +DEFINE_SUITE("IBS via core pmu", ibs_via_core_pmu); diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h index 9a0f3904e53d..36339fdf9c36 100644 --- a/tools/perf/tests/tests.h +++ b/tools/perf/tests/tests.h @@ -149,6 +149,7 @@ DECLARE_SUITE(dlfilter); DECLARE_SUITE(sigtrap); DECLARE_SUITE(event_groups); DECLARE_SUITE(symbols); +DECLARE_SUITE(ibs_via_core_pmu); /* * PowerPC and S390 do not support creation of instruction breakpoints using the -- 2.39.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] perf test: Add selftest to test IBS invocation via core pmu events 2023-03-02 9:21 ` [PATCH 2/2] perf test: Add selftest to test IBS invocation " Ravi Bangoria @ 2023-03-02 22:12 ` Namhyung Kim 2023-03-03 5:55 ` Ravi Bangoria 0 siblings, 1 reply; 9+ messages in thread From: Namhyung Kim @ 2023-03-02 22:12 UTC (permalink / raw) To: Ravi Bangoria Cc: peterz, eranian, acme, mark.rutland, jolsa, irogers, bp, x86, linux-perf-users, linux-kernel, sandipan.das, ananth.narayan, santosh.shukla On Thu, Mar 2, 2023 at 1:22 AM Ravi Bangoria <ravi.bangoria@amd.com> wrote: > > IBS pmu can be invoked via fixed set of core pmu events. Add simple > event open test for all these events. > > Without kernel fix: > $ sudo ./perf test -vv 76 > 76: IBS via core pmu : > --- start --- > test child forked, pid 6553 > Using CPUID AuthenticAMD-25-1-1 > type: 0x0, config: 0x0, fd: 3 - Pass > type: 0x0, config: 0x1, fd: -1 - Pass > type: 0x4, config: 0x76, fd: -1 - Fail > type: 0x4, config: 0xc1, fd: -1 - Fail > type: 0x4, config: 0x12, fd: -1 - Pass > test child finished with -1 > ---- end ---- > IBS via core pmu: FAILED! > > With kernel fix: > $ sudo ./perf test -vv 76 > 76: IBS via core pmu : > --- start --- > test child forked, pid 7526 > Using CPUID AuthenticAMD-25-1-1 > type: 0x0, config: 0x0, fd: 3 - Pass > type: 0x0, config: 0x1, fd: -1 - Pass > type: 0x4, config: 0x76, fd: 3 - Pass > type: 0x4, config: 0xc1, fd: 3 - Pass > type: 0x4, config: 0x12, fd: -1 - Pass > test child finished with 0 > ---- end ---- > IBS via core pmu: Ok > > Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com> > --- > tools/perf/tests/Build | 1 + > tools/perf/tests/builtin-test.c | 1 + > tools/perf/tests/ibs-via-core-pmu.c | 70 +++++++++++++++++++++++++++++ > tools/perf/tests/tests.h | 1 + > 4 files changed, 73 insertions(+) > create mode 100644 tools/perf/tests/ibs-via-core-pmu.c > > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build > index fb9ac5dc4079..1a232cf13c33 100644 > --- a/tools/perf/tests/Build > +++ b/tools/perf/tests/Build > @@ -69,6 +69,7 @@ perf-y += dlfilter-test.o > perf-y += sigtrap.o > perf-y += event_groups.o > perf-y += symbols.o > +perf-y += ibs-via-core-pmu.o > > $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build > $(call rule_mkdir) > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > index 35cc3807cc9e..aed887234500 100644 > --- a/tools/perf/tests/builtin-test.c > +++ b/tools/perf/tests/builtin-test.c > @@ -119,6 +119,7 @@ static struct test_suite *generic_tests[] = { > &suite__sigtrap, > &suite__event_groups, > &suite__symbols, > + &suite__ibs_via_core_pmu, > NULL, > }; > > diff --git a/tools/perf/tests/ibs-via-core-pmu.c b/tools/perf/tests/ibs-via-core-pmu.c > new file mode 100644 > index 000000000000..6ac539509791 > --- /dev/null > +++ b/tools/perf/tests/ibs-via-core-pmu.c > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include "linux/perf_event.h" > +#include "tests.h" > +#include "pmu.h" > +#include "pmus.h" > +#include "../perf-sys.h" > +#include "debug.h" > + > +#define NR_SUB_TESTS 5 > + > +static struct sub_tests { > + int type; > + unsigned long config; > + bool valid; > +} sub_tests[NR_SUB_TESTS] = { > + { PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES, true }, > + { PERF_TYPE_HARDWARE, PERF_COUNT_HW_INSTRUCTIONS, false }, > + { PERF_TYPE_RAW, 0x076, true }, > + { PERF_TYPE_RAW, 0x0C1, true }, > + { PERF_TYPE_RAW, 0x012, false }, > +}; > + > +static int event_open(int type, unsigned long config) > +{ > + struct perf_event_attr attr; > + > + memset(&attr, 0, sizeof(struct perf_event_attr)); > + attr.type = type; > + attr.size = sizeof(struct perf_event_attr); > + attr.config = config; > + attr.disabled = 1; > + attr.precise_ip = 1; > + attr.sample_type = PERF_SAMPLE_IP | PERF_SAMPLE_TID; > + attr.sample_period = 100000; > + > + return sys_perf_event_open(&attr, -1, 0, -1, 0); > +} > + > +static int test__ibs_via_core_pmu(struct test_suite *text __maybe_unused, > + int subtest __maybe_unused) > +{ > + struct perf_pmu *ibs_pmu; > + int ret = TEST_OK; > + int fd, i; > + > + if (list_empty(&pmus)) > + perf_pmu__scan(NULL); > + > + ibs_pmu = perf_pmu__find("ibs_op"); > + if (!ibs_pmu) > + return TEST_SKIP; > + > + for (i = 0; i < NR_SUB_TESTS; i++) { > + fd = event_open(sub_tests[i].type, sub_tests[i].config); > + pr_debug("type: 0x%x, config: 0x%lx, fd: %d - ", sub_tests[i].type, > + sub_tests[i].config, fd); > + if ((sub_tests[i].valid && fd == -1) || > + (!sub_tests[i].valid && fd > 0)) { > + pr_debug("Fail\n"); > + ret = TEST_FAIL; You need to close(fd) when it's succeeded for invalid one. Thanks, Namhyung > + } else { > + pr_debug("Pass\n"); > + close(fd); > + } > + } > + > + return ret; > +} > + > +DEFINE_SUITE("IBS via core pmu", ibs_via_core_pmu); > diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h > index 9a0f3904e53d..36339fdf9c36 100644 > --- a/tools/perf/tests/tests.h > +++ b/tools/perf/tests/tests.h > @@ -149,6 +149,7 @@ DECLARE_SUITE(dlfilter); > DECLARE_SUITE(sigtrap); > DECLARE_SUITE(event_groups); > DECLARE_SUITE(symbols); > +DECLARE_SUITE(ibs_via_core_pmu); > > /* > * PowerPC and S390 do not support creation of instruction breakpoints using the > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] perf test: Add selftest to test IBS invocation via core pmu events 2023-03-02 22:12 ` Namhyung Kim @ 2023-03-03 5:55 ` Ravi Bangoria 0 siblings, 0 replies; 9+ messages in thread From: Ravi Bangoria @ 2023-03-03 5:55 UTC (permalink / raw) To: Namhyung Kim Cc: peterz, eranian, acme, mark.rutland, jolsa, irogers, bp, x86, linux-perf-users, linux-kernel, sandipan.das, ananth.narayan, santosh.shukla, Ravi Bangoria Hi Namhyung, >> +static int test__ibs_via_core_pmu(struct test_suite *text __maybe_unused, >> + int subtest __maybe_unused) >> +{ >> + struct perf_pmu *ibs_pmu; >> + int ret = TEST_OK; >> + int fd, i; >> + >> + if (list_empty(&pmus)) >> + perf_pmu__scan(NULL); >> + >> + ibs_pmu = perf_pmu__find("ibs_op"); >> + if (!ibs_pmu) >> + return TEST_SKIP; >> + >> + for (i = 0; i < NR_SUB_TESTS; i++) { >> + fd = event_open(sub_tests[i].type, sub_tests[i].config); >> + pr_debug("type: 0x%x, config: 0x%lx, fd: %d - ", sub_tests[i].type, >> + sub_tests[i].config, fd); >> + if ((sub_tests[i].valid && fd == -1) || >> + (!sub_tests[i].valid && fd > 0)) { >> + pr_debug("Fail\n"); >> + ret = TEST_FAIL; > > You need to close(fd) when it's succeeded for invalid one. Yup. Will fix it. Thanks, Ravi ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-03-07 3:22 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-02 9:21 [PATCH 0/2] perf/ibs: Fix interface via core pmu events Ravi Bangoria 2023-03-02 9:21 ` [PATCH 1/2] " Ravi Bangoria 2023-03-02 22:10 ` Namhyung Kim 2023-03-03 5:54 ` Ravi Bangoria 2023-03-06 22:29 ` Namhyung Kim 2023-03-07 3:19 ` Ravi Bangoria 2023-03-02 9:21 ` [PATCH 2/2] perf test: Add selftest to test IBS invocation " Ravi Bangoria 2023-03-02 22:12 ` Namhyung Kim 2023-03-03 5:55 ` Ravi Bangoria
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).