Subject: cell: clean up oprofile code - don't use SilLyCApS - don't use typedef - simplify rtas handling - make rtas call reentrant - handle rtas call missing better - use pr_debug instead of homegrown macros - make mmcr file creation depend on cpu features instead of incorrect CONFIG_PPC_CELL - remove wrong CPU_FTR_MMCRA from cell features - add a few FIXME for potentially broken code - remove some write-only variables Signed-off-by: Arnd Bergmann Index: linux-2.6/arch/powerpc/oprofile/op_model_cell.c =================================================================== --- linux-2.6.orig/arch/powerpc/oprofile/op_model_cell.c +++ linux-2.6/arch/powerpc/oprofile/op_model_cell.c @@ -29,53 +29,44 @@ #include "../platforms/cell/perfmon.h" #include "../platforms/cell/interrupt.h" -#define dbg(args...) printk(args) -#define dbg_entry(args...) -#define dbg_exit(args...) -#define dbg_hw(args...) - - /* There is only 1 Performance Monitor for every dual-threaded processor */ +/* FIXME: this should not be hardcoded here! */ #define NR_NODES (NR_CPUS >> 1) /* * ibm,cbe-perftools rtas parameters */ -typedef struct { +struct pm_signal { u16 cpu; /* Processor to modify */ u16 sub_unit; /* hw subunit this applies to (if applicable) */ u16 signal_group; /* Signal Group to Enable/Disable */ u8 bus_word; /* Enable/Disable on this Trace/Trigger/Event Bus Word(s) (bitmask) */ u8 bit; /* Trigger/Event bit (if applicable) */ -} PM_Signal; - -typedef struct { - u32 subfunc; - #define subfunc_RESET 1 - #define subfunc_ACTIVATE 2 - #define subfunc_DEACTIVATE 3 - - u32 passthru; - #define passthru_IGNORE 0 - #define passthru_ENABLE 1 - #define passthru_DISABLE 2 - - u32 buffer_addr_hi; /* addr of buffer containing PM_Signal array */ - u32 buffer_addr_lo; - u32 buffer_length; /* buffer size (in bytes) */ +}; - PM_Signal signal[NR_NODES * OP_MAX_COUNTER]; -} PM_Rtas_Args; +/* + * rtas call arguments, the firmware crammed multiple calls into + * a multiplexer... + */ +enum { + SUBFUNC_RESET = 1, + SUBFUNC_ACTIVATE = 2, + SUBFUNC_DEACTIVATE = 3, + + PASSTHRU_IGNORE = 0, + PASSTHRU_ENABLE = 1, + PASSTHRU_DISABLE = 2, +}; -static PM_Rtas_Args rtas_args; +static struct pm_signal pm_signal[NR_NODES * OP_MAX_COUNTER]; +static int pm_rtas_token; static u32 reset_value[OP_MAX_COUNTER]; static int num_counters; -static int oprofile_running; static char ctrSize[4]; -static u32 ctr_enabled = 0; +static u32 ctr_enabled; static struct { u32 group_control; @@ -110,66 +101,48 @@ struct oprofile_umask { /* * Firmware interface functions */ - -static void FW_Call_Rtas(PM_Rtas_Args *pArgs) +static int rtas_ibm_cbe_perftools(int subfunc, int passthru, + void *address, unsigned long length) { -#ifdef SDK2_0 - int error; - - error = rtas_call(rtas_token("ibm,cbe-perftools"), 5, 1, NULL, - pArgs->subfunc, - pArgs->passthru, - pArgs->buffer_addr_hi, - pArgs->buffer_addr_lo, - pArgs->buffer_length); + u64 paddr = __pa(address); - if (error) - printk(KERN_WARNING "error: ibm,cbe-perftools returned: %d\n", - error); -#endif + return rtas_call(pm_rtas_token, 5, 1, NULL, subfunc, passthru, + paddr >> 32, paddr & 0xffffffff, length); } -static void FW_ResetSignals(u32 cpu) +static void pm_rtas_reset_signals(u32 cpu) { - PM_Rtas_Args *pArgs; - u64 real_address; - - pArgs = &rtas_args; + int ret; - real_address = (u64) virt_to_phys(&(pArgs->signal[0])); + pm_signal[0].cpu = cpu; + pm_regs.passthru = PASSTHRU_DISABLE; - pArgs->subfunc = subfunc_RESET; - pArgs->passthru = passthru_DISABLE; - pArgs->buffer_addr_hi = real_address >> 32; - pArgs->buffer_addr_lo = real_address & 0xffffffff; - pArgs->buffer_length = sizeof(PM_Signal); - pArgs->signal[0].cpu = cpu; + ret = rtas_ibm_cbe_perftools(SUBFUNC_RESET, PASSTHRU_DISABLE, + pm_signal, sizeof(struct pm_signal)); - pm_regs.passthru = passthru_DISABLE; - - FW_Call_Rtas(pArgs); + if (ret) + printk(KERN_WARNING "%s: rtas returned: %d\n", + __FUNCTION__, ret); } -static void FW_ActivateSignals(u32 cpu, u32 count) +static void pm_rtas_activate_signals(u32 cpu, u32 count) { - PM_Rtas_Args *pArgs; - u64 real_address; - - pArgs = &rtas_args; - - real_address = (u64) virt_to_phys(&(pArgs->signal[0])); + int passthru; + int ret; - pArgs->subfunc = subfunc_ACTIVATE; - pArgs->buffer_addr_hi = real_address >> 32; - pArgs->buffer_addr_lo = real_address & 0xffffffff; - pArgs->buffer_length = count * sizeof(PM_Signal); + passthru = PASSTHRU_IGNORE; - if (pm_regs.passthru == passthru_DISABLE) { - pm_regs.passthru = passthru_ENABLE; - pArgs->passthru = passthru_ENABLE; + if (pm_regs.passthru == PASSTHRU_DISABLE) { + pm_regs.passthru = PASSTHRU_ENABLE; + passthru = PASSTHRU_ENABLE; } - FW_Call_Rtas(pArgs); + ret = rtas_ibm_cbe_perftools(SUBFUNC_ACTIVATE, passthru, pm_signal, + count * sizeof(struct pm_signal)); + + if (ret) + printk(KERN_WARNING "%s: rtas returned: %d\n", + __FUNCTION__, ret); } /* @@ -178,7 +151,7 @@ static void FW_ActivateSignals(u32 cpu, static void set_pm_event(u32 ctr, int event, u32 unit_mask) { - PM_Signal *p; + struct pm_signal *p; u32 bus_word; u32 bus_type; u32 signal_bit; @@ -198,7 +171,7 @@ static void set_pm_event(u32 ctr, int ev signal_bit = (event % 100); - p = &(rtas_args.signal[ctr]); + p = &(pm_signal[ctr]); p->signal_group = event / 100; p->bus_word = bus_word; @@ -277,6 +250,10 @@ static void cell_reg_setup(struct op_cou int nEnabled = 0; int i; + pm_rtas_token = rtas_token("ibm,cbe-perftools"); + if (pm_rtas_token == RTAS_UNKNOWN_SERVICE) + return; + /* This function is called once for all cpus combined */ num_counters = num_ctrs; @@ -336,28 +313,30 @@ static void cell_cpu_setup(void *unused) if (cpu & 1) return; + if (pm_rtas_token == RTAS_UNKNOWN_SERVICE) + return; + /* Stop all counters */ cbe_disable_pm(cpu); cbe_disable_pm_interrupts(cpu); - cbe_write_pm (cpu, pm_interval, 0); - cbe_write_pm (cpu, pm_start_stop, 0); - cbe_write_pm (cpu, group_control, pm_regs.group_control); - cbe_write_pm (cpu, debug_bus_control, pm_regs.debug_bus_control); - cbe_write_pm (cpu, pm_control, pm_regs.pm_control.val); + cbe_write_pm(cpu, pm_interval, 0); + cbe_write_pm(cpu, pm_start_stop, 0); + cbe_write_pm(cpu, group_control, pm_regs.group_control); + cbe_write_pm(cpu, debug_bus_control, pm_regs.debug_bus_control); + cbe_write_pm(cpu, pm_control, pm_regs.pm_control.val); mask.val = 0; for (i = 0; i < num_counters; ++i) { if (ctr_enabled & (1 << i)) { - rtas_args.signal[nEnabled].cpu = cpu; + pm_signal[nEnabled].cpu = cpu; nEnabled++; mask.ctr_overflow |= CBE_CTR_OVERFLOW(i); } } - - FW_ActivateSignals(cpu, nEnabled); + pm_rtas_activate_signals(cpu, nEnabled); } static void cell_start(struct op_counter_config *ctr) @@ -387,9 +366,7 @@ static void cell_start(struct op_counter cbe_enable_pm_interrupts(cpu, 0, mask.val); cbe_enable_pm(cpu); - oprofile_running = 1; - - dbg("oprofile started on BE %d\n", cpu >> 1); + pr_debug("oprofile started on BE %d\n", cpu >> 1); } static void cell_stop(void) @@ -398,6 +375,8 @@ static void cell_stop(void) /* There is one performance monitor for cpu 0/1 and one for cpu 2/3 */ + /* FIXME: this check is wrong, we can't assume any meaningful + values inside cpu */ if (cpu & 1) return; @@ -405,16 +384,12 @@ static void cell_stop(void) cbe_disable_pm(cpu); /* Deactivate the signals */ - FW_ResetSignals(cpu); + pm_rtas_reset_signals(cpu); /* Deactivate interrupts */ cbe_disable_pm_interrupts(cpu); - oprofile_running = 0; - - dbg("oprofile stopped on BE %d\n", cpu >> 1); - - mb(); + pr_debug("oprofile stopped on BE %d\n", cpu >> 1); } static void cell_handle_interrupt(struct pt_regs *regs, @@ -430,9 +405,9 @@ static void cell_handle_interrupt(struct cpu = smp_processor_id(); handler_thread = cpu & 1; - dbg_hw ("\n---------> cell_handle_interrupt\n"); - dbg_hw (" interrupt on: node %d\n", node); - dbg_hw (" handled by: node %d, cpu %d, thread %d\n", + pr_debug("---------> cell_handle_interrupt\n"); + pr_debug(" interrupt on: node %d\n", node); + pr_debug(" handled by: node %d, cpu %d, thread %d\n", cpu >> 1, cpu, handler_thread); cbe_disable_pm(cpu); Index: linux-2.6/arch/powerpc/oprofile/common.c =================================================================== --- linux-2.6.orig/arch/powerpc/oprofile/common.c +++ linux-2.6/arch/powerpc/oprofile/common.c @@ -82,17 +82,15 @@ static int op_powerpc_create_files(struc { int i; -#ifdef CONFIG_PPC64 -#ifndef CONFIG_PPC_CELL /* * There is one mmcr0, mmcr1 and mmcra for setting the events for * all of the counters. */ - oprofilefs_create_ulong(sb, root, "mmcr0", &sys.mmcr0); - oprofilefs_create_ulong(sb, root, "mmcr1", &sys.mmcr1); - oprofilefs_create_ulong(sb, root, "mmcra", &sys.mmcra); -#endif -#endif + if (cpu_has_feature(CPU_FTR_MMCRA)) { + oprofilefs_create_ulong(sb, root, "mmcr0", &sys.mmcr0); + oprofilefs_create_ulong(sb, root, "mmcr1", &sys.mmcr1); + oprofilefs_create_ulong(sb, root, "mmcra", &sys.mmcra); + } for (i = 0; i < model->num_counters; ++i) { struct dentry *dir; @@ -137,12 +135,12 @@ int __init oprofile_arch_init(struct opr return -ENODEV; switch (cur_cpu_spec->oprofile_type) { +#ifdef CONFIG_PPC64 #ifdef CONFIG_PPC_CELL case PPC_OPROFILE_CELL: model = &op_model_cell; break; -#else -#ifdef CONFIG_PPC64 +#endif case PPC_OPROFILE_RS64: model = &op_model_rs64; break; @@ -154,7 +152,6 @@ int __init oprofile_arch_init(struct opr model = &op_model_7450; break; #endif -#endif #ifdef CONFIG_FSL_BOOKE case PPC_OPROFILE_BOOKE: model = &op_model_fsl_booke; Index: linux-2.6/include/asm-powerpc/cputable.h =================================================================== --- linux-2.6.orig/include/asm-powerpc/cputable.h +++ linux-2.6/include/asm-powerpc/cputable.h @@ -327,7 +327,7 @@ extern void do_cpu_ftr_fixups(unsigned l CPU_FTR_PURR | CPU_FTR_CI_LARGE_PAGE | CPU_FTR_REAL_LE) #define CPU_FTRS_CELL (CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB | \ CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2 | \ - CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \ + CPU_FTR_ALTIVEC_COMP | CPU_FTR_SMT | \ CPU_FTR_CTRL | CPU_FTR_PAUSE_ZERO | CPU_FTR_CI_LARGE_PAGE) #define CPU_FTRS_COMPATIBLE (CPU_FTR_SPLIT_ID_CACHE | CPU_FTR_USE_TB | \ CPU_FTR_HPTE_TABLE | CPU_FTR_PPCAS_ARCH_V2)