[PATCH] Fix oprofile on hyperthreaded P4's

From: Philippe Elie <phil.el@wanadoo.fr>

- oprofile is currently only profiling one sibling.  Fix that with
  appropriate register settings.

- fix an oops which could occur if the userspace driver were to request a
  non-existent resource.

- in NMI handler counter_config[i].event is accessible from user space so
  user can change the event during profiling by echo xxx >
  /dev/oprofile/event

- event mask was wrong, the bit field is 6 bits length not 5, events
  SSE_INPUT_ASSIST and X87_SIMD_MOVES_UOP was affected by masking high bit of
  event number.

arch/i386/oprofile/op_model_p4.c

@@ -31,7 +31,6 @@
 #define NUM_CONTROLS_HT2 (NUM_ESCRS_HT2 + NUM_CCCRS_HT2)
 
 static unsigned int num_counters = NUM_COUNTERS_NON_HT;
-static unsigned int num_cccrs = NUM_CCCRS_NON_HT;
 
 
 /* this has to be checked dynamically since the
@@ -40,10 +39,17 @@ static unsigned int num_cccrs = NUM_CCCRS_NON_HT;
 static inline void setup_num_counters(void)
 {
 #ifdef CONFIG_SMP
-	if (smp_num_siblings == 2) {		
+	if (smp_num_siblings == 2)
 		num_counters = NUM_COUNTERS_HT2;
-		num_cccrs = NUM_CCCRS_HT2;
-	}
+#endif
+}
+
+static int inline addr_increment(void)
+{
+#ifdef CONFIG_SMP
+	return smp_num_siblings == 2 ? 2 : 1;
+#else
+	return 1;
 #endif
 }
 
@@ -65,7 +71,7 @@ struct p4_event_binding {
 };
 
 /* nb: these CTR_* defines are a duplicate of defines in
-   libop/op_events.c. */
+   event/i386.p4*events. */
 
 
 #define CTR_BPU_0      (1 << 0)
@@ -88,6 +94,17 @@ static struct p4_counter_binding p4_counters [NUM_COUNTERS_NON_HT] = {
 	{ CTR_IQ_5,    MSR_P4_IQ_PERFCTR5,    MSR_P4_IQ_CCCR5 }
 };
 
