[PATCH 0/24] oprofile: Add IBS support for AMD CPUs

[PATCH 0/24] oprofile: Add IBS support for AMD CPUs

[Robert Richter]

Patches #1-3 are not directly related to IBS.
Patch #4 adds generic support of model specific initialization.
Patches #10, #11 add the core implementation of IBS.
Patches #12-24 contain code improvements and small fixes.

[PATCH 01/24] x86: Add PCI IDs for AMD Barcelona PCI devices

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 include/linux/pci_ids.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index d8507eb..d1a52d8 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -497,6 +497,11 @@
 #define PCI_DEVICE_ID_AMD_K8_NB_ADDRMAP	0x1101
 #define PCI_DEVICE_ID_AMD_K8_NB_MEMCTL	0x1102
 #define PCI_DEVICE_ID_AMD_K8_NB_MISC	0x1103
+#define PCI_DEVICE_ID_AMD_10H_NB_HT	0x1200
+#define PCI_DEVICE_ID_AMD_10H_NB_MAP	0x1201
+#define PCI_DEVICE_ID_AMD_10H_NB_DRAM	0x1202
+#define PCI_DEVICE_ID_AMD_10H_NB_MISC	0x1203
+#define PCI_DEVICE_ID_AMD_10H_NB_LINK	0x1204
 #define PCI_DEVICE_ID_AMD_LANCE		0x2000
 #define PCI_DEVICE_ID_AMD_LANCE_HOME	0x2001
 #define PCI_DEVICE_ID_AMD_SCSI		0x2020
-- 
1.5.5.4

[PATCH 02/24] x86: apic_*.c: Add description to AMD's extended LVT functions

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/kernel/apic_32.c |    3 +++
 arch/x86/kernel/apic_64.c |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic_32.c b/arch/x86/kernel/apic_32.c
index d6c8983..fad94b0 100644
--- a/arch/x86/kernel/apic_32.c
+++ b/arch/x86/kernel/apic_32.c
@@ -646,6 +646,9 @@ int setup_profiling_timer(unsigned int multiplier)
  *
  * Vector mappings are hard coded. On K8 only offset 0 (APIC500) and
  * MCE interrupts are supported. Thus MCE offset must be set to 0.
+ *
+ * If mask=1, the LVT entry does not generate interrupts while mask=0
+ * enables the vector. See also the BKDGs.
  */
 
 #define APIC_EILVT_LVTOFF_MCE 0
diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index 7f1f030..42bf69f 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -205,6 +205,9 @@ static void __setup_APIC_LVTT(unsigned int clocks, int oneshot, int irqen)
  *
  * Vector mappings are hard coded. On K8 only offset 0 (APIC500) and
  * MCE interrupts are supported. Thus MCE offset must be set to 0.
+ *
+ * If mask=1, the LVT entry does not generate interrupts while mask=0
+ * enables the vector. See also the BKDGs.
  */
 
 #define APIC_EILVT_LVTOFF_MCE 0
-- 
1.5.5.4

[PATCH 03/24] oprofile: Add support for AMD Family 11h

[Robert Richter]

From: Barry Kasindorf <barry.kasindorf@amd.com>

This patch add support for AMD Family 11h CPUs.

Signed-off-by: Barry Kasindorf <barry.kasindorf@amd.com>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 7f3329b..cf28a02 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -422,6 +422,10 @@ int __init op_nmi_init(struct oprofile_operations *ops)
 			model = &op_athlon_spec;
 			cpu_type = "x86-64/family10";
 			break;
+		case 0x11:
+			model = &op_athlon_spec;
+			cpu_type = "x86-64/family11h";
+			break;
 		}
 		break;
 
-- 
1.5.5.4

Re: [PATCH 03/24] oprofile: Add support for AMD Family 11h

[Daniel K.]

Robert Richter wrote:
> diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
> index 7f3329b..cf28a02 100644
> --- a/arch/x86/oprofile/nmi_int.c
> +++ b/arch/x86/oprofile/nmi_int.c
> @@ -422,6 +422,10 @@ int __init op_nmi_init(struct oprofile_operations *ops)
>  			model = &op_athlon_spec;
>  			cpu_type = "x86-64/family10";
>  			break;
> +		case 0x11:
> +			model = &op_athlon_spec;
> +			cpu_type = "x86-64/family11h";

Should the 'h' be dropped, as for family10 above, to keep them similar.


Daniel K.

Re: [PATCH 03/24] oprofile: Add support for AMD Family 11h

[Ingo Molnar]

* Daniel K. <dk@uw.no> wrote:

> Robert Richter wrote:
>> diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
>> index 7f3329b..cf28a02 100644
>> --- a/arch/x86/oprofile/nmi_int.c
>> +++ b/arch/x86/oprofile/nmi_int.c
>> @@ -422,6 +422,10 @@ int __init op_nmi_init(struct oprofile_operations *ops)
>>  			model = &op_athlon_spec;
>>  			cpu_type = "x86-64/family10";
>>  			break;
>> +		case 0x11:
>> +			model = &op_athlon_spec;
>> +			cpu_type = "x86-64/family11h";
>
> Should the 'h' be dropped, as for family10 above, to keep them similar.

'family10' is a typo we've got to live with - but there's no need to 
repeat it for family11h.

	Ingo

[PATCH 04/24] x86/oprofile: Introduce model specific init/exit functions

[Robert Richter]

This patch implements model specific OProfile init/exit functions for
x86 CPUs. Though there is more rework needed at the initialization
code, this new introduced functions allow it to keep model specific
code in the corresponding op_model_*.c files.

The function interface is the same as for oprofile_arch_init/exit().

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c         |   11 ++++++++++-
 arch/x86/oprofile/op_model_athlon.c |   18 +++++++++++++++---
 arch/x86/oprofile/op_x86_model.h    |    2 ++
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index cf28a02..b96cfd3 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -1,10 +1,11 @@
 /**
  * @file nmi_int.c
  *
- * @remark Copyright 2002 OProfile authors
+ * @remark Copyright 2002-2008 OProfile authors
  * @remark Read the file COPYING
  *
  * @author John Levon <levon@movementarian.org>
+ * @author Robert Richter <robert.richter@amd.com>
  */
 
 #include <linux/init.h>
@@ -397,6 +398,7 @@ int __init op_nmi_init(struct oprofile_operations *ops)
 	__u8 vendor = boot_cpu_data.x86_vendor;
 	__u8 family = boot_cpu_data.x86;
 	char *cpu_type;
+	int ret = 0;
 
 	if (!cpu_has_apic)
 		return -ENODEV;
@@ -452,6 +454,11 @@ int __init op_nmi_init(struct oprofile_operations *ops)
 		return -ENODEV;
 	}
 
+	if (model->init)
+		ret = model->init(ops);
+	if (ret)
+		return ret;
+
 	init_sysfs();
 	using_nmi = 1;
 	ops->create_files = nmi_create_files;
@@ -468,4 +475,6 @@ void op_nmi_exit(void)
 {
 	if (using_nmi)
 		exit_sysfs();
+	if (model->exit)
+		model->exit();
 }
diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index 3d53487..dd8b1dc 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -1,14 +1,15 @@
 /*
- * @file op_model_athlon.h
+ * @file op_model_athlon.c
  * athlon / K7 / K8 / Family 10h model-specific MSR operations
  *
- * @remark Copyright 2002 OProfile authors
+ * @remark Copyright 2002-2008 OProfile authors
  * @remark Read the file COPYING
  *
  * @author John Levon
  * @author Philippe Elie
  * @author Graydon Hoare
- */
+ * @author Robert Richter <robert.richter@amd.com>
+*/
 
 #include <linux/oprofile.h>
 #include <asm/ptrace.h>
@@ -178,7 +179,18 @@ static void athlon_shutdown(struct op_msrs const * const msrs)
 	}
 }
 
