[RFC PATCH 0/1] perf tools: Add 'm' event modifier for counting

[RFC PATCH 0/1] perf tools: Add 'm' event modifier for counting

[Nikita Shubin]

From: Nikita Shubin <n.shubin@yadro.com>

This patch is a proposal to add Machine Mode modifier that is currently 
available only for RISC-V for perf tool.

We don't currently have a level that is lower than kernel, so i don't 
see we can reuse something existing for this purpose.

The exclude_machine bit seems harmless to as no one will proccess it apart 
of riscv.

Patch depends on Atish Patra PMU series:
https://lwn.net/Articles/879905/
 + some modifications to pass execlude event to SBI PMU extension

Tested with qemu.

Nikita Shubin (1):
  perf tools: Add 'm' event modifier for counting machine

 include/uapi/linux/perf_event.h        |  3 ++-
 tools/include/uapi/linux/perf_event.h  |  3 ++-
 tools/perf/Documentation/perf-list.txt |  1 +
 tools/perf/tests/parse-events.c        | 18 ++++++++++++++++++
 tools/perf/util/evsel.c                |  4 +++-
 tools/perf/util/parse-events.c         | 18 +++++++++++++-----
 tools/perf/util/parse-events.l         |  2 +-
 7 files changed, 40 insertions(+), 9 deletions(-)

-- 
2.31.1

[RFC PATCH 1/1] perf tools: Add 'm' event modifier for counting machine

[Nikita Shubin]

From: Nikita Shubin <n.shubin@yadro.com>

Adding 'm' event modifier for machine mode counting.

Any event specified with 'm', will set exclude_machine to zero.

Default behavior is setting exclude_machine to zero, unless kernel or
user specified explicitly.

Adding automated tests.

Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
---
 include/uapi/linux/perf_event.h        |  3 ++-
 tools/include/uapi/linux/perf_event.h  |  3 ++-
 tools/perf/Documentation/perf-list.txt |  1 +
 tools/perf/tests/parse-events.c        | 18 ++++++++++++++++++
 tools/perf/util/evsel.c                |  4 +++-
 tools/perf/util/parse-events.c         | 18 +++++++++++++-----
 tools/perf/util/parse-events.l         |  2 +-
 7 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index bd8860eeb291..b70c245664eb 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -409,7 +409,8 @@ struct perf_event_attr {
 				inherit_thread :  1, /* children only inherit if cloned with CLONE_THREAD */
 				remove_on_exec :  1, /* event is removed from task on exec */
 				sigtrap        :  1, /* send synchronous SIGTRAP on event */
-				__reserved_1   : 26;
+				exclude_machine:  1, /* don't count M-Mode */
+				__reserved_1   : 25;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index bd8860eeb291..17f4d3ac10da 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -409,7 +409,8 @@ struct perf_event_attr {
 				inherit_thread :  1, /* children only inherit if cloned with CLONE_THREAD */
 				remove_on_exec :  1, /* event is removed from task on exec */
 				sigtrap        :  1, /* send synchronous SIGTRAP on event */
-				__reserved_1   : 26;
+				exclude_machine:  1, /* don't count events in M-Mode */
+				__reserved_1   : 25;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 4dc8d0af19df..e6d10d95df70 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -53,6 +53,7 @@ counted. The following modifiers exist:
 
  u - user-space counting
  k - kernel counting
+ m - machine counting
  h - hypervisor counting
  I - non idle counting
  G - guest counting (in KVM guests)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index a508f1dbcb2a..617a66003648 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1323,6 +1323,19 @@ static int test__exclusive_group(struct evlist *evlist)
 
 	return 0;
 }
+
+static int test__checkevent_exclude_machine_modifier(struct evlist *evlist)
+{
+	struct evsel *evsel = evlist__first(evlist);
+
+	TEST_ASSERT_VAL("wrong exclude_user", evsel->core.attr.exclude_user);
+	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
+	TEST_ASSERT_VAL("wrong exclude_machine", !evsel->core.attr.exclude_machine);
+	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
+
+	return test__checkevent_symbolic_name(evlist);
+}
+
 static int test__checkevent_breakpoint_len(struct evlist *evlist)
 {
 	struct evsel *evsel = evlist__first(evlist);
@@ -1943,6 +1956,11 @@ static struct evlist_test test__events[] = {
 		.check = test__exclusive_group,
 		.id    = 57,
 	},
+	{
+		.name  = "instructions:m",
+		.check = test__checkevent_exclude_machine_modifier,
+		.id    = 58,
+	},
 };
 
 static struct evlist_test test__events_pmu[] = {
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a59fb2ecb84e..a49c995908dc 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -542,9 +542,11 @@ static int evsel__add_modifiers(struct evsel *evsel, char *bf, size_t size)
 			r += scnprintf(bf + r, size - r, "%c", mod);	\
 		} } while(0)
 