+#define NUM_UNUSED_CCCRS	NUM_CCCRS_NON_HT - NUM_COUNTERS_NON_HT
+
+/* All cccr we don't use. */
+static int p4_unused_cccr[NUM_UNUSED_CCCRS] = {
+	MSR_P4_BPU_CCCR1,	MSR_P4_BPU_CCCR3,
+	MSR_P4_MS_CCCR1,	MSR_P4_MS_CCCR3,
+	MSR_P4_FLAME_CCCR1,	MSR_P4_FLAME_CCCR3,
+	MSR_P4_IQ_CCCR0,	MSR_P4_IQ_CCCR1,
+	MSR_P4_IQ_CCCR2,	MSR_P4_IQ_CCCR3
+};
+
 /* p4 event codes in libop/op_event.h are indices into this table. */
 
 static struct p4_event_binding p4_events[NUM_EVENTS] = {
@@ -167,13 +184,13 @@ static struct p4_event_binding p4_events[NUM_EVENTS] = {
 	{ /* IOQ_ALLOCATION */
 		0x06, 0x03, 
 		{ { CTR_BPU_0, MSR_P4_FSB_ESCR0},
-		  {-1,-1} }
+		  { 0, 0 } }
 	},
 
 	{ /* IOQ_ACTIVE_ENTRIES */
 		0x06, 0x1a, 
 		{ { CTR_BPU_2, MSR_P4_FSB_ESCR1},
-		  {-1,-1} }
+		  { 0, 0 } }
 	},
 
 	{ /* FSB_DATA_ACTIVITY */
@@ -185,13 +202,13 @@ static struct p4_event_binding p4_events[NUM_EVENTS] = {
 	{ /* BSQ_ALLOCATION */
 		0x07, 0x05, 
 		{ { CTR_BPU_0, MSR_P4_BSU_ESCR0},
-		  {-1,-1} }
+		  { 0, 0 } }
 	},
 
 	{ /* BSQ_ACTIVE_ENTRIES */
 		0x07, 0x06,
 		{ { CTR_BPU_2, MSR_P4_BSU_ESCR1 /* guess */},  
-		  {-1,-1} }
+		  { 0, 0 } }
 	},
 
 	{ /* X87_ASSIST */
@@ -336,7 +353,7 @@ static struct p4_event_binding p4_events[NUM_EVENTS] = {
 #define ESCR_SET_OS_0(escr, os) ((escr) |= (((os) & 1) << 3))
 #define ESCR_SET_USR_1(escr, usr) ((escr) |= (((usr) & 1)))
 #define ESCR_SET_OS_1(escr, os) ((escr) |= (((os) & 1) << 1))
-#define ESCR_SET_EVENT_SELECT(escr, sel) ((escr) |= (((sel) & 0x1f) << 25))
+#define ESCR_SET_EVENT_SELECT(escr, sel) ((escr) |= (((sel) & 0x3f) << 25))
 #define ESCR_SET_EVENT_MASK(escr, mask) ((escr) |= (((mask) & 0xffff) << 9))
 #define ESCR_READ(escr,high,ev,i) do {rdmsr(ev->bindings[(i)].escr_address, (escr), (high));} while (0);
 #define ESCR_WRITE(escr,high,ev,i) do {wrmsr(ev->bindings[(i)].escr_address, (escr), (high));} while (0);
@@ -358,15 +375,11 @@ static struct p4_event_binding p4_events[NUM_EVENTS] = {
 #define CTR_WRITE(l,i) do {wrmsr(p4_counters[(i)].counter_address, -(u32)(l), -1);} while (0);
 #define CTR_OVERFLOW_P(ctr) (!((ctr) & 0x80000000))
 
-/* these access the underlying cccrs 1-18, not the subset of 8 bound to "virtual counters" */
-#define RAW_CCCR_READ(low, high, i) do {rdmsr (MSR_P4_BPU_CCCR0 + (i), (low), (high));} while (0);
-#define RAW_CCCR_WRITE(low, high, i) do {wrmsr (MSR_P4_BPU_CCCR0 + (i), (low), (high));} while (0);
-
 
 /* this assigns a "stagger" to the current CPU, which is used throughout
    the code in this module as an extra array offset, to select the "even"
    or "odd" part of all the divided resources. */
-static inline unsigned int get_stagger(void)
+static unsigned int get_stagger(void)
 {
 #ifdef CONFIG_SMP
 	int cpu;
@@ -395,29 +408,33 @@ static void p4_fill_in_addresses(struct op_msrs * const msrs)
 	setup_num_counters();
 	stag = get_stagger();
 
-	/* the 8 counter registers we pay attention to */
-	for (i = 0; i < num_counters; ++i)
+	/* the counter registers we pay attention to */
+	for (i = 0; i < num_counters; ++i) {
 		msrs->counters.addrs[i] = 
 			p4_counters[VIRT_CTR(stag, i)].counter_address;
+	}
+
+	/* FIXME: bad feeling, we don't save the 10 counters we don't use. */
 
 	/* 18 CCCR registers */
-	for (i=stag, addr = MSR_P4_BPU_CCCR0;
-	     addr <= MSR_P4_IQ_CCCR5; ++i, addr += (1 + stag)) 
+	for (i = 0, addr = MSR_P4_BPU_CCCR0 + stag;
+	     addr <= MSR_P4_IQ_CCCR5; ++i, addr += addr_increment()) {
 		msrs->controls.addrs[i] = addr;
+	}
 	
-	/* 43 ESCR registers */
-	for (addr = MSR_P4_BSU_ESCR0;
-	     addr <= MSR_P4_SSU_ESCR0; ++i, addr += (1 + stag)){ 
+	/* 43 ESCR registers in three discontiguous group */
+	for (addr = MSR_P4_BSU_ESCR0 + stag;
+	     addr <= MSR_P4_SSU_ESCR0; ++i, addr += addr_increment()) { 
 		msrs->controls.addrs[i] = addr;
 	}
 	
-	for (addr = MSR_P4_MS_ESCR0;
-	     addr <= MSR_P4_TC_ESCR1; ++i, addr += (1 + stag)){ 
+	for (addr = MSR_P4_MS_ESCR0 + stag;
+	     addr <= MSR_P4_TC_ESCR1; ++i, addr += addr_increment()) { 
 		msrs->controls.addrs[i] = addr;
 	}
 	
-	for (addr = MSR_P4_IX_ESCR0;
-	     addr <= MSR_P4_CRU_ESCR3; ++i, addr += (1 + stag)){ 
+	for (addr = MSR_P4_IX_ESCR0 + stag;
+	     addr <= MSR_P4_CRU_ESCR3; ++i, addr += addr_increment()) { 
 		msrs->controls.addrs[i] = addr;
 	}
 
@@ -435,7 +452,7 @@ static void p4_fill_in_addresses(struct op_msrs * const msrs)
 
 	} else {
 		/* and two copies of the second to the odd thread,
-		   for the 31st and 32nd control registers */
+		   for the 22st and 23nd control registers */
 		msrs->controls.addrs[i++] = MSR_P4_CRU_ESCR5;
 		msrs->controls.addrs[i++] = MSR_P4_CRU_ESCR5;
 	}
@@ -456,7 +473,7 @@ static void pmc_setup_one_p4_counter(unsigned int ctr)
 	stag = get_stagger();
 	
 	/* convert from counter *number* to counter *bit* */
-	counter_bit = 1 << ctr;
+	counter_bit = 1 << VIRT_CTR(stag, ctr);
 	
 	/* find our event binding structure. */
 	if (counter_config[ctr].event <= 0 || counter_config[ctr].event > NUM_EVENTS) {
@@ -470,7 +487,7 @@ static void pmc_setup_one_p4_counter(unsigned int ctr)
 	
 	for (i = 0; i < maxbind; i++) {
 		if (ev->bindings[i].virt_counter & counter_bit) {
-			
+
 			/* modify ESCR */
 			ESCR_READ(escr, high, ev, i);
 			ESCR_CLEAR(escr);
@@ -499,6 +516,10 @@ static void pmc_setup_one_p4_counter(unsigned int ctr)
 			return;
 		}
 	}
+
+	printk(KERN_ERR 
+	       "oprofile: P4 event code 0x%lx no binding, stag %d ctr %d\n",
+	       counter_config[ctr].event, stag, ctr);
 }
 
 
@@ -517,27 +538,35 @@ static void p4_setup_ctrs(struct op_msrs const * const msrs)
 		return;
 	}
 
-	/* clear all cccrs (including those outside our concern) */
-	for (i = stag ; i < num_cccrs ; i += (1 + stag)) {
-		RAW_CCCR_READ(low, high, i);
+	/* clear the cccrs we will use */
+	for (i = 0 ; i < num_counters ; i++) {
+		rdmsr(p4_counters[VIRT_CTR(stag, i)].cccr_address, low, high);
+		CCCR_CLEAR(low);
+		CCCR_SET_REQUIRED_BITS(low);
+		wrmsr(p4_counters[VIRT_CTR(stag, i)].cccr_address, low, high);
+	}
+
+	/* clear cccrs outside our concern */
+	for (i = stag ; i < NUM_UNUSED_CCCRS ; i += addr_increment()) {
+		rdmsr(p4_unused_cccr[i], low, high);
 		CCCR_CLEAR(low);
 		CCCR_SET_REQUIRED_BITS(low);
-		RAW_CCCR_WRITE(low, high, i);
+		wrmsr(p4_unused_cccr[i], low, high);
 	}
 
-	/* clear all escrs (including those outside out concern) */
+	/* clear all escrs (including those outside our concern) */
 	for (addr = MSR_P4_BSU_ESCR0 + stag;
-	     addr <= MSR_P4_SSU_ESCR0; addr += (1 + stag)){ 
+	     addr <= MSR_P4_SSU_ESCR0; addr += addr_increment()) { 
 		wrmsr(addr, 0, 0);
 	}
 	
 	for (addr = MSR_P4_MS_ESCR0 + stag;
-	     addr <= MSR_P4_TC_ESCR1; addr += (1 + stag)){ 
+	     addr <= MSR_P4_TC_ESCR1; addr += addr_increment()){ 
 		wrmsr(addr, 0, 0);
 	}
 	
 	for (addr = MSR_P4_IX_ESCR0 + stag;
-	     addr <= MSR_P4_CRU_ESCR3; addr += (1 + stag)){ 
+	     addr <= MSR_P4_CRU_ESCR3; addr += addr_increment()){ 
 		wrmsr(addr, 0, 0);
 	}
 
@@ -576,7 +605,7 @@ static int p4_check_ctrs(unsigned int const cpu,
 
 	for (i = 0; i < num_counters; ++i) {
 		
-		if (!counter_config[i].event) 
+		if (!reset_value[i]) 
 			continue;
 
 		/* 
@@ -606,8 +635,6 @@ static int p4_check_ctrs(unsigned int const cpu,
 			CCCR_CLEAR_OVF(low);
 			CCCR_WRITE(low, high, real);
  			CTR_WRITE(reset_value[i], real);
-			/* P4 quirk: you have to re-unmask the apic vector */
-			apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);
 		}
 	}