+static int op_amd_init(struct oprofile_operations *ops)
+{
+	return 0;
+}
+
+static void op_amd_exit(void)
+{
+}
+
 struct op_x86_model_spec const op_athlon_spec = {
+	.init = op_amd_init,
+	.exit = op_amd_exit,
 	.num_counters = NUM_COUNTERS,
 	.num_controls = NUM_CONTROLS,
 	.fill_in_addresses = &athlon_fill_in_addresses,
diff --git a/arch/x86/oprofile/op_x86_model.h b/arch/x86/oprofile/op_x86_model.h
index 45b605f..ee9ca96 100644
--- a/arch/x86/oprofile/op_x86_model.h
+++ b/arch/x86/oprofile/op_x86_model.h
@@ -32,6 +32,8 @@ struct pt_regs;
  * various x86 CPU models' perfctr support.
  */
 struct op_x86_model_spec {
+	int (*init)(struct oprofile_operations *ops);
+	void (*exit)(void);
 	unsigned int const num_counters;
 	unsigned int const num_controls;
 	void (*fill_in_addresses)(struct op_msrs * const msrs);
-- 
1.5.5.4

[PATCH 05/24] x86/oprofile: Minor changes in op_model_athlon.c

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index dd8b1dc..d25d7f1 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -45,6 +45,8 @@
 
 static unsigned long reset_value[NUM_COUNTERS];
 
+/* functions for op_athlon_spec */
+
 static void athlon_fill_in_addresses(struct op_msrs * const msrs)
 {
 	int i;
-- 
1.5.5.4

[PATCH 06/24] x86/oprofile: Renaming athlon_*() into op_amd_*()

[Robert Richter]

These functions contain code for all AMD CPUs. The new names fit
better.

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c         |    8 ++++----
 arch/x86/oprofile/op_model_athlon.c |   28 ++++++++++++++--------------
 arch/x86/oprofile/op_x86_model.h    |    2 +-
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index b96cfd3..4e42b50 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -411,21 +411,21 @@ int __init op_nmi_init(struct oprofile_operations *ops)
 		default:
 			return -ENODEV;
 		case 6:
-			model = &op_athlon_spec;
+			model = &op_amd_spec;
 			cpu_type = "i386/athlon";
 			break;
 		case 0xf:
-			model = &op_athlon_spec;
+			model = &op_amd_spec;
 			/* Actually it could be i386/hammer too, but give
 			 user space an consistent name. */
 			cpu_type = "x86-64/hammer";
 			break;
 		case 0x10:
-			model = &op_athlon_spec;
+			model = &op_amd_spec;
 			cpu_type = "x86-64/family10";
 			break;
 		case 0x11:
-			model = &op_athlon_spec;
+			model = &op_amd_spec;
 			cpu_type = "x86-64/family11h";
 			break;
 		}
diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index d25d7f1..40ecb02 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -45,9 +45,9 @@
 
 static unsigned long reset_value[NUM_COUNTERS];
 
-/* functions for op_athlon_spec */
+/* functions for op_amd_spec */
 
-static void athlon_fill_in_addresses(struct op_msrs * const msrs)
+static void op_amd_fill_in_addresses(struct op_msrs * const msrs)
 {
 	int i;
 
@@ -67,7 +67,7 @@ static void athlon_fill_in_addresses(struct op_msrs * const msrs)
 }
 
 
-static void athlon_setup_ctrs(struct op_msrs const * const msrs)
+static void op_amd_setup_ctrs(struct op_msrs const * const msrs)
 {
 	unsigned int low, high;
 	int i;
@@ -116,7 +116,7 @@ static void athlon_setup_ctrs(struct op_msrs const * const msrs)
 }
 
 
-static int athlon_check_ctrs(struct pt_regs * const regs,
+static int op_amd_check_ctrs(struct pt_regs * const regs,
 			     struct op_msrs const * const msrs)
 {
 	unsigned int low, high;
@@ -137,7 +137,7 @@ static int athlon_check_ctrs(struct pt_regs * const regs,
 }
 
 
-static void athlon_start(struct op_msrs const * const msrs)
+static void op_amd_start(struct op_msrs const * const msrs)
 {
 	unsigned int low, high;
 	int i;
@@ -151,7 +151,7 @@ static void athlon_start(struct op_msrs const * const msrs)
 }
 
 
-static void athlon_stop(struct op_msrs const * const msrs)
+static void op_amd_stop(struct op_msrs const * const msrs)
 {
 	unsigned int low, high;
 	int i;
@@ -167,7 +167,7 @@ static void athlon_stop(struct op_msrs const * const msrs)
 	}
 }
 
-static void athlon_shutdown(struct op_msrs const * const msrs)
+static void op_amd_shutdown(struct op_msrs const * const msrs)
 {
 	int i;
 
@@ -190,15 +190,15 @@ static void op_amd_exit(void)
 {
 }
 
-struct op_x86_model_spec const op_athlon_spec = {
+struct op_x86_model_spec const op_amd_spec = {
 	.init = op_amd_init,
 	.exit = op_amd_exit,
 	.num_counters = NUM_COUNTERS,
 	.num_controls = NUM_CONTROLS,
-	.fill_in_addresses = &athlon_fill_in_addresses,
-	.setup_ctrs = &athlon_setup_ctrs,
-	.check_ctrs = &athlon_check_ctrs,
-	.start = &athlon_start,
-	.stop = &athlon_stop,
-	.shutdown = &athlon_shutdown
+	.fill_in_addresses = &op_amd_fill_in_addresses,
+	.setup_ctrs = &op_amd_setup_ctrs,
+	.check_ctrs = &op_amd_check_ctrs,
+	.start = &op_amd_start,
+	.stop = &op_amd_stop,
+	.shutdown = &op_amd_shutdown
 };
diff --git a/arch/x86/oprofile/op_x86_model.h b/arch/x86/oprofile/op_x86_model.h
index ee9ca96..05a0261 100644
--- a/arch/x86/oprofile/op_x86_model.h
+++ b/arch/x86/oprofile/op_x86_model.h
@@ -48,6 +48,6 @@ struct op_x86_model_spec {
 extern struct op_x86_model_spec const op_ppro_spec;
 extern struct op_x86_model_spec const op_p4_spec;
 extern struct op_x86_model_spec const op_p4_ht2_spec;
-extern struct op_x86_model_spec const op_athlon_spec;
+extern struct op_x86_model_spec const op_amd_spec;
 
 #endif /* OP_X86_MODEL_H */
-- 
1.5.5.4

Re: [PATCH 06/24] x86/oprofile: Renaming athlon_*() into op_amd_*()

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> -struct op_x86_model_spec const op_athlon_spec = {
> +struct op_x86_model_spec const op_amd_spec = {
>  	.init = op_amd_init,
>  	.exit = op_amd_exit,
>  	.num_counters = NUM_COUNTERS,
>  	.num_controls = NUM_CONTROLS,
> -	.fill_in_addresses = &athlon_fill_in_addresses,
> -	.setup_ctrs = &athlon_setup_ctrs,
> -	.check_ctrs = &athlon_check_ctrs,
> -	.start = &athlon_start,
> -	.stop = &athlon_stop,
> -	.shutdown = &athlon_shutdown
> +	.fill_in_addresses = &op_amd_fill_in_addresses,
> +	.setup_ctrs = &op_amd_setup_ctrs,
> +	.check_ctrs = &op_amd_check_ctrs,
> +	.start = &op_amd_start,
> +	.stop = &op_amd_stop,
> +	.shutdown = &op_amd_shutdown
>  };

minor style comment: while we are at touching these initializers, could 
you also do a followup cleanup patch that uses the standard initializer 
style of arch/x86 that have the initializer values aligned vertically? 

Sample:

static const struct file_operations ptdump_fops = {
	.open		= ptdump_open,
	.read		= seq_read,
	.llseek		= seq_lseek,
	.release	= single_release,
};

Thanks,

	Ingo

[PATCH 07/24] drivers/oprofile: Coding style fixes in buffer_sync.c

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 drivers/oprofile/buffer_sync.c |  111 ++++++++++++++++++++-------------------
 1 files changed, 57 insertions(+), 54 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index 9304c45..69a7327 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -33,7 +33,7 @@
 #include "event_buffer.h"
 #include "cpu_buffer.h"
 #include "buffer_sync.h"
- 
+
 static LIST_HEAD(dying_tasks);
 static LIST_HEAD(dead_tasks);
 static cpumask_t marked_cpus = CPU_MASK_NONE;
@@ -48,10 +48,11 @@ static void process_task_mortuary(void);
  * Can be invoked from softirq via RCU callback due to
  * call_rcu() of the task struct, hence the _irqsave.
  */
-static int task_free_notify(struct notifier_block * self, unsigned long val, void * data)
+static int
+task_free_notify(struct notifier_block *self, unsigned long val, void *data)
 {
 	unsigned long flags;
-	struct task_struct * task = data;
+	struct task_struct *task = data;
 	spin_lock_irqsave(&task_mortuary, flags);
 	list_add(&task->tasks, &dying_tasks);
 	spin_unlock_irqrestore(&task_mortuary, flags);
@@ -62,13 +63,14 @@ static int task_free_notify(struct notifier_block * self, unsigned long val, voi
 /* The task is on its way out. A sync of the buffer means we can catch
  * any remaining samples for this task.
  */
-static int task_exit_notify(struct notifier_block * self, unsigned long val, void * data)
+static int
+task_exit_notify(struct notifier_block *self, unsigned long val, void *data)
 {
 	/* To avoid latency problems, we only process the current CPU,
 	 * hoping that most samples for the task are on this CPU
 	 */
 	sync_buffer(raw_smp_processor_id());
-  	return 0;
+	return 0;
 }
 
 
@@ -77,11 +79,12 @@ static int task_exit_notify(struct notifier_block * self, unsigned long val, voi
  * we don't lose any. This does not have to be exact, it's a QoI issue
  * only.
  */
-static int munmap_notify(struct notifier_block * self, unsigned long val, void * data)
+static int
+munmap_notify(struct notifier_block *self, unsigned long val, void *data)
 {
 	unsigned long addr = (unsigned long)data;
-	struct mm_struct * mm = current->mm;
-	struct vm_area_struct * mpnt;
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *mpnt;
 
 	down_read(&mm->mmap_sem);
 
@@ -99,11 +102,12 @@ static int munmap_notify(struct notifier_block * self, unsigned long val, void *
 	return 0;
 }
 
- 
+
 /* We need to be told about new modules so we don't attribute to a previously
  * loaded module, or drop the samples on the floor.
  */
-static int module_load_notify(struct notifier_block * self, unsigned long val, void * data)
+static int
+module_load_notify(struct notifier_block *self, unsigned long val, void *data)
 {
 #ifdef CONFIG_MODULES
 	if (val != MODULE_STATE_COMING)
@@ -118,7 +122,7 @@ static int module_load_notify(struct notifier_block * self, unsigned long val, v
 	return 0;
 }
 
- 
+
 static struct notifier_block task_free_nb = {
 	.notifier_call	= task_free_notify,
 };
@@ -135,7 +139,7 @@ static struct notifier_block module_load_nb = {
 	.notifier_call = module_load_notify,
 };
 
- 
+
 static void end_sync(void)
 {
 	end_cpu_work();
@@ -208,14 +212,14 @@ static inline unsigned long fast_get_dcookie(struct path *path)
  * not strictly necessary but allows oprofile to associate
  * shared-library samples with particular applications
  */
-static unsigned long get_exec_dcookie(struct mm_struct * mm)
+static unsigned long get_exec_dcookie(struct mm_struct *mm)
 {
 	unsigned long cookie = NO_COOKIE;
-	struct vm_area_struct * vma;
- 
+	struct vm_area_struct *vma;
+
 	if (!mm)
 		goto out;
- 
+
 	for (vma = mm->mmap; vma; vma = vma->vm_next) {
 		if (!vma->vm_file)
 			continue;
@@ -235,13 +239,14 @@ out:
  * sure to do this lookup before a mm->mmap modification happens so
  * we don't lose track.
  */
-static unsigned long lookup_dcookie(struct mm_struct * mm, unsigned long addr, off_t * offset)
+static unsigned long
+lookup_dcookie(struct mm_struct *mm, unsigned long addr, off_t *offset)
 {
 	unsigned long cookie = NO_COOKIE;
-	struct vm_area_struct * vma;
+	struct vm_area_struct *vma;
 
 	for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
- 
+
 		if (addr < vma->vm_start || addr >= vma->vm_end)
 			continue;
 
@@ -265,7 +270,7 @@ static unsigned long lookup_dcookie(struct mm_struct * mm, unsigned long addr, o
 
 
 static unsigned long last_cookie = INVALID_COOKIE;
- 
+
 static void add_cpu_switch(int i)
 {
 	add_event_entry(ESCAPE_CODE);
@@ -278,16 +283,16 @@ static void add_kernel_ctx_switch(unsigned int in_kernel)
 {
 	add_event_entry(ESCAPE_CODE);
 	if (in_kernel)
-		add_event_entry(KERNEL_ENTER_SWITCH_CODE); 
+		add_event_entry(KERNEL_ENTER_SWITCH_CODE);
 	else
-		add_event_entry(KERNEL_EXIT_SWITCH_CODE); 
+		add_event_entry(KERNEL_EXIT_SWITCH_CODE);
 }
- 
+
 static void
-add_user_ctx_switch(struct task_struct const * task, unsigned long cookie)
+add_user_ctx_switch(struct task_struct const *task, unsigned long cookie)
 {
 	add_event_entry(ESCAPE_CODE);
-	add_event_entry(CTX_SWITCH_CODE); 
+	add_event_entry(CTX_SWITCH_CODE);
 	add_event_entry(task->pid);
 	add_event_entry(cookie);
 	/* Another code for daemon back-compat */
@@ -296,7 +301,7 @@ add_user_ctx_switch(struct task_struct const * task, unsigned long cookie)
 	add_event_entry(task->tgid);
 }
 
- 
+
 static void add_cookie_switch(unsigned long cookie)
 {
 	add_event_entry(ESCAPE_CODE);
@@ -304,7 +309,7 @@ static void add_cookie_switch(unsigned long cookie)
 	add_event_entry(cookie);
 }
 
- 
+
 static void add_trace_begin(void)
 {
 	add_event_entry(ESCAPE_CODE);
@@ -319,13 +324,13 @@ static void add_sample_entry(unsigned long offset, unsigned long event)
 }
 
 
-static int add_us_sample(struct mm_struct * mm, struct op_sample * s)
+static int add_us_sample(struct mm_struct *mm, struct op_sample *s)
 {
 	unsigned long cookie;
 	off_t offset;
- 
- 	cookie = lookup_dcookie(mm, s->eip, &offset);
- 
+
+	cookie = lookup_dcookie(mm, s->eip, &offset);
+
 	if (cookie == INVALID_COOKIE) {
 		atomic_inc(&oprofile_stats.sample_lost_no_mapping);
 		return 0;
@@ -341,13 +346,13 @@ static int add_us_sample(struct mm_struct * mm, struct op_sample * s)
 	return 1;
 }
 
- 
+
 /* Add a sample to the global event buffer. If possible the
  * sample is converted into a persistent dentry/offset pair
  * for later lookup from userspace.
  */
 static int
-add_sample(struct mm_struct * mm, struct op_sample * s, int in_kernel)
+add_sample(struct mm_struct *mm, struct op_sample *s, int in_kernel)
 {
 	if (in_kernel) {
 		add_sample_entry(s->eip, s->event);
@@ -359,9 +364,9 @@ add_sample(struct mm_struct * mm, struct op_sample * s, int in_kernel)
 	}
 	return 0;
 }
- 
 
-static void release_mm(struct mm_struct * mm)
+
+static void release_mm(struct mm_struct *mm)
 {
 	if (!mm)
 		return;
@@ -370,9 +375,9 @@ static void release_mm(struct mm_struct * mm)
 }
 
 
-static struct mm_struct * take_tasks_mm(struct task_struct * task)
+static struct mm_struct *take_tasks_mm(struct task_struct *task)
 {
-	struct mm_struct * mm = get_task_mm(task);
+	struct mm_struct *mm = get_task_mm(task);
 	if (mm)
 		down_read(&mm->mmap_sem);
 	return mm;
@@ -383,10 +388,10 @@ static inline int is_code(unsigned long val)
 {
 	return val == ESCAPE_CODE;
 }
- 
+
 
 /* "acquire" as many cpu buffer slots as we can */
-static unsigned long get_slots(struct oprofile_cpu_buffer * b)
+static unsigned long get_slots(struct oprofile_cpu_buffer *b)
 {
 	unsigned long head = b->head_pos;
 	unsigned long tail = b->tail_pos;
@@ -412,7 +417,7 @@ static unsigned long get_slots(struct oprofile_cpu_buffer * b)
 }
 
 
-static void increment_tail(struct oprofile_cpu_buffer * b)
+static void increment_tail(struct oprofile_cpu_buffer *b)
 {
 	unsigned long new_tail = b->tail_pos + 1;
 
@@ -435,8 +440,8 @@ static void process_task_mortuary(void)
 {
 	unsigned long flags;
 	LIST_HEAD(local_dead_tasks);
-	struct task_struct * task;
-	struct task_struct * ttask;
+	struct task_struct *task;
+	struct task_struct *ttask;
 
 	spin_lock_irqsave(&task_mortuary, flags);
 
@@ -493,7 +498,7 @@ void sync_buffer(int cpu)
 {
 	struct oprofile_cpu_buffer *cpu_buf = &per_cpu(cpu_buffer, cpu);
 	struct mm_struct *mm = NULL;
-	struct task_struct * new;
+	struct task_struct *new;
 	unsigned long cookie = 0;
 	int in_kernel = 1;
 	unsigned int i;
@@ -501,7 +506,7 @@ void sync_buffer(int cpu)
 	unsigned long available;
 
 	mutex_lock(&buffer_mutex);
- 
+
 	add_cpu_switch(cpu);
 
 	/* Remember, only we can modify tail_pos */
@@ -509,8 +514,8 @@ void sync_buffer(int cpu)
 	available = get_slots(cpu_buf);
 
 	for (i = 0; i < available; ++i) {
-		struct op_sample * s = &cpu_buf->buffer[cpu_buf->tail_pos];
- 
+		struct op_sample *s = &cpu_buf->buffer[cpu_buf->tail_pos];
+
 		if (is_code(s->eip)) {
 			if (s->event <= CPU_IS_KERNEL) {
 				/* kernel/userspace switch */
@@ -522,7 +527,7 @@ void sync_buffer(int cpu)
 				state = sb_bt_start;
 				add_trace_begin();
 			} else {
-				struct mm_struct * oldmm = mm;
+				struct mm_struct *oldmm = mm;
 
 				/* userspace context switch */
 				new = (struct task_struct *)s->event;
@@ -533,13 +538,11 @@ void sync_buffer(int cpu)
 					cookie = get_exec_dcookie(mm);
 				add_user_ctx_switch(new, cookie);
 			}
-		} else {
-			if (state >= sb_bt_start &&
-			    !add_sample(mm, s, in_kernel)) {
-				if (state == sb_bt_start) {
-					state = sb_bt_ignore;
-					atomic_inc(&oprofile_stats.bt_lost_no_mapping);
-				}
+		} else if (state >= sb_bt_start &&
+			   !add_sample(mm, s, in_kernel)) {
+			if (state == sb_bt_start) {
+				state = sb_bt_ignore;
+				atomic_inc(&oprofile_stats.bt_lost_no_mapping);
 			}
 		}
 
-- 
1.5.5.4

[PATCH 08/24] OProfile: Moving increment_tail() in buffer_sync.c

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 drivers/oprofile/buffer_sync.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index 69a7327..615929f 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -268,6 +268,17 @@ lookup_dcookie(struct mm_struct *mm, unsigned long addr, off_t *offset)
 	return cookie;
 }
 
+static void increment_tail(struct oprofile_cpu_buffer *b)
+{
+	unsigned long new_tail = b->tail_pos + 1;
+
+	rmb();
+
+	if (new_tail < b->buffer_size)
+		b->tail_pos = new_tail;
+	else
+		b->tail_pos = 0;
+}
 
 static unsigned long last_cookie = INVALID_COOKIE;
 
@@ -417,19 +428,6 @@ static unsigned long get_slots(struct oprofile_cpu_buffer *b)
 }
 
 
-static void increment_tail(struct oprofile_cpu_buffer *b)
-{
-	unsigned long new_tail = b->tail_pos + 1;
-
-	rmb();
-
-	if (new_tail < b->buffer_size)
-		b->tail_pos = new_tail;
-	else
-		b->tail_pos = 0;
-}
-
-
 /* Move tasks along towards death. Any tasks on dead_tasks
  * will definitely have no remaining references in any
  * CPU buffers at this point, because we use two lists,
-- 
1.5.5.4

Re: [PATCH 08/24] OProfile: Moving increment_tail() in buffer_sync.c

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> +static void increment_tail(struct oprofile_cpu_buffer *b)
> +{
> +	unsigned long new_tail = b->tail_pos + 1;
> +
> +	rmb();

this barrier needs a comment.

	Ingo

[PATCH 09/24] OProfile: Add IBS code macros

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 include/linux/oprofile.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/oprofile.h b/include/linux/oprofile.h
index 041bb31..bcb8f72 100644
--- a/include/linux/oprofile.h
+++ b/include/linux/oprofile.h
@@ -36,6 +36,8 @@
 #define XEN_ENTER_SWITCH_CODE		10
 #define SPU_PROFILING_CODE		11
 #define SPU_CTX_SWITCH_CODE		12
+#define IBS_FETCH_CODE			13
+#define IBS_OP_CODE			14
 
 struct super_block;
 struct dentry;
-- 
1.5.5.4

[PATCH 10/24] x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines

[Robert Richter]

From: Barry Kasindorf <barry.kasindorf@amd.com>

This patchset supports the new profiling hardware available in the
latest AMD CPUs in the oProfile driver.

Signed-off-by: Barry Kasindorf <barry.kasindorf@amd.com>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 drivers/oprofile/buffer_sync.c |   72 +++++++++++++++++++++++++++++++++++++++-
 drivers/oprofile/cpu_buffer.c  |   68 +++++++++++++++++++++++++++++++++++++-
 drivers/oprofile/cpu_buffer.h  |    2 +
 3 files changed, 140 insertions(+), 2 deletions(-)

diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index 615929f..e1782d2 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -5,6 +5,7 @@
  * @remark Read the file COPYING
  *
  * @author John Levon <levon@movementarian.org>
+ * @author Barry Kasindorf
  *
  * This is the core of the buffer management. Each
  * CPU buffer is processed and entered into the
@@ -272,7 +273,7 @@ static void increment_tail(struct oprofile_cpu_buffer *b)
 {
 	unsigned long new_tail = b->tail_pos + 1;
 
-	rmb();
+	rmb();	/* be sure fifo pointers are synchromized */
 
 	if (new_tail < b->buffer_size)
 		b->tail_pos = new_tail;
@@ -327,6 +328,67 @@ static void add_trace_begin(void)
 	add_event_entry(TRACE_BEGIN_CODE);
 }
 
+#define IBS_FETCH_CODE_SIZE	2
+#define IBS_OP_CODE_SIZE	5
+#define IBS_EIP(offset)				\
+	(((struct op_sample *)&cpu_buf->buffer[(offset)])->eip)
+#define IBS_EVENT(offset)				\
+	(((struct op_sample *)&cpu_buf->buffer[(offset)])->event)
+
+/*
+ * Add IBS fetch and op entries to event buffer
+ */
+static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code,
+	int in_kernel, struct mm_struct *mm)
+{
+	unsigned long rip;
+	int i, count;
+	unsigned long ibs_cookie = 0;
+	off_t offset;
+
+	increment_tail(cpu_buf);	/* move to RIP entry */
+
+	rip = IBS_EIP(cpu_buf->tail_pos);
+
+#ifdef __LP64__
+	rip += IBS_EVENT(cpu_buf->tail_pos) << 32;
+#endif
+
+	if (mm) {
+		ibs_cookie = lookup_dcookie(mm, rip, &offset);
+
+		if (ibs_cookie == NO_COOKIE)
+			offset = rip;
+		if (ibs_cookie == INVALID_COOKIE) {
+			atomic_inc(&oprofile_stats.sample_lost_no_mapping);
+			offset = rip;
+		}
+		if (ibs_cookie != last_cookie) {
+			add_cookie_switch(ibs_cookie);
+			last_cookie = ibs_cookie;
+		}
+	} else
+		offset = rip;
+
+	add_event_entry(ESCAPE_CODE);
+	add_event_entry(code);
+	add_event_entry(offset);	/* Offset from Dcookie */
+
+	/* we send the Dcookie offset, but send the raw Linear Add also*/
+	add_event_entry(IBS_EIP(cpu_buf->tail_pos));
+	add_event_entry(IBS_EVENT(cpu_buf->tail_pos));
+
+	if (code == IBS_FETCH_CODE)
+		count = IBS_FETCH_CODE_SIZE;	/*IBS FETCH is 2 int64s*/
+	else
+		count = IBS_OP_CODE_SIZE;	/*IBS OP is 5 int64s*/
+
+	for (i = 0; i < count; i++) {
+		increment_tail(cpu_buf);
+		add_event_entry(IBS_EIP(cpu_buf->tail_pos));
+		add_event_entry(IBS_EVENT(cpu_buf->tail_pos));
+	}
+}
 
 static void add_sample_entry(unsigned long offset, unsigned long event)
 {
@@ -524,6 +586,14 @@ void sync_buffer(int cpu)
 			} else if (s->event == CPU_TRACE_BEGIN) {
 				state = sb_bt_start;
 				add_trace_begin();
+			} else if (s->event == IBS_FETCH_BEGIN) {
+				state = sb_bt_start;
+				add_ibs_begin(cpu_buf,
+					IBS_FETCH_CODE, in_kernel, mm);
+			} else if (s->event == IBS_OP_BEGIN) {
+				state = sb_bt_start;
+				add_ibs_begin(cpu_buf,
+					IBS_OP_CODE, in_kernel, mm);
 			} else {
 				struct mm_struct *oldmm = mm;
 
diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index 2450b3a..c9ac4e1 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -5,6 +5,7 @@
  * @remark Read the file COPYING
  *
  * @author John Levon <levon@movementarian.org>
+ * @author Barry Kasindorf <barry.kasindorf@amd.com>
  *
  * Each CPU has a local buffer that stores PC value/event
  * pairs. We also log context switches when we notice them.
@@ -207,7 +208,7 @@ static int log_sample(struct oprofile_cpu_buffer * cpu_buf, unsigned long pc,
 	return 1;
 }
 
-static int oprofile_begin_trace(struct oprofile_cpu_buffer * cpu_buf)
+static int oprofile_begin_trace(struct oprofile_cpu_buffer *cpu_buf)
 {
 	if (nr_available_slots(cpu_buf) < 4) {
 		cpu_buf->sample_lost_overflow++;
@@ -252,6 +253,71 @@ void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
 	oprofile_add_ext_sample(pc, regs, event, is_kernel);
 }
 
+#define MAX_IBS_SAMPLE_SIZE	14
+static int log_ibs_sample(struct oprofile_cpu_buffer *cpu_buf,
+	unsigned long pc, int is_kernel, unsigned  int *ibs, int ibs_code)
+{
+	struct task_struct *task;
+
+	cpu_buf->sample_received++;
+
+	if (nr_available_slots(cpu_buf) < MAX_IBS_SAMPLE_SIZE) {
+		cpu_buf->sample_lost_overflow++;
+		return 0;
+	}
+
+	is_kernel = !!is_kernel;
+
+	/* notice a switch from user->kernel or vice versa */
+	if (cpu_buf->last_is_kernel != is_kernel) {
+		cpu_buf->last_is_kernel = is_kernel;
+		add_code(cpu_buf, is_kernel);
+	}
+
+	/* notice a task switch */
+	if (!is_kernel) {
+		task = current;
+
+		if (cpu_buf->last_task != task) {
+			cpu_buf->last_task = task;
+			add_code(cpu_buf, (unsigned long)task);
+		}
+	}
+
+	add_code(cpu_buf, ibs_code);
+	add_sample(cpu_buf, ibs[0], ibs[1]);
+	add_sample(cpu_buf, ibs[2], ibs[3]);
+	add_sample(cpu_buf, ibs[4], ibs[5]);
+
+	if (ibs_code == IBS_OP_BEGIN) {
+	add_sample(cpu_buf, ibs[6], ibs[7]);
+	add_sample(cpu_buf, ibs[8], ibs[9]);
+	add_sample(cpu_buf, ibs[10], ibs[11]);
+	}
+
+	return 1;
+}
+
+void oprofile_add_ibs_sample(struct pt_regs *const regs,
+				unsigned int * const ibs_sample, u8 code)
+{
+	int is_kernel = !user_mode(regs);
+	unsigned long pc = profile_pc(regs);
+
+	struct oprofile_cpu_buffer *cpu_buf =
+			 &per_cpu(cpu_buffer, smp_processor_id());
+
+	if (!backtrace_depth) {
+		log_ibs_sample(cpu_buf, pc, is_kernel, ibs_sample, code);
+		return;
+	}
+
+	/* if log_sample() fails we can't backtrace since we lost the source
+	* of this event */
+	if (log_ibs_sample(cpu_buf, pc, is_kernel, ibs_sample, code))
+		oprofile_ops.backtrace(regs, backtrace_depth);
+}
+
 void oprofile_add_pc(unsigned long pc, int is_kernel, unsigned long event)
 {
 	struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer);
diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h
index c3e366b..9c44d00 100644
--- a/drivers/oprofile/cpu_buffer.h
+++ b/drivers/oprofile/cpu_buffer.h
@@ -55,5 +55,7 @@ void cpu_buffer_reset(struct oprofile_cpu_buffer * cpu_buf);
 /* transient events for the CPU buffer -> event buffer */
 #define CPU_IS_KERNEL 1
 #define CPU_TRACE_BEGIN 2
+#define IBS_FETCH_BEGIN 3
+#define IBS_OP_BEGIN    4
 
 #endif /* OPROFILE_CPU_BUFFER_H */
-- 
1.5.5.4

Re: [PATCH 10/24] x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines

[Maynard Johnson]

Robert Richter wrote:
> From: Barry Kasindorf <barry.kasindorf@amd.com>
>
> This patchset supports the new profiling hardware available in the
> latest AMD CPUs in the oProfile driver.
>
> Signed-off-by: Barry Kasindorf <barry.kasindorf@amd.com>
> Signed-off-by: Robert Richter <robert.richter@amd.com>
> ---
>  drivers/oprofile/buffer_sync.c |   72 +++++++++++++++++++++++++++++++++++++++-
>  drivers/oprofile/cpu_buffer.c  |   68 +++++++++++++++++++++++++++++++++++++-
>  drivers/oprofile/cpu_buffer.h  |    2 +
>  3 files changed, 140 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
> index 615929f..e1782d2 100644
> --- a/drivers/oprofile/buffer_sync.c
> +++ b/drivers/oprofile/buffer_sync.c
> @@ -5,6 +5,7 @@
>   * @remark Read the file COPYING
>   *
>   * @author John Levon <levon@movementarian.org>
> + * @author Barry Kasindorf
>   *
>   * This is the core of the buffer management. Each
>   * CPU buffer is processed and entered into the
> @@ -272,7 +273,7 @@ static void increment_tail(struct oprofile_cpu_buffer *b)
>  {
>  	unsigned long new_tail = b->tail_pos + 1;
>
> -	rmb();
> +	rmb();	/* be sure fifo pointers are synchromized */
>
>  	if (new_tail < b->buffer_size)
>  		b->tail_pos = new_tail;
> @@ -327,6 +328,67 @@ static void add_trace_begin(void)
>  	add_event_entry(TRACE_BEGIN_CODE);
>  }
>
> +#define IBS_FETCH_CODE_SIZE	2
> +#define IBS_OP_CODE_SIZE	5
> +#define IBS_EIP(offset)				\
> +	(((struct op_sample *)&cpu_buf->buffer[(offset)])->eip)
> +#define IBS_EVENT(offset)				\
> +	(((struct op_sample *)&cpu_buf->buffer[(offset)])->event)
> +
> +/*
> + * Add IBS fetch and op entries to event buffer
> + */
> +static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code,
> +	int in_kernel, struct mm_struct *mm)
> +{
> +	unsigned long rip;
> +	int i, count;
> +	unsigned long ibs_cookie = 0;
> +	off_t offset;
> +
> +	increment_tail(cpu_buf);	/* move to RIP entry */
> +
> +	rip = IBS_EIP(cpu_buf->tail_pos);
> +
> +#ifdef __LP64__
> +	rip += IBS_EVENT(cpu_buf->tail_pos) << 32;
> +#endif
> +
> +	if (mm) {
> +		ibs_cookie = lookup_dcookie(mm, rip, &offset);
> +
> +		if (ibs_cookie == NO_COOKIE)
> +			offset = rip;
> +		if (ibs_cookie == INVALID_COOKIE) {
> +			atomic_inc(&oprofile_stats.sample_lost_no_mapping);
> +			offset = rip;
> +		}
> +		if (ibs_cookie != last_cookie) {
> +			add_cookie_switch(ibs_cookie);
> +			last_cookie = ibs_cookie;
> +		}
> +	} else
> +		offset = rip;
> +
> +	add_event_entry(ESCAPE_CODE);
> +	add_event_entry(code);
> +	add_event_entry(offset);	/* Offset from Dcookie */
> +
> +	/* we send the Dcookie offset, but send the raw Linear Add also*/
> +	add_event_entry(IBS_EIP(cpu_buf->tail_pos));
> +	add_event_entry(IBS_EVENT(cpu_buf->tail_pos));
> +
> +	if (code == IBS_FETCH_CODE)
> +		count = IBS_FETCH_CODE_SIZE;	/*IBS FETCH is 2 int64s*/
> +	else
> +		count = IBS_OP_CODE_SIZE;	/*IBS OP is 5 int64s*/
> +
> +	for (i = 0; i < count; i++) {
> +		increment_tail(cpu_buf);
> +		add_event_entry(IBS_EIP(cpu_buf->tail_pos));
> +		add_event_entry(IBS_EVENT(cpu_buf->tail_pos));
> +	}
> +}
>   
>  static void add_sample_entry(unsigned long offset, unsigned long event)
>  {
> @@ -524,6 +586,14 @@ void sync_buffer(int cpu)
>  			} else if (s->event == CPU_TRACE_BEGIN) {
>  				state = sb_bt_start;
>  				add_trace_begin();
> +			} else if (s->event == IBS_FETCH_BEGIN) {
> +				state = sb_bt_start;
> +				add_ibs_begin(cpu_buf,
> +					IBS_FETCH_CODE, in_kernel, mm);
> +			} else if (s->event == IBS_OP_BEGIN) {
> +				state = sb_bt_start;
> +				add_ibs_begin(cpu_buf,
> +					IBS_OP_CODE, in_kernel, mm);
>  			} else {
>  				struct mm_struct *oldmm = mm;
>
> diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
> index 2450b3a..c9ac4e1 100644
> --- a/drivers/oprofile/cpu_buffer.c
> +++ b/drivers/oprofile/cpu_buffer.c
> @@ -5,6 +5,7 @@
>   * @remark Read the file COPYING
>   *
>   * @author John Levon <levon@movementarian.org>
> + * @author Barry Kasindorf <barry.kasindorf@amd.com>
>   *
>   * Each CPU has a local buffer that stores PC value/event
>   * pairs. We also log context switches when we notice them.
> @@ -207,7 +208,7 @@ static int log_sample(struct oprofile_cpu_buffer * cpu_buf, unsigned long pc,
>  	return 1;
>  }
>
> -static int oprofile_begin_trace(struct oprofile_cpu_buffer * cpu_buf)
> +static int oprofile_begin_trace(struct oprofile_cpu_buffer *cpu_buf)
>  {
>  	if (nr_available_slots(cpu_buf) < 4) {
>  		cpu_buf->sample_lost_overflow++;
> @@ -252,6 +253,71 @@ void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
>  	oprofile_add_ext_sample(pc, regs, event, is_kernel);
>  }
>
> +#define MAX_IBS_SAMPLE_SIZE	14
> +static int log_ibs_sample(struct oprofile_cpu_buffer *cpu_buf,
> +	unsigned long pc, int is_kernel, unsigned  int *ibs, int ibs_code)
> +{
> +	struct task_struct *task;
> +
> +	cpu_buf->sample_received++;
> +
> +	if (nr_available_slots(cpu_buf) < MAX_IBS_SAMPLE_SIZE) {
> +		cpu_buf->sample_lost_overflow++;
> +		return 0;
> +	}
> +
> +	is_kernel = !!is_kernel;
> +
> +	/* notice a switch from user->kernel or vice versa */
> +	if (cpu_buf->last_is_kernel != is_kernel) {
> +		cpu_buf->last_is_kernel = is_kernel;
> +		add_code(cpu_buf, is_kernel);
> +	}
> +
> +	/* notice a task switch */
> +	if (!is_kernel) {
> +		task = current;
> +
> +		if (cpu_buf->last_task != task) {
> +			cpu_buf->last_task = task;
> +			add_code(cpu_buf, (unsigned long)task);
> +		}
> +	}
> +
> +	add_code(cpu_buf, ibs_code);
> +	add_sample(cpu_buf, ibs[0], ibs[1]);
> +	add_sample(cpu_buf, ibs[2], ibs[3]);
> +	add_sample(cpu_buf, ibs[4], ibs[5]);
> +
> +	if (ibs_code == IBS_OP_BEGIN) {
> +	add_sample(cpu_buf, ibs[6], ibs[7]);
> +	add_sample(cpu_buf, ibs[8], ibs[9]);
> +	add_sample(cpu_buf, ibs[10], ibs[11]);
> +	}
> +
> +	return 1;
> +}
> +
>   
I'm concerned about the arch-specific nature of the "ibs" functions
above being placed here in the generic portion of the oprofile driver. 
Better to generalize the external function defined below (and rename it)
by invoking arch-specific handlers via function pointers.  Hopefully
find a way to move the arch-specific code back to where it belongs,
under the appropriate arch/ directory.

-Maynard
> +void oprofile_add_ibs_sample(struct pt_regs *const regs,
> +				unsigned int * const ibs_sample, u8 code)
> +{
> +	int is_kernel = !user_mode(regs);
> +	unsigned long pc = profile_pc(regs);
> +
> +	struct oprofile_cpu_buffer *cpu_buf =
> +			 &per_cpu(cpu_buffer, smp_processor_id());
> +
> +	if (!backtrace_depth) {
> +		log_ibs_sample(cpu_buf, pc, is_kernel, ibs_sample, code);
> +		return;
> +	}
> +
> +	/* if log_sample() fails we can't backtrace since we lost the source
> +	* of this event */
> +	if (log_ibs_sample(cpu_buf, pc, is_kernel, ibs_sample, code))
> +		oprofile_ops.backtrace(regs, backtrace_depth);
> +}
> +
>  void oprofile_add_pc(unsigned long pc, int is_kernel, unsigned long event)
>  {
>  	struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer);
> diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h
> index c3e366b..9c44d00 100644
> --- a/drivers/oprofile/cpu_buffer.h
> +++ b/drivers/oprofile/cpu_buffer.h
> @@ -55,5 +55,7 @@ void cpu_buffer_reset(struct oprofile_cpu_buffer * cpu_buf);
>  /* transient events for the CPU buffer -> event buffer */
>  #define CPU_IS_KERNEL 1
>  #define CPU_TRACE_BEGIN 2
> +#define IBS_FETCH_BEGIN 3
> +#define IBS_OP_BEGIN    4
>
>  #endif /* OPROFILE_CPU_BUFFER_H */
>   

Re: [PATCH 10/24] x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines

[Robert Richter]

On 23.07.08 14:20:37, Maynard Johnson wrote:
> Robert Richter wrote:

[...]

> > @@ -252,6 +253,71 @@ void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
> >  	oprofile_add_ext_sample(pc, regs, event, is_kernel);
> >  }
> >
> > +#define MAX_IBS_SAMPLE_SIZE	14
> > +static int log_ibs_sample(struct oprofile_cpu_buffer *cpu_buf,
> > +	unsigned long pc, int is_kernel, unsigned  int *ibs, int ibs_code)
> > +{
> > +	struct task_struct *task;
> > +
> > +	cpu_buf->sample_received++;
> > +
> > +	if (nr_available_slots(cpu_buf) < MAX_IBS_SAMPLE_SIZE) {
> > +		cpu_buf->sample_lost_overflow++;
> > +		return 0;
> > +	}
> > +
> > +	is_kernel = !!is_kernel;
> > +
> > +	/* notice a switch from user->kernel or vice versa */
> > +	if (cpu_buf->last_is_kernel != is_kernel) {
> > +		cpu_buf->last_is_kernel = is_kernel;
> > +		add_code(cpu_buf, is_kernel);
> > +	}
> > +
> > +	/* notice a task switch */
> > +	if (!is_kernel) {
> > +		task = current;
> > +
> > +		if (cpu_buf->last_task != task) {
> > +			cpu_buf->last_task = task;
> > +			add_code(cpu_buf, (unsigned long)task);
> > +		}
> > +	}
> > +
> > +	add_code(cpu_buf, ibs_code);
> > +	add_sample(cpu_buf, ibs[0], ibs[1]);
> > +	add_sample(cpu_buf, ibs[2], ibs[3]);
> > +	add_sample(cpu_buf, ibs[4], ibs[5]);
> > +
> > +	if (ibs_code == IBS_OP_BEGIN) {
> > +	add_sample(cpu_buf, ibs[6], ibs[7]);
> > +	add_sample(cpu_buf, ibs[8], ibs[9]);
> > +	add_sample(cpu_buf, ibs[10], ibs[11]);
> > +	}
> > +
> > +	return 1;
> > +}
> > +
> >   
> I'm concerned about the arch-specific nature of the "ibs" functions
> above being placed here in the generic portion of the oprofile driver. 
> Better to generalize the external function defined below (and rename it)
> by invoking arch-specific handlers via function pointers.  Hopefully
> find a way to move the arch-specific code back to where it belongs,
> under the appropriate arch/ directory.

Yes, this is true. The plan is to rework the code so that IBS is only
in op_model_amd.c. The only thing visible outside will then be the
code macros introduced in patch #9. The code will change into generic
with something like add_u64(). This function will then be called from
the IBS handler.

This is also the reason I did not add op_amd_handle_ibs() to the API
definition in linux/oprofile.h.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [PATCH 10/24] x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines

[Carl Love]

On Tue, 2008-07-22 at 21:08 +0200, Robert Richter wrote:
> From: Barry Kasindorf <barry.kasindorf@amd.com>
> 
> This patchset supports the new profiling hardware available in the
> latest AMD CPUs in the oProfile driver.
> 
> Signed-off-by: Barry Kasindorf <barry.kasindorf@amd.com>
> Signed-off-by: Robert Richter <robert.richter@amd.com>
> ---
>  drivers/oprofile/buffer_sync.c |   72 +++++++++++++++++++++++++++++++++++++++-
>  drivers/oprofile/cpu_buffer.c  |   68 +++++++++++++++++++++++++++++++++++++-
>  drivers/oprofile/cpu_buffer.h  |    2 +
>  3 files changed, 140 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
> index 615929f..e1782d2 100644
> --- a/drivers/oprofile/buffer_sync.c
> +++ b/drivers/oprofile/buffer_sync.c
> @@ -5,6 +5,7 @@
>   * @remark Read the file COPYING
>   *
>   * @author John Levon <levon@movementarian.org>
> + * @author Barry Kasindorf
>   *
>   * This is the core of the buffer management. Each
>   * CPU buffer is processed and entered into the
> @@ -272,7 +273,7 @@ static void increment_tail(struct oprofile_cpu_buffer *b)
>  {
>  	unsigned long new_tail = b->tail_pos + 1;
> 
> -	rmb();
> +	rmb();	/* be sure fifo pointers are synchromized */
> 
>  	if (new_tail < b->buffer_size)
>  		b->tail_pos = new_tail;
> @@ -327,6 +328,67 @@ static void add_trace_begin(void)
>  	add_event_entry(TRACE_BEGIN_CODE);
>  }
> 
> +#define IBS_FETCH_CODE_SIZE	2
> +#define IBS_OP_CODE_SIZE	5
> +#define IBS_EIP(offset)				\
> +	(((struct op_sample *)&cpu_buf->buffer[(offset)])->eip)
> +#define IBS_EVENT(offset)				\
> +	(((struct op_sample *)&cpu_buf->buffer[(offset)])->event)
> +
> +/*
> + * Add IBS fetch and op entries to event buffer
> + */
> +static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code,
> +	int in_kernel, struct mm_struct *mm)
> +{
> +	unsigned long rip;
> +	int i, count;
> +	unsigned long ibs_cookie = 0;
> +	off_t offset;
> +
> +	increment_tail(cpu_buf);	/* move to RIP entry */
> +
> +	rip = IBS_EIP(cpu_buf->tail_pos);
> +
> +#ifdef __LP64__
> +	rip += IBS_EVENT(cpu_buf->tail_pos) << 32;
> +#endif
> +
> +	if (mm) {
> +		ibs_cookie = lookup_dcookie(mm, rip, &offset);
> +
> +		if (ibs_cookie == NO_COOKIE)
> +			offset = rip;
> +		if (ibs_cookie == INVALID_COOKIE) {
> +			atomic_inc(&oprofile_stats.sample_lost_no_mapping);
> +			offset = rip;
> +		}
> +		if (ibs_cookie != last_cookie) {
> +			add_cookie_switch(ibs_cookie);
> +			last_cookie = ibs_cookie;
> +		}
> +	} else
> +		offset = rip;
> +
> +	add_event_entry(ESCAPE_CODE);
> +	add_event_entry(code);
> +	add_event_entry(offset);	/* Offset from Dcookie */
> +
> +	/* we send the Dcookie offset, but send the raw Linear Add also*/
> +	add_event_entry(IBS_EIP(cpu_buf->tail_pos));
> +	add_event_entry(IBS_EVENT(cpu_buf->tail_pos));
> +
> +	if (code == IBS_FETCH_CODE)
> +		count = IBS_FETCH_CODE_SIZE;	/*IBS FETCH is 2 int64s*/
> +	else
> +		count = IBS_OP_CODE_SIZE;	/*IBS OP is 5 int64s*/
> +
> +	for (i = 0; i < count; i++) {
> +		increment_tail(cpu_buf);
> +		add_event_entry(IBS_EIP(cpu_buf->tail_pos));
> +		add_event_entry(IBS_EVENT(cpu_buf->tail_pos));
> +	}
> +}

In general, I think it would be good to make the IBS stuff as arch
independent as possible.  Could you move the above function to the arch
specific code.  Here a generic function pointer could be exported which
would be assigned to the arch specific routine.  This would enable
another architecture to reuse this code.  

> 
>  static void add_sample_entry(unsigned long offset, unsigned long event)
>  {
> @@ -524,6 +586,14 @@ void sync_buffer(int cpu)
>  			} else if (s->event == CPU_TRACE_BEGIN) {
>  				state = sb_bt_start;
>  				add_trace_begin();
> +			} else if (s->event == IBS_FETCH_BEGIN) {
> +				state = sb_bt_start;
> +				add_ibs_begin(cpu_buf,
> +					IBS_FETCH_CODE, in_kernel, mm);
> +			} else if (s->event == IBS_OP_BEGIN) {
> +				state = sb_bt_start;
> +				add_ibs_begin(cpu_buf,
> +					IBS_OP_CODE, in_kernel, mm);
>  			} else {
>  				struct mm_struct *oldmm = mm;
> 

If you made the log_ibs_sample() more generic, as discussed below, then
the #defines could be changed to generic names and thus allow other
architectures to leverage the same code.  


> diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
> index 2450b3a..c9ac4e1 100644
> --- a/drivers/oprofile/cpu_buffer.c
> +++ b/drivers/oprofile/cpu_buffer.c
> @@ -5,6 +5,7 @@
>   * @remark Read the file COPYING
>   *
>   * @author John Levon <levon@movementarian.org>
> + * @author Barry Kasindorf <barry.kasindorf@amd.com>
>   *
>   * Each CPU has a local buffer that stores PC value/event
>   * pairs. We also log context switches when we notice them.
> @@ -207,7 +208,7 @@ static int log_sample(struct oprofile_cpu_buffer * cpu_buf, unsigned long pc,
>  	return 1;
>  }
> 
> -static int oprofile_begin_trace(struct oprofile_cpu_buffer * cpu_buf)
> +static int oprofile_begin_trace(struct oprofile_cpu_buffer *cpu_buf)
>  {
>  	if (nr_available_slots(cpu_buf) < 4) {
>  		cpu_buf->sample_lost_overflow++;
> @@ -252,6 +253,71 @@ void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
>  	oprofile_add_ext_sample(pc, regs, event, is_kernel);
>  }
> 
> +#define MAX_IBS_SAMPLE_SIZE	14
> +static int log_ibs_sample(struct oprofile_cpu_buffer *cpu_buf,
> +	unsigned long pc, int is_kernel, unsigned  int *ibs, int ibs_code)
> +{
> +	struct task_struct *task;
> +
> +	cpu_buf->sample_received++;
> +
> +	if (nr_available_slots(cpu_buf) < MAX_IBS_SAMPLE_SIZE) {
> +		cpu_buf->sample_lost_overflow++;
> +		return 0;
> +	}
> +
> +	is_kernel = !!is_kernel;
> +
> +	/* notice a switch from user->kernel or vice versa */
> +	if (cpu_buf->last_is_kernel != is_kernel) {
> +		cpu_buf->last_is_kernel = is_kernel;
> +		add_code(cpu_buf, is_kernel);
> +	}
> +
> +	/* notice a task switch */
> +	if (!is_kernel) {
> +		task = current;
> +
> +		if (cpu_buf->last_task != task) {
> +			cpu_buf->last_task = task;
> +			add_code(cpu_buf, (unsigned long)task);
> +		}
> +	}
> +
> +	add_code(cpu_buf, ibs_code);
> +	add_sample(cpu_buf, ibs[0], ibs[1]);
> +	add_sample(cpu_buf, ibs[2], ibs[3]);
> +	add_sample(cpu_buf, ibs[4], ibs[5]);
> +
> +	if (ibs_code == IBS_OP_BEGIN) {
> +	add_sample(cpu_buf, ibs[6], ibs[7]);
> +	add_sample(cpu_buf, ibs[8], ibs[9]);
> +	add_sample(cpu_buf, ibs[10], ibs[11]);
> +	}
> +
> +	return 1;
> +}
> +

Can we make this function a bit more general?  Specifically, suppose 

static int log_generic_sample (struct oprofile_cpu_buffer *cpu_buf, 
                                 unsigned int *array, int num_entries) 

If we just passed in an array of data that is ready to just be copied
directly to 
the kernel buffer in a for loop: 

for (i=0; i<num_entries; i=i+2) 

        add_sample(cpu_buf, array[i], array[i+1]); 

The arch specific code would have to do the if(ibs_code == IBS_OP_BEGIN)
code to either add the stuff into the input array or not.  The
appropriate number of entries would be passed.   

The  if (cpu_buf->last_is_kernel != is_kernel) statement might be hard
to do from the architecture code and might have to stay in the
log_generic_sample. It would be good if we could find a way to also move
this line of code to the arch specific code to setup the generic array
of data to pass to log_generic_sample(). Maybe the arch specific code
could have an array of last_is_kernel to refer to when preparing the
data for the log_generic_sample() call.

By making this more generic and flexible in the number of entries and
pushing the logic of what to put into the array into the arch specific
code, it could be used by other architectures.  I had something along
the lines of the above log_generic_sample() in a version of a patch for
the CELL architecture.  On CELL, I have to take samples from the various
SPUs and push them into the kernel buffer.  I had tried to do it using
the per CPU buffers.  I ended up dropping the approach for other
reasons.  The point is I can see where something a little more
generic/flexible could be used by other architectures.

> +void oprofile_add_ibs_sample(struct pt_regs *const regs,
> +				unsigned int * const ibs_sample, u8 code)
> +{
> +	int is_kernel = !user_mode(regs);
> +	unsigned long pc = profile_pc(regs);
> +
> +	struct oprofile_cpu_buffer *cpu_buf =
> +			 &per_cpu(cpu_buffer, smp_processor_id());
> +
> +	if (!backtrace_depth) {
> +		log_ibs_sample(cpu_buf, pc, is_kernel, ibs_sample, code);
> +		return;
> +	}
> +
> +	/* if log_sample() fails we can't backtrace since we lost the source
> +	* of this event */
> +	if (log_ibs_sample(cpu_buf, pc, is_kernel, ibs_sample, code))
> +		oprofile_ops.backtrace(regs, backtrace_depth);
> +}
> +
>  void oprofile_add_pc(unsigned long pc, int is_kernel, unsigned long event)
>  {
>  	struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer);
> diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h
> index c3e366b..9c44d00 100644
> --- a/drivers/oprofile/cpu_buffer.h
> +++ b/drivers/oprofile/cpu_buffer.h
> @@ -55,5 +55,7 @@ void cpu_buffer_reset(struct oprofile_cpu_buffer * cpu_buf);
>  /* transient events for the CPU buffer -> event buffer */
>  #define CPU_IS_KERNEL 1
>  #define CPU_TRACE_BEGIN 2
> +#define IBS_FETCH_BEGIN 3
> +#define IBS_OP_BEGIN    4
> 
>  #endif /* OPROFILE_CPU_BUFFER_H */

Re: [PATCH 10/24] x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines

[Robert Richter]

On 23.07.08 13:01:57, Carl Love wrote:
> 
> On Tue, 2008-07-22 at 21:08 +0200, Robert Richter wrote:
> > From: Barry Kasindorf <barry.kasindorf@amd.com>
> > 
> > This patchset supports the new profiling hardware available in the
> > latest AMD CPUs in the oProfile driver.
> > 
> > Signed-off-by: Barry Kasindorf <barry.kasindorf@amd.com>
> > Signed-off-by: Robert Richter <robert.richter@amd.com>
> > ---
> >  drivers/oprofile/buffer_sync.c |   72 +++++++++++++++++++++++++++++++++++++++-
> >  drivers/oprofile/cpu_buffer.c  |   68 +++++++++++++++++++++++++++++++++++++-
> >  drivers/oprofile/cpu_buffer.h  |    2 +
> >  3 files changed, 140 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
> > index 615929f..e1782d2 100644
> > --- a/drivers/oprofile/buffer_sync.c
> > +++ b/drivers/oprofile/buffer_sync.c
> > @@ -5,6 +5,7 @@
> >   * @remark Read the file COPYING
> >   *
> >   * @author John Levon <levon@movementarian.org>
> > + * @author Barry Kasindorf
> >   *
> >   * This is the core of the buffer management. Each
> >   * CPU buffer is processed and entered into the
> > @@ -272,7 +273,7 @@ static void increment_tail(struct oprofile_cpu_buffer *b)
> >  {
> >  	unsigned long new_tail = b->tail_pos + 1;
> > 
> > -	rmb();
> > +	rmb();	/* be sure fifo pointers are synchromized */
> > 
> >  	if (new_tail < b->buffer_size)
> >  		b->tail_pos = new_tail;
> > @@ -327,6 +328,67 @@ static void add_trace_begin(void)
> >  	add_event_entry(TRACE_BEGIN_CODE);
> >  }
> > 
> > +#define IBS_FETCH_CODE_SIZE	2
> > +#define IBS_OP_CODE_SIZE	5
> > +#define IBS_EIP(offset)				\
> > +	(((struct op_sample *)&cpu_buf->buffer[(offset)])->eip)
> > +#define IBS_EVENT(offset)				\
> > +	(((struct op_sample *)&cpu_buf->buffer[(offset)])->event)
> > +
> > +/*
> > + * Add IBS fetch and op entries to event buffer
> > + */
> > +static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code,
> > +	int in_kernel, struct mm_struct *mm)
> > +{
> > +	unsigned long rip;
> > +	int i, count;
> > +	unsigned long ibs_cookie = 0;
> > +	off_t offset;
> > +
> > +	increment_tail(cpu_buf);	/* move to RIP entry */
> > +
> > +	rip = IBS_EIP(cpu_buf->tail_pos);
> > +
> > +#ifdef __LP64__
> > +	rip += IBS_EVENT(cpu_buf->tail_pos) << 32;
> > +#endif
> > +
> > +	if (mm) {
> > +		ibs_cookie = lookup_dcookie(mm, rip, &offset);
> > +
> > +		if (ibs_cookie == NO_COOKIE)
> > +			offset = rip;
> > +		if (ibs_cookie == INVALID_COOKIE) {
> > +			atomic_inc(&oprofile_stats.sample_lost_no_mapping);
> > +			offset = rip;
> > +		}
> > +		if (ibs_cookie != last_cookie) {
> > +			add_cookie_switch(ibs_cookie);
> > +			last_cookie = ibs_cookie;
> > +		}
> > +	} else
> > +		offset = rip;
> > +
> > +	add_event_entry(ESCAPE_CODE);
> > +	add_event_entry(code);
> > +	add_event_entry(offset);	/* Offset from Dcookie */
> > +
> > +	/* we send the Dcookie offset, but send the raw Linear Add also*/
> > +	add_event_entry(IBS_EIP(cpu_buf->tail_pos));
> > +	add_event_entry(IBS_EVENT(cpu_buf->tail_pos));
> > +
> > +	if (code == IBS_FETCH_CODE)
> > +		count = IBS_FETCH_CODE_SIZE;	/*IBS FETCH is 2 int64s*/
> > +	else
> > +		count = IBS_OP_CODE_SIZE;	/*IBS OP is 5 int64s*/
> > +
> > +	for (i = 0; i < count; i++) {
> > +		increment_tail(cpu_buf);
> > +		add_event_entry(IBS_EIP(cpu_buf->tail_pos));
> > +		add_event_entry(IBS_EVENT(cpu_buf->tail_pos));
> > +	}
> > +}
> 
> In general, I think it would be good to make the IBS stuff as arch
> independent as possible.  Could you move the above function to the arch
> specific code.  Here a generic function pointer could be exported which
> would be assigned to the arch specific routine.  This would enable
> another architecture to reuse this code.  

Yes, this will change too (see my mail to Maynard). No IBS outside of
op_model_amd.c. The solution is copy the samples directly in
sync_buffer() until a new escape code is received. Internals of the
samples (type, size, etc.) are not needed in this case. The address
convertion will use existion functions as well.

-Robert

> 
> > 
> >  static void add_sample_entry(unsigned long offset, unsigned long event)
> >  {
> > @@ -524,6 +586,14 @@ void sync_buffer(int cpu)
> >  			} else if (s->event == CPU_TRACE_BEGIN) {
> >  				state = sb_bt_start;
> >  				add_trace_begin();
> > +			} else if (s->event == IBS_FETCH_BEGIN) {
> > +				state = sb_bt_start;
> > +				add_ibs_begin(cpu_buf,
> > +					IBS_FETCH_CODE, in_kernel, mm);
> > +			} else if (s->event == IBS_OP_BEGIN) {
> > +				state = sb_bt_start;
> > +				add_ibs_begin(cpu_buf,
> > +					IBS_OP_CODE, in_kernel, mm);
> >  			} else {
> >  				struct mm_struct *oldmm = mm;
> > 
> 
> If you made the log_ibs_sample() more generic, as discussed below, then
> the #defines could be changed to generic names and thus allow other
> architectures to leverage the same code.  
> 
> 
> > diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
> > index 2450b3a..c9ac4e1 100644
> > --- a/drivers/oprofile/cpu_buffer.c
> > +++ b/drivers/oprofile/cpu_buffer.c
> > @@ -5,6 +5,7 @@
> >   * @remark Read the file COPYING
> >   *
> >   * @author John Levon <levon@movementarian.org>
> > + * @author Barry Kasindorf <barry.kasindorf@amd.com>
> >   *
> >   * Each CPU has a local buffer that stores PC value/event
> >   * pairs. We also log context switches when we notice them.
> > @@ -207,7 +208,7 @@ static int log_sample(struct oprofile_cpu_buffer * cpu_buf, unsigned long pc,
> >  	return 1;
> >  }
> > 
> > -static int oprofile_begin_trace(struct oprofile_cpu_buffer * cpu_buf)
> > +static int oprofile_begin_trace(struct oprofile_cpu_buffer *cpu_buf)
> >  {
> >  	if (nr_available_slots(cpu_buf) < 4) {
> >  		cpu_buf->sample_lost_overflow++;
> > @@ -252,6 +253,71 @@ void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
> >  	oprofile_add_ext_sample(pc, regs, event, is_kernel);
> >  }
> > 
> > +#define MAX_IBS_SAMPLE_SIZE	14
> > +static int log_ibs_sample(struct oprofile_cpu_buffer *cpu_buf,
> > +	unsigned long pc, int is_kernel, unsigned  int *ibs, int ibs_code)
> > +{
> > +	struct task_struct *task;
> > +
> > +	cpu_buf->sample_received++;
> > +
> > +	if (nr_available_slots(cpu_buf) < MAX_IBS_SAMPLE_SIZE) {
> > +		cpu_buf->sample_lost_overflow++;
> > +		return 0;
> > +	}
> > +
> > +	is_kernel = !!is_kernel;
> > +
> > +	/* notice a switch from user->kernel or vice versa */
> > +	if (cpu_buf->last_is_kernel != is_kernel) {
> > +		cpu_buf->last_is_kernel = is_kernel;
> > +		add_code(cpu_buf, is_kernel);
> > +	}
> > +
> > +	/* notice a task switch */
> > +	if (!is_kernel) {
> > +		task = current;
> > +
> > +		if (cpu_buf->last_task != task) {
> > +			cpu_buf->last_task = task;
> > +			add_code(cpu_buf, (unsigned long)task);
> > +		}
> > +	}
> > +
> > +	add_code(cpu_buf, ibs_code);
> > +	add_sample(cpu_buf, ibs[0], ibs[1]);
> > +	add_sample(cpu_buf, ibs[2], ibs[3]);
> > +	add_sample(cpu_buf, ibs[4], ibs[5]);
> > +
> > +	if (ibs_code == IBS_OP_BEGIN) {
> > +	add_sample(cpu_buf, ibs[6], ibs[7]);
> > +	add_sample(cpu_buf, ibs[8], ibs[9]);
> > +	add_sample(cpu_buf, ibs[10], ibs[11]);
> > +	}
> > +
> > +	return 1;
> > +}
> > +
> 
> Can we make this function a bit more general?  Specifically, suppose 
> 
> static int log_generic_sample (struct oprofile_cpu_buffer *cpu_buf, 
>                                  unsigned int *array, int num_entries) 
> 
> If we just passed in an array of data that is ready to just be copied
> directly to 
> the kernel buffer in a for loop: 
> 
> for (i=0; i<num_entries; i=i+2) 
> 
>         add_sample(cpu_buf, array[i], array[i+1]); 
> 
> The arch specific code would have to do the if(ibs_code == IBS_OP_BEGIN)
> code to either add the stuff into the input array or not.  The
> appropriate number of entries would be passed.   
> 
> The  if (cpu_buf->last_is_kernel != is_kernel) statement might be hard
> to do from the architecture code and might have to stay in the
> log_generic_sample. It would be good if we could find a way to also move
> this line of code to the arch specific code to setup the generic array
> of data to pass to log_generic_sample(). Maybe the arch specific code
> could have an array of last_is_kernel to refer to when preparing the
> data for the log_generic_sample() call.
> 
> By making this more generic and flexible in the number of entries and
> pushing the logic of what to put into the array into the arch specific
> code, it could be used by other architectures.  I had something along
> the lines of the above log_generic_sample() in a version of a patch for
> the CELL architecture.  On CELL, I have to take samples from the various
> SPUs and push them into the kernel buffer.  I had tried to do it using
> the per CPU buffers.  I ended up dropping the approach for other
> reasons.  The point is I can see where something a little more
> generic/flexible could be used by other architectures.
> 
> > +void oprofile_add_ibs_sample(struct pt_regs *const regs,
> > +				unsigned int * const ibs_sample, u8 code)
> > +{
> > +	int is_kernel = !user_mode(regs);
> > +	unsigned long pc = profile_pc(regs);
> > +
> > +	struct oprofile_cpu_buffer *cpu_buf =
> > +			 &per_cpu(cpu_buffer, smp_processor_id());
> > +
> > +	if (!backtrace_depth) {
> > +		log_ibs_sample(cpu_buf, pc, is_kernel, ibs_sample, code);
> > +		return;
> > +	}
> > +
> > +	/* if log_sample() fails we can't backtrace since we lost the source
> > +	* of this event */
> > +	if (log_ibs_sample(cpu_buf, pc, is_kernel, ibs_sample, code))
> > +		oprofile_ops.backtrace(regs, backtrace_depth);
> > +}
> > +
> >  void oprofile_add_pc(unsigned long pc, int is_kernel, unsigned long event)
> >  {
> >  	struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer);
> > diff --git a/drivers/oprofile/cpu_buffer.h b/drivers/oprofile/cpu_buffer.h
> > index c3e366b..9c44d00 100644
> > --- a/drivers/oprofile/cpu_buffer.h
> > +++ b/drivers/oprofile/cpu_buffer.h
> > @@ -55,5 +55,7 @@ void cpu_buffer_reset(struct oprofile_cpu_buffer * cpu_buf);
> >  /* transient events for the CPU buffer -> event buffer */
> >  #define CPU_IS_KERNEL 1
> >  #define CPU_TRACE_BEGIN 2
> > +#define IBS_FETCH_BEGIN 3
> > +#define IBS_OP_BEGIN    4
> > 
> >  #endif /* OPROFILE_CPU_BUFFER_H */
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [PATCH 10/24] x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> @@ -272,7 +273,7 @@ static void increment_tail(struct oprofile_cpu_buffer *b)
>  {
>  	unsigned long new_tail = b->tail_pos + 1;
>  
> -	rmb();
> +	rmb();	/* be sure fifo pointers are synchromized */

ignore my previous mail :-)

> +	if (ibs_code == IBS_OP_BEGIN) {
> +	add_sample(cpu_buf, ibs[6], ibs[7]);
> +	add_sample(cpu_buf, ibs[8], ibs[9]);
> +	add_sample(cpu_buf, ibs[10], ibs[11]);
> +	}

style problem.

> +	int is_kernel = !user_mode(regs);
> +	unsigned long pc = profile_pc(regs);
> +
> +	struct oprofile_cpu_buffer *cpu_buf =
> +			 &per_cpu(cpu_buffer, smp_processor_id());

please dont put newlines in the middle of variable definitions.

>  /* transient events for the CPU buffer -> event buffer */
>  #define CPU_IS_KERNEL 1
>  #define CPU_TRACE_BEGIN 2
> +#define IBS_FETCH_BEGIN 3
> +#define IBS_OP_BEGIN    4

vertical alignment would help readability i guess. Plus use an enum 
instead of a macro.

	Ingo

[PATCH 11/24] x86/oprofile: Add IBS support for AMD CPUs, model specific code

[Robert Richter]

From: Barry Kasindorf <barry.kasindorf@amd.com>

This patchset supports the new profiling hardware available in the
latest AMD CPUs in the oProfile driver.

Signed-off-by: Barry Kasindorf <barry.kasindorf@amd.com>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |  257 +++++++++++++++++++++++++++++++++++
 1 files changed, 257 insertions(+), 0 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index 40ecb02..229e0b4 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -9,9 +9,13 @@
  * @author Philippe Elie
  * @author Graydon Hoare
  * @author Robert Richter <robert.richter@amd.com>
+ * @author Barry Kasindorf
 */
 
 #include <linux/oprofile.h>
+#include <linux/device.h>
+#include <linux/pci.h>
+
 #include <asm/ptrace.h>
 #include <asm/msr.h>
 #include <asm/nmi.h>
@@ -43,7 +47,83 @@
 #define CTRL_SET_HOST_ONLY(val, h) (val |= ((h & 1) << 9))
 #define CTRL_SET_GUEST_ONLY(val, h) (val |= ((h & 1) << 8))
 
+#define IBS_FETCH_CTL_HIGH_MASK		0xFFFFFFFF
+/* high dword bit IbsFetchCtl[bit 49] */
+#define IBS_FETCH_VALID_BIT		(1UL << 17)
+/* high dword bit IbsFetchCtl[bit 52] */
+#define IBS_FETCH_PHY_ADDR_VALID_BIT 	(1UL << 20)
+/* high dword bit IbsFetchCtl[bit 48] */
+#define IBS_FETCH_ENABLE		(1UL << 16)
+
+#define IBS_FETCH_CTL_CNT_MASK 		0x00000000FFFF0000UL
+#define IBS_FETCH_CTL_MAX_CNT_MASK 	0x000000000000FFFFUL
+
+/*IbsOpCtl masks/bits */
+#define IBS_OP_VALID_BIT	(1ULL<<18)	/* IbsOpCtl[bit18] */
+#define IBS_OP_ENABLE		(1ULL<<17)	/* IBS_OP_ENABLE[bit17]*/
+
+/* Codes used in cpu_buffer.c */
+#define IBS_FETCH_BEGIN 3
+#define IBS_OP_BEGIN    4
+
+/*IbsOpData3 masks */
+#define IBS_CTL_LVT_OFFSET_VALID_BIT		(1ULL<<8)
+
+/*PCI Extended Configuration Constants */
+/* MSR to set the IBS control register APIC LVT offset */
+#define IBS_LVT_OFFSET_PCI		0x1CC
+
+struct ibs_fetch_sample {
+	/* MSRC001_1031 IBS Fetch Linear Address Register */
+	unsigned int ibs_fetch_lin_addr_low;
+	unsigned int ibs_fetch_lin_addr_high;
+	/* MSRC001_1030 IBS Fetch Control Register */
+	unsigned int ibs_fetch_ctl_low;
+	unsigned int ibs_fetch_ctl_high;
+	/* MSRC001_1032 IBS Fetch Physical Address Register */
+	unsigned int ibs_fetch_phys_addr_low;
+	unsigned int ibs_fetch_phys_addr_high;
+};
+
+struct ibs_op_sample {
+	/* MSRC001_1034 IBS Op Logical Address Register (IbsRIP) */
+	unsigned int ibs_op_rip_low;
+	unsigned int ibs_op_rip_high;
+	/* MSRC001_1035 IBS Op Data Register */
+	unsigned int ibs_op_data1_low;
+	unsigned int ibs_op_data1_high;
+	/* MSRC001_1036 IBS Op Data 2 Register */
+	unsigned int ibs_op_data2_low;
+	unsigned int ibs_op_data2_high;
+	/* MSRC001_1037 IBS Op Data 3 Register */
+	unsigned int ibs_op_data3_low;
+	unsigned int ibs_op_data3_high;
+	/* MSRC001_1038 IBS DC Linear Address Register (IbsDcLinAd) */
+	unsigned int ibs_dc_linear_low;
+	unsigned int ibs_dc_linear_high;
+	/* MSRC001_1039 IBS DC Physical Address Register (IbsDcPhysAd) */
+	unsigned int ibs_dc_phys_low;
+	unsigned int ibs_dc_phys_high;
+};
+
+/*
+ * unitialize the APIC for the IBS interrupts if needed on AMD Family10h+
+*/
+static void clear_ibs_nmi(void);
+
 static unsigned long reset_value[NUM_COUNTERS];
+static int ibs_allowed;	/* AMD Family10h and later */
+
+struct op_ibs_config {
+	unsigned long op_enabled;
+	unsigned long fetch_enabled;
+	unsigned long max_cnt_fetch;
+	unsigned long max_cnt_op;
+	unsigned long rand_en;
+	unsigned long dispatched_ops;
+};
+
+static struct op_ibs_config ibs_config;
 
 /* functions for op_amd_spec */
 
@@ -121,6 +201,8 @@ static int op_amd_check_ctrs(struct pt_regs * const regs,
 {
 	unsigned int low, high;
 	int i;
+	struct ibs_fetch_sample ibs_fetch;
+	struct ibs_op_sample ibs_op;
 
 	for (i = 0 ; i < NUM_COUNTERS; ++i) {
 		if (!reset_value[i])
@@ -132,6 +214,65 @@ static int op_amd_check_ctrs(struct pt_regs * const regs,
 		}
 	}
 
+	/*If AMD and IBS is available */
+	if (ibs_allowed && ibs_config.fetch_enabled) {
+		rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+		if (high & IBS_FETCH_VALID_BIT) {
+			ibs_fetch.ibs_fetch_ctl_high = high;
+			ibs_fetch.ibs_fetch_ctl_low = low;
+			rdmsr(MSR_AMD64_IBSFETCHLINAD, low, high);
+			ibs_fetch.ibs_fetch_lin_addr_high = high;
+			ibs_fetch.ibs_fetch_lin_addr_low = low;
+			rdmsr(MSR_AMD64_IBSFETCHPHYSAD, low, high);
+			ibs_fetch.ibs_fetch_phys_addr_high = high;
+			ibs_fetch.ibs_fetch_phys_addr_low = low;
+
+			oprofile_add_ibs_sample(regs,
+						(unsigned int *)&ibs_fetch,
+						IBS_FETCH_BEGIN);
+
+			/*reenable the IRQ */
+			rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+			high &= ~(IBS_FETCH_VALID_BIT);
+			high |= IBS_FETCH_ENABLE;
+			low &= IBS_FETCH_CTL_MAX_CNT_MASK;
+			wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+		}
+	}
+
+	if (ibs_allowed && ibs_config.op_enabled) {
+		rdmsr(MSR_AMD64_IBSOPCTL, low, high);
+		if (low & IBS_OP_VALID_BIT) {
+			rdmsr(MSR_AMD64_IBSOPRIP, low, high);
+			ibs_op.ibs_op_rip_low = low;
+			ibs_op.ibs_op_rip_high = high;
+			rdmsr(MSR_AMD64_IBSOPDATA, low, high);
+			ibs_op.ibs_op_data1_low = low;
+			ibs_op.ibs_op_data1_high = high;
+			rdmsr(MSR_AMD64_IBSOPDATA2, low, high);
+			ibs_op.ibs_op_data2_low = low;
+			ibs_op.ibs_op_data2_high = high;
+			rdmsr(MSR_AMD64_IBSOPDATA3, low, high);
+			ibs_op.ibs_op_data3_low = low;
+			ibs_op.ibs_op_data3_high = high;
+			rdmsr(MSR_AMD64_IBSDCLINAD, low, high);
+			ibs_op.ibs_dc_linear_low = low;
+			ibs_op.ibs_dc_linear_high = high;
+			rdmsr(MSR_AMD64_IBSDCPHYSAD, low, high);
+			ibs_op.ibs_dc_phys_low = low;
+			ibs_op.ibs_dc_phys_high = high;
+
+			/* reenable the IRQ */
+			oprofile_add_ibs_sample(regs,
+						(unsigned int *)&ibs_op,
+						IBS_OP_BEGIN);
+			rdmsr(MSR_AMD64_IBSOPCTL, low, high);
+			low &= ~(IBS_OP_VALID_BIT);
+			low |= IBS_OP_ENABLE;
+			wrmsr(MSR_AMD64_IBSOPCTL, low, high);
+		}
+	}
+
 	/* See op_model_ppro.c */
 	return 1;
 }
@@ -148,6 +289,17 @@ static void op_amd_start(struct op_msrs const * const msrs)
 			CTRL_WRITE(low, high, msrs, i);
 		}
 	}
+	if (ibs_allowed && ibs_config.fetch_enabled) {
+		low = (ibs_config.max_cnt_fetch >> 4) & 0xFFFF;
+		high = IBS_FETCH_ENABLE;
+		wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+	}
+
+	if (ibs_allowed && ibs_config.op_enabled) {
+		low = ((ibs_config.max_cnt_op >> 4) & 0xFFFF) + IBS_OP_ENABLE;
+		high = 0;
+		wrmsr(MSR_AMD64_IBSOPCTL, low, high);
+	}
 }
 
 
@@ -165,6 +317,18 @@ static void op_amd_stop(struct op_msrs const * const msrs)
 		CTRL_SET_INACTIVE(low);
 		CTRL_WRITE(low, high, msrs, i);
 	}
+
+	if (ibs_allowed && ibs_config.fetch_enabled) {
+		low = 0;		/* clear max count and enable */
+		high = 0;
+		wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+	}
+
+	if (ibs_allowed && ibs_config.op_enabled) {
+		low = 0;		/* clear max count and enable */
+		high = 0;
+		wrmsr(MSR_AMD64_IBSOPCTL, low, high);
+	}
 }
 
 static void op_amd_shutdown(struct op_msrs const * const msrs)
@@ -181,6 +345,99 @@ static void op_amd_shutdown(struct op_msrs const * const msrs)
 	}
 }
 
+static inline void apic_init_ibs_nmi_per_cpu(void *arg)
+{
+	setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_NMI, 0);
+}
+
+static inline void apic_clear_ibs_nmi_per_cpu(void *arg)
+{
+	setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_FIX, 1);
+}
+
+/*
+ * initialize the APIC for the IBS interrupts
+ * if needed on AMD Family10h rev B0 and later
+ */
+static void setup_ibs(void)
+{
+	struct pci_dev *gh_device = NULL;
+	u32 low, high;
+	u8 vector;
+
+	ibs_allowed = boot_cpu_has(X86_FEATURE_IBS);
+
+	if (!ibs_allowed)
+		return;
+
+	/* This gets the APIC_EILVT_LVTOFF_IBS value */
+	vector = setup_APIC_eilvt_ibs(0, 0, 1);
+
+	/*see if the IBS control register is already set correctly*/
+	/*remove this when we know for sure it is done
+	  in the kernel init*/
+	rdmsr(MSR_AMD64_IBSCTL, low, high);
+	if ((low & (IBS_CTL_LVT_OFFSET_VALID_BIT | vector)) !=
+		(IBS_CTL_LVT_OFFSET_VALID_BIT | vector)) {
+
+		/**** Be sure to run loop until NULL is returned to
+		decrement reference count on any pci_dev structures
+		returned ****/
+		while ((gh_device = pci_get_device(PCI_VENDOR_ID_AMD,
+			PCI_DEVICE_ID_AMD_10H_NB_MISC, gh_device))
+			!= NULL) {
+			/* This code may change if we can find a proper
+			* way to get at the PCI extended config space */
+			pci_write_config_dword(
+				gh_device, IBS_LVT_OFFSET_PCI,
+				(vector | IBS_CTL_LVT_OFFSET_VALID_BIT));
+		}
+	}
+	on_each_cpu(apic_init_ibs_nmi_per_cpu, NULL, 1, 1);
+}
+
+
+/*
+ * unitialize the APIC for the IBS interrupts if needed on AMD Family10h
+ * rev B0 and later */
+static void clear_ibs_nmi(void)
+{
+	if (ibs_allowed)
+		on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1, 1);
+}
+
+static void setup_ibs_files(struct super_block *sb, struct dentry *root)
+{
+	char buf[12];
+	struct dentry *dir;
+
+	if (!ibs_allowed)
+		return;
+
+	/* setup some reasonable defaults */
+	ibs_config.max_cnt_fetch = 250000;
+	ibs_config.fetch_enabled = 0;
+	ibs_config.max_cnt_op = 250000;
+	ibs_config.op_enabled = 0;
+	ibs_config.dispatched_ops = 1;
+	snprintf(buf,  sizeof(buf), "ibs_fetch");
+	dir = oprofilefs_mkdir(sb, root, buf);
+	oprofilefs_create_ulong(sb, dir, "rand_enable",
+				&ibs_config.rand_en);
+	oprofilefs_create_ulong(sb, dir, "enable",
+		&ibs_config.fetch_enabled);
+	oprofilefs_create_ulong(sb, dir, "max_count",
+		&ibs_config.max_cnt_fetch);
+	snprintf(buf,  sizeof(buf), "ibs_uops");
+	dir = oprofilefs_mkdir(sb, root, buf);
+	oprofilefs_create_ulong(sb, dir, "enable",
+		&ibs_config.op_enabled);
+	oprofilefs_create_ulong(sb, dir, "max_count",
+		&ibs_config.max_cnt_op);
+	oprofilefs_create_ulong(sb, dir, "dispatched_ops",
+		&ibs_config.dispatched_ops);
+}
+
 static int op_amd_init(struct oprofile_operations *ops)
 {
 	return 0;
-- 
1.5.5.4

Re: [PATCH 11/24] x86/oprofile: Add IBS support for AMD CPUs, model specific code

[Maynard Johnson]

Robert Richter wrote:
> From: Barry Kasindorf <barry.kasindorf@amd.com>
> 
> This patchset supports the new profiling hardware available in the
> latest AMD CPUs in the oProfile driver.
> 
> Signed-off-by: Barry Kasindorf <barry.kasindorf@amd.com>
> Signed-off-by: Robert Richter <robert.richter@amd.com>
> ---
>  arch/x86/oprofile/op_model_athlon.c |  257 +++++++++++++++++++++++++++++++++++
>  1 files changed, 257 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
> index 40ecb02..229e0b4 100644
> --- a/arch/x86/oprofile/op_model_athlon.c
> +++ b/arch/x86/oprofile/op_model_athlon.c
> @@ -9,9 +9,13 @@
>   * @author Philippe Elie
>   * @author Graydon Hoare
>   * @author Robert Richter <robert.richter@amd.com>
> + * @author Barry Kasindorf
>  */
> 
[snip]
> +
> +struct op_ibs_config {
> +	unsigned long op_enabled;
> +	unsigned long fetch_enabled;
> +	unsigned long max_cnt_fetch;
> +	unsigned long max_cnt_op;
> +	unsigned long rand_en;
> +	unsigned long dispatched_ops;
I don't see dispatched_ops being used either by your kernel driver changes or by 
the userspace changes submitted by Jason.

-Maynard
[snip]

Re: [PATCH 11/24] x86/oprofile: Add IBS support for AMD CPUs, model specific code

[Robert Richter]

On 24.07.08 09:15:24, Maynard Johnson wrote:
> Robert Richter wrote:
>> From: Barry Kasindorf <barry.kasindorf@amd.com>
>> This patchset supports the new profiling hardware available in the
>> latest AMD CPUs in the oProfile driver.
>> Signed-off-by: Barry Kasindorf <barry.kasindorf@amd.com>
>> Signed-off-by: Robert Richter <robert.richter@amd.com>
>> ---
>>  arch/x86/oprofile/op_model_athlon.c |  257 
>> +++++++++++++++++++++++++++++++++++
>>  1 files changed, 257 insertions(+), 0 deletions(-)
>> diff --git a/arch/x86/oprofile/op_model_athlon.c 
>> b/arch/x86/oprofile/op_model_athlon.c
>> index 40ecb02..229e0b4 100644
>> --- a/arch/x86/oprofile/op_model_athlon.c
>> +++ b/arch/x86/oprofile/op_model_athlon.c
>> @@ -9,9 +9,13 @@
>>   * @author Philippe Elie
>>   * @author Graydon Hoare
>>   * @author Robert Richter <robert.richter@amd.com>
>> + * @author Barry Kasindorf
>>  */
> [snip]
>> +
>> +struct op_ibs_config {
>> +	unsigned long op_enabled;
>> +	unsigned long fetch_enabled;
>> +	unsigned long max_cnt_fetch;
>> +	unsigned long max_cnt_op;
>> +	unsigned long rand_en;
>> +	unsigned long dispatched_ops;
> I don't see dispatched_ops being used either by your kernel driver changes 
> or by the userspace changes submitted by Jason.

This is a RevC feature. I think there is a revision check needed and
thus Barry did not yet implement it.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [PATCH 11/24] x86/oprofile: Add IBS support for AMD CPUs, model specific code

[Robert Richter]

On 22.07.08 21:08:55, Robert Richter wrote:
> From: Barry Kasindorf <barry.kasindorf@amd.com>
> 
> This patchset supports the new profiling hardware available in the
> latest AMD CPUs in the oProfile driver.
> 
> Signed-off-by: Barry Kasindorf <barry.kasindorf@amd.com>
> Signed-off-by: Robert Richter <robert.richter@amd.com>
> ---
>  arch/x86/oprofile/op_model_athlon.c |  257 +++++++++++++++++++++++++++++++++++
>  1 files changed, 257 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
> index 40ecb02..229e0b4 100644

[...]

> @@ -181,6 +345,99 @@ static void op_amd_shutdown(struct op_msrs const * const msrs)
>  	}
>  }
>  
> +static inline void apic_init_ibs_nmi_per_cpu(void *arg)
> +{
> +	setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_NMI, 0);
> +}
> +
> +static inline void apic_clear_ibs_nmi_per_cpu(void *arg)
> +{
> +	setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_FIX, 1);
> +}
> +
> +/*
> + * initialize the APIC for the IBS interrupts
> + * if needed on AMD Family10h rev B0 and later
> + */
> +static void setup_ibs(void)
> +{
> +	struct pci_dev *gh_device = NULL;
> +	u32 low, high;
> +	u8 vector;
> +
> +	ibs_allowed = boot_cpu_has(X86_FEATURE_IBS);
> +
> +	if (!ibs_allowed)
> +		return;
> +
> +	/* This gets the APIC_EILVT_LVTOFF_IBS value */
> +	vector = setup_APIC_eilvt_ibs(0, 0, 1);
> +
> +	/*see if the IBS control register is already set correctly*/
> +	/*remove this when we know for sure it is done
> +	  in the kernel init*/
> +	rdmsr(MSR_AMD64_IBSCTL, low, high);
> +	if ((low & (IBS_CTL_LVT_OFFSET_VALID_BIT | vector)) !=
> +		(IBS_CTL_LVT_OFFSET_VALID_BIT | vector)) {
> +
> +		/**** Be sure to run loop until NULL is returned to
> +		decrement reference count on any pci_dev structures
> +		returned ****/
> +		while ((gh_device = pci_get_device(PCI_VENDOR_ID_AMD,
> +			PCI_DEVICE_ID_AMD_10H_NB_MISC, gh_device))
> +			!= NULL) {
> +			/* This code may change if we can find a proper
> +			* way to get at the PCI extended config space */
> +			pci_write_config_dword(
> +				gh_device, IBS_LVT_OFFSET_PCI,
> +				(vector | IBS_CTL_LVT_OFFSET_VALID_BIT));
> +		}
> +	}
> +	on_each_cpu(apic_init_ibs_nmi_per_cpu, NULL, 1, 1);
> +}
> +
> +
> +/*
> + * unitialize the APIC for the IBS interrupts if needed on AMD Family10h
> + * rev B0 and later */
> +static void clear_ibs_nmi(void)
> +{
> +	if (ibs_allowed)
> +		on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1, 1);
> +}
> +
> +static void setup_ibs_files(struct super_block *sb, struct dentry *root)
> +{
> +	char buf[12];
> +	struct dentry *dir;
> +
> +	if (!ibs_allowed)
> +		return;
> +
> +	/* setup some reasonable defaults */
> +	ibs_config.max_cnt_fetch = 250000;
> +	ibs_config.fetch_enabled = 0;
> +	ibs_config.max_cnt_op = 250000;
> +	ibs_config.op_enabled = 0;
> +	ibs_config.dispatched_ops = 1;
> +	snprintf(buf,  sizeof(buf), "ibs_fetch");
> +	dir = oprofilefs_mkdir(sb, root, buf);
> +	oprofilefs_create_ulong(sb, dir, "rand_enable",
> +				&ibs_config.rand_en);
> +	oprofilefs_create_ulong(sb, dir, "enable",
> +		&ibs_config.fetch_enabled);
> +	oprofilefs_create_ulong(sb, dir, "max_count",
> +		&ibs_config.max_cnt_fetch);
> +	snprintf(buf,  sizeof(buf), "ibs_uops");

This should be renamed to "ibs_op" to be close to the register
specification in the BKDG.

-Robert

> +	dir = oprofilefs_mkdir(sb, root, buf);
> +	oprofilefs_create_ulong(sb, dir, "enable",
> +		&ibs_config.op_enabled);
> +	oprofilefs_create_ulong(sb, dir, "max_count",
> +		&ibs_config.max_cnt_op);
> +	oprofilefs_create_ulong(sb, dir, "dispatched_ops",
> +		&ibs_config.dispatched_ops);
> +}
> +
>  static int op_amd_init(struct oprofile_operations *ops)
>  {
>  	return 0;
> -- 
> 1.5.5.4
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [PATCH 11/24] x86/oprofile: Add IBS support for AMD CPUs, model specific code

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> @@ -132,6 +214,65 @@ static int op_amd_check_ctrs(struct pt_regs * const regs,
>  		}
>  	}
>  
> +	/*If AMD and IBS is available */
> +	if (ibs_allowed && ibs_config.fetch_enabled) {
> +		rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
> +		if (high & IBS_FETCH_VALID_BIT) {
> +			ibs_fetch.ibs_fetch_ctl_high = high;
> +			ibs_fetch.ibs_fetch_ctl_low = low;
> +			rdmsr(MSR_AMD64_IBSFETCHLINAD, low, high);
> +			ibs_fetch.ibs_fetch_lin_addr_high = high;
> +			ibs_fetch.ibs_fetch_lin_addr_low = low;
> +			rdmsr(MSR_AMD64_IBSFETCHPHYSAD, low, high);
> +			ibs_fetch.ibs_fetch_phys_addr_high = high;
> +			ibs_fetch.ibs_fetch_phys_addr_low = low;
> +
> +			oprofile_add_ibs_sample(regs,
> +						(unsigned int *)&ibs_fetch,
> +						IBS_FETCH_BEGIN);

please move this innermost code into a helper function. That will help 
eliminate line 80 artifacts as well.

> +	if (ibs_allowed && ibs_config.op_enabled) {
> +		rdmsr(MSR_AMD64_IBSOPCTL, low, high);
> +		if (low & IBS_OP_VALID_BIT) {
> +			rdmsr(MSR_AMD64_IBSOPRIP, low, high);
> +			ibs_op.ibs_op_rip_low = low;
> +			ibs_op.ibs_op_rip_high = high;
> +			rdmsr(MSR_AMD64_IBSOPDATA, low, high);
> +			ibs_op.ibs_op_data1_low = low;
> +			ibs_op.ibs_op_data1_high = high;
> +			rdmsr(MSR_AMD64_IBSOPDATA2, low, high);
> +			ibs_op.ibs_op_data2_low = low;
> +			ibs_op.ibs_op_data2_high = high;
> +			rdmsr(MSR_AMD64_IBSOPDATA3, low, high);
> +			ibs_op.ibs_op_data3_low = low;
> +			ibs_op.ibs_op_data3_high = high;
> +			rdmsr(MSR_AMD64_IBSDCLINAD, low, high);
> +			ibs_op.ibs_dc_linear_low = low;
> +			ibs_op.ibs_dc_linear_high = high;
> +			rdmsr(MSR_AMD64_IBSDCPHYSAD, low, high);
> +			ibs_op.ibs_dc_phys_low = low;
> +			ibs_op.ibs_dc_phys_high = high;
> +
> +			/* reenable the IRQ */
> +			oprofile_add_ibs_sample(regs,
> +						(unsigned int *)&ibs_op,
> +						IBS_OP_BEGIN);
> +			rdmsr(MSR_AMD64_IBSOPCTL, low, high);
> +			low &= ~(IBS_OP_VALID_BIT);
> +			low |= IBS_OP_ENABLE;
> +			wrmsr(MSR_AMD64_IBSOPCTL, low, high);
> +		}
> +	}

ditto.

> +		/**** Be sure to run loop until NULL is returned to
> +		decrement reference count on any pci_dev structures
> +		returned ****/

please use standard comment style:

 /*
  * Multi-line ..........
  * ....................... comment:
  */

> +		while ((gh_device = pci_get_device(PCI_VENDOR_ID_AMD,
> +			PCI_DEVICE_ID_AMD_10H_NB_MISC, gh_device))
> +			!= NULL) {
> +			/* This code may change if we can find a proper
> +			* way to get at the PCI extended config space */

ditto.

> +			pci_write_config_dword(
> +				gh_device, IBS_LVT_OFFSET_PCI,
> +				(vector | IBS_CTL_LVT_OFFSET_VALID_BIT));

perhaps use a helper function to reduce col 80 artifacts.

> +/*
> + * unitialize the APIC for the IBS interrupts if needed on AMD Family10h
> + * rev B0 and later */

comment style.

> +static void setup_ibs_files(struct super_block *sb, struct dentry *root)
> +{
> +	char buf[12];

magic limit.

> +	struct dentry *dir;
> +
> +	if (!ibs_allowed)
> +		return;
> +
> +	/* setup some reasonable defaults */
> +	ibs_config.max_cnt_fetch = 250000;
> +	ibs_config.fetch_enabled = 0;
> +	ibs_config.max_cnt_op = 250000;
> +	ibs_config.op_enabled = 0;
> +	ibs_config.dispatched_ops = 1;
> +	snprintf(buf,  sizeof(buf), "ibs_fetch");
> +	dir = oprofilefs_mkdir(sb, root, buf);

you could make use of a small style trick here: use a newline before 
this statement, to make blocks of code stand out in a clearer way. The 
construction of 'buf' is finished after the snprintf above, then comes 
the next step, to create the oprofilefs entries.

	Ingo

[PATCH 12/24] x86/oprofile: Separating the IBS handler

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |   45 +++++++++++++++++++++-------------
 1 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index 229e0b4..a2c8e2e 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -195,27 +195,18 @@ static void op_amd_setup_ctrs(struct op_msrs const * const msrs)
 	}
 }
 
-
-static int op_amd_check_ctrs(struct pt_regs * const regs,
-			     struct op_msrs const * const msrs)
+static inline int
+op_amd_handle_ibs(struct pt_regs * const regs,
+		  struct op_msrs const * const msrs)
 {
 	unsigned int low, high;
-	int i;
 	struct ibs_fetch_sample ibs_fetch;
 	struct ibs_op_sample ibs_op;
 
-	for (i = 0 ; i < NUM_COUNTERS; ++i) {
-		if (!reset_value[i])
-			continue;
-		CTR_READ(low, high, msrs, i);
-		if (CTR_OVERFLOWED(low)) {
-			oprofile_add_sample(regs, i);
-			CTR_WRITE(reset_value[i], msrs, i);
-		}
-	}
+	if (!ibs_allowed)
+		return 1;
 
-	/*If AMD and IBS is available */
-	if (ibs_allowed && ibs_config.fetch_enabled) {
+	if (ibs_config.fetch_enabled) {
 		rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
 		if (high & IBS_FETCH_VALID_BIT) {
 			ibs_fetch.ibs_fetch_ctl_high = high;
@@ -240,7 +231,7 @@ static int op_amd_check_ctrs(struct pt_regs * const regs,
 		}
 	}
 
-	if (ibs_allowed && ibs_config.op_enabled) {
+	if (ibs_config.op_enabled) {
 		rdmsr(MSR_AMD64_IBSOPCTL, low, high);
 		if (low & IBS_OP_VALID_BIT) {
 			rdmsr(MSR_AMD64_IBSOPRIP, low, high);
@@ -273,10 +264,30 @@ static int op_amd_check_ctrs(struct pt_regs * const regs,
 		}
 	}
 
-	/* See op_model_ppro.c */
 	return 1;
 }
 
+static int op_amd_check_ctrs(struct pt_regs * const regs,
+			     struct op_msrs const * const msrs)
+{
+	unsigned int low, high;
+	int i;
+
+	for (i = 0 ; i < NUM_COUNTERS; ++i) {
+		if (!reset_value[i])
+			continue;
+		CTR_READ(low, high, msrs, i);
+		if (CTR_OVERFLOWED(low)) {
+			oprofile_add_sample(regs, i);
+			CTR_WRITE(reset_value[i], msrs, i);
+		}
+	}
+
+	op_amd_handle_ibs(regs, msrs);
+
+	/* See op_model_ppro.c */
+	return 1;
+}
 
 static void op_amd_start(struct op_msrs const * const msrs)
 {
-- 
1.5.5.4

[PATCH 13/24] OProfile: Change IBS interrupt initialization

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |   84 ++++++++++++++++++++++------------
 1 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index a2c8e2e..90193b1 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -356,9 +356,11 @@ static void op_amd_shutdown(struct op_msrs const * const msrs)
 	}
 }
 
+static u8 ibs_eilvt_off;
+
 static inline void apic_init_ibs_nmi_per_cpu(void *arg)
 {
-	setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_NMI, 0);
+	ibs_eilvt_off = setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_NMI, 0);
 }
 
 static inline void apic_clear_ibs_nmi_per_cpu(void *arg)
@@ -366,45 +368,67 @@ static inline void apic_clear_ibs_nmi_per_cpu(void *arg)
 	setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_FIX, 1);
 }
 
+static int pfm_amd64_setup_eilvt(void)
+{
+#define IBSCTL_LVTOFFSETVAL		(1 << 8)
+#define IBSCTL				0x1cc
+	struct pci_dev *cpu_cfg;
+	int nodes;
+	u32 value = 0;
+
+	/* per CPU setup */
+	on_each_cpu(apic_init_ibs_nmi_per_cpu, NULL, 0, 1);
+
+	nodes = 0;
+	cpu_cfg = NULL;
+	do {
+		cpu_cfg = pci_get_device(PCI_VENDOR_ID_AMD,
+					 PCI_DEVICE_ID_AMD_10H_NB_MISC,
+					 cpu_cfg);
+		if (!cpu_cfg)
+			break;
+		++nodes;
+		pci_write_config_dword(cpu_cfg, IBSCTL, ibs_eilvt_off
+				       | IBSCTL_LVTOFFSETVAL);
+		pci_read_config_dword(cpu_cfg, IBSCTL, &value);
+		if (value != (ibs_eilvt_off | IBSCTL_LVTOFFSETVAL)) {
+			printk(KERN_DEBUG "Failed to setup IBS LVT offset, "
+				"IBSCTL = 0x%08x", value);
+			return 1;
+		}
+	} while (1);
+
+	if (!nodes) {
+		printk(KERN_DEBUG "No CPU node configured for IBS");
+		return 1;
+	}
+
+#ifdef CONFIG_NUMA
+	/* Sanity check */
+	/* Works only for 64bit with proper numa implementation. */
+	if (nodes != num_possible_nodes()) {
+		printk(KERN_DEBUG "Failed to setup CPU node(s) for IBS, "
+			"found: %d, expected %d",
+			nodes, num_possible_nodes());
+		return 1;
+	}
+#endif
+	return 0;
+}
+
 /*
  * initialize the APIC for the IBS interrupts
- * if needed on AMD Family10h rev B0 and later
+ * if available (AMD Family10h rev B0 and later)
  */
 static void setup_ibs(void)
 {
-	struct pci_dev *gh_device = NULL;
-	u32 low, high;
-	u8 vector;
-
 	ibs_allowed = boot_cpu_has(X86_FEATURE_IBS);
 
 	if (!ibs_allowed)
 		return;
 
-	/* This gets the APIC_EILVT_LVTOFF_IBS value */
-	vector = setup_APIC_eilvt_ibs(0, 0, 1);
-
-	/*see if the IBS control register is already set correctly*/
-	/*remove this when we know for sure it is done
-	  in the kernel init*/
-	rdmsr(MSR_AMD64_IBSCTL, low, high);
-	if ((low & (IBS_CTL_LVT_OFFSET_VALID_BIT | vector)) !=
-		(IBS_CTL_LVT_OFFSET_VALID_BIT | vector)) {
-
-		/**** Be sure to run loop until NULL is returned to
-		decrement reference count on any pci_dev structures
-		returned ****/
-		while ((gh_device = pci_get_device(PCI_VENDOR_ID_AMD,
-			PCI_DEVICE_ID_AMD_10H_NB_MISC, gh_device))
-			!= NULL) {
-			/* This code may change if we can find a proper
-			* way to get at the PCI extended config space */
-			pci_write_config_dword(
-				gh_device, IBS_LVT_OFFSET_PCI,
-				(vector | IBS_CTL_LVT_OFFSET_VALID_BIT));
-		}
-	}
-	on_each_cpu(apic_init_ibs_nmi_per_cpu, NULL, 1, 1);
+	if (pfm_amd64_setup_eilvt())
+		ibs_allowed = 0;
 }
 
 
-- 
1.5.5.4

Re: [PATCH 13/24] OProfile: Change IBS interrupt initialization

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> +static int pfm_amd64_setup_eilvt(void)
> +{
> +#define IBSCTL_LVTOFFSETVAL		(1 << 8)
> +#define IBSCTL				0x1cc

move this into a header file please.

> +	cpu_cfg = NULL;
> +	do {
> +		cpu_cfg = pci_get_device(PCI_VENDOR_ID_AMD,
> +					 PCI_DEVICE_ID_AMD_10H_NB_MISC,
> +					 cpu_cfg);
> +		if (!cpu_cfg)
> +			break;
> +		++nodes;
> +		pci_write_config_dword(cpu_cfg, IBSCTL, ibs_eilvt_off
> +				       | IBSCTL_LVTOFFSETVAL);
> +		pci_read_config_dword(cpu_cfg, IBSCTL, &value);
> +		if (value != (ibs_eilvt_off | IBSCTL_LVTOFFSETVAL)) {
> +			printk(KERN_DEBUG "Failed to setup IBS LVT offset, "
> +				"IBSCTL = 0x%08x", value);
> +			return 1;
> +		}
> +	} while (1);

helper function for the iterator would help a bit i guess.

	Ingo

[PATCH 14/24] OProfile: Fix build error in op_model_athlon.c

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index 90193b1..284c456 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -73,6 +73,11 @@
 /* MSR to set the IBS control register APIC LVT offset */
 #define IBS_LVT_OFFSET_PCI		0x1CC
 
+/* The function interface needs to be fixed, something like add
+   data. Should then be added to linux/oprofile.h. */
+extern void oprofile_add_ibs_sample(struct pt_regs *const regs,
+				    unsigned int * const ibs_sample, u8 code);
+
 struct ibs_fetch_sample {
 	/* MSRC001_1031 IBS Fetch Linear Address Register */
 	unsigned int ibs_fetch_lin_addr_low;
-- 
1.5.5.4

Re: [PATCH 14/24] OProfile: Fix build error in op_model_athlon.c

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> Signed-off-by: Robert Richter <robert.richter@amd.com>
> ---
>  arch/x86/oprofile/op_model_athlon.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
> index 90193b1..284c456 100644
> --- a/arch/x86/oprofile/op_model_athlon.c
> +++ b/arch/x86/oprofile/op_model_athlon.c
> @@ -73,6 +73,11 @@
>  /* MSR to set the IBS control register APIC LVT offset */
>  #define IBS_LVT_OFFSET_PCI		0x1CC
>  
> +/* The function interface needs to be fixed, something like add
> +   data. Should then be added to linux/oprofile.h. */

please use standard comment style.

	Ingo

[PATCH 15/24] OProfile: on_each_cpu(): kill unused retry parameter

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index 284c456..ce73236 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -382,7 +382,7 @@ static int pfm_amd64_setup_eilvt(void)
 	u32 value = 0;
 
 	/* per CPU setup */
-	on_each_cpu(apic_init_ibs_nmi_per_cpu, NULL, 0, 1);
+	on_each_cpu(apic_init_ibs_nmi_per_cpu, NULL, 1);
 
 	nodes = 0;
 	cpu_cfg = NULL;
@@ -443,7 +443,7 @@ static void setup_ibs(void)
 static void clear_ibs_nmi(void)
 {
 	if (ibs_allowed)
-		on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1, 1);
+		on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1);
 }
 
 static void setup_ibs_files(struct super_block *sb, struct dentry *root)
-- 
1.5.5.4

[PATCH 16/24] OProfile: Fix setup_ibs_files() function interface

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index ce73236..2650b12 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -446,13 +446,13 @@ static void clear_ibs_nmi(void)
 		on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1);
 }
 
-static void setup_ibs_files(struct super_block *sb, struct dentry *root)
+static int setup_ibs_files(struct super_block * sb, struct dentry * root)
 {
 	char buf[12];
 	struct dentry *dir;
 
 	if (!ibs_allowed)
-		return;
+		return 0;
 
 	/* setup some reasonable defaults */
 	ibs_config.max_cnt_fetch = 250000;
@@ -476,6 +476,8 @@ static void setup_ibs_files(struct super_block *sb, struct dentry *root)
 		&ibs_config.max_cnt_op);
 	oprofilefs_create_ulong(sb, dir, "dispatched_ops",
 		&ibs_config.dispatched_ops);
+
+	return 0;
 }
 
 static int op_amd_init(struct oprofile_operations *ops)
-- 
1.5.5.4

[PATCH 17/24] OProfile: Enable IBS for AMD CPUs

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/nmi_int.c         |   14 ++++++++------
 arch/x86/oprofile/op_model_athlon.c |   18 +++++++++++++++++-
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 4e42b50..f46d8fc 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -454,6 +454,14 @@ int __init op_nmi_init(struct oprofile_operations *ops)
 		return -ENODEV;
 	}
 
+	/* default values, can be overwritten by model */
+	ops->create_files = nmi_create_files;
+	ops->setup = nmi_setup;
+	ops->shutdown = nmi_shutdown;
+	ops->start = nmi_start;
+	ops->stop = nmi_stop;
+	ops->cpu_type = cpu_type;
+
 	if (model->init)
 		ret = model->init(ops);
 	if (ret)
@@ -461,12 +469,6 @@ int __init op_nmi_init(struct oprofile_operations *ops)
 
 	init_sysfs();
 	using_nmi = 1;
-	ops->create_files = nmi_create_files;
-	ops->setup = nmi_setup;
-	ops->shutdown = nmi_shutdown;
-	ops->start = nmi_start;
-	ops->stop = nmi_stop;
-	ops->cpu_type = cpu_type;
 	printk(KERN_INFO "oprofile: using NMI interrupt.\n");
 	return 0;
 }
diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index 2650b12..0d83903 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -446,13 +446,25 @@ static void clear_ibs_nmi(void)
 		on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1);
 }
 
+static int (*create_arch_files)(struct super_block * sb, struct dentry * root);
+
 static int setup_ibs_files(struct super_block * sb, struct dentry * root)
 {
 	char buf[12];
 	struct dentry *dir;
+	int ret = 0;
+
+	/* architecture specific files */
+	if (create_arch_files)
+		ret = create_arch_files(sb, root);
+
+	if (ret)
+		return ret;
 
 	if (!ibs_allowed)
-		return 0;
+		return ret;
+
+	/* model specific files */
 
 	/* setup some reasonable defaults */
 	ibs_config.max_cnt_fetch = 250000;
@@ -482,11 +494,15 @@ static int setup_ibs_files(struct super_block * sb, struct dentry * root)
 
 static int op_amd_init(struct oprofile_operations *ops)
 {
+	setup_ibs();
+	create_arch_files = ops->create_files;
+	ops->create_files = setup_ibs_files;
 	return 0;
 }
 
 static void op_amd_exit(void)
 {
+	clear_ibs_nmi();
 }
 
 struct op_x86_model_spec const op_amd_spec = {
-- 
1.5.5.4

Re: [PATCH 17/24] OProfile: Enable IBS for AMD CPUs

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> +	/* default values, can be overwritten by model */
> +	ops->create_files = nmi_create_files;
> +	ops->setup = nmi_setup;
> +	ops->shutdown = nmi_shutdown;
> +	ops->start = nmi_start;
> +	ops->stop = nmi_stop;
> +	ops->cpu_type = cpu_type;

i know you are moving existing code around, but sill it helps 
readability if you align these vertically too, like:

> +	/* default values, can be overwritten by model */
> +	ops->create_files	= nmi_create_files;
> +	ops->setup		= nmi_setup;
> +	ops->shutdown		= nmi_shutdown;
> +	ops->start		= nmi_start;
> +	ops->stop		= nmi_stop;
> +	ops->cpu_type		= cpu_type;

if i look at the aligned variant during review, i can see it immediately 
that it's fine and that left and right matches up conceptually. I can 
also see it, without having to look anywhere else, that ->cpu_type is 
special.

> @@ -482,11 +494,15 @@ static int setup_ibs_files(struct super_block * sb, struct dentry * root)
>  
>  static int op_amd_init(struct oprofile_operations *ops)
>  {
> +	setup_ibs();
> +	create_arch_files = ops->create_files;
> +	ops->create_files = setup_ibs_files;
>  	return 0;

the (non-)locking might be a bit racy here: the oprofilefs entries are 
set up first in setup_ibs() and made visible before you override them. 
oprofilefs entries should be made visible at the very last step, when 
all pointers are at their final versions and are stable already.

	Ingo

[PATCH 18/24] OProfile: Fix IBS build error for UP

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index 0d83903..1acb067 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -361,6 +361,26 @@ static void op_amd_shutdown(struct op_msrs const * const msrs)
 	}
 }
 
+#ifndef CONFIG_SMP
+
+/* no IBS support */
+
+static void setup_ibs(void)
+{
+	ibs_allowed = 0;
+}
+
+static void clear_ibs_nmi(void) {}
+
+static int op_amd_init(struct oprofile_operations *ops)
+{
+	return 0;
+}
+
+static void op_amd_exit(void) {}
+
+#else
+
 static u8 ibs_eilvt_off;
 
 static inline void apic_init_ibs_nmi_per_cpu(void *arg)
@@ -505,6 +525,8 @@ static void op_amd_exit(void)
 	clear_ibs_nmi();
 }
 
+#endif
+
 struct op_x86_model_spec const op_amd_spec = {
 	.init = op_amd_init,
 	.exit = op_amd_exit,
-- 
1.5.5.4

[PATCH 19/24] x86/oprofile: Macro definition cleanup in op_model_athlon.c

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |   46 +++++++++++++----------------------
 1 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index 1acb067..a3a2058 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -47,32 +47,20 @@
 #define CTRL_SET_HOST_ONLY(val, h) (val |= ((h & 1) << 9))
 #define CTRL_SET_GUEST_ONLY(val, h) (val |= ((h & 1) << 8))
 
-#define IBS_FETCH_CTL_HIGH_MASK		0xFFFFFFFF
-/* high dword bit IbsFetchCtl[bit 49] */
-#define IBS_FETCH_VALID_BIT		(1UL << 17)
-/* high dword bit IbsFetchCtl[bit 52] */
-#define IBS_FETCH_PHY_ADDR_VALID_BIT 	(1UL << 20)
-/* high dword bit IbsFetchCtl[bit 48] */
-#define IBS_FETCH_ENABLE		(1UL << 16)
+/* IbsFetchCtl bits/masks */
+#define IBS_FETCH_HIGH_VALID_BIT	(1UL << 17)	/* bit 49 */
+#define IBS_FETCH_HIGH_ENABLE		(1UL << 16)	/* bit 48 */
+#define IBS_FETCH_LOW_MAX_CNT_MASK	0x0000FFFFUL	/* MaxCnt mask */
 
-#define IBS_FETCH_CTL_CNT_MASK 		0x00000000FFFF0000UL
-#define IBS_FETCH_CTL_MAX_CNT_MASK 	0x000000000000FFFFUL
-
-/*IbsOpCtl masks/bits */
-#define IBS_OP_VALID_BIT	(1ULL<<18)	/* IbsOpCtl[bit18] */
-#define IBS_OP_ENABLE		(1ULL<<17)	/* IBS_OP_ENABLE[bit17]*/
+/*IbsOpCtl bits */
+#define IBS_OP_LOW_VALID_BIT		(1ULL<<18)	/* bit 18 */
+#define IBS_OP_LOW_ENABLE		(1ULL<<17)	/* bit 17 */
 
 /* Codes used in cpu_buffer.c */
+/* This produces duplicate code, need to be fixed */
 #define IBS_FETCH_BEGIN 3
 #define IBS_OP_BEGIN    4
 
-/*IbsOpData3 masks */
-#define IBS_CTL_LVT_OFFSET_VALID_BIT		(1ULL<<8)
-
-/*PCI Extended Configuration Constants */
-/* MSR to set the IBS control register APIC LVT offset */
-#define IBS_LVT_OFFSET_PCI		0x1CC
-
 /* The function interface needs to be fixed, something like add
    data. Should then be added to linux/oprofile.h. */
 extern void oprofile_add_ibs_sample(struct pt_regs *const regs,
@@ -213,7 +201,7 @@ op_amd_handle_ibs(struct pt_regs * const regs,
 
 	if (ibs_config.fetch_enabled) {
 		rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
-		if (high & IBS_FETCH_VALID_BIT) {
+		if (high & IBS_FETCH_HIGH_VALID_BIT) {
 			ibs_fetch.ibs_fetch_ctl_high = high;
 			ibs_fetch.ibs_fetch_ctl_low = low;
 			rdmsr(MSR_AMD64_IBSFETCHLINAD, low, high);
@@ -229,16 +217,16 @@ op_amd_handle_ibs(struct pt_regs * const regs,
 
 			/*reenable the IRQ */
 			rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
-			high &= ~(IBS_FETCH_VALID_BIT);
-			high |= IBS_FETCH_ENABLE;
-			low &= IBS_FETCH_CTL_MAX_CNT_MASK;
+			high &= ~IBS_FETCH_HIGH_VALID_BIT;
+			high |= IBS_FETCH_HIGH_ENABLE;
+			low &= IBS_FETCH_LOW_MAX_CNT_MASK;
 			wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
 		}
 	}
 
 	if (ibs_config.op_enabled) {
 		rdmsr(MSR_AMD64_IBSOPCTL, low, high);
-		if (low & IBS_OP_VALID_BIT) {
+		if (low & IBS_OP_LOW_VALID_BIT) {
 			rdmsr(MSR_AMD64_IBSOPRIP, low, high);
 			ibs_op.ibs_op_rip_low = low;
 			ibs_op.ibs_op_rip_high = high;
@@ -263,8 +251,8 @@ op_amd_handle_ibs(struct pt_regs * const regs,
 						(unsigned int *)&ibs_op,
 						IBS_OP_BEGIN);
 			rdmsr(MSR_AMD64_IBSOPCTL, low, high);
-			low &= ~(IBS_OP_VALID_BIT);
-			low |= IBS_OP_ENABLE;
+			low &= ~IBS_OP_LOW_VALID_BIT;
+			low |= IBS_OP_LOW_ENABLE;
 			wrmsr(MSR_AMD64_IBSOPCTL, low, high);
 		}
 	}
@@ -307,12 +295,12 @@ static void op_amd_start(struct op_msrs const * const msrs)
 	}
 	if (ibs_allowed && ibs_config.fetch_enabled) {
 		low = (ibs_config.max_cnt_fetch >> 4) & 0xFFFF;
-		high = IBS_FETCH_ENABLE;
+		high = IBS_FETCH_HIGH_ENABLE;
 		wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
 	}
 
 	if (ibs_allowed && ibs_config.op_enabled) {
-		low = ((ibs_config.max_cnt_op >> 4) & 0xFFFF) + IBS_OP_ENABLE;
+		low = ((ibs_config.max_cnt_op >> 4) & 0xFFFF) + IBS_OP_LOW_ENABLE;
 		high = 0;
 		wrmsr(MSR_AMD64_IBSOPCTL, low, high);
 	}
-- 
1.5.5.4

[PATCH 20/24] x86/oprofile: op_model_athlon.c: Fix counter reset when reenabling IBS OP

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/op_model_athlon.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index a3a2058..9c8c8c5 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -251,6 +251,7 @@ op_amd_handle_ibs(struct pt_regs * const regs,
 						(unsigned int *)&ibs_op,
 						IBS_OP_BEGIN);
 			rdmsr(MSR_AMD64_IBSOPCTL, low, high);
+			high = 0;
 			low &= ~IBS_OP_LOW_VALID_BIT;
 			low |= IBS_OP_LOW_ENABLE;
 			wrmsr(MSR_AMD64_IBSOPCTL, low, high);
-- 
1.5.5.4

[PATCH 21/24] x86: apic: Export symbols for extended interrupt LVT functions

[Robert Richter]

This patch adds EXPORT_SYMBOLs to allow OProfile to be built as
module.

Cc: Arjan van de Ven <arjan@infradead.org>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/kernel/apic_32.c |    1 +
 arch/x86/kernel/apic_64.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic_32.c b/arch/x86/kernel/apic_32.c
index fad94b0..ed3a6d7 100644
--- a/arch/x86/kernel/apic_32.c
+++ b/arch/x86/kernel/apic_32.c
@@ -672,6 +672,7 @@ u8 setup_APIC_eilvt_ibs(u8 vector, u8 msg_type, u8 mask)
 	setup_APIC_eilvt(APIC_EILVT_LVTOFF_IBS, vector, msg_type, mask);
 	return APIC_EILVT_LVTOFF_IBS;
 }
+EXPORT_SYMBOL(setup_APIC_eilvt_ibs);
 
 /*
  * Local APIC start and shutdown
diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index 42bf69f..45ec90e 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -232,6 +232,7 @@ u8 setup_APIC_eilvt_ibs(u8 vector, u8 msg_type, u8 mask)
 	setup_APIC_eilvt(APIC_EILVT_LVTOFF_IBS, vector, msg_type, mask);
 	return APIC_EILVT_LVTOFF_IBS;
 }
+EXPORT_SYMBOL(setup_APIC_eilvt_ibs);
 
 /*
  * Program the next event, relative to now
-- 
1.5.5.4

Re: [PATCH 21/24] x86: apic: Export symbols for extended interrupt LVT functions

[Arjan van de Ven]

surely such deep internals warrant a _GPL export...


On Tue, 22 Jul 2008 21:09:05 +0200
Robert Richter <robert.richter@amd.com> wrote:

> This patch adds EXPORT_SYMBOLs to allow OProfile to be built as
> module.
> 
> Cc: Arjan van de Ven <arjan@infradead.org>
> Signed-off-by: Robert Richter <robert.richter@amd.com>
> ---
>  arch/x86/kernel/apic_32.c |    1 +
>  arch/x86/kernel/apic_64.c |    1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic_32.c b/arch/x86/kernel/apic_32.c
> index fad94b0..ed3a6d7 100644
> --- a/arch/x86/kernel/apic_32.c
> +++ b/arch/x86/kernel/apic_32.c
> @@ -672,6 +672,7 @@ u8 setup_APIC_eilvt_ibs(u8 vector, u8 msg_type,
> u8 mask) setup_APIC_eilvt(APIC_EILVT_LVTOFF_IBS, vector, msg_type,
> mask); return APIC_EILVT_LVTOFF_IBS;
>  }
> +EXPORT_SYMBOL(setup_APIC_eilvt_ibs);
>  
>  /*
>   * Local APIC start and shutdown
> diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
> index 42bf69f..45ec90e 100644
> --- a/arch/x86/kernel/apic_64.c
> +++ b/arch/x86/kernel/apic_64.c
> @@ -232,6 +232,7 @@ u8 setup_APIC_eilvt_ibs(u8 vector, u8 msg_type,
> u8 mask) setup_APIC_eilvt(APIC_EILVT_LVTOFF_IBS, vector, msg_type,
> mask); return APIC_EILVT_LVTOFF_IBS;
>  }
> +EXPORT_SYMBOL(setup_APIC_eilvt_ibs);
>  
>  /*
>   * Program the next event, relative to now


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

[PATCH] x86: apic: Changing export symbols to *_GPL

[Robert Richter]

This fits better here.

Cc: Arjan van de Ven <arjan@infradead.org>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/kernel/apic_32.c |    2 +-
 arch/x86/kernel/apic_64.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic_32.c b/arch/x86/kernel/apic_32.c
index ed3a6d7..0059e7a 100644
--- a/arch/x86/kernel/apic_32.c
+++ b/arch/x86/kernel/apic_32.c
@@ -672,7 +672,7 @@ u8 setup_APIC_eilvt_ibs(u8 vector, u8 msg_type, u8 mask)
 	setup_APIC_eilvt(APIC_EILVT_LVTOFF_IBS, vector, msg_type, mask);
 	return APIC_EILVT_LVTOFF_IBS;
 }
-EXPORT_SYMBOL(setup_APIC_eilvt_ibs);
+EXPORT_SYMBOL_GPL(setup_APIC_eilvt_ibs);
 
 /*
  * Local APIC start and shutdown
diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index 45ec90e..e571351 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -232,7 +232,7 @@ u8 setup_APIC_eilvt_ibs(u8 vector, u8 msg_type, u8 mask)
 	setup_APIC_eilvt(APIC_EILVT_LVTOFF_IBS, vector, msg_type, mask);
 	return APIC_EILVT_LVTOFF_IBS;
 }
-EXPORT_SYMBOL(setup_APIC_eilvt_ibs);
+EXPORT_SYMBOL_GPL(setup_APIC_eilvt_ibs);
 
 /*
  * Program the next event, relative to now
-- 
1.5.5.4

Re: [PATCH] x86: apic: Changing export symbols to *_GPL

[linux-os (Dick Johnson)]

On Wed, 23 Jul 2008, Robert Richter wrote:

> This fits better here.

[Snipped...]
This question was raised before and simply dismissed.

If somebody wrote a procedure in the kernel that exported
some symbols. And that person wanted anybody to use that
symbol, upon what authority does one change "EXPORT_SYMBOL"
to "EXPORT_SYMBOL_GPL?" This will need to eventually be
answered.

Cheers,
Dick Johnson
Penguin : Linux version 2.6.22.1 on an i686 machine (5588.29 BogoMips).
My book : http://www.AbominableFirebug.com/
_

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: [PATCH] x86: apic: Changing export symbols to *_GPL

[Robert Richter]

On 23.07.08 15:29:59, linux-os (Dick Johnson) wrote:
> 
> On Wed, 23 Jul 2008, Robert Richter wrote:
> 
> > This fits better here.
> 
> [Snipped...]
> This question was raised before and simply dismissed.
> 
> If somebody wrote a procedure in the kernel that exported
> some symbols. And that person wanted anybody to use that
> symbol, upon what authority does one change "EXPORT_SYMBOL"
> to "EXPORT_SYMBOL_GPL?" This will need to eventually be
> answered.

This corrects a patch _I_ introduced before in my OProfile patch
series. Ok, maybe a revert would have been better. But sending 2 new
patches for this littls change?

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [PATCH] x86: apic: Changing export symbols to *_GPL

[Stefan Richter]

Robert Richter wrote:
> This corrects a patch _I_ introduced before in my OProfile patch
> series.

Subject/changelog of the correcting patch could mention this.
-- 
Stefan Richter
-=====-==--- -=== ==---
http://arcgraph.de/sr/

[PATCH 22/24] x86/oprofile: Add CONFIG_OPROFILE_IBS option

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/Kconfig                        |   14 ++++++++++++++
 arch/x86/oprofile/op_model_athlon.c |   33 +++++++++++++++++++++++----------
 drivers/oprofile/buffer_sync.c      |    6 ++++++
 drivers/oprofile/cpu_buffer.c       |    4 ++++
 4 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index ad89a33..62be4e2 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -13,6 +13,20 @@ config OPROFILE
 
 	  If unsure, say N.
 
+config OPROFILE_IBS
+	bool "OProfile AMD IBS support (EXPERIMENTAL)"
+	default n
+	depends on OPROFILE && SMP && X86
+	help
+          Instruction-Based Sampling (IBS) is a new profiling
+          technique that provides rich, precise program performance
+          information. IBS is introduced by AMD Family10h processors
+          (AMD Opteron Quad-Core processor “Barcelona”) to overcome
+          the limitations of conventional performance counter
+          sampling.
+
+	  If unsure, say N.
+
 config HAVE_OPROFILE
 	def_bool n
 
diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
index 9c8c8c5..fb6015c 100644
--- a/arch/x86/oprofile/op_model_athlon.c
+++ b/arch/x86/oprofile/op_model_athlon.c
@@ -47,6 +47,10 @@
 #define CTRL_SET_HOST_ONLY(val, h) (val |= ((h & 1) << 9))
 #define CTRL_SET_GUEST_ONLY(val, h) (val |= ((h & 1) << 8))
 
+static unsigned long reset_value[NUM_COUNTERS];
+
+#ifdef CONFIG_OPROFILE_IBS
+
 /* IbsFetchCtl bits/masks */
 #define IBS_FETCH_HIGH_VALID_BIT	(1UL << 17)	/* bit 49 */
 #define IBS_FETCH_HIGH_ENABLE		(1UL << 16)	/* bit 48 */
@@ -104,7 +108,6 @@ struct ibs_op_sample {
 */
 static void clear_ibs_nmi(void);
 
-static unsigned long reset_value[NUM_COUNTERS];
 static int ibs_allowed;	/* AMD Family10h and later */
 
 struct op_ibs_config {
@@ -118,6 +121,8 @@ struct op_ibs_config {
 
 static struct op_ibs_config ibs_config;
 
+#endif
+
 /* functions for op_amd_spec */
 
 static void op_amd_fill_in_addresses(struct op_msrs * const msrs)
@@ -188,6 +193,8 @@ static void op_amd_setup_ctrs(struct op_msrs const * const msrs)
 	}
 }
 
+#ifdef CONFIG_OPROFILE_IBS
+
 static inline int
 op_amd_handle_ibs(struct pt_regs * const regs,
 		  struct op_msrs const * const msrs)
@@ -261,6 +268,8 @@ op_amd_handle_ibs(struct pt_regs * const regs,
 	return 1;
 }
 
+#endif
+
 static int op_amd_check_ctrs(struct pt_regs * const regs,
 			     struct op_msrs const * const msrs)
 {
@@ -277,7 +286,9 @@ static int op_amd_check_ctrs(struct pt_regs * const regs,
 		}
 	}
 
+#ifdef CONFIG_OPROFILE_IBS
 	op_amd_handle_ibs(regs, msrs);
+#endif
 
 	/* See op_model_ppro.c */
 	return 1;
@@ -294,6 +305,8 @@ static void op_amd_start(struct op_msrs const * const msrs)
 			CTRL_WRITE(low, high, msrs, i);
 		}
 	}
+
+#ifdef CONFIG_OPROFILE_IBS
 	if (ibs_allowed && ibs_config.fetch_enabled) {
 		low = (ibs_config.max_cnt_fetch >> 4) & 0xFFFF;
 		high = IBS_FETCH_HIGH_ENABLE;
@@ -305,6 +318,7 @@ static void op_amd_start(struct op_msrs const * const msrs)
 		high = 0;
 		wrmsr(MSR_AMD64_IBSOPCTL, low, high);
 	}
+#endif
 }
 
 
@@ -323,6 +337,7 @@ static void op_amd_stop(struct op_msrs const * const msrs)
 		CTRL_WRITE(low, high, msrs, i);
 	}
 
+#ifdef CONFIG_OPROFILE_IBS
 	if (ibs_allowed && ibs_config.fetch_enabled) {
 		low = 0;		/* clear max count and enable */
 		high = 0;
@@ -334,6 +349,7 @@ static void op_amd_stop(struct op_msrs const * const msrs)
 		high = 0;
 		wrmsr(MSR_AMD64_IBSOPCTL, low, high);
 	}
+#endif
 }
 
 static void op_amd_shutdown(struct op_msrs const * const msrs)
@@ -350,17 +366,10 @@ static void op_amd_shutdown(struct op_msrs const * const msrs)
 	}
 }
 
-#ifndef CONFIG_SMP
+#ifndef CONFIG_OPROFILE_IBS
 
 /* no IBS support */
 
-static void setup_ibs(void)
-{
-	ibs_allowed = 0;
-}
-
-static void clear_ibs_nmi(void) {}
-
 static int op_amd_init(struct oprofile_operations *ops)
 {
 	return 0;
@@ -441,8 +450,12 @@ static void setup_ibs(void)
 	if (!ibs_allowed)
 		return;
 
-	if (pfm_amd64_setup_eilvt())
+	if (pfm_amd64_setup_eilvt()) {
 		ibs_allowed = 0;
+		return;
+	}
+
+	printk(KERN_INFO "oprofile: AMD IBS detected\n");
 }
 
 
diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index e1782d2..ed98227 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -328,6 +328,8 @@ static void add_trace_begin(void)
 	add_event_entry(TRACE_BEGIN_CODE);
 }
 
+#ifdef CONFIG_OPROFILE_IBS
+
 #define IBS_FETCH_CODE_SIZE	2
 #define IBS_OP_CODE_SIZE	5
 #define IBS_EIP(offset)				\
@@ -390,6 +392,8 @@ static void add_ibs_begin(struct oprofile_cpu_buffer *cpu_buf, int code,
 	}
 }
 
+#endif
+
 static void add_sample_entry(unsigned long offset, unsigned long event)
 {
 	add_event_entry(offset);
@@ -586,6 +590,7 @@ void sync_buffer(int cpu)
 			} else if (s->event == CPU_TRACE_BEGIN) {
 				state = sb_bt_start;
 				add_trace_begin();
+#ifdef CONFIG_OPROFILE_IBS
 			} else if (s->event == IBS_FETCH_BEGIN) {
 				state = sb_bt_start;
 				add_ibs_begin(cpu_buf,
@@ -594,6 +599,7 @@ void sync_buffer(int cpu)
 				state = sb_bt_start;
 				add_ibs_begin(cpu_buf,
 					IBS_OP_CODE, in_kernel, mm);
+#endif
 			} else {
 				struct mm_struct *oldmm = mm;
 
diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index c9ac4e1..aba905b 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -253,6 +253,8 @@ void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
 	oprofile_add_ext_sample(pc, regs, event, is_kernel);
 }
 
+#ifdef CONFIG_OPROFILE_IBS
+
 #define MAX_IBS_SAMPLE_SIZE	14
 static int log_ibs_sample(struct oprofile_cpu_buffer *cpu_buf,
 	unsigned long pc, int is_kernel, unsigned  int *ibs, int ibs_code)
@@ -318,6 +320,8 @@ void oprofile_add_ibs_sample(struct pt_regs *const regs,
 		oprofile_ops.backtrace(regs, backtrace_depth);
 }
 
+#endif
+
 void oprofile_add_pc(unsigned long pc, int is_kernel, unsigned long event)
 {
 	struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var(cpu_buffer);
-- 
1.5.5.4

Re: [PATCH 22/24] x86/oprofile: Add CONFIG_OPROFILE_IBS option

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> +config OPROFILE_IBS
> +	bool "OProfile AMD IBS support (EXPERIMENTAL)"
> +	default n
> +	depends on OPROFILE && SMP && X86
> +	help
> +          Instruction-Based Sampling (IBS) is a new profiling
> +          technique that provides rich, precise program performance
> +          information. IBS is introduced by AMD Family10h processors
> +          (AMD Opteron Quad-Core processor ???Barcelona???) to overcome
> +          the limitations of conventional performance counter
> +          sampling.

weird UTF characters in the description - please try to use ASCII in 
such general Kconfig help-texts.

also, it would be nice to have a pure technical description. I.e. 
something like:

> +          Instruction-Based Sampling (IBS) is a new hardware feature
> +          that provides precise, non-statistical profiling 
> +          information. IBS is available in AMD Family10h processors.
> +          (AMD Barcelona).

> +#ifdef CONFIG_OPROFILE_IBS
>  	op_amd_handle_ibs(regs, msrs);
> +#endif

please hide #ifdefs by making the op_amd_handle_ibs() function general.

	Ingo

[PATCH 23/24] oprofile: Fix printk in cpu_buffer.c

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 drivers/oprofile/cpu_buffer.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/oprofile/cpu_buffer.c b/drivers/oprofile/cpu_buffer.c
index aba905b..4decab6 100644
--- a/drivers/oprofile/cpu_buffer.c
+++ b/drivers/oprofile/cpu_buffer.c
@@ -364,7 +364,7 @@ static void wq_sync_buffer(struct work_struct *work)
 	struct oprofile_cpu_buffer * b =
 		container_of(work, struct oprofile_cpu_buffer, work.work);
 	if (b->cpu != smp_processor_id()) {
-		printk("WQ on CPU%d, prefer CPU%d\n",
+		printk(KERN_DEBUG "WQ on CPU%d, prefer CPU%d\n",
 		       smp_processor_id(), b->cpu);
 	}
 	sync_buffer(b->cpu);
-- 
1.5.5.4

[PATCH 24/24] x86/oprofile: Reanaming op_model_athlon.c to op_model_amd.c

[Robert Richter]

Signed-off-by: Robert Richter <robert.richter@amd.com>
---
 arch/x86/oprofile/Makefile          |    2 +-
 arch/x86/oprofile/op_model_amd.c    |  543 +++++++++++++++++++++++++++++++++++
 arch/x86/oprofile/op_model_athlon.c |  543 -----------------------------------
 3 files changed, 544 insertions(+), 544 deletions(-)
 create mode 100644 arch/x86/oprofile/op_model_amd.c
 delete mode 100644 arch/x86/oprofile/op_model_athlon.c

diff --git a/arch/x86/oprofile/Makefile b/arch/x86/oprofile/Makefile
index 30f3eb3..446902b 100644
--- a/arch/x86/oprofile/Makefile
+++ b/arch/x86/oprofile/Makefile
@@ -7,6 +7,6 @@ DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 		timer_int.o )
 
 oprofile-y				:= $(DRIVER_OBJS) init.o backtrace.o
-oprofile-$(CONFIG_X86_LOCAL_APIC) 	+= nmi_int.o op_model_athlon.o \
+oprofile-$(CONFIG_X86_LOCAL_APIC) 	+= nmi_int.o op_model_amd.o \
 					   op_model_ppro.o op_model_p4.o
 oprofile-$(CONFIG_X86_IO_APIC)		+= nmi_timer_int.o
diff --git a/arch/x86/oprofile/op_model_amd.c b/arch/x86/oprofile/op_model_amd.c
new file mode 100644
index 0000000..d9faf60
--- /dev/null
+++ b/arch/x86/oprofile/op_model_amd.c
@@ -0,0 +1,543 @@
+/*
+ * @file op_model_amd.c
+ * athlon / K7 / K8 / Family 10h model-specific MSR operations
+ *
+ * @remark Copyright 2002-2008 OProfile authors
+ * @remark Read the file COPYING
+ *
+ * @author John Levon
+ * @author Philippe Elie
+ * @author Graydon Hoare
+ * @author Robert Richter <robert.richter@amd.com>
+ * @author Barry Kasindorf
+*/
+
+#include <linux/oprofile.h>
+#include <linux/device.h>
+#include <linux/pci.h>
+
+#include <asm/ptrace.h>
+#include <asm/msr.h>
+#include <asm/nmi.h>
+
+#include "op_x86_model.h"
+#include "op_counter.h"
+
+#define NUM_COUNTERS 4
+#define NUM_CONTROLS 4
+
+#define CTR_IS_RESERVED(msrs, c) (msrs->counters[(c)].addr ? 1 : 0)
+#define CTR_READ(l, h, msrs, c) do {rdmsr(msrs->counters[(c)].addr, (l), (h)); } 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 CTRL_IS_RESERVED(msrs, c) (msrs->controls[(c)].addr ? 1 : 0)
+#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(x) (x &= (1<<21))
+#define CTRL_CLEAR_HI(x) (x &= 0xfffffcf0)
+#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_SET_UM(val, m) (val |= (m << 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))
+
+static unsigned long reset_value[NUM_COUNTERS];
+
+#ifdef CONFIG_OPROFILE_IBS
+
+/* IbsFetchCtl bits/masks */
+#define IBS_FETCH_HIGH_VALID_BIT	(1UL << 17)	/* bit 49 */
+#define IBS_FETCH_HIGH_ENABLE		(1UL << 16)	/* bit 48 */
+#define IBS_FETCH_LOW_MAX_CNT_MASK	0x0000FFFFUL	/* MaxCnt mask */
+
+/*IbsOpCtl bits */
+#define IBS_OP_LOW_VALID_BIT		(1ULL<<18)	/* bit 18 */
+#define IBS_OP_LOW_ENABLE		(1ULL<<17)	/* bit 17 */
+
+/* Codes used in cpu_buffer.c */
+/* This produces duplicate code, need to be fixed */
+#define IBS_FETCH_BEGIN 3
+#define IBS_OP_BEGIN    4
+
+/* The function interface needs to be fixed, something like add
+   data. Should then be added to linux/oprofile.h. */
+extern void oprofile_add_ibs_sample(struct pt_regs *const regs,
+				    unsigned int * const ibs_sample, u8 code);
+
+struct ibs_fetch_sample {
+	/* MSRC001_1031 IBS Fetch Linear Address Register */
+	unsigned int ibs_fetch_lin_addr_low;
+	unsigned int ibs_fetch_lin_addr_high;
+	/* MSRC001_1030 IBS Fetch Control Register */
+	unsigned int ibs_fetch_ctl_low;
+	unsigned int ibs_fetch_ctl_high;
+	/* MSRC001_1032 IBS Fetch Physical Address Register */
+	unsigned int ibs_fetch_phys_addr_low;
+	unsigned int ibs_fetch_phys_addr_high;
+};
+
+struct ibs_op_sample {
+	/* MSRC001_1034 IBS Op Logical Address Register (IbsRIP) */
+	unsigned int ibs_op_rip_low;
+	unsigned int ibs_op_rip_high;
+	/* MSRC001_1035 IBS Op Data Register */
+	unsigned int ibs_op_data1_low;
+	unsigned int ibs_op_data1_high;
+	/* MSRC001_1036 IBS Op Data 2 Register */
+	unsigned int ibs_op_data2_low;
+	unsigned int ibs_op_data2_high;
+	/* MSRC001_1037 IBS Op Data 3 Register */
+	unsigned int ibs_op_data3_low;
+	unsigned int ibs_op_data3_high;
+	/* MSRC001_1038 IBS DC Linear Address Register (IbsDcLinAd) */
+	unsigned int ibs_dc_linear_low;
+	unsigned int ibs_dc_linear_high;
+	/* MSRC001_1039 IBS DC Physical Address Register (IbsDcPhysAd) */
+	unsigned int ibs_dc_phys_low;
+	unsigned int ibs_dc_phys_high;
+};
+
+/*
+ * unitialize the APIC for the IBS interrupts if needed on AMD Family10h+
+*/
+static void clear_ibs_nmi(void);
+
+static int ibs_allowed;	/* AMD Family10h and later */
+
+struct op_ibs_config {
+	unsigned long op_enabled;
+	unsigned long fetch_enabled;
+	unsigned long max_cnt_fetch;
+	unsigned long max_cnt_op;
+	unsigned long rand_en;
+	unsigned long dispatched_ops;
+};
+
+static struct op_ibs_config ibs_config;
+
+#endif
+
+/* functions for op_amd_spec */
+
+static void op_amd_fill_in_addresses(struct op_msrs * const msrs)
+{
+	int i;
+
+	for (i = 0; i < NUM_COUNTERS; i++) {
+		if (reserve_perfctr_nmi(MSR_K7_PERFCTR0 + i))
+			msrs->counters[i].addr = MSR_K7_PERFCTR0 + i;
+		else
+			msrs->counters[i].addr = 0;
+	}
+
+	for (i = 0; i < NUM_CONTROLS; i++) {
+		if (reserve_evntsel_nmi(MSR_K7_EVNTSEL0 + i))
+			msrs->controls[i].addr = MSR_K7_EVNTSEL0 + i;
+		else
+			msrs->controls[i].addr = 0;
+	}
+}
+
+
+static void op_amd_setup_ctrs(struct op_msrs const * const msrs)
+{
+	unsigned int low, high;
+	int i;
+
+	/* clear all counters */
+	for (i = 0 ; i < NUM_CONTROLS; ++i) {
+		if (unlikely(!CTRL_IS_RESERVED(msrs, i)))
+			continue;
+		CTRL_READ(low, high, msrs, i);
+		CTRL_CLEAR_LO(low);
+		CTRL_CLEAR_HI(high);
+		CTRL_WRITE(low, high, msrs, i);
+	}
+
+	/* avoid a false detection of ctr overflows in NMI handler */
+	for (i = 0; i < NUM_COUNTERS; ++i) {
+		if (unlikely(!CTR_IS_RESERVED(msrs, i)))
+			continue;
+		CTR_WRITE(1, msrs, i);
+	}
+
+	/* enable active counters */
+	for (i = 0; i < NUM_COUNTERS; ++i) {
+		if ((counter_config[i].enabled) && (CTR_IS_RESERVED(msrs, i))) {
+			reset_value[i] = counter_config[i].count;
+
+			CTR_WRITE(counter_config[i].count, msrs, i);
+
+			CTRL_READ(low, high, msrs, i);
+			CTRL_CLEAR_LO(low);
+			CTRL_CLEAR_HI(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);
+		} else {
+			reset_value[i] = 0;
+		}
+	}
+}
+
+#ifdef CONFIG_OPROFILE_IBS
+
+static inline int
+op_amd_handle_ibs(struct pt_regs * const regs,
+		  struct op_msrs const * const msrs)
+{
+	unsigned int low, high;
+	struct ibs_fetch_sample ibs_fetch;
+	struct ibs_op_sample ibs_op;
+
+	if (!ibs_allowed)
+		return 1;
+
+	if (ibs_config.fetch_enabled) {
+		rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+		if (high & IBS_FETCH_HIGH_VALID_BIT) {
+			ibs_fetch.ibs_fetch_ctl_high = high;
+			ibs_fetch.ibs_fetch_ctl_low = low;
+			rdmsr(MSR_AMD64_IBSFETCHLINAD, low, high);
+			ibs_fetch.ibs_fetch_lin_addr_high = high;
+			ibs_fetch.ibs_fetch_lin_addr_low = low;
+			rdmsr(MSR_AMD64_IBSFETCHPHYSAD, low, high);
+			ibs_fetch.ibs_fetch_phys_addr_high = high;
+			ibs_fetch.ibs_fetch_phys_addr_low = low;
+
+			oprofile_add_ibs_sample(regs,
+						(unsigned int *)&ibs_fetch,
+						IBS_FETCH_BEGIN);
+
+			/*reenable the IRQ */
+			rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+			high &= ~IBS_FETCH_HIGH_VALID_BIT;
+			high |= IBS_FETCH_HIGH_ENABLE;
+			low &= IBS_FETCH_LOW_MAX_CNT_MASK;
+			wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+		}
+	}
+
+	if (ibs_config.op_enabled) {
+		rdmsr(MSR_AMD64_IBSOPCTL, low, high);
+		if (low & IBS_OP_LOW_VALID_BIT) {
+			rdmsr(MSR_AMD64_IBSOPRIP, low, high);
+			ibs_op.ibs_op_rip_low = low;
+			ibs_op.ibs_op_rip_high = high;
+			rdmsr(MSR_AMD64_IBSOPDATA, low, high);
+			ibs_op.ibs_op_data1_low = low;
+			ibs_op.ibs_op_data1_high = high;
+			rdmsr(MSR_AMD64_IBSOPDATA2, low, high);
+			ibs_op.ibs_op_data2_low = low;
+			ibs_op.ibs_op_data2_high = high;
+			rdmsr(MSR_AMD64_IBSOPDATA3, low, high);
+			ibs_op.ibs_op_data3_low = low;
+			ibs_op.ibs_op_data3_high = high;
+			rdmsr(MSR_AMD64_IBSDCLINAD, low, high);
+			ibs_op.ibs_dc_linear_low = low;
+			ibs_op.ibs_dc_linear_high = high;
+			rdmsr(MSR_AMD64_IBSDCPHYSAD, low, high);
+			ibs_op.ibs_dc_phys_low = low;
+			ibs_op.ibs_dc_phys_high = high;
+
+			/* reenable the IRQ */
+			oprofile_add_ibs_sample(regs,
+						(unsigned int *)&ibs_op,
+						IBS_OP_BEGIN);
+			rdmsr(MSR_AMD64_IBSOPCTL, low, high);
+			high = 0;
+			low &= ~IBS_OP_LOW_VALID_BIT;
+			low |= IBS_OP_LOW_ENABLE;
+			wrmsr(MSR_AMD64_IBSOPCTL, low, high);
+		}
+	}
+
+	return 1;
+}
+
+#endif
+
+static int op_amd_check_ctrs(struct pt_regs * const regs,
+			     struct op_msrs const * const msrs)
+{
+	unsigned int low, high;
+	int i;
+
+	for (i = 0 ; i < NUM_COUNTERS; ++i) {
+		if (!reset_value[i])
+			continue;
+		CTR_READ(low, high, msrs, i);
+		if (CTR_OVERFLOWED(low)) {
+			oprofile_add_sample(regs, i);
+			CTR_WRITE(reset_value[i], msrs, i);
+		}
+	}
+
+#ifdef CONFIG_OPROFILE_IBS
+	op_amd_handle_ibs(regs, msrs);
+#endif
+
+	/* See op_model_ppro.c */
+	return 1;
+}
+
+static void op_amd_start(struct op_msrs const * const msrs)
+{
+	unsigned int low, high;
+	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);
+		}
+	}
+
+#ifdef CONFIG_OPROFILE_IBS
+	if (ibs_allowed && ibs_config.fetch_enabled) {
+		low = (ibs_config.max_cnt_fetch >> 4) & 0xFFFF;
+		high = IBS_FETCH_HIGH_ENABLE;
+		wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+	}
+
+	if (ibs_allowed && ibs_config.op_enabled) {
+		low = ((ibs_config.max_cnt_op >> 4) & 0xFFFF) + IBS_OP_LOW_ENABLE;
+		high = 0;
+		wrmsr(MSR_AMD64_IBSOPCTL, low, high);
+	}
+#endif
+}
+
+
+static void op_amd_stop(struct op_msrs const * const msrs)
+{
+	unsigned int low, high;
+	int i;
+
+	/* Subtle: stop on all counters to avoid race with
+	 * setting our pm callback */
+	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);
+	}
+
+#ifdef CONFIG_OPROFILE_IBS
+	if (ibs_allowed && ibs_config.fetch_enabled) {
+		low = 0;		/* clear max count and enable */
+		high = 0;
+		wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
+	}
+
+	if (ibs_allowed && ibs_config.op_enabled) {
+		low = 0;		/* clear max count and enable */
+		high = 0;
+		wrmsr(MSR_AMD64_IBSOPCTL, low, high);
+	}
+#endif
+}
+
+static void op_amd_shutdown(struct op_msrs const * const msrs)
+{
+	int i;
+
+	for (i = 0 ; i < NUM_COUNTERS ; ++i) {
+		if (CTR_IS_RESERVED(msrs, i))
+			release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
+	}
+	for (i = 0 ; i < NUM_CONTROLS ; ++i) {
+		if (CTRL_IS_RESERVED(msrs, i))
+			release_evntsel_nmi(MSR_K7_EVNTSEL0 + i);
+	}
+}
+
+#ifndef CONFIG_OPROFILE_IBS
+
+/* no IBS support */
+
+static int op_amd_init(struct oprofile_operations *ops)
+{
+	return 0;
+}
+
+static void op_amd_exit(void) {}
+
+#else
+
+static u8 ibs_eilvt_off;
+
+static inline void apic_init_ibs_nmi_per_cpu(void *arg)
+{
+	ibs_eilvt_off = setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_NMI, 0);
+}
+
+static inline void apic_clear_ibs_nmi_per_cpu(void *arg)
+{
+	setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_FIX, 1);
+}
+
+static int pfm_amd64_setup_eilvt(void)
+{
+#define IBSCTL_LVTOFFSETVAL		(1 << 8)
+#define IBSCTL				0x1cc
+	struct pci_dev *cpu_cfg;
+	int nodes;
+	u32 value = 0;
+
+	/* per CPU setup */
+	on_each_cpu(apic_init_ibs_nmi_per_cpu, NULL, 1);
+
+	nodes = 0;
+	cpu_cfg = NULL;
+	do {
+		cpu_cfg = pci_get_device(PCI_VENDOR_ID_AMD,
+					 PCI_DEVICE_ID_AMD_10H_NB_MISC,
+					 cpu_cfg);
+		if (!cpu_cfg)
+			break;
+		++nodes;
+		pci_write_config_dword(cpu_cfg, IBSCTL, ibs_eilvt_off
+				       | IBSCTL_LVTOFFSETVAL);
+		pci_read_config_dword(cpu_cfg, IBSCTL, &value);
+		if (value != (ibs_eilvt_off | IBSCTL_LVTOFFSETVAL)) {
+			printk(KERN_DEBUG "Failed to setup IBS LVT offset, "
+				"IBSCTL = 0x%08x", value);
+			return 1;
+		}
+	} while (1);
+
+	if (!nodes) {
+		printk(KERN_DEBUG "No CPU node configured for IBS");
+		return 1;
+	}
+
+#ifdef CONFIG_NUMA
+	/* Sanity check */
+	/* Works only for 64bit with proper numa implementation. */
+	if (nodes != num_possible_nodes()) {
+		printk(KERN_DEBUG "Failed to setup CPU node(s) for IBS, "
+			"found: %d, expected %d",
+			nodes, num_possible_nodes());
+		return 1;
+	}
+#endif
+	return 0;
+}
+
+/*
+ * initialize the APIC for the IBS interrupts
+ * if available (AMD Family10h rev B0 and later)
+ */
+static void setup_ibs(void)
+{
+	ibs_allowed = boot_cpu_has(X86_FEATURE_IBS);
+
+	if (!ibs_allowed)
+		return;
+
+	if (pfm_amd64_setup_eilvt()) {
+		ibs_allowed = 0;
+		return;
+	}
+
+	printk(KERN_INFO "oprofile: AMD IBS detected\n");
+}
+
+
+/*
+ * unitialize the APIC for the IBS interrupts if needed on AMD Family10h
+ * rev B0 and later */
+static void clear_ibs_nmi(void)
+{
+	if (ibs_allowed)
+		on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1);
+}
+
+static int (*create_arch_files)(struct super_block * sb, struct dentry * root);
+
+static int setup_ibs_files(struct super_block * sb, struct dentry * root)
+{
+	char buf[12];
+	struct dentry *dir;
+	int ret = 0;
+
+	/* architecture specific files */
+	if (create_arch_files)
+		ret = create_arch_files(sb, root);
+
+	if (ret)
+		return ret;
+
+	if (!ibs_allowed)
+		return ret;
+
+	/* model specific files */
+
+	/* setup some reasonable defaults */
+	ibs_config.max_cnt_fetch = 250000;
+	ibs_config.fetch_enabled = 0;
+	ibs_config.max_cnt_op = 250000;
+	ibs_config.op_enabled = 0;
+	ibs_config.dispatched_ops = 1;
+	snprintf(buf,  sizeof(buf), "ibs_fetch");
+	dir = oprofilefs_mkdir(sb, root, buf);
+	oprofilefs_create_ulong(sb, dir, "rand_enable",
+				&ibs_config.rand_en);
+	oprofilefs_create_ulong(sb, dir, "enable",
+		&ibs_config.fetch_enabled);
+	oprofilefs_create_ulong(sb, dir, "max_count",
+		&ibs_config.max_cnt_fetch);
+	snprintf(buf,  sizeof(buf), "ibs_uops");
+	dir = oprofilefs_mkdir(sb, root, buf);
+	oprofilefs_create_ulong(sb, dir, "enable",
+		&ibs_config.op_enabled);
+	oprofilefs_create_ulong(sb, dir, "max_count",
+		&ibs_config.max_cnt_op);
+	oprofilefs_create_ulong(sb, dir, "dispatched_ops",
+		&ibs_config.dispatched_ops);
+
+	return 0;
+}
+
+static int op_amd_init(struct oprofile_operations *ops)
+{
+	setup_ibs();
+	create_arch_files = ops->create_files;
+	ops->create_files = setup_ibs_files;
+	return 0;
+}
+
+static void op_amd_exit(void)
+{
+	clear_ibs_nmi();
+}
+
+#endif
+
+struct op_x86_model_spec const op_amd_spec = {
+	.init = op_amd_init,
+	.exit = op_amd_exit,
+	.num_counters = NUM_COUNTERS,
+	.num_controls = NUM_CONTROLS,
+	.fill_in_addresses = &op_amd_fill_in_addresses,
+	.setup_ctrs = &op_amd_setup_ctrs,
+	.check_ctrs = &op_amd_check_ctrs,
+	.start = &op_amd_start,
+	.stop = &op_amd_stop,
+	.shutdown = &op_amd_shutdown
+};
diff --git a/arch/x86/oprofile/op_model_athlon.c b/arch/x86/oprofile/op_model_athlon.c
deleted file mode 100644
index fb6015c..0000000
--- a/arch/x86/oprofile/op_model_athlon.c
+++ /dev/null
@@ -1,543 +0,0 @@
-/*
- * @file op_model_athlon.c
- * athlon / K7 / K8 / Family 10h model-specific MSR operations
- *
- * @remark Copyright 2002-2008 OProfile authors
- * @remark Read the file COPYING
- *
- * @author John Levon
- * @author Philippe Elie
- * @author Graydon Hoare
- * @author Robert Richter <robert.richter@amd.com>
- * @author Barry Kasindorf
-*/
-
-#include <linux/oprofile.h>
-#include <linux/device.h>
-#include <linux/pci.h>
-
-#include <asm/ptrace.h>
-#include <asm/msr.h>
-#include <asm/nmi.h>
-
-#include "op_x86_model.h"
-#include "op_counter.h"
-
-#define NUM_COUNTERS 4
-#define NUM_CONTROLS 4
-
-#define CTR_IS_RESERVED(msrs, c) (msrs->counters[(c)].addr ? 1 : 0)
-#define CTR_READ(l, h, msrs, c) do {rdmsr(msrs->counters[(c)].addr, (l), (h)); } 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 CTRL_IS_RESERVED(msrs, c) (msrs->controls[(c)].addr ? 1 : 0)
-#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(x) (x &= (1<<21))
-#define CTRL_CLEAR_HI(x) (x &= 0xfffffcf0)
-#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_SET_UM(val, m) (val |= (m << 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))
-
-static unsigned long reset_value[NUM_COUNTERS];
-
-#ifdef CONFIG_OPROFILE_IBS
-
-/* IbsFetchCtl bits/masks */
-#define IBS_FETCH_HIGH_VALID_BIT	(1UL << 17)	/* bit 49 */
-#define IBS_FETCH_HIGH_ENABLE		(1UL << 16)	/* bit 48 */
-#define IBS_FETCH_LOW_MAX_CNT_MASK	0x0000FFFFUL	/* MaxCnt mask */
-
-/*IbsOpCtl bits */
-#define IBS_OP_LOW_VALID_BIT		(1ULL<<18)	/* bit 18 */
-#define IBS_OP_LOW_ENABLE		(1ULL<<17)	/* bit 17 */
-
-/* Codes used in cpu_buffer.c */
-/* This produces duplicate code, need to be fixed */
-#define IBS_FETCH_BEGIN 3
-#define IBS_OP_BEGIN    4
-
-/* The function interface needs to be fixed, something like add
-   data. Should then be added to linux/oprofile.h. */
-extern void oprofile_add_ibs_sample(struct pt_regs *const regs,
-				    unsigned int * const ibs_sample, u8 code);
-
-struct ibs_fetch_sample {
-	/* MSRC001_1031 IBS Fetch Linear Address Register */
-	unsigned int ibs_fetch_lin_addr_low;
-	unsigned int ibs_fetch_lin_addr_high;
-	/* MSRC001_1030 IBS Fetch Control Register */
-	unsigned int ibs_fetch_ctl_low;
-	unsigned int ibs_fetch_ctl_high;
-	/* MSRC001_1032 IBS Fetch Physical Address Register */
-	unsigned int ibs_fetch_phys_addr_low;
-	unsigned int ibs_fetch_phys_addr_high;
-};
-
-struct ibs_op_sample {
-	/* MSRC001_1034 IBS Op Logical Address Register (IbsRIP) */
-	unsigned int ibs_op_rip_low;
-	unsigned int ibs_op_rip_high;
-	/* MSRC001_1035 IBS Op Data Register */
-	unsigned int ibs_op_data1_low;
-	unsigned int ibs_op_data1_high;
-	/* MSRC001_1036 IBS Op Data 2 Register */
-	unsigned int ibs_op_data2_low;
-	unsigned int ibs_op_data2_high;
-	/* MSRC001_1037 IBS Op Data 3 Register */
-	unsigned int ibs_op_data3_low;
-	unsigned int ibs_op_data3_high;
-	/* MSRC001_1038 IBS DC Linear Address Register (IbsDcLinAd) */
-	unsigned int ibs_dc_linear_low;
-	unsigned int ibs_dc_linear_high;
-	/* MSRC001_1039 IBS DC Physical Address Register (IbsDcPhysAd) */
-	unsigned int ibs_dc_phys_low;
-	unsigned int ibs_dc_phys_high;
-};
-
-/*
- * unitialize the APIC for the IBS interrupts if needed on AMD Family10h+
-*/
-static void clear_ibs_nmi(void);
-
-static int ibs_allowed;	/* AMD Family10h and later */
-
-struct op_ibs_config {
-	unsigned long op_enabled;
-	unsigned long fetch_enabled;
-	unsigned long max_cnt_fetch;
-	unsigned long max_cnt_op;
-	unsigned long rand_en;
-	unsigned long dispatched_ops;
-};
-
-static struct op_ibs_config ibs_config;
-
-#endif
-
-/* functions for op_amd_spec */
-
-static void op_amd_fill_in_addresses(struct op_msrs * const msrs)
-{
-	int i;
-
-	for (i = 0; i < NUM_COUNTERS; i++) {
-		if (reserve_perfctr_nmi(MSR_K7_PERFCTR0 + i))
-			msrs->counters[i].addr = MSR_K7_PERFCTR0 + i;
-		else
-			msrs->counters[i].addr = 0;
-	}
-
-	for (i = 0; i < NUM_CONTROLS; i++) {
-		if (reserve_evntsel_nmi(MSR_K7_EVNTSEL0 + i))
-			msrs->controls[i].addr = MSR_K7_EVNTSEL0 + i;
-		else
-			msrs->controls[i].addr = 0;
-	}
-}
-
-
-static void op_amd_setup_ctrs(struct op_msrs const * const msrs)
-{
-	unsigned int low, high;
-	int i;
-
-	/* clear all counters */
-	for (i = 0 ; i < NUM_CONTROLS; ++i) {
-		if (unlikely(!CTRL_IS_RESERVED(msrs, i)))
-			continue;
-		CTRL_READ(low, high, msrs, i);
-		CTRL_CLEAR_LO(low);
-		CTRL_CLEAR_HI(high);
-		CTRL_WRITE(low, high, msrs, i);
-	}
-
-	/* avoid a false detection of ctr overflows in NMI handler */
-	for (i = 0; i < NUM_COUNTERS; ++i) {
-		if (unlikely(!CTR_IS_RESERVED(msrs, i)))
-			continue;
-		CTR_WRITE(1, msrs, i);
-	}
-
-	/* enable active counters */
-	for (i = 0; i < NUM_COUNTERS; ++i) {
-		if ((counter_config[i].enabled) && (CTR_IS_RESERVED(msrs, i))) {
-			reset_value[i] = counter_config[i].count;
-
-			CTR_WRITE(counter_config[i].count, msrs, i);
-
-			CTRL_READ(low, high, msrs, i);
-			CTRL_CLEAR_LO(low);
-			CTRL_CLEAR_HI(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);
-		} else {
-			reset_value[i] = 0;
-		}
-	}
-}
-
-#ifdef CONFIG_OPROFILE_IBS
-
-static inline int
-op_amd_handle_ibs(struct pt_regs * const regs,
-		  struct op_msrs const * const msrs)
-{
-	unsigned int low, high;
-	struct ibs_fetch_sample ibs_fetch;
-	struct ibs_op_sample ibs_op;
-
-	if (!ibs_allowed)
-		return 1;
-
-	if (ibs_config.fetch_enabled) {
-		rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
-		if (high & IBS_FETCH_HIGH_VALID_BIT) {
-			ibs_fetch.ibs_fetch_ctl_high = high;
-			ibs_fetch.ibs_fetch_ctl_low = low;
-			rdmsr(MSR_AMD64_IBSFETCHLINAD, low, high);
-			ibs_fetch.ibs_fetch_lin_addr_high = high;
-			ibs_fetch.ibs_fetch_lin_addr_low = low;
-			rdmsr(MSR_AMD64_IBSFETCHPHYSAD, low, high);
-			ibs_fetch.ibs_fetch_phys_addr_high = high;
-			ibs_fetch.ibs_fetch_phys_addr_low = low;
-
-			oprofile_add_ibs_sample(regs,
-						(unsigned int *)&ibs_fetch,
-						IBS_FETCH_BEGIN);
-
-			/*reenable the IRQ */
-			rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
-			high &= ~IBS_FETCH_HIGH_VALID_BIT;
-			high |= IBS_FETCH_HIGH_ENABLE;
-			low &= IBS_FETCH_LOW_MAX_CNT_MASK;
-			wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
-		}
-	}
-
-	if (ibs_config.op_enabled) {
-		rdmsr(MSR_AMD64_IBSOPCTL, low, high);
-		if (low & IBS_OP_LOW_VALID_BIT) {
-			rdmsr(MSR_AMD64_IBSOPRIP, low, high);
-			ibs_op.ibs_op_rip_low = low;
-			ibs_op.ibs_op_rip_high = high;
-			rdmsr(MSR_AMD64_IBSOPDATA, low, high);
-			ibs_op.ibs_op_data1_low = low;
-			ibs_op.ibs_op_data1_high = high;
-			rdmsr(MSR_AMD64_IBSOPDATA2, low, high);
-			ibs_op.ibs_op_data2_low = low;
-			ibs_op.ibs_op_data2_high = high;
-			rdmsr(MSR_AMD64_IBSOPDATA3, low, high);
-			ibs_op.ibs_op_data3_low = low;
-			ibs_op.ibs_op_data3_high = high;
-			rdmsr(MSR_AMD64_IBSDCLINAD, low, high);
-			ibs_op.ibs_dc_linear_low = low;
-			ibs_op.ibs_dc_linear_high = high;
-			rdmsr(MSR_AMD64_IBSDCPHYSAD, low, high);
-			ibs_op.ibs_dc_phys_low = low;
-			ibs_op.ibs_dc_phys_high = high;
-
-			/* reenable the IRQ */
-			oprofile_add_ibs_sample(regs,
-						(unsigned int *)&ibs_op,
-						IBS_OP_BEGIN);
-			rdmsr(MSR_AMD64_IBSOPCTL, low, high);
-			high = 0;
-			low &= ~IBS_OP_LOW_VALID_BIT;
-			low |= IBS_OP_LOW_ENABLE;
-			wrmsr(MSR_AMD64_IBSOPCTL, low, high);
-		}
-	}
-
-	return 1;
-}
-
-#endif
-
-static int op_amd_check_ctrs(struct pt_regs * const regs,
-			     struct op_msrs const * const msrs)
-{
-	unsigned int low, high;
-	int i;
-
-	for (i = 0 ; i < NUM_COUNTERS; ++i) {
-		if (!reset_value[i])
-			continue;
-		CTR_READ(low, high, msrs, i);
-		if (CTR_OVERFLOWED(low)) {
-			oprofile_add_sample(regs, i);
-			CTR_WRITE(reset_value[i], msrs, i);
-		}
-	}
-
-#ifdef CONFIG_OPROFILE_IBS
-	op_amd_handle_ibs(regs, msrs);
-#endif
-
-	/* See op_model_ppro.c */
-	return 1;
-}
-
-static void op_amd_start(struct op_msrs const * const msrs)
-{
-	unsigned int low, high;
-	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);
-		}
-	}
-
-#ifdef CONFIG_OPROFILE_IBS
-	if (ibs_allowed && ibs_config.fetch_enabled) {
-		low = (ibs_config.max_cnt_fetch >> 4) & 0xFFFF;
-		high = IBS_FETCH_HIGH_ENABLE;
-		wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
-	}
-
-	if (ibs_allowed && ibs_config.op_enabled) {
-		low = ((ibs_config.max_cnt_op >> 4) & 0xFFFF) + IBS_OP_LOW_ENABLE;
-		high = 0;
-		wrmsr(MSR_AMD64_IBSOPCTL, low, high);
-	}
-#endif
-}
-
-
-static void op_amd_stop(struct op_msrs const * const msrs)
-{
-	unsigned int low, high;
-	int i;
-
-	/* Subtle: stop on all counters to avoid race with
-	 * setting our pm callback */
-	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);
-	}
-
-#ifdef CONFIG_OPROFILE_IBS
-	if (ibs_allowed && ibs_config.fetch_enabled) {
-		low = 0;		/* clear max count and enable */
-		high = 0;
-		wrmsr(MSR_AMD64_IBSFETCHCTL, low, high);
-	}
-
-	if (ibs_allowed && ibs_config.op_enabled) {
-		low = 0;		/* clear max count and enable */
-		high = 0;
-		wrmsr(MSR_AMD64_IBSOPCTL, low, high);
-	}
-#endif
-}
-
-static void op_amd_shutdown(struct op_msrs const * const msrs)
-{
-	int i;
-
-	for (i = 0 ; i < NUM_COUNTERS ; ++i) {
-		if (CTR_IS_RESERVED(msrs, i))
-			release_perfctr_nmi(MSR_K7_PERFCTR0 + i);
-	}
-	for (i = 0 ; i < NUM_CONTROLS ; ++i) {
-		if (CTRL_IS_RESERVED(msrs, i))
-			release_evntsel_nmi(MSR_K7_EVNTSEL0 + i);
-	}
-}
-
-#ifndef CONFIG_OPROFILE_IBS
-
-/* no IBS support */
-
-static int op_amd_init(struct oprofile_operations *ops)
-{
-	return 0;
-}
-
-static void op_amd_exit(void) {}
-
-#else
-
-static u8 ibs_eilvt_off;
-
-static inline void apic_init_ibs_nmi_per_cpu(void *arg)
-{
-	ibs_eilvt_off = setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_NMI, 0);
-}
-
-static inline void apic_clear_ibs_nmi_per_cpu(void *arg)
-{
-	setup_APIC_eilvt_ibs(0, APIC_EILVT_MSG_FIX, 1);
-}
-
-static int pfm_amd64_setup_eilvt(void)
-{
-#define IBSCTL_LVTOFFSETVAL		(1 << 8)
-#define IBSCTL				0x1cc
-	struct pci_dev *cpu_cfg;
-	int nodes;
-	u32 value = 0;
-
-	/* per CPU setup */
-	on_each_cpu(apic_init_ibs_nmi_per_cpu, NULL, 1);
-
-	nodes = 0;
-	cpu_cfg = NULL;
-	do {
-		cpu_cfg = pci_get_device(PCI_VENDOR_ID_AMD,
-					 PCI_DEVICE_ID_AMD_10H_NB_MISC,
-					 cpu_cfg);
-		if (!cpu_cfg)
-			break;
-		++nodes;
-		pci_write_config_dword(cpu_cfg, IBSCTL, ibs_eilvt_off
-				       | IBSCTL_LVTOFFSETVAL);
-		pci_read_config_dword(cpu_cfg, IBSCTL, &value);
-		if (value != (ibs_eilvt_off | IBSCTL_LVTOFFSETVAL)) {
-			printk(KERN_DEBUG "Failed to setup IBS LVT offset, "
-				"IBSCTL = 0x%08x", value);
-			return 1;
-		}
-	} while (1);
-
-	if (!nodes) {
-		printk(KERN_DEBUG "No CPU node configured for IBS");
-		return 1;
-	}
-
-#ifdef CONFIG_NUMA
-	/* Sanity check */
-	/* Works only for 64bit with proper numa implementation. */
-	if (nodes != num_possible_nodes()) {
-		printk(KERN_DEBUG "Failed to setup CPU node(s) for IBS, "
-			"found: %d, expected %d",
-			nodes, num_possible_nodes());
-		return 1;
-	}
-#endif
-	return 0;
-}
-
-/*
- * initialize the APIC for the IBS interrupts
- * if available (AMD Family10h rev B0 and later)
- */
-static void setup_ibs(void)
-{
-	ibs_allowed = boot_cpu_has(X86_FEATURE_IBS);
-
-	if (!ibs_allowed)
-		return;
-
-	if (pfm_amd64_setup_eilvt()) {
-		ibs_allowed = 0;
-		return;
-	}
-
-	printk(KERN_INFO "oprofile: AMD IBS detected\n");
-}
-
-
-/*
- * unitialize the APIC for the IBS interrupts if needed on AMD Family10h
- * rev B0 and later */
-static void clear_ibs_nmi(void)
-{
-	if (ibs_allowed)
-		on_each_cpu(apic_clear_ibs_nmi_per_cpu, NULL, 1);
-}
-
-static int (*create_arch_files)(struct super_block * sb, struct dentry * root);
-
-static int setup_ibs_files(struct super_block * sb, struct dentry * root)
-{
-	char buf[12];
-	struct dentry *dir;
-	int ret = 0;
-
-	/* architecture specific files */
-	if (create_arch_files)
-		ret = create_arch_files(sb, root);
-
-	if (ret)
-		return ret;
-
-	if (!ibs_allowed)
-		return ret;
-
-	/* model specific files */
-
-	/* setup some reasonable defaults */
-	ibs_config.max_cnt_fetch = 250000;
-	ibs_config.fetch_enabled = 0;
-	ibs_config.max_cnt_op = 250000;
-	ibs_config.op_enabled = 0;
-	ibs_config.dispatched_ops = 1;
-	snprintf(buf,  sizeof(buf), "ibs_fetch");
-	dir = oprofilefs_mkdir(sb, root, buf);
-	oprofilefs_create_ulong(sb, dir, "rand_enable",
-				&ibs_config.rand_en);
-	oprofilefs_create_ulong(sb, dir, "enable",
-		&ibs_config.fetch_enabled);
-	oprofilefs_create_ulong(sb, dir, "max_count",
-		&ibs_config.max_cnt_fetch);
-	snprintf(buf,  sizeof(buf), "ibs_uops");
-	dir = oprofilefs_mkdir(sb, root, buf);
-	oprofilefs_create_ulong(sb, dir, "enable",
-		&ibs_config.op_enabled);
-	oprofilefs_create_ulong(sb, dir, "max_count",
-		&ibs_config.max_cnt_op);
-	oprofilefs_create_ulong(sb, dir, "dispatched_ops",
-		&ibs_config.dispatched_ops);
-
-	return 0;
-}
-
-static int op_amd_init(struct oprofile_operations *ops)
-{
-	setup_ibs();
-	create_arch_files = ops->create_files;
-	ops->create_files = setup_ibs_files;
-	return 0;
-}
-
-static void op_amd_exit(void)
-{
-	clear_ibs_nmi();
-}
-
-#endif
-
-struct op_x86_model_spec const op_amd_spec = {
-	.init = op_amd_init,
-	.exit = op_amd_exit,
-	.num_counters = NUM_COUNTERS,
-	.num_controls = NUM_CONTROLS,
-	.fill_in_addresses = &op_amd_fill_in_addresses,
-	.setup_ctrs = &op_amd_setup_ctrs,
-	.check_ctrs = &op_amd_check_ctrs,
-	.start = &op_amd_start,
-	.stop = &op_amd_stop,
-	.shutdown = &op_amd_shutdown
-};
-- 
1.5.5.4

