All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf regs x86: Fix arch__intr_reg_mask() for the hybrid platform
@ 2022-05-18 14:51 kan.liang
  2022-05-19  4:38 ` Ian Rogers
  0 siblings, 1 reply; 3+ messages in thread
From: kan.liang @ 2022-05-18 14:51 UTC (permalink / raw)
  To: acme, mingo, namhyung, jolsa, irogers, linux-kernel, linux-perf-users
  Cc: peterz, zhengjun.xing, Kan Liang, Ammy Yi

From: Kan Liang <kan.liang@linux.intel.com>

The X86 specific arch__intr_reg_mask() is to check whether the kernel
and hardware can collect XMM registers. But it doesn't work on some
hybrid platform.

Without the patch on ADL-N,

$perf record -I?
available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10
R11 R12 R13 R14 R15

The config of the test event doesn't contain the PMU information. The
kernel may fail to initialize it on the correct hybrid PMU and return
the wrong non-supported information.

Add the PMU information into the config for the hybrid platform. The
same register set is supported among different hybrid PMUs. Checking
the first available one is good enough.

With the patch on ADL-N,

$perf record -I?
available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10
R11 R12 R13 R14 R15 XMM0 XMM1 XMM2 XMM3 XMM4 XMM5 XMM6 XMM7 XMM8 XMM9
XMM10 XMM11 XMM12 XMM13 XMM14 XMM15

Fixes: 6466ec14aaf4 ("perf regs x86: Add X86 specific arch__intr_reg_mask()")
Reported-by: Ammy Yi <ammy.yi@intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 tools/perf/arch/x86/util/perf_regs.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index 207c56805c55..0ed177991ad0 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -9,6 +9,8 @@
 #include "../../../util/perf_regs.h"
 #include "../../../util/debug.h"
 #include "../../../util/event.h"
+#include "../../../util/pmu.h"
+#include "../../../util/pmu-hybrid.h"
 
 const struct sample_reg sample_reg_masks[] = {
 	SMPL_REG(AX, PERF_REG_X86_AX),
@@ -284,12 +286,22 @@ uint64_t arch__intr_reg_mask(void)
 		.disabled 		= 1,
 		.exclude_kernel		= 1,
 	};
+	struct perf_pmu *pmu;
 	int fd;
 	/*
 	 * In an unnamed union, init it here to build on older gcc versions
 	 */
 	attr.sample_period = 1;
 
+	if (perf_pmu__has_hybrid()) {
+		/*
+		 * The same register set is supported among different hybrid PMUs.
+		 * Only check the first available one.
+		 */
+		pmu = list_first_entry(&perf_pmu__hybrid_pmus, typeof(*pmu), hybrid_list);
+		attr.config |= (__u64)pmu->type << PERF_PMU_TYPE_SHIFT;
+	}
+
 	event_attr_init(&attr);
 
 	fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
-- 
2.35.1


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

* Re: [PATCH] perf regs x86: Fix arch__intr_reg_mask() for the hybrid platform
  2022-05-18 14:51 [PATCH] perf regs x86: Fix arch__intr_reg_mask() for the hybrid platform kan.liang
@ 2022-05-19  4:38 ` Ian Rogers
  2022-05-20 14:15   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2022-05-19  4:38 UTC (permalink / raw)
  To: kan.liang
  Cc: acme, mingo, namhyung, jolsa, linux-kernel, linux-perf-users,
	peterz, zhengjun.xing, Ammy Yi

On Wed, May 18, 2022 at 7:52 AM <kan.liang@linux.intel.com> wrote:
>
> From: Kan Liang <kan.liang@linux.intel.com>
>
> The X86 specific arch__intr_reg_mask() is to check whether the kernel
> and hardware can collect XMM registers. But it doesn't work on some
> hybrid platform.
>
> Without the patch on ADL-N,
>
> $perf record -I?
> available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10
> R11 R12 R13 R14 R15
>
> The config of the test event doesn't contain the PMU information. The
> kernel may fail to initialize it on the correct hybrid PMU and return
> the wrong non-supported information.
>
> Add the PMU information into the config for the hybrid platform. The
> same register set is supported among different hybrid PMUs. Checking
> the first available one is good enough.
>
> With the patch on ADL-N,
>
> $perf record -I?
> available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10
> R11 R12 R13 R14 R15 XMM0 XMM1 XMM2 XMM3 XMM4 XMM5 XMM6 XMM7 XMM8 XMM9
> XMM10 XMM11 XMM12 XMM13 XMM14 XMM15
>
> Fixes: 6466ec14aaf4 ("perf regs x86: Add X86 specific arch__intr_reg_mask()")
> Reported-by: Ammy Yi <ammy.yi@intel.com>
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>

Acked-by: Ian Rogers <irogers@google.com>

