[PATCH] x86 oprofile: use rdmsrl/wrmsrl

[PATCH] x86 oprofile: use rdmsrl/wrmsrl

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]


Hi!

Attached patch makes x86 oprofile code use rdmsrl/wrmsrl

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_oprofile.diff --]
[-- Type: text/x-diff, Size: 19789 bytes --]

diff -r 7b00193bd033 xen/arch/x86/oprofile/op_model_athlon.c
--- a/xen/arch/x86/oprofile/op_model_athlon.c	Mon Jun 28 17:40:16 2010 +0100
+++ b/xen/arch/x86/oprofile/op_model_athlon.c	Tue Jun 29 17:45:20 2010 +0200
@@ -26,23 +26,22 @@
 #define NUM_COUNTERS 4
 #define NUM_CONTROLS 4
 
-#define CTR_READ(l,h,msrs,c) do {rdmsr(msrs->counters[(c)].addr, (l), (h));} while (0)
+#define CTR_READ(msr_content,msrs,c) do {rdmsrl(msrs->counters[(c)].addr, (msr_content));} while (0)
 #define CTR_WRITE(l,msrs,c) do {wrmsr(msrs->counters[(c)].addr, -(unsigned int)(l), -1);} while (0)
-#define CTR_OVERFLOWED(n) (!((n) & (1U<<31)))
+#define CTR_OVERFLOWED(n) (!((n) & (1ULL<<31)))
 
-#define CTRL_READ(l,h,msrs,c) do {rdmsr(msrs->controls[(c)].addr, (l), (h));} while (0)
-#define CTRL_WRITE(l,h,msrs,c) do {wrmsr(msrs->controls[(c)].addr, (l), (h));} while (0)
-#define CTRL_SET_ACTIVE(n) (n |= (1<<22))
-#define CTRL_SET_INACTIVE(n) (n &= ~(1<<22))
-#define CTRL_CLEAR(lo, hi) (lo &= (1<<21), hi = 0)
-#define CTRL_SET_ENABLE(val) (val |= 1<<20)
+#define CTRL_READ(msr_content,msrs,c) do {rdmsrl(msrs->controls[(c)].addr, (msr_content));} while (0)
+#define CTRL_WRITE(msr_content,msrs,c) do {wrmsrl(msrs->controls[(c)].addr, (msr_content));} while (0)
+#define CTRL_SET_ACTIVE(n) (n |= (1ULL<<22))
+#define CTRL_SET_INACTIVE(n) (n &= ~(1ULL<<22))
+#define CTRL_CLEAR(val) (val &= (1ULL<<21))
+#define CTRL_SET_ENABLE(val) (val |= 1ULL<<20)
 #define CTRL_SET_USR(val,u) (val |= ((u & 1) << 16))
 #define CTRL_SET_KERN(val,k) (val |= ((k & 1) << 17))
 #define CTRL_SET_UM(val, m) (val |= ((m & 0xff) << 8))
-#define CTRL_SET_EVENT_LOW(val, e) (val |= (e & 0xff))
-#define CTRL_SET_EVENT_HIGH(val, e) (val |= ((e >> 8) & 0xf))
-#define CTRL_SET_HOST_ONLY(val, h) (val |= ((h & 1) << 9))
-#define CTRL_SET_GUEST_ONLY(val, h) (val |= ((h & 1) << 8))
+#define CTRL_SET_EVENT(val, e) (val |= (((e >> 8) & 0xf) | (e & 0xff)))
+#define CTRL_SET_HOST_ONLY(val, h) (val |= ((h & 0x1ULL) << 41))
+#define CTRL_SET_GUEST_ONLY(val, h) (val |= ((h & 0x1ULL) << 40))
 
 static unsigned long reset_value[NUM_COUNTERS];
 