Re: [PATCH 24/24] x86/oprofile: Reanaming op_model_athlon.c to op_model_amd.c

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> +#define NUM_COUNTERS 4
> +#define NUM_CONTROLS 4
> +
> +#define CTR_IS_RESERVED(msrs, c) (msrs->counters[(c)].addr ? 1 : 0)
> +#define CTR_READ(l, h, msrs, c) do {rdmsr(msrs->counters[(c)].addr, (l), (h)); } 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 CTRL_IS_RESERVED(msrs, c) (msrs->controls[(c)].addr ? 1 : 0)
> +#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(x) (x &= (1<<21))
> +#define CTRL_CLEAR_HI(x) (x &= 0xfffffcf0)
> +#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_SET_UM(val, m) (val |= (m << 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))

while at it, it would be nice to have a followup cleanup that aligns 
these vertically.

> +#ifdef CONFIG_OPROFILE_IBS

btw., why not include IBS support unconditionally in the _amd.ko module? 
It's all loadable anyway - and the size difference is small. That would 
get rid of a ton of #ifdef complexity.

	Ingo

Re: [PATCH 0/24] oprofile: Add IBS support for AMD CPUs

[Maynard Johnson]

Robert Richter wrote:
> Patches #1-3 are not directly related to IBS.
> Patch #4 adds generic support of model specific initialization.
>   
And patches 5-9 are for . . .