> ---
>  tools/perf/arch/x86/util/perf_regs.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
> index 207c56805c55..0ed177991ad0 100644
> --- a/tools/perf/arch/x86/util/perf_regs.c
> +++ b/tools/perf/arch/x86/util/perf_regs.c
> @@ -9,6 +9,8 @@
>  #include "../../../util/perf_regs.h"
>  #include "../../../util/debug.h"
>  #include "../../../util/event.h"
> +#include "../../../util/pmu.h"
> +#include "../../../util/pmu-hybrid.h"
>
>  const struct sample_reg sample_reg_masks[] = {
>         SMPL_REG(AX, PERF_REG_X86_AX),
> @@ -284,12 +286,22 @@ uint64_t arch__intr_reg_mask(void)
>                 .disabled               = 1,
>                 .exclude_kernel         = 1,
>         };
> +       struct perf_pmu *pmu;

nit: this could have smaller scope if just be declared in the if-block.

Thanks,
Ian

>         int fd;
>         /*
>          * In an unnamed union, init it here to build on older gcc versions
>          */
>         attr.sample_period = 1;
>
> +       if (perf_pmu__has_hybrid()) {
> +               /*
> +                * The same register set is supported among different hybrid PMUs.
> +                * Only check the first available one.
> +                */
> +               pmu = list_first_entry(&perf_pmu__hybrid_pmus, typeof(*pmu), hybrid_list);
> +               attr.config |= (__u64)pmu->type << PERF_PMU_TYPE_SHIFT;
> +       }
> +
>         event_attr_init(&attr);
>
>         fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
> --
> 2.35.1
>

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

* Re: [PATCH] perf regs x86: Fix arch__intr_reg_mask() for the hybrid platform
  2022-05-19  4:38 ` Ian Rogers
@ 2022-05-20 14:15   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-05-20 14:15 UTC (permalink / raw)
  To: Ian Rogers
  Cc: kan.liang, mingo, namhyung, jolsa, linux-kernel,
	linux-perf-users, peterz, zhengjun.xing, Ammy Yi

Em Wed, May 18, 2022 at 09:38:20PM -0700, Ian Rogers escreveu:
> On Wed, May 18, 2022 at 7:52 AM <kan.liang@linux.intel.com> wrote:
> >
> > From: Kan Liang <kan.liang@linux.intel.com>
> >
> > The X86 specific arch__intr_reg_mask() is to check whether the kernel
> > and hardware can collect XMM registers. But it doesn't work on some
> > hybrid platform.
> >
> > Without the patch on ADL-N,
> >
> > $perf record -I?
> > available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10
> > R11 R12 R13 R14 R15
> >
> > The config of the test event doesn't contain the PMU information. The
> > kernel may fail to initialize it on the correct hybrid PMU and return
> > the wrong non-supported information.
> >
> > Add the PMU information into the config for the hybrid platform. The
> > same register set is supported among different hybrid PMUs. Checking
> > the first available one is good enough.
> >
> > With the patch on ADL-N,
> >
> > $perf record -I?
> > available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10
> > R11 R12 R13 R14 R15 XMM0 XMM1 XMM2 XMM3 XMM4 XMM5 XMM6 XMM7 XMM8 XMM9
> > XMM10 XMM11 XMM12 XMM13 XMM14 XMM15
> >
> > Fixes: 6466ec14aaf4 ("perf regs x86: Add X86 specific arch__intr_reg_mask()")
> > Reported-by: Ammy Yi <ammy.yi@intel.com>
> > Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> 
> Acked-by: Ian Rogers <irogers@google.com>

Thanks, applied.

- Arnaldo

 
> > ---
> >  tools/perf/arch/x86/util/perf_regs.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
> > index 207c56805c55..0ed177991ad0 100644
> > --- a/tools/perf/arch/x86/util/perf_regs.c
> > +++ b/tools/perf/arch/x86/util/perf_regs.c
> > @@ -9,6 +9,8 @@
> >  #include "../../../util/perf_regs.h"
> >  #include "../../../util/debug.h"
> >  #include "../../../util/event.h"
> > +#include "../../../util/pmu.h"
> > +#include "../../../util/pmu-hybrid.h"
> >
> >  const struct sample_reg sample_reg_masks[] = {
> >         SMPL_REG(AX, PERF_REG_X86_AX),
> > @@ -284,12 +286,22 @@ uint64_t arch__intr_reg_mask(void)
> >                 .disabled               = 1,
> >                 .exclude_kernel         = 1,
> >         };
> > +       struct perf_pmu *pmu;
> 
> nit: this could have smaller scope if just be declared in the if-block.
> 
> Thanks,
> Ian
> 
> >         int fd;
> >         /*
> >          * In an unnamed union, init it here to build on older gcc versions
> >          */
> >         attr.sample_period = 1;
> >
> > +       if (perf_pmu__has_hybrid()) {
> > +               /*
> > +                * The same register set is supported among different hybrid PMUs.
> > +                * Only check the first available one.
> > +                */
> > +               pmu = list_first_entry(&perf_pmu__hybrid_pmus, typeof(*pmu), hybrid_list);
> > +               attr.config |= (__u64)pmu->type << PERF_PMU_TYPE_SHIFT;
> > +       }
> > +
> >         event_attr_init(&attr);
> >
> >         fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
> > --
> > 2.35.1
> >

-- 

- Arnaldo

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

end of thread, other threads:[~2022-05-20 14:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-18 14:51 [PATCH] perf regs x86: Fix arch__intr_reg_mask() for the hybrid platform kan.liang
2022-05-19  4:38 ` Ian Rogers
2022-05-20 14:15   ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.