Re: [PATCH v5 04/12] MIPS: add support for hardware performance events (skeleton)

[David Daney <david.s.daney@gmail.com>]

On 05/27/2010 06:03 AM, Deng-Cheng Zhu wrote:
> This patch provides the skeleton of the HW perf event support. To enable
> this feature, we can not choose the SMTC kernel; Oprofile should be
> disabled; kernel performance events be selected. Then we can enable it in
> Kernel type menu.
>
> Oprofile for MIPS platforms initializes irq at arch init time. Currently
> we do not change this logic to allow PMU reservation.
>
> If a platform has EIC, we can use the irq base and perf counter irq
> offset defines for the interrupt controller in mipspmu_get_irq().
>
> Based on this skeleton patch, the 3 different kinds of MIPS PMU, namely,
> mipsxx/loongson2/rm9000, can be supported by adding corresponding lower
> level C files at the bottom. The suggested names of these files are
> perf_event_mipsxx.c/perf_event_loongson2.c/perf_event_rm9000.c. So, for
> example, we can do this by adding "#include perf_event_mipsxx.c" at the
> bottom of perf_event.c.
>
> Signed-off-by: Deng-Cheng Zhu<dengcheng.zhu@gmail.com>
> ---
>   arch/mips/Kconfig                  |    8 +
>   arch/mips/include/asm/perf_event.h |   28 ++
>   arch/mips/kernel/Makefile          |    2 +
>   arch/mips/kernel/perf_event.c      |  503 ++++++++++++++++++++++++++++++++++++
>   4 files changed, 541 insertions(+), 0 deletions(-)
>   create mode 100644 arch/mips/include/asm/perf_event.h
>   create mode 100644 arch/mips/kernel/perf_event.c
>
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 1bccfe5..27577b4 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -1888,6 +1888,14 @@ config NODES_SHIFT
>   	default "6"
>   	depends on NEED_MULTIPLE_NODES
>
> +config HW_PERF_EVENTS
> +	bool "Enable hardware performance counter support for perf events"
> +	depends on PERF_EVENTS&&  !MIPS_MT_SMTC&&  OPROFILE=n&&  CPU_MIPS32


This depends on not consistent with the #if conditions in [01/12] for 
pmu.h.  They should be I think.

Probably removing the tests from pmu.h and encoding them here is better.


> +	default y
> +	help
> +	  Enable hardware performance counter support for perf events. If
> +	  disabled, perf events will use software events only.
> +
>   source "mm/Kconfig"
>
>   config SMP
> diff --git a/arch/mips/include/asm/perf_event.h b/arch/mips/include/asm/perf_event.h
> new file mode 100644
> index 0000000..bcf54bc
> --- /dev/null
> +++ b/arch/mips/include/asm/perf_event.h
> @@ -0,0 +1,28 @@
> +/*
> + * linux/arch/mips/include/asm/perf_event.h
> + *
> + * Copyright (C) 2010 MIPS Technologies, Inc. Deng-Cheng Zhu

IANAL, but who holds the copyright?  You or MTI ?



> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#ifndef __MIPS_PERF_EVENT_H__
> +#define __MIPS_PERF_EVENT_H__
> +
> +extern int (*perf_irq)(void);
> +

This shadows the declaration in asm/time.h.  Declare it in exactly one 
place please.


[...]
> diff --git a/arch/mips/kernel/perf_event.c b/arch/mips/kernel/perf_event.c
> new file mode 100644
> index 0000000..788815f
> --- /dev/null
> +++ b/arch/mips/kernel/perf_event.c
> @@ -0,0 +1,503 @@
> +/*
> + * Linux performance counter support for MIPS.
> + *
> + * Copyright (C) 2010 MIPS Technologies, Inc. Deng-Cheng Zhu
> + *

Same thing about the copyright.


> + * This code is based on the implementation for ARM, which is in turn
> + * based on the sparc64 perf event code and the x86 code. Performance
> + * counter access is based on the MIPS Oprofile code.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include<linux/cpumask.h>
> +#include<linux/interrupt.h>
> +#include<linux/smp.h>
> +#include<linux/kernel.h>
> +#include<linux/perf_event.h>
> +#include<linux/uaccess.h>
> +
> +#include<asm/irq.h>
> +#include<asm/irq_regs.h>
> +#include<asm/stacktrace.h>
> +#include<asm/pmu.h>
> +
> +
> +#define MAX_PERIOD ((1ULL<<  32) - 1)
> +
> +struct cpu_hw_events {
> +	/* Array of events on this cpu. */
> +	struct perf_event	*events[MIPS_MAX_HWEVENTS];
> +
> +	/*
> +	 * Set the bit (indexed by the counter number) when the counter
> +	 * is used for an event.
> +	 */
> +	unsigned long		used_mask[BITS_TO_LONGS(MIPS_MAX_HWEVENTS)];
> +
> +	/*
> +	 * The borrowed MSB for the performance counter. A MIPS performance
> +	 * counter uses its bit 31 as a factor of determining whether a counter

Not quite true.  They use the high bit, that can be either 31 or 63 
depending on the width of the counters.


[...]
> +
> +struct mips_pmu {
> +	const char	*name;
> +	irqreturn_t	(*handle_irq)(int irq, void *dev);
> +	int		(*handle_shared_irq)(void);
> +	void		(*start)(void);
> +	void		(*stop)(void);
> +	int		(*alloc_counter)(struct cpu_hw_events *cpuc,
> +					struct hw_perf_event *hwc);
> +	unsigned int	(*read_counter)(unsigned int idx);
> +	void		(*write_counter)(unsigned int idx, unsigned int val);

Counters can be 64-bits wide, unsigned int is only 32-bits wide.


[...]

David Daney