It seems that you have 5 independent patch sets here.  I don't see any
advantage in posting them together in one large group -- maybe more of a
disadvantage, since some are pretty non-controversial and should get
accepted fairly easily, while others will probably result in extended
discussion.  You might consider reposting these 5 patch sets
individually.  But I'm not so familiar with kernel patch
review/acceptance process, so maybe this is fine as it is if subsets of
the 24 patches are accepted incrementally.

-Maynard
> Patches #10, #11 add the core implementation of IBS.
> Patches #12-24 contain code improvements and small fixes.
>
>
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> oprofile-list mailing list
> oprofile-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/oprofile-list
>   

Re: [PATCH 0/24] oprofile: Add IBS support for AMD CPUs

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> Patches #1-3 are not directly related to IBS.
> Patch #4 adds generic support of model specific initialization.
> Patches #10, #11 add the core implementation of IBS.
> Patches #12-24 contain code improvements and small fixes.

cool stuff! I've created the tip/oprofile topic branch for the patches 
from you, Barry and Jason and have applied all the current patches (and 
followup fixes) to it. Below are the access coordinates and the log 
summary.

It's not yet integrated into tip/master - will do some testing first. 
I'll send review feedback separately as well, on a per patch basis.

Thanks,

	Ingo

------------->
You can pull the latest tip/oprofile git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git oprofile

