linux-kernel - [PATCH] oprofile, x86: Add comments to IBS LVT offset 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20110530144324.GM20052@erda.amd.com>
Date:	Mon, 30 May 2011 16:43:24 +0200
From:	Robert Richter <robert.richter@....com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"oprofile-list@...ts.sourceforge.net" 
	<oprofile-list@...ts.sourceforge.net>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: [PATCH] oprofile, x86: Add comments to IBS LVT offset initialization

(resent with modified subject)

On 30.05.11 12:16:19, Robert Richter wrote:
> On 30.05.11 06:09:56, Ingo Molnar wrote:
> > The way IBS is set up is not very obvious and atypical of existing 
> > preempt_*() patters. Might be worth adding a comment to the 
> > preempt_disable(), to unconfuse unsuspecting readers who are 
> > wondering about its oddness?
> 
> Yes, will add some comments.

Ingo,

please see below and apply the patch to tip, if it makes sense to you.

Thanks,

-Robert



>From cbf74cea070fa1f705de4712e25d9e56ae6543c7 Mon Sep 17 00:00:00 2001
From: Robert Richter <robert.richter@....com>
Date: Mon, 30 May 2011 16:31:11 +0200
Subject: [PATCH] oprofile, x86: Add comments to IBS LVT offset initialization

Adding a comment in the code as IBS LVT setup is not obvious at all ...

Signed-off-by: Robert Richter <robert.richter@....com>
---
 arch/x86/kernel/apic/apic.c      |    3 ++-
 arch/x86/oprofile/op_model_amd.c |   13 +++++++++----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index fabf01e..a0bf78a 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -390,7 +390,8 @@ static unsigned int reserve_eilvt_offset(int offset, unsigned int new)
 
 /*
  * If mask=1, the LVT entry does not generate interrupts while mask=0
- * enables the vector. See also the BKDGs.
+ * enables the vector. See also the BKDGs. Must be called with
+ * preemption disabled.
  */
 
 int setup_APIC_eilvt(u8 offset, u8 vector, u8 msg_type, u8 mask)
diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
index 9fd8a56..9cbb710 100644
--- a/arch/x86/oprofile/op_model_amd.c
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -609,16 +609,21 @@ static int setup_ibs_ctl(int ibs_eilvt_off)
 	return 0;
 }
 
+/*
+ * This runs only on the current cpu. We try to find an LVT offset and
+ * setup the local APIC. For this we must disable preemption. On
+ * success we initialize all nodes with this offset. This updates then
+ * the offset in the IBS_CTL per-node msr. The per-core APIC setup of
+ * the IBS interrupt vector is called from op_amd_setup_ctrs()/op_-
+ * amd_cpu_shutdown() using the new offset.
+ */
 static int force_ibs_eilvt_setup(void)
 {
 	int offset;
 	int ret;
 
-	/*
-	 * find the next free available EILVT entry, skip offset 0,
-	 * pin search to this cpu
-	 */
 	preempt_disable();
+	/* find the next free available EILVT entry, skip offset 0 */
 	for (offset = 1; offset < APIC_EILVT_NR_MAX; offset++) {
 		if (get_eilvt(offset))
 			break;
-- 
1.7.5.rc3



-- 
Advanced Micro Devices, Inc.
Operating System Research Center

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