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 03:43:32 +0000
From:	"Liang, Kan" <kan.liang@...el.com>
To:	Andi Kleen <ak@...ux.intel.com>,
	Stephane Eranian <eranian@...gle.com>
CC:	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


>
> > 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>
---
 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/