-	if (attr->exclude_kernel || attr->exclude_user || attr->exclude_hv) {
+	if (attr->exclude_kernel || attr->exclude_user ||
+	    attr->exclude_hv || attr->exclude_machine) {
 		MOD_PRINT(kernel, 'k');
 		MOD_PRINT(user, 'u');
+		MOD_PRINT(machine, 'm');
 		MOD_PRINT(hv, 'h');
 		exclude_guest_default = true;
 	}
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 5bfb6f892489..920397e1b959 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1867,6 +1867,7 @@ struct event_modifier {
 	int weak;
 	int exclusive;
 	int bpf_counter;
+	int em;
 };
 
 static int get_event_modifier(struct event_modifier *mod, char *str,
@@ -1874,6 +1875,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 {
 	int eu = evsel ? evsel->core.attr.exclude_user : 0;
 	int ek = evsel ? evsel->core.attr.exclude_kernel : 0;
+	int em = evsel ? evsel->core.attr.exclude_machine : 0;
 	int eh = evsel ? evsel->core.attr.exclude_hv : 0;
 	int eH = evsel ? evsel->core.attr.exclude_host : 0;
 	int eG = evsel ? evsel->core.attr.exclude_guest : 0;
@@ -1884,7 +1886,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 	int pinned = evsel ? evsel->core.attr.pinned : 0;
 	int exclusive = evsel ? evsel->core.attr.exclusive : 0;
 
-	int exclude = eu | ek | eh;
+	int exclude = eu | ek | eh | em;
 	int exclude_GH = evsel ? evsel->exclude_GH : 0;
 	int weak = 0;
 	int bpf_counter = 0;
@@ -1894,17 +1896,17 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 	while (*str) {
 		if (*str == 'u') {
 			if (!exclude)
-				exclude = eu = ek = eh = 1;
+				exclude = eu = ek = eh = em = 1;
 			if (!exclude_GH && !perf_guest)
 				eG = 1;
 			eu = 0;
 		} else if (*str == 'k') {
 			if (!exclude)
-				exclude = eu = ek = eh = 1;
+				exclude = eu = ek = eh = em = 1;
 			ek = 0;
 		} else if (*str == 'h') {
 			if (!exclude)
-				exclude = eu = ek = eh = 1;
+				exclude = eu = ek = eh = em = 1;
 			eh = 0;
 		} else if (*str == 'G') {
 			if (!exclude_GH)
@@ -1933,6 +1935,10 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 			weak = 1;
 		} else if (*str == 'b') {
 			bpf_counter = 1;
+		} else if (*str == 'm') {
+			if (!exclude)
+				exclude = eu = ek = eh = em = 1;
+			em = 0;
 		} else
 			break;
 
@@ -1958,6 +1964,7 @@ static int get_event_modifier(struct event_modifier *mod, char *str,
 	mod->eH = eH;
 	mod->eG = eG;
 	mod->eI = eI;
+	mod->em = em;
 	mod->precise = precise;
 	mod->precise_max = precise_max;
 	mod->exclude_GH = exclude_GH;
@@ -1979,7 +1986,7 @@ static int check_modifier(char *str)
 	char *p = str;
 
 	/* The sizeof includes 0 byte as well. */
-	if (strlen(str) > (sizeof("ukhGHpppPSDIWeb") - 1))
+	if (strlen(str) > (sizeof("ukmhGHpppPSDIWeb") - 1))
 		return -1;
 
 	while (*p) {
@@ -2011,6 +2018,7 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add)
 
 		evsel->core.attr.exclude_user   = mod.eu;
 		evsel->core.attr.exclude_kernel = mod.ek;
+		evsel->core.attr.exclude_machine= mod.em;
 		evsel->core.attr.exclude_hv     = mod.eh;
 		evsel->core.attr.precise_ip     = mod.precise;
 		evsel->core.attr.exclude_host   = mod.eH;
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 4efe9872c667..5528a52611de 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -214,7 +214,7 @@ name_tag	[\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
 name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
 drv_cfg_term	[a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
 /* If you add a modifier you need to update check_modifier() */
-modifier_event	[ukhpPGHSDIWeb]+
+modifier_event	[ukhpPGHSDIWebm]+
 modifier_bp	[rwx]{1,3}
 
 %%
-- 
2.31.1

Re: [RFC PATCH 0/1] perf tools: Add 'm' event modifier for counting

[Jessica Clarke]

On 11 Jan 2022, at 14:20, Nikita Shubin <nikita.shubin@maquefel.me> wrote:
> 
> From: Nikita Shubin <n.shubin@yadro.com>
> 
> This patch is a proposal to add Machine Mode modifier that is currently 
> available only for RISC-V for perf tool.
> 
> We don't currently have a level that is lower than kernel, so i don't 
> see we can reuse something existing for this purpose.
> 
> The exclude_machine bit seems harmless to as no one will proccess it apart 
> of riscv.

Armv8 has equivalent bits in PMEVTYPER<n>_EL0 and PMCCFILTR_EL0 for
EL-based filtering, which includes filtering EL3.

Jess

> Patch depends on Atish Patra PMU series:
> https://lwn.net/Articles/879905/
> + some modifications to pass execlude event to SBI PMU extension
> 
> Tested with qemu.
> 
> Nikita Shubin (1):
>  perf tools: Add 'm' event modifier for counting machine
> 
> include/uapi/linux/perf_event.h        |  3 ++-
> tools/include/uapi/linux/perf_event.h  |  3 ++-
> tools/perf/Documentation/perf-list.txt |  1 +
> tools/perf/tests/parse-events.c        | 18 ++++++++++++++++++
> tools/perf/util/evsel.c                |  4 +++-
> tools/perf/util/parse-events.c         | 18 +++++++++++++-----
> tools/perf/util/parse-events.l         |  2 +-
> 7 files changed, 40 insertions(+), 9 deletions(-)
> 
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH 0/1] perf tools: Add 'm' event modifier for counting

[Atish Patra]

On Tue, Jan 11, 2022 at 6:29 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> On 11 Jan 2022, at 14:20, Nikita Shubin <nikita.shubin@maquefel.me> wrote:
> >
> > From: Nikita Shubin <n.shubin@yadro.com>
> >
> > This patch is a proposal to add Machine Mode modifier that is currently
> > available only for RISC-V for perf tool.
> >
> > We don't currently have a level that is lower than kernel, so i don't
> > see we can reuse something existing for this purpose.
> >

Thanks for a quick turnaround with a working patch!!

However, the use case can be applied to other arch as well. It would
be good to classify this
as excluding "firmware" events instead of machine mode which is a
RISC-V terminology.
Ofcourse, the current documentation should indicate that this option
is only useful
for RISC-V right now.

I would like to understand if this idea is appealing to the broader community.

> > The exclude_machine bit seems harmless to as no one will proccess it apart
> > of riscv.
>
> Armv8 has equivalent bits in PMEVTYPER<n>_EL0 and PMCCFILTR_EL0 for
> EL-based filtering, which includes filtering EL3.
>
> Jess
>
> > Patch depends on Atish Patra PMU series:
> > https://lwn.net/Articles/879905/
> > + some modifications to pass execlude event to SBI PMU extension
> >
> > Tested with qemu.
> >
> > Nikita Shubin (1):
> >  perf tools: Add 'm' event modifier for counting machine
> >
> > include/uapi/linux/perf_event.h        |  3 ++-
> > tools/include/uapi/linux/perf_event.h  |  3 ++-
> > tools/perf/Documentation/perf-list.txt |  1 +
> > tools/perf/tests/parse-events.c        | 18 ++++++++++++++++++
> > tools/perf/util/evsel.c                |  4 +++-
> > tools/perf/util/parse-events.c         | 18 +++++++++++++-----
> > tools/perf/util/parse-events.l         |  2 +-
> > 7 files changed, 40 insertions(+), 9 deletions(-)
> >
> > --
> > 2.31.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

Re: [RFC PATCH 1/1] perf tools: Add 'm' event modifier for counting machine

[Andi Kleen]

>   
>    u - user-space counting
>    k - kernel counting
> + m - machine counting


You really need to explain what "machine counting" actually is, as well 
that is likely irrelevant for most CPUs.

I also didn't see that anywhere in the commit logs.

-Andi

Re: [RFC PATCH 1/1] perf tools: Add 'm' event modifier for counting machine

[Atish Patra]

On Tue, Jan 11, 2022 at 10:53 AM Andi Kleen <ak@linux.intel.com> wrote:
>
>
> >
> >    u - user-space counting
> >    k - kernel counting
> > + m - machine counting
>
>
> You really need to explain what "machine counting" actually is, as well
> that is likely irrelevant for most CPUs.
>

Ideally, this should firmware counting instead of "machine counting"
which is a RISC-V specific terminology
(Machine mode is highest privilege mode similar to EL3 in ARM where
the firmware runs)

> I also didn't see that anywhere in the commit logs.
>
> -Andi
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Regards,
Atish

Re: [RFC PATCH 0/1] perf tools: Add 'm' event modifier for counting

[Nikita Shubin]

On Tue, 11 Jan 2022 14:29:15 +0000
Jessica Clarke <jrtc27@jrtc27.com> wrote:

Hello Jessica!

> On 11 Jan 2022, at 14:20, Nikita Shubin <nikita.shubin@maquefel.me>
> wrote:
> > 
> > From: Nikita Shubin <n.shubin@yadro.com>
> > 
> > This patch is a proposal to add Machine Mode modifier that is
> > currently available only for RISC-V for perf tool.
> > 
> > We don't currently have a level that is lower than kernel, so i
> > don't see we can reuse something existing for this purpose.
> > 
> > The exclude_machine bit seems harmless to as no one will proccess
> > it apart of riscv.  
> 
> Armv8 has equivalent bits in PMEVTYPER<n>_EL0 and PMCCFILTR_EL0 for
> EL-based filtering, which includes filtering EL3.
> 

Thank your for your tip, i ll study it and see if this is a good way to
go.

> Jess
> 
> > Patch depends on Atish Patra PMU series:
> > https://lwn.net/Articles/879905/
> > + some modifications to pass execlude event to SBI PMU extension
> > 
> > Tested with qemu.
> > 
> > Nikita Shubin (1):
> >  perf tools: Add 'm' event modifier for counting machine
> > 
> > include/uapi/linux/perf_event.h        |  3 ++-
> > tools/include/uapi/linux/perf_event.h  |  3 ++-
> > tools/perf/Documentation/perf-list.txt |  1 +
> > tools/perf/tests/parse-events.c        | 18 ++++++++++++++++++
> > tools/perf/util/evsel.c                |  4 +++-
> > tools/perf/util/parse-events.c         | 18 +++++++++++++-----
> > tools/perf/util/parse-events.l         |  2 +-
> > 7 files changed, 40 insertions(+), 9 deletions(-)
> > 
> > -- 
> > 2.31.1
> > 
> > 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv  
> 

Re: [RFC PATCH 0/1] perf tools: Add 'm' event modifier for counting

[Nikita Shubin]

Hello Atish!

On Tue, 11 Jan 2022 10:45:56 -0800
Atish Patra <atishp@atishpatra.org> wrote:

> On Tue, Jan 11, 2022 at 6:29 AM Jessica Clarke <jrtc27@jrtc27.com>
> wrote:
> >
> > On 11 Jan 2022, at 14:20, Nikita Shubin <nikita.shubin@maquefel.me>
> > wrote:  
> > >
> > > From: Nikita Shubin <n.shubin@yadro.com>
> > >
> > > This patch is a proposal to add Machine Mode modifier that is
> > > currently available only for RISC-V for perf tool.
> > >
> > > We don't currently have a level that is lower than kernel, so i
> > > don't see we can reuse something existing for this purpose.
> > >  
> 
> Thanks for a quick turnaround with a working patch!!
> 

Nothing please - your RISC-V PMU series as well as your work on
sscofpmf extension are great, so i am glad i can help you with that.

> However, the use case can be applied to other arch as well. It would
> be good to classify this
> as excluding "firmware" events instead of machine mode which is a
> RISC-V terminology.

That's a bit confusing cause, "firmware" resides in M-Mode in RISC-V
case and it's another level of privilege, i don't think it's possible
to exclude firmware that is on the same level as kernel.

For example we have PSCI in ARM, which falls in "firmware" category, in
my option, but we can't distinguish from other stuff on same privilege
level. 

> Ofcourse, the current documentation should indicate that this option
> is only useful
> for RISC-V right now.
> 
> I would like to understand if this idea is appealing to the broader
> community.

Yes - i would like to hear if adding event modifier is a way to go or
not.

> 
> > > The exclude_machine bit seems harmless to as no one will proccess
> > > it apart of riscv.  
> >
> > Armv8 has equivalent bits in PMEVTYPER<n>_EL0 and PMCCFILTR_EL0 for
> > EL-based filtering, which includes filtering EL3.
> >
> > Jess
> >  
> > > Patch depends on Atish Patra PMU series:
> > > https://lwn.net/Articles/879905/
> > > + some modifications to pass execlude event to SBI PMU extension
> > >
> > > Tested with qemu.
> > >
> > > Nikita Shubin (1):
> > >  perf tools: Add 'm' event modifier for counting machine
> > >
> > > include/uapi/linux/perf_event.h        |  3 ++-
> > > tools/include/uapi/linux/perf_event.h  |  3 ++-
> > > tools/perf/Documentation/perf-list.txt |  1 +
> > > tools/perf/tests/parse-events.c        | 18 ++++++++++++++++++
> > > tools/perf/util/evsel.c                |  4 +++-
> > > tools/perf/util/parse-events.c         | 18 +++++++++++++-----
> > > tools/perf/util/parse-events.l         |  2 +-
> > > 7 files changed, 40 insertions(+), 9 deletions(-)
> > >
> > > --
> > > 2.31.1
> > >
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv  
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv  
> 
> 
>