------------------>
Barry Kasindorf (3):
      oprofile: Add support for AMD Family 11h
      x86/oprofile: add IBS support for AMD CPUs, IBS buffer handling routines
      x86/oprofile: add IBS support for AMD CPUs, model specific code

Jason Yeh (1):
      Oprofile Multiplexing Patch

Robert Richter (23):
      x86: add PCI IDs for AMD Barcelona PCI devices
      x86: apic_*.c: add description to AMD's extended LVT functions
      x86/oprofile: introduce model specific init/exit functions
      x86/oprofile: Minor changes in op_model_athlon.c
      x86/oprofile: renaming athlon_*() into op_amd_*()
      drivers/oprofile: coding style fixes in buffer_sync.c
      OProfile: moving increment_tail() in buffer_sync.c
      OProfile: add IBS code macros
      x86/oprofile: separating the IBS handler
      OProfile: change IBS interrupt initialization
      OProfile: Fix build error in op_model_athlon.c
      OProfile: on_each_cpu(): kill unused retry parameter
      OProfile: fix setup_ibs_files() function interface
      OProfile: enable IBS for AMD CPUs
      OProfile: fix IBS build error for UP
      x86/oprofile: macro definition cleanup in op_model_athlon.c
      x86/oprofile: op_model_athlon.c: fix counter reset when reenabling IBS OP
      x86: apic: export symbols for extended interrupt LVT functions
      x86: apic: changing export symbols to *_GPL
      x86/oprofile: add CONFIG_OPROFILE_IBS option
      oprofile: fix printk in cpu_buffer.c
      x86/oprofile: reanaming op_model_athlon.c to op_model_amd.c
      x86/oprofile: fix on_each_cpu build error


 arch/Kconfig                        |   14 +
 arch/x86/kernel/apic_32.c           |    4 +
 arch/x86/kernel/apic_64.c           |    4 +
 arch/x86/oprofile/Makefile          |    2 +-
 arch/x86/oprofile/nmi_int.c         |  127 +++++++-
 arch/x86/oprofile/op_counter.h      |    3 +-
 arch/x86/oprofile/op_model_amd.c    |  559 +++++++++++++++++++++++++++++++++++
 arch/x86/oprofile/op_model_athlon.c |  190 ------------
 arch/x86/oprofile/op_model_p4.c     |    4 +
 arch/x86/oprofile/op_model_ppro.c   |    2 +
 arch/x86/oprofile/op_x86_model.h    |    7 +-
 drivers/oprofile/buffer_sync.c      |  209 +++++++++----
 drivers/oprofile/cpu_buffer.c       |   74 +++++-
 drivers/oprofile/cpu_buffer.h       |    2 +
 drivers/oprofile/oprof.c            |   58 ++++-
 drivers/oprofile/oprof.h            |    4 +-
 drivers/oprofile/oprofile_files.c   |   39 +++-
 include/linux/oprofile.h            |    5 +
 include/linux/pci_ids.h             |    5 +
 19 files changed, 1031 insertions(+), 281 deletions(-)
 create mode 100644 arch/x86/oprofile/op_model_amd.c
 delete mode 100644 arch/x86/oprofile/op_model_athlon.c