@@ -64,14 +63,14 @@ static void athlon_fill_in_addresses(str
  
 static void athlon_setup_ctrs(struct op_msrs const * const msrs)
 {
-	unsigned int low, high;
+	uint64_t msr_content;
 	int i;
  
 	/* clear all counters */
 	for (i = 0 ; i < NUM_CONTROLS; ++i) {
-		CTRL_READ(low, high, msrs, i);
-		CTRL_CLEAR(low, high);
-		CTRL_WRITE(low, high, msrs, i);
+		CTRL_READ(msr_content, msrs, i);
+		CTRL_CLEAR(msr_content);
+		CTRL_WRITE(msr_content, msrs, i);
 	}
 	
 	/* avoid a false detection of ctr overflows in NMI handler */
@@ -86,17 +85,16 @@ static void athlon_setup_ctrs(struct op_
 
 			CTR_WRITE(counter_config[i].count, msrs, i);
 
-			CTRL_READ(low, high, msrs, i);
-			CTRL_CLEAR(low, high);
-			CTRL_SET_ENABLE(low);
-			CTRL_SET_USR(low, counter_config[i].user);
-			CTRL_SET_KERN(low, counter_config[i].kernel);
-			CTRL_SET_UM(low, counter_config[i].unit_mask);
-			CTRL_SET_EVENT_LOW(low, counter_config[i].event);
-			CTRL_SET_EVENT_HIGH(high, counter_config[i].event);
-			CTRL_SET_HOST_ONLY(high, 0);
-			CTRL_SET_GUEST_ONLY(high, 0);
-			CTRL_WRITE(low, high, msrs, i);
+			CTRL_READ(msr_content, msrs, i);
+			CTRL_CLEAR(msr_content);
+			CTRL_SET_ENABLE(msr_content);
+			CTRL_SET_USR(msr_content, counter_config[i].user);
+			CTRL_SET_KERN(msr_content, counter_config[i].kernel);
+			CTRL_SET_UM(msr_content, counter_config[i].unit_mask);
+			CTRL_SET_EVENT(msr_content, counter_config[i].event);
+			CTRL_SET_HOST_ONLY(msr_content, 0);
+			CTRL_SET_GUEST_ONLY(msr_content, 0);
+			CTRL_WRITE(msr_content, msrs, i);
 		} else {
 			reset_value[i] = 0;
 		}
@@ -108,7 +106,7 @@ static int athlon_check_ctrs(unsigned in
 			     struct cpu_user_regs * const regs)
 
 {
-	unsigned int low, high;
+	uint64_t msr_content;
 	int i;
 	int ovf = 0;
 	unsigned long eip = regs->eip;
@@ -128,8 +126,8 @@ static int athlon_check_ctrs(unsigned in
 	}
 
 	for (i = 0 ; i < NUM_COUNTERS; ++i) {
-		CTR_READ(low, high, msrs, i);
-		if (CTR_OVERFLOWED(low)) {
+		CTR_READ(msr_content, msrs, i);
+		if (CTR_OVERFLOWED(msr_content)) {
 			xenoprof_log_event(current, regs, eip, mode, i);
 			CTR_WRITE(reset_value[i], msrs, i);
 			ovf = 1;
@@ -143,13 +141,13 @@ static int athlon_check_ctrs(unsigned in
  
 static void athlon_start(struct op_msrs const * const msrs)
 {
-	unsigned int low, high;
+	uint64_t msr_content;
 	int i;
 	for (i = 0 ; i < NUM_COUNTERS ; ++i) {
 		if (reset_value[i]) {
-			CTRL_READ(low, high, msrs, i);
-			CTRL_SET_ACTIVE(low);
-			CTRL_WRITE(low, high, msrs, i);
+			CTRL_READ(msr_content, msrs, i);
+			CTRL_SET_ACTIVE(msr_content);
+			CTRL_WRITE(msr_content, msrs, i);
 		}
 	}
 }
@@ -157,15 +155,15 @@ static void athlon_start(struct op_msrs 
 
 static void athlon_stop(struct op_msrs const * const msrs)
 {
-	unsigned int low,high;
+	uint64_t msr_content;
 	int i;
 
 	/* Subtle: stop on all counters to avoid race with
 	 * setting our pm callback */
 	for (i = 0 ; i < NUM_COUNTERS ; ++i) {
-		CTRL_READ(low, high, msrs, i);
-		CTRL_SET_INACTIVE(low);
-		CTRL_WRITE(low, high, msrs, i);
+		CTRL_READ(msr_content, msrs, i);
+		CTRL_SET_INACTIVE(msr_content);
+		CTRL_WRITE(msr_content, msrs, i);
 	}
 }
 
diff -r 7b00193bd033 xen/arch/x86/oprofile/op_model_p4.c
--- a/xen/arch/x86/oprofile/op_model_p4.c	Mon Jun 28 17:40:16 2010 +0100
+++ b/xen/arch/x86/oprofile/op_model_p4.c	Tue Jun 29 17:45:20 2010 +0200
@@ -347,35 +347,35 @@ static const struct p4_event_binding p4_
 };
 
 
-#define MISC_PMC_ENABLED_P(x) ((x) & 1 << 7)
+#define MISC_PMC_ENABLED_P(x) ((x) & 1ULL << 7)
 
-#define ESCR_RESERVED_BITS 0x80000003
+#define ESCR_RESERVED_BITS 0x80000003ULL
 #define ESCR_CLEAR(escr) ((escr) &= ESCR_RESERVED_BITS)
-#define ESCR_SET_USR_0(escr, usr) ((escr) |= (((usr) & 1) << 2))
-#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) & 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)
+#define ESCR_SET_USR_0(escr, usr) ((escr) |= (((usr) & 1ULL) << 2))
+#define ESCR_SET_OS_0(escr, os) ((escr) |= (((os) & 1ULL) << 3))
+#define ESCR_SET_USR_1(escr, usr) ((escr) |= (((usr) & 1ULL)))
+#define ESCR_SET_OS_1(escr, os) ((escr) |= (((os) & 1ULL) << 1))
+#define ESCR_SET_EVENT_SELECT(escr, sel) ((escr) |= (((sel) & 0x3fULL) << 25))
+#define ESCR_SET_EVENT_MASK(escr, mask) ((escr) |= (((mask) & 0xffffULL) << 9))
+#define ESCR_READ(escr,ev,i) do {rdmsrl(ev->bindings[(i)].escr_address, (escr));} while (0)
+#define ESCR_WRITE(escr,ev,i) do {wrmsrl(ev->bindings[(i)].escr_address, (escr));} while (0)
 
-#define CCCR_RESERVED_BITS 0x38030FFF
+#define CCCR_RESERVED_BITS 0x38030FFFULL
 #define CCCR_CLEAR(cccr) ((cccr) &= CCCR_RESERVED_BITS)
-#define CCCR_SET_REQUIRED_BITS(cccr) ((cccr) |= 0x00030000)
-#define CCCR_SET_ESCR_SELECT(cccr, sel) ((cccr) |= (((sel) & 0x07) << 13))
-#define CCCR_SET_PMI_OVF_0(cccr) ((cccr) |= (1<<26))
-#define CCCR_SET_PMI_OVF_1(cccr) ((cccr) |= (1<<27))
-#define CCCR_SET_ENABLE(cccr) ((cccr) |= (1<<12))
-#define CCCR_SET_DISABLE(cccr) ((cccr) &= ~(1<<12))
-#define CCCR_READ(low, high, i) do {rdmsr(p4_counters[(i)].cccr_address, (low), (high));} while (0)
-#define CCCR_WRITE(low, high, i) do {wrmsr(p4_counters[(i)].cccr_address, (low), (high));} while (0)
-#define CCCR_OVF_P(cccr) ((cccr) & (1U<<31))
-#define CCCR_CLEAR_OVF(cccr) ((cccr) &= (~(1U<<31)))
+#define CCCR_SET_REQUIRED_BITS(cccr) ((cccr) |= 0x00030000ULL)
+#define CCCR_SET_ESCR_SELECT(cccr, sel) ((cccr) |= (((sel) & 0x07ULL) << 13))
+#define CCCR_SET_PMI_OVF_0(cccr) ((cccr) |= (1ULL<<26))
+#define CCCR_SET_PMI_OVF_1(cccr) ((cccr) |= (1ULL<<27))
+#define CCCR_SET_ENABLE(cccr) ((cccr) |= (1ULL<<12))
+#define CCCR_SET_DISABLE(cccr) ((cccr) &= ~(1ULL<<12))
+#define CCCR_READ(msr_content, i) do {rdmsrl(p4_counters[(i)].cccr_address, (msr_content));} while (0)
+#define CCCR_WRITE(msr_content, i) do {wrmsrl(p4_counters[(i)].cccr_address, (msr_content));} while (0)
+#define CCCR_OVF_P(cccr) ((cccr) & (1ULL<<31))
+#define CCCR_CLEAR_OVF(cccr) ((cccr) &= (~(1ULL<<31)))
 
-#define CTR_READ(l,h,i) do {rdmsr(p4_counters[(i)].counter_address, (l), (h));} while (0)
-#define CTR_WRITE(l,i) do {wrmsr(p4_counters[(i)].counter_address, -(u32)(l), -1);} while (0)
-#define CTR_OVERFLOW_P(ctr) (!((ctr) & 0x80000000))
+#define CTR_READ(msr_content,i) do {rdmsrl(p4_counters[(i)].counter_address, (msr_content));} while (0)
+#define CTR_WRITE(msr_content,i) do {wrmsrl(p4_counters[(i)].counter_address, -(msr_content));} while (0)
+#define CTR_OVERFLOW_P(ctr) (!((ctr) & 0x80000000ULL))
 
 
 /* this assigns a "stagger" to the current CPU, which is used throughout
@@ -481,9 +481,8 @@ static void pmc_setup_one_p4_counter(uns
 {
 	int i;
 	int const maxbind = 2;
-	unsigned int cccr = 0;
-	unsigned int escr = 0;
-	unsigned int high = 0;
+	uint64_t cccr = 0;
+	uint64_t escr = 0;
 	unsigned int counter_bit;
 	const struct p4_event_binding *ev = NULL;
 	unsigned int stag;
@@ -507,7 +506,7 @@ static void pmc_setup_one_p4_counter(uns
 		if (ev->bindings[i].virt_counter & counter_bit) {
 
 			/* modify ESCR */
-			ESCR_READ(escr, high, ev, i);
+			ESCR_READ(escr, ev, i);
 			ESCR_CLEAR(escr);
 			if (stag == 0) {
 				ESCR_SET_USR_0(escr, counter_config[ctr].user);
@@ -518,10 +517,10 @@ static void pmc_setup_one_p4_counter(uns
 			}
 			ESCR_SET_EVENT_SELECT(escr, ev->event_select);
 			ESCR_SET_EVENT_MASK(escr, counter_config[ctr].unit_mask);			
-			ESCR_WRITE(escr, high, ev, i);
+			ESCR_WRITE(escr, ev, i);
 		       
 			/* modify CCCR */
-			CCCR_READ(cccr, high, VIRT_CTR(stag, ctr));
+			CCCR_READ(cccr, VIRT_CTR(stag, ctr));
 			CCCR_CLEAR(cccr);
 			CCCR_SET_REQUIRED_BITS(cccr);
 			CCCR_SET_ESCR_SELECT(cccr, ev->escr_select);
@@ -530,7 +529,7 @@ static void pmc_setup_one_p4_counter(uns
 			} else {
 				CCCR_SET_PMI_OVF_1(cccr);
 			}
-			CCCR_WRITE(cccr, high, VIRT_CTR(stag, ctr));
+			CCCR_WRITE(cccr, VIRT_CTR(stag, ctr));
 			return;
 		}
 	}
@@ -544,68 +543,68 @@ static void pmc_setup_one_p4_counter(uns
 static void p4_setup_ctrs(struct op_msrs const * const msrs)
 {
 	unsigned int i;
-	unsigned int low, high;
+	uint64_t msr_content;
 	unsigned int addr;
 	unsigned int stag;
 
 	stag = get_stagger();
 
-	rdmsr(MSR_IA32_MISC_ENABLE, low, high);
-	if (! MISC_PMC_ENABLED_P(low)) {
+	rdmsrl(MSR_IA32_MISC_ENABLE, msr_content);
+	if (! MISC_PMC_ENABLED_P(msr_content)) {
 		printk(KERN_ERR "oprofile: P4 PMC not available\n");
 		return;
 	}
 
 	/* 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);
+		rdmsrl(p4_counters[VIRT_CTR(stag, i)].cccr_address, msr_content);
+		CCCR_CLEAR(msr_content);
+		CCCR_SET_REQUIRED_BITS(msr_content);
+		wrmsrl(p4_counters[VIRT_CTR(stag, i)].cccr_address, msr_content);
 	}
 
 	/* 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);
-		wrmsr(p4_unused_cccr[i], low, high);
+		rdmsrl(p4_unused_cccr[i], msr_content);
+		CCCR_CLEAR(msr_content);
+		CCCR_SET_REQUIRED_BITS(msr_content);
+		wrmsrl(p4_unused_cccr[i], msr_content);
 	}
 
 	/* clear all escrs (including those outside our concern) */
 	for (addr = MSR_P4_BSU_ESCR0 + stag;
 	     addr <  MSR_P4_IQ_ESCR0; addr += addr_increment()) {
-		wrmsr(addr, 0, 0);
+		wrmsrl(addr, 0x0ULL);
 	}
 
 	/* On older models clear also MSR_P4_IQ_ESCR0/1 */
 	if (boot_cpu_data.x86_model < 0x3) {
-		wrmsr(MSR_P4_IQ_ESCR0, 0, 0);
-		wrmsr(MSR_P4_IQ_ESCR1, 0, 0);
+		wrmsrl(MSR_P4_IQ_ESCR0, 0x0ULL);
+		wrmsrl(MSR_P4_IQ_ESCR1, 0x0ULL);
 	}
 
 	for (addr = MSR_P4_RAT_ESCR0 + stag;
 	     addr <= MSR_P4_SSU_ESCR0; ++i, addr += addr_increment()) {
-		wrmsr(addr, 0, 0);
+		wrmsrl(addr, 0x0ULL);
 	}
 	
 	for (addr = MSR_P4_MS_ESCR0 + stag;
 	     addr <= MSR_P4_TC_ESCR1; addr += addr_increment()){ 
-		wrmsr(addr, 0, 0);
+		wrmsrl(addr, 0x0ULL);
 	}
 	
 	for (addr = MSR_P4_IX_ESCR0 + stag;
 	     addr <= MSR_P4_CRU_ESCR3; addr += addr_increment()){ 
-		wrmsr(addr, 0, 0);
+		wrmsrl(addr, 0x0ULL);
 	}
 
 	if (num_counters == NUM_COUNTERS_NON_HT) {		
-		wrmsr(MSR_P4_CRU_ESCR4, 0, 0);
-		wrmsr(MSR_P4_CRU_ESCR5, 0, 0);
+		wrmsrl(MSR_P4_CRU_ESCR4, 0x0ULL);
+		wrmsrl(MSR_P4_CRU_ESCR5, 0x0ULL);
 	} else if (stag == 0) {
-		wrmsr(MSR_P4_CRU_ESCR4, 0, 0);
+		wrmsrl(MSR_P4_CRU_ESCR4, 0x0ULL);
 	} else {
-		wrmsr(MSR_P4_CRU_ESCR5, 0, 0);
+		wrmsrl(MSR_P4_CRU_ESCR5, 0x0ULL);
 	}		
 	
 	/* setup all counters */
@@ -624,7 +623,8 @@ static int p4_check_ctrs(unsigned int co
                          struct op_msrs const * const msrs,
                          struct cpu_user_regs * const regs)
 {
-	unsigned long ctr, low, high, stag, real;
+	unsigned long ctr, stag, real;
+	uint64_t msr_content;
 	int i;
 	int ovf = 0;
 	unsigned long eip = regs->eip;
@@ -656,13 +656,13 @@ static int p4_check_ctrs(unsigned int co
 		
 		real = VIRT_CTR(stag, i);
 
-		CCCR_READ(low, high, real);
- 		CTR_READ(ctr, high, real);
-		if (CCCR_OVF_P(low) || CTR_OVERFLOW_P(ctr)) {
+		CCCR_READ(msr_content, real);
+ 		CTR_READ(ctr, real);
+		if (CCCR_OVF_P(msr_content) || CTR_OVERFLOW_P(ctr)) {
 			xenoprof_log_event(current, regs, eip, mode, i);
 			CTR_WRITE(reset_value[i], real);
-			CCCR_CLEAR_OVF(low);
-			CCCR_WRITE(low, high, real);
+			CCCR_CLEAR_OVF(msr_content);
+			CCCR_WRITE(msr_content, real);
  			CTR_WRITE(reset_value[i], real);
 			ovf = 1;
 		}
@@ -677,7 +677,8 @@ static int p4_check_ctrs(unsigned int co
 
 static void p4_start(struct op_msrs const * const msrs)
 {
-	unsigned int low, high, stag;
+	unsigned int stag;
+	uint64_t msr_content;
 	int i;
 
 	stag = get_stagger();
@@ -685,24 +686,25 @@ static void p4_start(struct op_msrs cons
 	for (i = 0; i < num_counters; ++i) {
 		if (!reset_value[i])
 			continue;
-		CCCR_READ(low, high, VIRT_CTR(stag, i));
-		CCCR_SET_ENABLE(low);
-		CCCR_WRITE(low, high, VIRT_CTR(stag, i));
+		CCCR_READ(msr_content, VIRT_CTR(stag, i));
+		CCCR_SET_ENABLE(msr_content);
+		CCCR_WRITE(msr_content, VIRT_CTR(stag, i));
 	}
 }
 
 
 static void p4_stop(struct op_msrs const * const msrs)
 {
-	unsigned int low, high, stag;
+	unsigned int stag;
+	uint64_t msr_content;
 	int i;
 
 	stag = get_stagger();
 
 	for (i = 0; i < num_counters; ++i) {
-		CCCR_READ(low, high, VIRT_CTR(stag, i));
-		CCCR_SET_DISABLE(low);
-		CCCR_WRITE(low, high, VIRT_CTR(stag, i));
+		CCCR_READ(msr_content, VIRT_CTR(stag, i));
+		CCCR_SET_DISABLE(msr_content);
+		CCCR_WRITE(msr_content, VIRT_CTR(stag, i));
 	}
 }
 
diff -r 7b00193bd033 xen/arch/x86/oprofile/op_model_ppro.c
--- a/xen/arch/x86/oprofile/op_model_ppro.c	Mon Jun 28 17:40:16 2010 +0100
+++ b/xen/arch/x86/oprofile/op_model_ppro.c	Tue Jun 29 17:45:20 2010 +0200
@@ -43,18 +43,18 @@ static int counter_width = 32;
 
 #define CTR_OVERFLOWED(n) (!((n) & (1ULL<<(counter_width-1)))) 
 
-#define CTRL_READ(l,h,msrs,c) do {rdmsr((msrs->controls[(c)].addr), (l), (h));} while (0)
-#define CTRL_WRITE(l,h,msrs,c) do {wrmsr((msrs->controls[(c)].addr), (l), (h));} while (0)
-#define CTRL_SET_ACTIVE(n) (n |= (1<<22))
-#define CTRL_SET_INACTIVE(n) (n &= ~(1<<22))
-#define CTRL_CLEAR(x) (x &= (1<<21))
-#define CTRL_SET_ENABLE(val) (val |= 1<<20)
-#define CTRL_SET_USR(val,u) (val |= ((u & 1) << 16))
-#define CTRL_SET_KERN(val,k) (val |= ((k & 1) << 17))
+#define CTRL_READ(msr_content,msrs,c) do {rdmsrl((msrs->controls[(c)].addr), (msr_content));} while (0)
+#define CTRL_WRITE(msr_content,msrs,c) do {wrmsrl((msrs->controls[(c)].addr), (msr_content));} while (0)
+#define CTRL_SET_ACTIVE(n) (n |= (1ULL<<22))
+#define CTRL_SET_INACTIVE(n) (n &= ~(1ULL<<22))
+#define CTRL_CLEAR(x) (x &= (1ULL<<21))
+#define CTRL_SET_ENABLE(val) (val |= 1ULL<<20)
+#define CTRL_SET_USR(val,u) (val |= ((u & 1ULL) << 16))
+#define CTRL_SET_KERN(val,k) (val |= ((k & 1ULL) << 17))
 #define CTRL_SET_UM(val, m) (val |= (m << 8))
 #define CTRL_SET_EVENT(val, e) (val |= e)
-#define IS_ACTIVE(val) (val & (1 << 22) )  
-#define IS_ENABLE(val) (val & (1 << 20) )
+#define IS_ACTIVE(val) (val & (1ULL << 22) )  
+#define IS_ENABLE(val) (val & (1ULL << 20) )
 static unsigned long reset_value[OP_MAX_COUNTER];
 int ppro_has_global_ctrl = 0;
  
@@ -71,7 +71,7 @@ static void ppro_fill_in_addresses(struc
 
 static void ppro_setup_ctrs(struct op_msrs const * const msrs)
 {
-	unsigned int low, high;
+	uint64_t msr_content;
 	int i;
 	
 	if (cpu_has_arch_perfmon) {
@@ -93,14 +93,14 @@ static void ppro_setup_ctrs(struct op_ms
 
 	/* clear all counters */
 	for (i = 0 ; i < num_counters; ++i) {
-		CTRL_READ(low, high, msrs, i);
-		CTRL_CLEAR(low);
-		CTRL_WRITE(low, high, msrs, i);
+		CTRL_READ(msr_content, msrs, i);
+		CTRL_CLEAR(msr_content);
+		CTRL_WRITE(msr_content, msrs, i);
 	}
 	
 	/* avoid a false detection of ctr overflows in NMI handler */
 	for (i = 0; i < num_counters; ++i)
-		wrmsrl(msrs->counters[i].addr, -1LL);
+		wrmsrl(msrs->counters[i].addr, ~0x0ULL);
 
 	/* enable active counters */
 	for (i = 0; i < num_counters; ++i) {
@@ -109,14 +109,14 @@ static void ppro_setup_ctrs(struct op_ms
 
 			wrmsrl(msrs->counters[i].addr, -reset_value[i]);
 
-			CTRL_READ(low, high, msrs, i);
-			CTRL_CLEAR(low);
-			CTRL_SET_ENABLE(low);
-			CTRL_SET_USR(low, counter_config[i].user);
-			CTRL_SET_KERN(low, counter_config[i].kernel);
-			CTRL_SET_UM(low, counter_config[i].unit_mask);
-			CTRL_SET_EVENT(low, counter_config[i].event);
-			CTRL_WRITE(low, high, msrs, i);
+			CTRL_READ(msr_content, msrs, i);
+			CTRL_CLEAR(msr_content);
+			CTRL_SET_ENABLE(msr_content);
+			CTRL_SET_USR(msr_content, counter_config[i].user);
+			CTRL_SET_KERN(msr_content, counter_config[i].kernel);
+			CTRL_SET_UM(msr_content, counter_config[i].unit_mask);
+			CTRL_SET_EVENT(msr_content, counter_config[i].event);
+			CTRL_WRITE(msr_content, msrs, i);
 		} else {
 			reset_value[i] = 0;
 		}
@@ -166,38 +166,38 @@ static int ppro_check_ctrs(unsigned int 
  
 static void ppro_start(struct op_msrs const * const msrs)
 {
-	unsigned int low,high;
+	uint64_t msr_content;
 	int i;
 
 	for (i = 0; i < num_counters; ++i) {
 		if (reset_value[i]) {
-			CTRL_READ(low, high, msrs, i);
-			CTRL_SET_ACTIVE(low);
-			CTRL_WRITE(low, high, msrs, i);
+			CTRL_READ(msr_content, msrs, i);
+			CTRL_SET_ACTIVE(msr_content);
+			CTRL_WRITE(msr_content, msrs, i);
 		}
 	}
     /* Global Control MSR is enabled by default when system power on.
      * However, this may not hold true when xenoprof starts to run.
      */
     if ( ppro_has_global_ctrl )
-        wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, (1<<num_counters) - 1);
+        wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, (1ULL<<num_counters) - 1);
 }
 
 
 static void ppro_stop(struct op_msrs const * const msrs)
 {
-	unsigned int low,high;
+	uint64_t msr_content;
 	int i;
 
 	for (i = 0; i < num_counters; ++i) {
 		if (!reset_value[i])
 			continue;
-		CTRL_READ(low, high, msrs, i);
-		CTRL_SET_INACTIVE(low);
-		CTRL_WRITE(low, high, msrs, i);
+		CTRL_READ(msr_content, msrs, i);
+		CTRL_SET_INACTIVE(msr_content);
+		CTRL_WRITE(msr_content, msrs, i);
 	}
     if ( ppro_has_global_ctrl )
-        wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+        wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, 0x0ULL);
 }
 
 static int ppro_is_arch_pmu_msr(u64 msr_index, int *type, int *index)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] x86 oprofile: use rdmsrl/wrmsrl

[Keir Fraser]

How many more of these patches do you have up your sleeve?

 -- Keir

On 29/06/2010 16:47, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> 
> Hi!
> 
> Attached patch makes x86 oprofile code use rdmsrl/wrmsrl
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

Re: [PATCH] x86 oprofile: use rdmsrl/wrmsrl

[Christoph Egger]

On Tuesday 29 June 2010 18:02:54 Keir Fraser wrote:
> How many more of these patches do you have up your sleeve?

Depends on how small/large the chunks should/may be.
My intention is to remove duplicated rdmsr/wrmsr functions
and make rdmsr usable as a real C function.
The large part of changes are the bit-operations surrounding
the rdmsr/wrmsr code lines.

The sent patches replace rdmsr/wrmsr by rdmsrl/wrmsrl
step by step. When this is done, rdmsr/wrmsr can be killed.

Christoph

>  -- Keir
>
> On 29/06/2010 16:47, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > Hi!
> >
> > Attached patch makes x86 oprofile code use rdmsrl/wrmsrl
> >
> > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] x86 oprofile: use rdmsrl/wrmsrl

[Keir Fraser]

On 30/06/2010 11:12, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> On Tuesday 29 June 2010 18:02:54 Keir Fraser wrote:
>> How many more of these patches do you have up your sleeve?
> 
> Depends on how small/large the chunks should/may be.
> My intention is to remove duplicated rdmsr/wrmsr functions
> and make rdmsr usable as a real C function.

It's a lot of effort for questionable value isn't it. And many of your
patches so far have contained bugs, and in some cases decreased code
readability by needing to manually merge/split values to be able to make use
of rdmsrl/wrmsrl, so it's not like it's even strictly improving the code.
Frankly, it smells like busy work.

 -- Keir

Re: [PATCH] x86 oprofile: use rdmsrl/wrmsrl

[Christoph Egger]

On Wednesday 30 June 2010 12:19:09 Keir Fraser wrote:
> On 30/06/2010 11:12, "Christoph Egger" <Christoph.Egger@amd.com> wrote:
> > On Tuesday 29 June 2010 18:02:54 Keir Fraser wrote:
> >> How many more of these patches do you have up your sleeve?
> >
> > Depends on how small/large the chunks should/may be.
> > My intention is to remove duplicated rdmsr/wrmsr functions
> > and make rdmsr usable as a real C function.
>
> It's a lot of effort for questionable value isn't it. And many of your
> patches so far have contained bugs, and in some cases decreased code
> readability by needing to manually merge/split values to be able to make
> use of rdmsrl/wrmsrl, so it's not like it's even strictly improving the
> code. Frankly, it smells like busy work.

Yes, you're right. Work like this is not funny and error-prone
(you can look over it many times, test it many times and still bugs
sit in there), hence the high number of small patches so that other
people look at it and make bisecting easier.
Whoever is doing such work always risks to get the head chopped off
for the breakages.

The parts decreasing code readability should actually be rewritten
but this is beyond of my intention.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] x86 oprofile: use rdmsrl/wrmsrl

[Joerg Roedel]

On Wed, Jun 30, 2010 at 01:02:50PM +0200, Christoph Egger wrote:
> The parts decreasing code readability should actually be rewritten
> but this is beyond of my intention.

That's interesting. What is your intention then?

	Joerg

Re: [PATCH] x86 oprofile: use rdmsrl/wrmsrl

[Christoph Egger]

On Wednesday 30 June 2010 16:29:44 Joerg Roedel wrote:
> On Wed, Jun 30, 2010 at 01:02:50PM +0200, Christoph Egger wrote:
> > The parts decreasing code readability should actually be rewritten
> > but this is beyond of my intention.
>
> That's interesting. What is your intention then?

As explained in an earlier mail, I want to get rid of the
redundant rdmsr/wrmsr functions.

Christoph
-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] x86 oprofile: use rdmsrl/wrmsrl

[Joerg Roedel]

On Wed, Jun 30, 2010 at 04:34:43PM +0200, Christoph Egger wrote:
> As explained in an earlier mail, I want to get rid of the
> redundant rdmsr/wrmsr functions.

What's the point of that if you decrease the code readability?

	Joerg