oprofile: Add IBS support for AMD CPUs
cover oprofile: Add IBS support for AMD CPUs
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 1/24 x86: Add PCI IDs for AMD Barcelona PCI devices
--- include/linux/pci_ids.h | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
include/linux/pci_ids.h +5 -0
@@ -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
Signature · 1 line
1.5.5.4
PATCH 2/24 x86: apic_*.c: Add description to AMD's extended LVT functions
In reply to Robert Richter--- arch/x86/kernel/apic_32.c | 3 +++ arch/x86/kernel/apic_64.c | 3 +++ 2 files changed, 6 insertions(+), 0 deletions(-)
arch/x86/kernel/apic_32.c +3 -0
@@ -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
arch/x86/kernel/apic_64.c +3 -0
@@ -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
Signature · 1 line
1.5.5.4
PATCH 3/24 oprofile: Add support for AMD Family 11h
In reply to Robert RichterFrom: Barry Kasindorf <[email protected]>
This patch add support for AMD Family 11h CPUs.
--- arch/x86/oprofile/nmi_int.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
arch/x86/oprofile/nmi_int.c +4 -0
@@ -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 ;
Signature · 1 line
1.5.5.4
Re: [PATCH 03/24] oprofile: Add support for AMD Family 11h
arch/x86/oprofile/nmi_int.c int __init op_nmi_init(struct oprofile_operations *ops) · line 427
Quoted 11 lines · Robert Richter
arch/x86/oprofile/nmi_int.c +3
-0@@ -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
Quoted 14 lines · Daniel K.
arch/x86/oprofile/nmi_int.c int __init op_nmi_init(struct oprofile_operations *ops) · line 427
Quoted 11 lines · Robert Richter
arch/x86/oprofile/nmi_int.c +3
-0@@ -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 4/24 x86/oprofile: Introduce model specific init/exit functions
In reply to Robert RichterThis 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().
--- 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(-)
arch/x86/oprofile/nmi_int.c +10 -1
@@ -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 <[email protected]> +* @author Robert Richter <[email protected]> */ #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 (); }
arch/x86/oprofile/op_model_athlon.c +15 -3
@@ -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 <[email protected]> +*/ #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,
arch/x86/oprofile/op_x86_model.h +2 -0
@@ -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);
Signature · 1 line
1.5.5.4
PATCH 5/24 x86/oprofile: Minor changes in op_model_athlon.c
In reply to Robert Richter--- arch/x86/oprofile/op_model_athlon.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
arch/x86/oprofile/op_model_athlon.c +2 -0
@@ -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;
Signature · 1 line
1.5.5.4
PATCH 6/24 x86/oprofile: Renaming athlon_*() into op_amd_*()
In reply to Robert RichterThese functions contain code for all AMD CPUs. The new names fit better.
--- 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(-)
arch/x86/oprofile/nmi_int.c +4 -4
@@ -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 ; }
arch/x86/oprofile/op_model_athlon.c +14 -14
@@ -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 };
arch/x86/oprofile/op_x86_model.h +1 -1
@@ -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 */
Signature · 1 line
1.5.5.4
Re: [PATCH 06/24] x86/oprofile: Renaming athlon_*() into op_amd_*()
Quoted 9 lines · Robert Richter
arch/x86/oprofile/op_model_athlon.c +1
-4-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,
arch/x86/oprofile/op_model_athlon.c static void op_amd_exit(void) · line 204
Quoted 10 lines
arch/x86/oprofile/op_model_athlon.c +6
-3- .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 7/24 drivers/oprofile: Coding style fixes in buffer_sync.c
In reply to Robert Richter--- drivers/oprofile/buffer_sync.c | 111 ++++++++++++++++++++------------------- 1 files changed, 57 insertions(+), 54 deletions(-)
drivers/oprofile/buffer_sync.c +57 -54
@@ -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, vreturn 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, ostatic 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); } }
Signature · 1 line
1.5.5.4
PATCH 8/24 OProfile: Moving increment_tail() in buffer_sync.c
In reply to Robert Richter--- drivers/oprofile/buffer_sync.c | 24 +++++++++++------------- 1 files changed, 11 insertions(+), 13 deletions(-)
drivers/oprofile/buffer_sync.c +11 -13
@@ -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,
Signature · 1 line
1.5.5.4
Re: [PATCH 08/24] OProfile: Moving increment_tail() in buffer_sync.c
drivers/oprofile/buffer_sync.c lookup_dcookie(struct mm_struct *mm, unsigned long addr, off_t *offset) · line 275
Quoted 5 lines · Robert Richter
drivers/oprofile/buffer_sync.c +5
-0+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 9/24 OProfile: Add IBS code macros
In reply to Robert Richter--- include/linux/oprofile.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
include/linux/oprofile.h +2 -0
@@ -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 14struct super_block ;struct dentry ;
Signature · 1 line
1.5.5.4
PATCH 10/24 x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines
In reply to Robert RichterFrom: Barry Kasindorf <[email protected]>
This patchset supports the new profiling hardware available in the latest AMD CPUs in the oProfile driver.
--- 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(-)
drivers/oprofile/buffer_sync.c +71 -1
@@ -5,6 +5,7 @@* @remark Read the file COPYING * * @author John Levon <[email protected]> +* @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;
drivers/oprofile/cpu_buffer.c +67 -1
86 lines not on this page · raw message
drivers/oprofile/cpu_buffer.h +2 -0
@@ -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 */
Signature · 1 line
1.5.5.4
Re: [PATCH 10/24] x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines
Quoted 178 lines · Robert Richter
From: Barry Kasindorf <[email protected]>
This patchset supports the new profiling hardware available in the latest AMD CPUs in the oProfile driver.
Signed-off-by Barry Kasindorf <[email protected]>Signed-off-by Robert Richter <[email protected]>--- 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(-)drivers/oprofile/buffer_sync.c +71
-1@@ -5,6 +5,7 @@* @remark Read the file COPYING * * @author John Levon <[email protected]> +* @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;drivers/oprofile/cpu_buffer.c +38
-1@@ -5,6 +5,7 @@* @remark Read the file COPYING * * @author John Levon <[email protected]> +* @author Barry Kasindorf <[email protected]> * * 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 ]); +
drivers/oprofile/cpu_buffer.c void oprofile_add_sample(struct pt_regs * const regs, unsigned long event) · line 301
Quoted 10 lines
drivers/oprofile/cpu_buffer.c +9
-0+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
Quoted 36 lines
drivers/oprofile/cpu_buffer.c +20
-0+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) {drivers/oprofile/cpu_buffer.c +0
-0struct oprofile_cpu_buffer *cpu_buf = &__get_cpu_var (cpu_buffer);drivers/oprofile/cpu_buffer.h +2
-0@@ -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
Quoted 1 line · Maynard Johnson
Robert Richter wrote:
[...]
Quoted 48 lines
Quoted 48 lines
drivers/oprofile/cpu_buffer.c +44
-0@@ -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 ; +}
Quoted 8 lines
Review void oprofile_add_sample(struct pt_regs * const regs, unsigned long event) · line 301
Quoted 2 lines
(quoted diff) +1
-0+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
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]
Re: [PATCH 10/24] x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines
In reply to Robert RichterQuoted 89 lines · Robert Richter
From: Barry Kasindorf <[email protected]>
This patchset supports the new profiling hardware available in the latest AMD CPUs in the oProfile driver.
Signed-off-by Barry Kasindorf <[email protected]>Signed-off-by Robert Richter <[email protected]>--- 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(-)drivers/oprofile/buffer_sync.c +53
-1@@ -5,6 +5,7 @@* @remark Read the file COPYING * * @author John Levon <[email protected]> +* @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 )
drivers/oprofile/buffer_sync.c static void add_trace_begin(void) · line 391
Quoted 10 lines
drivers/oprofile/buffer_sync.c +10
-0+ 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.
Quoted 8 lines
drivers/oprofile/buffer_sync.c +1
-0static 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 ) {
drivers/oprofile/buffer_sync.c void sync_buffer(int cpu) · line 599
Quoted 10 lines
drivers/oprofile/buffer_sync.c +7
-0+ 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.
Quoted 60 lines
drivers/oprofile/cpu_buffer.c +37
-1@@ -5,6 +5,7 @@* @remark Read the file COPYING * * @author John Levon <[email protected]> +* @author Barry Kasindorf <[email protected]> * * 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 ]);
drivers/oprofile/cpu_buffer.c void oprofile_add_sample(struct pt_regs * const regs, unsigned long event) · line 300
Quoted 10 lines
drivers/oprofile/cpu_buffer.c +10
-0+ +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.
Quoted 35 lines
drivers/oprofile/cpu_buffer.c +20
-0+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);drivers/oprofile/cpu_buffer.h +2
-0@@ -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
Quoted 99 lines · Carl Love
Quoted 97 lines · Robert Richter
From: Barry Kasindorf <[email protected]>
This patchset supports the new profiling hardware available in the latest AMD CPUs in the oProfile driver.
Signed-off-by Barry Kasindorf <[email protected]>Signed-off-by Robert Richter <[email protected]>--- 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(-)drivers/oprofile/buffer_sync.c +61
-1@@ -5,6 +5,7 @@* @remark Read the file COPYING * * @author John Levon <[email protected]> +* @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));
Quoted 8 lines
drivers/oprofile/buffer_sync.c static void add_trace_begin(void) · line 391
Quoted 2 lines
drivers/oprofile/buffer_sync.c +2
-0+ } +}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
Quoted 168 lines
Quoted 9 lines
drivers/oprofile/buffer_sync.c +1
-0static 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 ) {drivers/oprofile/buffer_sync.c void sync_buffer(int cpu) · line 599
Quoted 10 lines
drivers/oprofile/buffer_sync.c +7
-0+ 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.Quoted 60 lines
drivers/oprofile/cpu_buffer.c +37
-1@@ -5,6 +5,7 @@* @remark Read the file COPYING * * @author John Levon <[email protected]> +* @author Barry Kasindorf <[email protected]> * * 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 ]);drivers/oprofile/cpu_buffer.c void oprofile_add_sample(struct pt_regs * const regs, unsigned long event) · line 300
Quoted 10 lines
drivers/oprofile/cpu_buffer.c +10
-0+ +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(structoprofile_cpu_buffer*cpu_buf,
unsigned int *array, intnum_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 thelog_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 tolog_generic_sample(). Maybe the arch specific code could have an array oflast_is_kernelto refer to when preparing the data for thelog_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.Quoted 35 lines
drivers/oprofile/cpu_buffer.c +20
-0+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);drivers/oprofile/cpu_buffer.h +2
-0@@ -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 */
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]
Re: [PATCH 10/24] x86/oprofile: Add IBS support for AMD CPUs, IBS buffer handling routines
In reply to Robert Richterdrivers/oprofile/buffer_sync.c static void increment_tail(struct oprofile_cpu_buffer *b) · line 276
Quoted 6 lines · Robert Richter
drivers/oprofile/buffer_sync.c +1
-1@@ -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 :-)
drivers/oprofile/cpu_buffer.c void oprofile_add_sample(struct pt_regs * const regs, unsigned long event) · line 296
Quoted 5 lines
drivers/oprofile/cpu_buffer.c +5
-0+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.
drivers/oprofile/cpu_buffer.c void oprofile_add_sample(struct pt_regs * const regs, unsigned long event) · line 308
Quoted 5 lines
drivers/oprofile/cpu_buffer.c +5
-0+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.
drivers/oprofile/cpu_buffer.h void cpu_buffer_reset(struct oprofile_cpu_buffer * cpu_buf); · line 59
Quoted 5 lines
/* transient events for the CPU buffer -> event buffer */ #define
CPU_IS_KERNEL1
#defineCPU_TRACE_BEGIN2drivers/oprofile/cpu_buffer.h +2
-0+#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
In reply to Robert RichterFrom: Barry Kasindorf <[email protected]>
This patchset supports the new profiling hardware available in the latest AMD CPUs in the oProfile driver.
--- arch/x86/oprofile/op_model_athlon.c | 257 +++++++++++++++++++++++++++++++++++ 1 files changed, 257 insertions(+), 0 deletions(-)
arch/x86/oprofile/op_model_athlon.c +257 -0
@@ -9,9 +9,13 @@* @author Philippe Elie * @author Graydon Hoare * @author Robert Richter <[email protected]> +* @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 ;
Signature · 1 line
1.5.5.4
Re: [PATCH 11/24] x86/oprofile: Add IBS support for AMD CPUs, model specific code
Quoted 22 lines · Robert Richter
From: Barry Kasindorf <[email protected]>
This patchset supports the new profiling hardware available in the latest AMD CPUs in the oProfile driver.
Signed-off-by Barry Kasindorf <[email protected]>Signed-off-by Robert Richter <[email protected]>--- arch/x86/oprofile/op_model_athlon.c | 257 +++++++++++++++++++++++++++++++++++ 1 files changed, 257 insertions(+), 0 deletions(-)arch/x86/oprofile/op_model_athlon.c +1
-0@@ -9,9 +9,13 @@* @author Philippe Elie * @author Graydon Hoare * @author Robert Richter <[email protected]> +* @author Barry Kasindorf */
[snip]
arch/x86/oprofile/op_model_athlon.c line 123
Quoted 8 lines
arch/x86/oprofile/op_model_athlon.c +8
-0+ +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
Quoted 22 lines · Maynard Johnson
Quoted 20 lines · Robert Richter
From: Barry Kasindorf <[email protected]>
This patchset supports the new profiling hardware available in the latest AMD CPUs in the oProfile driver.Signed-off-by Barry Kasindorf <[email protected]>Signed-off-by Robert Richter <[email protected]>--- arch/x86/oprofile/op_model_athlon.c | 257 +++++++++++++++++++++++++++++++++++ 1 files changed, 257 insertions(+), 0 deletions(-)arch/x86/oprofile/op_model_athlon.c +1
-0@@ -9,9 +9,13 @@* @author Philippe Elie * @author Graydon Hoare * @author Robert Richter <[email protected]> +* @author Barry Kasindorf */ [snip]
Quoted 10 lines
arch/x86/oprofile/op_model_athlon.c line 123
Quoted 8 lines
arch/x86/oprofile/op_model_athlon.c +8
-0+ +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_opsbeing 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
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]
Re: [PATCH 11/24] x86/oprofile: Add IBS support for AMD CPUs, model specific code
In reply to Robert RichterQuoted 13 lines · Robert Richter
From: Barry Kasindorf <[email protected]>
This patchset supports the new profiling hardware available in the latest AMD CPUs in the oProfile driver.
Signed-off-by Barry Kasindorf <[email protected]>Signed-off-by Robert Richter <[email protected]>--- arch/x86/oprofile/op_model_athlon.c | 257 +++++++++++++++++++++++++++++++++++ 1 files changed, 257 insertions(+), 0 deletions(-)arch/x86/oprofile/op_model_athlon.c +0
-0
[...]
Quoted 78 lines
arch/x86/oprofile/op_model_athlon.c +74
-0@@ -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 ;
arch/x86/oprofile/op_model_athlon.c static void op_amd_shutdown(struct op_msrs const * const msrs) · line 431
Quoted 10 lines
arch/x86/oprofile/op_model_athlon.c +10
-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
Quoted 15 lines
arch/x86/oprofile/op_model_athlon.c +9
-0+ 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 ;Signature · 1 line
1.5.5.4
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]
Re: [PATCH 11/24] x86/oprofile: Add IBS support for AMD CPUs, model specific code
In reply to Robert RichterQuoted 10 lines · Robert Richter
arch/x86/oprofile/op_model_athlon.c +6
-0@@ -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;
arch/x86/oprofile/op_model_athlon.c static int op_amd_check_ctrs(struct pt_regs * const regs, · line 232
Quoted 10 lines
arch/x86/oprofile/op_model_athlon.c +10
-0+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.
Quoted 22 lines
arch/x86/oprofile/op_model_athlon.c +22
-0+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; +
arch/x86/oprofile/op_model_athlon.c static int op_amd_check_ctrs(struct pt_regs * const regs, · line 274
Quoted 10 lines
arch/x86/oprofile/op_model_athlon.c +10
-0+/* 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.
arch/x86/oprofile/op_model_athlon.c static void op_amd_shutdown(struct op_msrs const * const msrs) · line 385
Quoted 3 lines
arch/x86/oprofile/op_model_athlon.c +3
-0+/**** 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:
*/
arch/x86/oprofile/op_model_athlon.c static void op_amd_shutdown(struct op_msrs const * const msrs) · line 390
Quoted 5 lines
arch/x86/oprofile/op_model_athlon.c +5
-0+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.
arch/x86/oprofile/op_model_athlon.c static void op_amd_shutdown(struct op_msrs const * const msrs) · line 393
Quoted 3 lines
arch/x86/oprofile/op_model_athlon.c +3
-0+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.
arch/x86/oprofile/op_model_athlon.c static void op_amd_shutdown(struct op_msrs const * const msrs) · line 402
Quoted 3 lines
arch/x86/oprofile/op_model_athlon.c +3
-0+/* +* unitialize the APIC for the IBS interrupts if needed on AMD Family10h +* rev B0 and later */
comment style.
arch/x86/oprofile/op_model_athlon.c static void op_amd_shutdown(struct op_msrs const * const msrs) · line 411
Quoted 3 lines
arch/x86/oprofile/op_model_athlon.c +3
-0+static void setup_ibs_files (struct super_block *sb,struct dentry *root) +{ +char buf[12 ];
magic limit.
arch/x86/oprofile/op_model_athlon.c static void op_amd_shutdown(struct op_msrs const * const msrs) · line 424
Quoted 13 lines
arch/x86/oprofile/op_model_athlon.c +13
-0+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
In reply to Robert Richter--- arch/x86/oprofile/op_model_athlon.c | 45 +++++++++++++++++++++------------- 1 files changed, 28 insertions(+), 17 deletions(-)
arch/x86/oprofile/op_model_athlon.c +28 -17
@@ -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) {
Signature · 1 line
1.5.5.4
PATCH 13/24 OProfile: Change IBS interrupt initialization
In reply to Robert Richter--- arch/x86/oprofile/op_model_athlon.c | 84 ++++++++++++++++++++++------------ 1 files changed, 54 insertions(+), 30 deletions(-)
arch/x86/oprofile/op_model_athlon.c +54 -30
@@ -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 ; }
Signature · 1 line
1.5.5.4
Re: [PATCH 13/24] OProfile: Change IBS interrupt initialization
arch/x86/oprofile/op_model_athlon.c static inline void apic_clear_ibs_nmi_per_cpu(void *arg) · line 374
Quoted 4 lines · Robert Richter
arch/x86/oprofile/op_model_athlon.c +4
-0+static int pfm_amd64_setup_eilvt (void ) +{ +#define IBSCTL_LVTOFFSETVAL (1 << 8) +#define IBSCTL 0x1cc
move this into a header file please.
Quoted 7 lines
arch/x86/oprofile/op_model_athlon.c +7
-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;
arch/x86/oprofile/op_model_athlon.c static inline void apic_clear_ibs_nmi_per_cpu(void *arg) · line 399
Quoted 10 lines
arch/x86/oprofile/op_model_athlon.c +10
-0+ ++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
In reply to Robert Richter--- arch/x86/oprofile/op_model_athlon.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
arch/x86/oprofile/op_model_athlon.c +5 -0
@@ -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;
Signature · 1 line
1.5.5.4
Re: [PATCH 14/24] OProfile: Fix build error in op_model_athlon.c
Quoted 10 lines · Robert Richter
Signed-off-by Robert Richter <[email protected]>--- arch/x86/oprofile/op_model_athlon.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)arch/x86/oprofile/op_model_athlon.c +0
-0
arch/x86/oprofile/op_model_athlon.c line 77
Quoted 5 lines
arch/x86/oprofile/op_model_athlon.c +2
-0@@ -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
In reply to Robert Richter--- arch/x86/oprofile/op_model_athlon.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/oprofile/op_model_athlon.c +2 -2
@@ -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)
Signature · 1 line
1.5.5.4
PATCH 16/24 OProfile: Fix setup_ibs_files() function interface
In reply to Robert Richter--- arch/x86/oprofile/op_model_athlon.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
arch/x86/oprofile/op_model_athlon.c +4 -2
@@ -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)
Signature · 1 line
1.5.5.4
PATCH 17/24 OProfile: Enable IBS for AMD CPUs
In reply to Robert Richter--- arch/x86/oprofile/nmi_int.c | 14 ++++++++------ arch/x86/oprofile/op_model_athlon.c | 18 +++++++++++++++++- 2 files changed, 25 insertions(+), 7 deletions(-)
arch/x86/oprofile/nmi_int.c +8 -6
@@ -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 ; }
arch/x86/oprofile/op_model_athlon.c +17 -1
@@ -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 = {
Signature · 1 line
1.5.5.4
Re: [PATCH 17/24] OProfile: Enable IBS for AMD CPUs
arch/x86/oprofile/nmi_int.c int __init op_nmi_init(struct oprofile_operations *ops) · line 463
Quoted 7 lines · Robert Richter
arch/x86/oprofile/nmi_int.c +7
-0+/* 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:
arch/x86/oprofile/nmi_int.c int __init op_nmi_init(struct oprofile_operations *ops) · line 463
Quoted 7 lines
arch/x86/oprofile/nmi_int.c +7
-0+/* 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.
arch/x86/oprofile/op_model_athlon.c static int setup_ibs_files(struct super_block * sb, struct dentry * root) · line 500
Quoted 8 lines
arch/x86/oprofile/op_model_athlon.c +3
-0@@ -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
In reply to Robert Richter--- arch/x86/oprofile/op_model_athlon.c | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-)
arch/x86/oprofile/op_model_athlon.c +22 -0
@@ -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,
Signature · 1 line
1.5.5.4
PATCH 19/24 x86/oprofile: Macro definition cleanup in op_model_athlon.c
In reply to Robert Richter--- arch/x86/oprofile/op_model_athlon.c | 46 +++++++++++++---------------------- 1 files changed, 17 insertions(+), 29 deletions(-)
arch/x86/oprofile/op_model_athlon.c +17 -29
@@ -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); }
Signature · 1 line
1.5.5.4
PATCH 20/24 x86/oprofile: op_model_athlon.c: Fix counter reset when reenabling IBS OP
In reply to Robert Richter--- arch/x86/oprofile/op_model_athlon.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
arch/x86/oprofile/op_model_athlon.c +1 -0
@@ -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);
Signature · 1 line
1.5.5.4
PATCH 21/24 x86: apic: Export symbols for extended interrupt LVT functions
In reply to Robert RichterThis patch adds EXPORT_SYMBOLs to allow OProfile to be built as module.
--- arch/x86/kernel/apic_32.c | 1 + arch/x86/kernel/apic_64.c | 1 + 2 files changed, 2 insertions(+), 0 deletions(-)
arch/x86/kernel/apic_32.c +1 -0
@@ -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
arch/x86/kernel/apic_64.c +1 -0
@@ -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
Signature · 1 line
1.5.5.4
Re: [PATCH 21/24] x86: apic: Export symbols for extended interrupt LVT functions
surely such deep internals warrant a _GPL export...
Quoted 34 lines · On Tue, 22 Jul 2008 21:09:05 +0200 Robert Richter <[email protected]>…
This patch adds
EXPORT_SYMBOLsto allow OProfile to be built as module.Cc Arjan van de Ven <[email protected]>Signed-off-by Robert Richter <[email protected]>--- arch/x86/kernel/apic_32.c | 1 + arch/x86/kernel/apic_64.c | 1 + 2 files changed, 2 insertions(+), 0 deletions(-)arch/x86/kernel/apic_32.c +1
-0@@ -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 arch/x86/kernel/apic_64.c +1
-0@@ -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
Signature · 3 lines
If you want to reach me at my work email, use [email protected] For development, discussion and tips for power savings, visit http://www.lesswatts.org
PATCH 1/1 x86: apic: Changing export symbols to *_GPL
This fits better here.
--- arch/x86/kernel/apic_32.c | 2 +- arch/x86/kernel/apic_64.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/apic_32.c +1 -1
@@ -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
arch/x86/kernel/apic_64.c +1 -1
@@ -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
Signature · 1 line
1.5.5.4
Re: [PATCH] x86: apic: Changing export symbols to *_GPL
Quoted 1 line · Robert Richter
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 [email protected] - 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
Quoted 5 lines · linux-os (Dick Johnson)
Quoted 1 line · Robert Richter
This fits better here.
Quoted 8 lines
[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
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]
Re: [PATCH] x86: apic: Changing export symbols to *_GPL
Quoted 2 lines · Robert Richter
This corrects a patch _I_ introduced before in my OProfile patch series.
Subject/changelog of the correcting patch could mention this.
Signature · 3 lines
Stefan Richter -=====-==--- -=== ==--- http://arcgraph.de/sr/
PATCH 22/24 x86/oprofile: Add CONFIG_OPROFILE_IBS option
In reply to Robert Richter--- 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(-)
arch/Kconfig +14 -0
@@ -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
arch/x86/oprofile/op_model_athlon.c +23 -10
@@ -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" ); }
drivers/oprofile/buffer_sync.c +6 -0
@@ -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;
drivers/oprofile/cpu_buffer.c +4 -0
@@ -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 14static 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);
Signature · 1 line
1.5.5.4
Re: [PATCH 22/24] x86/oprofile: Add CONFIG_OPROFILE_IBS option
arch/Kconfig config OPROFILE · line 26
Quoted 11 lines · Robert Richter
arch/Kconfig +11
-0+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:
arch/x86/oprofile/op_model_athlon.c static int op_amd_check_ctrs(struct pt_regs * const regs, · line 291
Quoted 8 lines
arch/x86/oprofile/op_model_athlon.c +6
-0+ 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_IBSop_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
In reply to Robert Richter--- drivers/oprofile/cpu_buffer.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/oprofile/cpu_buffer.c +1 -1
@@ -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);
Signature · 1 line
1.5.5.4
PATCH 24/24 x86/oprofile: Reanaming op_model_athlon.c to op_model_amd.c
In reply to Robert Richter--- 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
arch/x86/oprofile/Makefile +1 -1
@@ -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
arch/x86/oprofile/op_model_amd.c new file +543 -0
543 lines not on this page · raw message
arch/x86/oprofile/op_model_athlon.c deleted +0 -543
543 lines not on this page · raw message
Signature · 1 line
1.5.5.4
Re: [PATCH 24/24] x86/oprofile: Reanaming op_model_athlon.c to op_model_amd.c
Quoted 13 lines · Robert Richter
arch/x86/oprofile/op_model_amd.c new file +13
-0+#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))
arch/x86/oprofile/op_model_amd.c line 48
Quoted 10 lines
arch/x86/oprofile/op_model_amd.c new file +10
-0+#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.
Quoted 1 line
+#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
Quoted 3 lines · Robert Richter
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
Quoted 16 lines
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/oprofile-list
Quoted 4 lines · 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.
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
Quoted 8 lines · Ingo Molnar
Quoted 4 lines · 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.
Quoted 7 lines
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
Signature · 3 lines
Advanced Micro Devices, Inc. Operating System Research Center email: [email protected]
Quoted 14 lines · Robert Richter
Quoted 13 lines · Ingo Molnar
Quoted 4 lines · 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.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.
Quoted 7 lines
Quoted 2 lines
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