Re: [PATCH 0/24] oprofile: Add IBS support for AMD CPUs

[Robert Richter]

On 26.07.08 11:52:21, Ingo Molnar wrote:
> 
> * Robert Richter <robert.richter@amd.com> wrote:
> 
> > Patches #1-3 are not directly related to IBS.
> > Patch #4 adds generic support of model specific initialization.
> > Patches #10, #11 add the core implementation of IBS.
> > Patches #12-24 contain code improvements and small fixes.
> 
> cool stuff! I've created the tip/oprofile topic branch for the patches 
> from you, Barry and Jason and have applied all the current patches (and 
> followup fixes) to it. Below are the access coordinates and the log 
> summary.
> 
> It's not yet integrated into tip/master - will do some testing first. 
> I'll send review feedback separately as well, on a per patch basis.

Ingo,

thanks for adding the patches to tip/oprofile. I will send you more
patches that reflect the comments you made.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@amd.com

Re: [PATCH 0/24] oprofile: Add IBS support for AMD CPUs

[Ingo Molnar]

* Robert Richter <robert.richter@amd.com> wrote:

> On 26.07.08 11:52:21, Ingo Molnar wrote:
> > 
> > * Robert Richter <robert.richter@amd.com> wrote:
> > 
> > > Patches #1-3 are not directly related to IBS.
> > > Patch #4 adds generic support of model specific initialization.
> > > Patches #10, #11 add the core implementation of IBS.
> > > Patches #12-24 contain code improvements and small fixes.
> > 
> > cool stuff! I've created the tip/oprofile topic branch for the patches 
> > from you, Barry and Jason and have applied all the current patches (and 
> > followup fixes) to it. Below are the access coordinates and the log 
> > summary.
> > 
> > It's not yet integrated into tip/master - will do some testing first. 
> > I'll send review feedback separately as well, on a per patch basis.
> 
> Ingo,
> 
> thanks for adding the patches to tip/oprofile. I will send you more 
> patches that reflect the comments you made.

thanks. FYI, i've merged tip/oprofile into tip/master 5 days ago, and 
it's been problem-free so far in testing. I'll set up a branch for 
linux-next testing as well.

	Ingo