linux-kernel - Re: [PATCH] perf/x86/intel/uncore: fix IMC missing box initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 27 Apr 2015 23:23:43 -0700
From:	Stephane Eranian <eranian@...gle.com>
To:	"Liang, Kan" <kan.liang@...el.com>
Cc:	Andi Kleen <ak@...ux.intel.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Vince Weaver <vincent.weaver@...ne.edu>,
	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	"mingo@...e.hu" <mingo@...e.hu>, Sonny Rao <sonnyrao@...omium.org>
Subject: Re: [PATCH] perf/x86/intel/uncore: fix IMC missing box initialization

On Sun, Apr 26, 2015 at 8:43 PM, Liang, Kan <kan.liang@...el.com> wrote:
>
>>
>> > This leads me to believe that this patch:
>> >
>> > commit c05199e5a57a579fea1e8fa65e2b511ceb524ffc
>> > Author: Kan Liang <kan.liang@...el.com>
>> > Date:   Tue Jan 20 04:54:25 2015 +0000
>> >
>> >     perf/x86/intel/uncore: Move uncore_box_init() out of driver
>> initialization
>> >
>> > If I revert it, I bet things will work again.
>>
>> Yes the initialization needs to be moved out of the IPI context.
>>
>
> Maybe we can move them to event init, which is not in IPI context.
>
> What do you think of this patch?
>
> ---
>
> From 8a61c48144921e9d1c841656829c3bae9bfb4408 Mon Sep 17 00:00:00 2001
> From: Kan Liang <kan.liang@...el.com>
> Date: Sun, 26 Apr 2015 16:24:59 -0400
> Subject: [PATCH 1/1] perf/x86/intel/uncore: move uncore_box_init to uncore
>  event init
>
> commit c05199e5a57a("perf/x86/intel/uncore: Move uncore_box_init() out
> of driver initialization") moves uncore_box_init into uncore_enable_box
> to prevent potential boot failures. However, uncore_enable_box is not
> called on some client platforms (SNB/IVB/HSW) for counting IMC event.
> When it is not called, the box is not initialized, which hard locks the
> system.
>
> Additionally, uncore_enable_box along with the initialization code in it
> is always called in uncore event start functions, which are in IPI
> context. But the initizlization code should not be in IPI context. This
> is because, for example, the IMC box initialization codes for client
> platforms include ioremap, which is not allowed to be called in IPI
> context.
>
> This patch moves uncore_box_init out of IPI context, to uncore event
> init. The box is initialized only when it has not yet been initialized.
>
> Signed-off-by: Kan Liang <kan.liang@...el.com>

Ok this works for me now. Thanks.

Tested-by: Stephane Eranian <eranian@...gle.com>

> ---
>  arch/x86/kernel/cpu/perf_event_intel_uncore.c     | 4 ++++
>  arch/x86/kernel/cpu/perf_event_intel_uncore.h     | 2 --
>  arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c | 3 +++
>  3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index c635b8b..cbc1a93 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -623,6 +623,10 @@ static int uncore_pmu_event_init(struct perf_event *event)
>         box = uncore_pmu_to_box(pmu, event->cpu);
>         if (!box || box->cpu < 0)
>                 return -EINVAL;
> +
> +       /* Init box if it's not initialized yet */
> +       uncore_box_init(box);
> +
>         event->cpu = box->cpu;
>
>         event->hw.idx = -1;
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> index 6c8c1e7..1fb2905 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> @@ -273,8 +273,6 @@ static inline void uncore_disable_box(struct intel_uncore_box *box)
>
>  static inline void uncore_enable_box(struct intel_uncore_box *box)
>  {
> -       uncore_box_init(box);
> -
>         if (box->pmu->type->ops->enable_box)
>                 box->pmu->type->ops->enable_box(box);
>  }
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c b/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c
> index 4562e9e..ead70a6 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore_snb.c
> @@ -279,6 +279,9 @@ static int snb_uncore_imc_event_init(struct perf_event *event)
>         if (!box || box->cpu < 0)
>                 return -EINVAL;
>
> +       /* Init box if it's not initialized yet */
> +       uncore_box_init(box);
> +
>         event->cpu = box->cpu;
>
>         event->hw.idx = -1;
>
> Thanks,
> Kan
>
>> -Andi
>>
>>
>> --
>> ak@...ux.intel.com -- Speaking for myself only
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/