[PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Mike Travis]

[Ingo - could you let me know what's holding up this driver?  Thanks!, Mike]

The SGI UV system has no LEDS but uses one of the system controller
regs to indicate the online internal state of the cpu.  There is a
heartbeat bit indicating that the cpu is responding to interrupts,
and an idle bit indicating whether the cpu has been more or less than
50% idle each heartbeat period.  The current period is one second.

When a cpu panics, an error code is written by BIOS to this same reg.

So the reg has been renamed the "System Controller Interface Reg".

This patchset provides the following:

  * x86_64: Add base functionality for writing to the specific SCIR's
    for each cpu.

  * idle: Add an idle callback to measure the idle "on" and "off" times.

  * heartbeat: Invert "heartbeat" bit to indicate the cpu is "active".

  * if hotplug enabled, all bits are set (0xff) when the cpu is disabled.

Based on linux-2.6.tip/master.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 arch/x86/kernel/genx2apic_uv_x.c |  138 +++++++++++++++++++++++++++++++++++++++
 include/asm-x86/uv/uv_hub.h      |   62 +++++++++++++++++
 2 files changed, 200 insertions(+)

--- linux-2.6.tip.orig/arch/x86/kernel/genx2apic_uv_x.c
+++ linux-2.6.tip/arch/x86/kernel/genx2apic_uv_x.c
@@ -10,6 +10,7 @@
 
 #include <linux/kernel.h>
 #include <linux/threads.h>
+#include <linux/cpu.h>
 #include <linux/cpumask.h>
 #include <linux/string.h>
 #include <linux/ctype.h>
@@ -18,6 +19,8 @@
 #include <linux/bootmem.h>
 #include <linux/module.h>
 #include <linux/hardirq.h>
+#include <linux/timer.h>
+#include <asm/idle.h>
 #include <asm/smp.h>
 #include <asm/ipi.h>
 #include <asm/genapic.h>
@@ -357,6 +360,139 @@ static __init void uv_rtc_init(void)
 		sn_rtc_cycles_per_second = ticks_per_sec;
 }
 
+/*
+ * percpu heartbeat timer
+ */
+static void uv_heartbeat(unsigned long ignored)
+{
+	struct timer_list *timer = &uv_hub_info->scir.timer;
+	unsigned char bits = uv_hub_info->scir.state;
+
+	/* flip heartbeat bit */
+	bits ^= SCIR_CPU_HEARTBEAT;
+
+	/* determine if we were mostly idle or not */
+	if (uv_hub_info->scir.idle_off && uv_hub_info->scir.idle_on) {
+		if (uv_hub_info->scir.idle_off > uv_hub_info->scir.idle_on)
+			bits |= SCIR_CPU_ACTIVITY;
+		else
+			bits &= ~SCIR_CPU_ACTIVITY;
+	}
+
+	/* reset idle counters */
+	uv_hub_info->scir.idle_on = 0;
+	uv_hub_info->scir.idle_off = 0;
+
+	/* update system controller interface reg */
+	uv_set_scir_bits(bits);
+
+	/* enable next timer period */
+	mod_timer(timer, jiffies + SCIR_CPU_HB_INTERVAL);
+}
+
+static int uv_idle(struct notifier_block *nfb, unsigned long action, void *junk)
+{
+	unsigned long elapsed = jiffies - uv_hub_info->scir.last;
+
+	/*
+	 * update activity to indicate current state,
+	 * measure time since last change
+	 */
+	if (action == IDLE_START) {
+
+		uv_hub_info->scir.state &= ~SCIR_CPU_ACTIVITY;
+		uv_hub_info->scir.idle_on += elapsed;
+		uv_hub_info->scir.last = jiffies;
+
+	} else if (action == IDLE_END) {
+
+		uv_hub_info->scir.state |= SCIR_CPU_ACTIVITY;
+		uv_hub_info->scir.idle_off += elapsed;
+		uv_hub_info->scir.last = jiffies;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block uv_idle_notifier = {
+	.notifier_call = uv_idle,
+};
+
+static void __cpuinit uv_heartbeat_enable(int cpu)
+{
+	if (!uv_cpu_hub_info(cpu)->scir.enabled) {
+		struct timer_list *timer = &uv_cpu_hub_info(cpu)->scir.timer;
+
+		uv_set_cpu_scir_bits(cpu, SCIR_CPU_HEARTBEAT|SCIR_CPU_ACTIVITY);
+		setup_timer(timer, uv_heartbeat, cpu);
+		timer->expires = jiffies + SCIR_CPU_HB_INTERVAL;
+		add_timer_on(timer, cpu);
+		uv_cpu_hub_info(cpu)->scir.enabled = 1;
+	}
+
+	/* check boot cpu */
+	if (!uv_cpu_hub_info(0)->scir.enabled)
+		uv_heartbeat_enable(0);
+}
+
+static void __cpuinit uv_heartbeat_disable(int cpu)
+{
+	if (uv_cpu_hub_info(cpu)->scir.enabled) {
+		uv_cpu_hub_info(cpu)->scir.enabled = 0;
+		del_timer(&uv_cpu_hub_info(cpu)->scir.timer);
+	}
+	uv_set_cpu_scir_bits(cpu, 0xff);
+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+/*
+ * cpu hotplug notifier
+ */
+static __cpuinit int uv_scir_cpu_notify(struct notifier_block *self,
+				       unsigned long action, void *hcpu)
+{
+	long cpu = (long)hcpu;
+
+	switch (action) {
+	case CPU_ONLINE:
+		uv_heartbeat_enable(cpu);
+		break;
+	case CPU_DOWN_PREPARE:
+		uv_heartbeat_disable(cpu);
+		break;
+	default:
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static __init void uv_scir_register_cpu_notifier(void)
+{
+	hotcpu_notifier(uv_scir_cpu_notify, 0);
+	idle_notifier_register(&uv_idle_notifier);
+}
+
+#else /* !CONFIG_HOTPLUG_CPU */
+
+static __init void uv_scir_register_cpu_notifier(void)
+{
+	idle_notifier_register(&uv_idle_notifier);
+}
+
+static __init int uv_init_heartbeat(void)
+{
+	int cpu;
+
+	if (is_uv_system())
+		for_each_online_cpu(cpu)
+			uv_heartbeat_enable(cpu);
+	return 0;
+}
+
+late_initcall(uv_init_heartbeat);
+
+#endif /* !CONFIG_HOTPLUG_CPU */
+
 static bool uv_system_inited;
 
 void __init uv_system_init(void)
@@ -435,6 +571,7 @@ void __init uv_system_init(void)
 		uv_cpu_hub_info(cpu)->gnode_upper = gnode_upper;
 		uv_cpu_hub_info(cpu)->global_mmr_base = mmr_base;
 		uv_cpu_hub_info(cpu)->coherency_domain_number = 0;/* ZZZ */
+		uv_cpu_hub_info(cpu)->scir.offset = SCIR_LOCAL_MMR_BASE + lcpu;
 		uv_node_to_blade[nid] = blade;
 		uv_cpu_to_blade[cpu] = blade;
 		max_pnode = max(pnode, max_pnode);
@@ -449,6 +586,7 @@ void __init uv_system_init(void)
 	map_mmr_high(max_pnode);
 	map_config_high(max_pnode);
 	map_mmioh_high(max_pnode);
+	uv_scir_register_cpu_notifier();
 	uv_system_inited = true;
 }
 
--- linux-2.6.tip.orig/include/asm-x86/uv/uv_hub.h
+++ linux-2.6.tip/include/asm-x86/uv/uv_hub.h
@@ -112,6 +112,16 @@
  */
 #define UV_MAX_NASID_VALUE	(UV_MAX_NUMALINK_NODES * 2)
 
+struct uv_scir_s {
+	struct timer_list timer;
+	unsigned long	offset;
+	unsigned long	last;
+	unsigned long	idle_on;
+	unsigned long	idle_off;
+	unsigned char	state;
+	unsigned char	enabled;
+};
+
 /*
  * The following defines attributes of the HUB chip. These attributes are
  * frequently referenced and are kept in the per-cpu data areas of each cpu.
@@ -130,7 +140,9 @@ struct uv_hub_info_s {
 	unsigned char	blade_processor_id;
 	unsigned char	m_val;
 	unsigned char	n_val;
+	struct uv_scir_s scir;
 };
+
 DECLARE_PER_CPU(struct uv_hub_info_s, __uv_hub_info);
 #define uv_hub_info 		(&__get_cpu_var(__uv_hub_info))
 #define uv_cpu_hub_info(cpu)	(&per_cpu(__uv_hub_info, cpu))
@@ -162,6 +174,30 @@ DECLARE_PER_CPU(struct uv_hub_info_s, __
 
 #define UV_APIC_PNODE_SHIFT	6
 
+/* Local Bus from cpu's perspective */
+#define LOCAL_BUS_BASE		0x1c00000
+#define LOCAL_BUS_SIZE		(4 * 1024 * 1024)
+
+/*
+ * System Controller Interface Reg
+ *
+ * Note there are NO leds on a UV system.  This register is only
+ * used by the system controller to monitor system-wide operation.
+ * There are 64 regs per node.  With Nahelem cpus (2 cores per node,
+ * 8 cpus per core, 2 threads per cpu) there are 32 cpu threads on
+ * a node.
+ *
+ * The window is located at top of ACPI MMR space
+ */
+#define SCIR_WINDOW_COUNT	64
+#define SCIR_LOCAL_MMR_BASE	(LOCAL_BUS_BASE + \
+				 LOCAL_BUS_SIZE - \
+				 SCIR_WINDOW_COUNT)
+
+#define SCIR_CPU_HEARTBEAT	0x01	/* timer interrupt */
+#define SCIR_CPU_ACTIVITY	0x02	/* not idle */
+#define SCIR_CPU_HB_INTERVAL	(HZ)	/* once per second */
+
 /*
  * Macros for converting between kernel virtual addresses, socket local physical
  * addresses, and UV global physical addresses.
@@ -276,6 +312,16 @@ static inline void uv_write_local_mmr(un
 	*uv_local_mmr_address(offset) = val;
 }
 
+static inline unsigned char uv_read_local_mmr8(unsigned long offset)
+{
+	return *((unsigned char *)uv_local_mmr_address(offset));
+}
+
+static inline void uv_write_local_mmr8(unsigned long offset, unsigned char val)
+{
+	*((unsigned char *)uv_local_mmr_address(offset)) = val;
+}
+
 /*
  * Structures and definitions for converting between cpu, node, pnode, and blade
  * numbers.
@@ -350,5 +396,21 @@ static inline int uv_num_possible_blades
 	return uv_possible_blades;
 }
 
+/* Update SCIR state */
+static inline void uv_set_scir_bits(unsigned char value)
+{
+	if (uv_hub_info->scir.state != value) {
+		uv_hub_info->scir.state = value;
+		uv_write_local_mmr8(uv_hub_info->scir.offset, value);
+	}
+}
+static inline void uv_set_cpu_scir_bits(int cpu, unsigned char value)
+{
+	if (uv_cpu_hub_info(cpu)->scir.state != value) {
+		uv_cpu_hub_info(cpu)->scir.state = value;
+		uv_write_local_mmr8(uv_cpu_hub_info(cpu)->scir.offset, value);
+	}
+}
+
 #endif /* ASM_X86__UV__UV_HUB_H */
 

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Pavel Machek]

On Fri 2008-10-24 04:19:04, Mike Travis wrote:
> [Ingo - could you let me know what's holding up this driver?
> Thanks!, Mike]

I thought that...

 > +static __init void uv_scir_register_cpu_notifier(void)
> +{
> +	hotcpu_notifier(uv_scir_cpu_notify, 0);
> +	idle_notifier_register(&uv_idle_notifier);
> +}

...Ingo said that idle_notifier_register is deprecated and going away?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Ingo Molnar]

* Pavel Machek <pavel@suse.cz> wrote:

> On Fri 2008-10-24 04:19:04, Mike Travis wrote:
> > [Ingo - could you let me know what's holding up this driver?
> > Thanks!, Mike]
> 
> I thought that...
> 
>  > +static __init void uv_scir_register_cpu_notifier(void)
> > +{
> > +	hotcpu_notifier(uv_scir_cpu_notify, 0);
> > +	idle_notifier_register(&uv_idle_notifier);
> > +}
> 
> ...Ingo said that idle_notifier_register is deprecated and going away?

yes, but i suspect Mike noticed that i acked Len's use of idle notifiers 
in the ACPI tree, and now he wants to refresh this discussion?

i cannot really believe why anyone would want to slow down the from-idle 
hotpath in such a lame way:

+       } else if (action == IDLE_END) {
+
+               uv_hub_info->scir.state |= SCIR_CPU_ACTIVITY;
+               uv_hub_info->scir.idle_off += elapsed;
+               uv_hub_info->scir.last = jiffies;
+       }

with such a lame and low-res timestamp:

+       unsigned long elapsed = jiffies - uv_hub_info->scir.last;

it's an absolutely pointless act of adding overhead. Just use a regular 
timer mechanism to update uv_hub_info->scir.idle_off.

	Ingo

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Mike Travis]

Ingo Molnar wrote:
> * Pavel Machek <pavel@suse.cz> wrote:
> 
>> On Fri 2008-10-24 04:19:04, Mike Travis wrote:
>>> [Ingo - could you let me know what's holding up this driver?
>>> Thanks!, Mike]
>> I thought that...
>>
>>  > +static __init void uv_scir_register_cpu_notifier(void)
>>> +{
>>> +	hotcpu_notifier(uv_scir_cpu_notify, 0);
>>> +	idle_notifier_register(&uv_idle_notifier);
>>> +}
>> ...Ingo said that idle_notifier_register is deprecated and going away?
> 
> yes, but i suspect Mike noticed that i acked Len's use of idle notifiers 
> in the ACPI tree, and now he wants to refresh this discussion?
> 
> i cannot really believe why anyone would want to slow down the from-idle 
> hotpath in such a lame way:
> 
> +       } else if (action == IDLE_END) {
> +
> +               uv_hub_info->scir.state |= SCIR_CPU_ACTIVITY;
> +               uv_hub_info->scir.idle_off += elapsed;
> +               uv_hub_info->scir.last = jiffies;
> +       }
> 
> with such a lame and low-res timestamp:
> 
> +       unsigned long elapsed = jiffies - uv_hub_info->scir.last;
> 
> it's an absolutely pointless act of adding overhead. Just use a regular 
> timer mechanism to update uv_hub_info->scir.idle_off.

I do use the timer mechanism (once per second) to update the scir state.
Unfortunately, the state is *always* not idle during the timer callback
(since we are actively executing), so I needed some way of knowing the
idle state prior to the timer callback.  The idle callback is only used
to record the amount of time this cpu's been idle during the last second
so on the once per second interrupt, I can determine if this cpu was more,
or less idle during that same time period.

The hardware saves the last 64 seconds for diagnostic purposes so this is
not meant to be a high-res indication but a longer term cpu history (along
with the other error records recorded, like memory/cpu/bus faults, etc.)
The System Controller copies these records into the system logs.

Also, this is only on a UV system, I'm not introducing any overhead for
any other x86_64 systems.

Thanks,
Mike

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Andi Kleen]

Mike Travis <travis@sgi.com> writes:
>
> I do use the timer mechanism (once per second) to update the scir state.
> Unfortunately, the state is *always* not idle during the timer callback
> (since we are actively executing), so I needed some way of knowing the
> idle state prior to the timer callback.

The idle thread always has pid 0. So if the timer sees current->pid == 0
it interrupted idle.

But sampling only once a second would be presumably not very accurate.
You could just check the per cpu statistics the kernel keeps
anyways in the timer.

I agree with Ingo that this doesn't belong in a idle notifier.

-Andi
-- 
ak@linux.intel.com

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Mike Travis]

Andi Kleen wrote:
> Mike Travis <travis@sgi.com> writes:
>> I do use the timer mechanism (once per second) to update the scir state.
>> Unfortunately, the state is *always* not idle during the timer callback
>> (since we are actively executing), so I needed some way of knowing the
>> idle state prior to the timer callback.
> 
> The idle thread always has pid 0. So if the timer sees current->pid == 0
> it interrupted idle.
> 
> But sampling only once a second would be presumably not very accurate.
> You could just check the per cpu statistics the kernel keeps
> anyways in the timer.
> 
> I agree with Ingo that this doesn't belong in a idle notifier.
> 
> -Andi

Cool, thanks!  I rewrote the patch to use current->pid, removing the
idle callback.

On ia64 the activity indicator was kept up to date by modifying the
register state every time the cpu went into and out of idle.  Ingo
thought this was excessive I/O overhead and since then, I've learned
that the UV system has a FIFO for these registers.  So flipping the
bit every time we changed idle state would quickly fill the FIFO and
wouldn't be keeping in sync with the companion bit which indicates
that the cpu is interruptible.

So the hw group decided that having a once per second indicator, even
if it's only a very narrow (instant) sampling period, would meet their 
requirements.

Thanks!
Mike

[PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Mike Travis]

Subject: SGI X86 UV: Provide a System Activity Indicator driver

The SGI UV system has no LEDS but uses one of the system controller
regs to indicate the online internal state of the cpu.  There is a
heartbeat bit indicating that the cpu is responding to interrupts,
and an idle bit indicating whether the cpu is idle when the heartbeat
interrupt occurs.  The current period is one second.

When a cpu panics, an error code is written by BIOS to this same reg.

This patchset provides the following:

  * x86_64: Add base functionality for writing to the specific SCIR's
    for each cpu.

  * heartbeat: Invert "heartbeat" bit to indicate the cpu is
    "interruptible".  If the current thread is the idle thread,
    then indicate system is "idle".

  * if hotplug enabled, all bits are set (0xff) when the cpu is disabled.

Based on linux-2.6.tip/master.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 arch/x86/include/asm/uv/uv_hub.h |   63 +++++++++++++++++++++++-
 arch/x86/kernel/genx2apic_uv_x.c |  102 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 164 insertions(+), 1 deletion(-)

--- test-tip-latest.orig/arch/x86/include/asm/uv/uv_hub.h
+++ test-tip-latest/arch/x86/include/asm/uv/uv_hub.h
@@ -112,6 +112,16 @@
  */
 #define UV_MAX_NASID_VALUE	(UV_MAX_NUMALINK_NODES * 2)
 
+struct uv_scir_s {
+	struct timer_list timer;
+	unsigned long	offset;
+	unsigned long	last;
+	unsigned long	idle_on;
+	unsigned long	idle_off;
+	unsigned char	state;
+	unsigned char	enabled;
+};
+
 /*
  * The following defines attributes of the HUB chip. These attributes are
  * frequently referenced and are kept in the per-cpu data areas of each cpu.
@@ -130,7 +140,9 @@ struct uv_hub_info_s {
 	unsigned char	blade_processor_id;
 	unsigned char	m_val;
 	unsigned char	n_val;
+	struct uv_scir_s scir;
 };
+
 DECLARE_PER_CPU(struct uv_hub_info_s, __uv_hub_info);
 #define uv_hub_info 		(&__get_cpu_var(__uv_hub_info))
 #define uv_cpu_hub_info(cpu)	(&per_cpu(__uv_hub_info, cpu))
@@ -162,6 +174,30 @@ DECLARE_PER_CPU(struct uv_hub_info_s, __
 
 #define UV_APIC_PNODE_SHIFT	6
 
+/* Local Bus from cpu's perspective */
+#define LOCAL_BUS_BASE		0x1c00000
+#define LOCAL_BUS_SIZE		(4 * 1024 * 1024)
+
+/*
+ * System Controller Interface Reg
+ *
+ * Note there are NO leds on a UV system.  This register is only
+ * used by the system controller to monitor system-wide operation.
+ * There are 64 regs per node.  With Nahelem cpus (2 cores per node,
+ * 8 cpus per core, 2 threads per cpu) there are 32 cpu threads on
+ * a node.
+ *
+ * The window is located at top of ACPI MMR space
+ */
+#define SCIR_WINDOW_COUNT	64
+#define SCIR_LOCAL_MMR_BASE	(LOCAL_BUS_BASE + \
+				 LOCAL_BUS_SIZE - \
+				 SCIR_WINDOW_COUNT)
+
+#define SCIR_CPU_HEARTBEAT	0x01	/* timer interrupt */
+#define SCIR_CPU_ACTIVITY	0x02	/* not idle */
+#define SCIR_CPU_HB_INTERVAL	(HZ)	/* once per second */
+
 /*
  * Macros for converting between kernel virtual addresses, socket local physical
  * addresses, and UV global physical addresses.
@@ -276,6 +312,16 @@ static inline void uv_write_local_mmr(un
 	*uv_local_mmr_address(offset) = val;
 }
 
+static inline unsigned char uv_read_local_mmr8(unsigned long offset)
+{
+	return *((unsigned char *)uv_local_mmr_address(offset));
+}
+
+static inline void uv_write_local_mmr8(unsigned long offset, unsigned char val)
+{
+	*((unsigned char *)uv_local_mmr_address(offset)) = val;
+}
+
 /*
  * Structures and definitions for converting between cpu, node, pnode, and blade
  * numbers.
@@ -350,5 +396,20 @@ static inline int uv_num_possible_blades
 	return uv_possible_blades;
 }
 
-#endif /* _ASM_X86_UV_UV_HUB_H */
+/* Update SCIR state */
+static inline void uv_set_scir_bits(unsigned char value)
+{
+	if (uv_hub_info->scir.state != value) {
+		uv_hub_info->scir.state = value;
+		uv_write_local_mmr8(uv_hub_info->scir.offset, value);
+	}
+}
+static inline void uv_set_cpu_scir_bits(int cpu, unsigned char value)
+{
+	if (uv_cpu_hub_info(cpu)->scir.state != value) {
+		uv_cpu_hub_info(cpu)->scir.state = value;
+		uv_write_local_mmr8(uv_cpu_hub_info(cpu)->scir.offset, value);
+	}
+}
 
+#endif /* _ASM_X86_UV_UV_HUB_H */
--- test-tip-latest.orig/arch/x86/kernel/genx2apic_uv_x.c
+++ test-tip-latest/arch/x86/kernel/genx2apic_uv_x.c
@@ -10,6 +10,7 @@
 
 #include <linux/kernel.h>
 #include <linux/threads.h>
+#include <linux/cpu.h>
 #include <linux/cpumask.h>
 #include <linux/string.h>
 #include <linux/ctype.h>
@@ -18,6 +19,8 @@
 #include <linux/bootmem.h>
 #include <linux/module.h>
 #include <linux/hardirq.h>
+#include <linux/timer.h>
+#include <asm/current.h>
 #include <asm/smp.h>
 #include <asm/ipi.h>
 #include <asm/genapic.h>
@@ -358,6 +361,103 @@ static __init void uv_rtc_init(void)
 }
 
 /*
+ * percpu heartbeat timer
+ */
+static void uv_heartbeat(unsigned long ignored)
+{
+	struct timer_list *timer = &uv_hub_info->scir.timer;
+	unsigned char bits = uv_hub_info->scir.state;
+
+	/* flip heartbeat bit */
+	bits ^= SCIR_CPU_HEARTBEAT;
+
+	/* are we the idle thread? */
+	if (current->pid == 0)
+		bits &= ~SCIR_CPU_ACTIVITY;
+	else
+		bits |= SCIR_CPU_ACTIVITY;
+
+	/* update system controller interface reg */
+	uv_set_scir_bits(bits);
+
+	/* enable next timer period */
+	mod_timer(timer, jiffies + SCIR_CPU_HB_INTERVAL);
+}
+
+static void __cpuinit uv_heartbeat_enable(int cpu)
+{
+	if (!uv_cpu_hub_info(cpu)->scir.enabled) {
+		struct timer_list *timer = &uv_cpu_hub_info(cpu)->scir.timer;
+
+		uv_set_cpu_scir_bits(cpu, SCIR_CPU_HEARTBEAT|SCIR_CPU_ACTIVITY);
+		setup_timer(timer, uv_heartbeat, cpu);
+		timer->expires = jiffies + SCIR_CPU_HB_INTERVAL;
+		add_timer_on(timer, cpu);
+		uv_cpu_hub_info(cpu)->scir.enabled = 1;
+	}
+
+	/* check boot cpu */
+	if (!uv_cpu_hub_info(0)->scir.enabled)
+		uv_heartbeat_enable(0);
+}
+
+static void __cpuinit uv_heartbeat_disable(int cpu)
+{
+	if (uv_cpu_hub_info(cpu)->scir.enabled) {
+		uv_cpu_hub_info(cpu)->scir.enabled = 0;
+		del_timer(&uv_cpu_hub_info(cpu)->scir.timer);
+	}
+	uv_set_cpu_scir_bits(cpu, 0xff);
+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+/*
+ * cpu hotplug notifier
+ */
+static __cpuinit int uv_scir_cpu_notify(struct notifier_block *self,
+				       unsigned long action, void *hcpu)
+{
+	long cpu = (long)hcpu;
+
+	switch (action) {
+	case CPU_ONLINE:
+		uv_heartbeat_enable(cpu);
+		break;
+	case CPU_DOWN_PREPARE:
+		uv_heartbeat_disable(cpu);
+		break;
+	default:
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static __init void uv_scir_register_cpu_notifier(void)
+{
+	hotcpu_notifier(uv_scir_cpu_notify, 0);
+}
+
+#else /* !CONFIG_HOTPLUG_CPU */
+
+static __init void uv_scir_register_cpu_notifier(void)
+{
+}
+
+static __init int uv_init_heartbeat(void)
+{
+	int cpu;
+
+	if (is_uv_system())
+		for_each_online_cpu(cpu)
+			uv_heartbeat_enable(cpu);
+	return 0;
+}
+
+late_initcall(uv_init_heartbeat);
+
+#endif /* !CONFIG_HOTPLUG_CPU */
+
+/*
  * Called on each cpu to initialize the per_cpu UV data area.
  * 	ZZZ hotplug not supported yet
  */
@@ -453,6 +553,7 @@ void __init uv_system_init(void)
 		uv_cpu_hub_info(cpu)->gnode_upper = gnode_upper;
 		uv_cpu_hub_info(cpu)->global_mmr_base = mmr_base;
 		uv_cpu_hub_info(cpu)->coherency_domain_number = sn_coherency_id;
+		uv_cpu_hub_info(cpu)->scir.offset = SCIR_LOCAL_MMR_BASE + lcpu;
 		uv_node_to_blade[nid] = blade;
 		uv_cpu_to_blade[cpu] = blade;
 		max_pnode = max(pnode, max_pnode);
@@ -469,4 +570,5 @@ void __init uv_system_init(void)
 	map_mmioh_high(max_pnode);
 
 	uv_cpu_init();
+	uv_scir_register_cpu_notifier();
 }

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> Subject: SGI X86 UV: Provide a System Activity Indicator driver
> 
> The SGI UV system has no LEDS but uses one of the system controller
> regs to indicate the online internal state of the cpu.  There is a
> heartbeat bit indicating that the cpu is responding to interrupts,
> and an idle bit indicating whether the cpu is idle when the heartbeat
> interrupt occurs.  The current period is one second.
> 
> When a cpu panics, an error code is written by BIOS to this same reg.
> 
> This patchset provides the following:
> 
>   * x86_64: Add base functionality for writing to the specific SCIR's
>     for each cpu.
> 
>   * heartbeat: Invert "heartbeat" bit to indicate the cpu is
>     "interruptible".  If the current thread is the idle thread,
>     then indicate system is "idle".
> 
>   * if hotplug enabled, all bits are set (0xff) when the cpu is disabled.
> 
> Based on linux-2.6.tip/master.
> 
> Signed-off-by: Mike Travis <travis@sgi.com>
> ---
>  arch/x86/include/asm/uv/uv_hub.h |   63 +++++++++++++++++++++++-
>  arch/x86/kernel/genx2apic_uv_x.c |  102 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 164 insertions(+), 1 deletion(-)

applied to tip/x86/uv, thanks Mike!

Please send the cpu_idle() cleanup patch separately.

Another minor thing i noticed:

@@ -130,7 +140,9 @@ struct uv_hub_info_s {
 	unsigned char	blade_processor_id;
 	unsigned char	m_val;
 	unsigned char	n_val;
+	struct uv_scir_s scir;

please align the new field vertically, like they were aligned before - 
by adding another tab to the whole lineup. (This will also make it 
appear nicer when viewed together with followup definitions below this 
section)

	Ingo

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Mike Travis]

Ingo Molnar wrote:
...
> 
> Please send the cpu_idle() cleanup patch separately.
> 
> Another minor thing i noticed:
> 
> @@ -130,7 +140,9 @@ struct uv_hub_info_s {
>  	unsigned char	blade_processor_id;
>  	unsigned char	m_val;
>  	unsigned char	n_val;
> +	struct uv_scir_s scir;
> 
> please align the new field vertically, like they were aligned before - 
> by adding another tab to the whole lineup. (This will also make it 
> appear nicer when viewed together with followup definitions below this 
> section)

Ok, will do, thanks!

Mike

[PATCH 1/1] SGI x86 UV: Update SCIR driver to use idle_cpu function

[Mike Travis]

Subject: SGI x86 UV: Update SCIR driver to use idle_cpu function

> Please send the cpu_idle() cleanup patch separately.

Change UV heartbeat function to use idle_cpu to determine cpu's
"idleness".  Realign uv_hub definitions.

Signed-of-by: Mike Travis <travis@sgi.com>
---
 arch/x86/include/asm/uv/uv_hub.h |   26 +++++++++++++-------------
 arch/x86/kernel/genx2apic_uv_x.c |    4 ++--
 2 files changed, 15 insertions(+), 15 deletions(-)

--- test-tip-latest.orig/arch/x86/include/asm/uv/uv_hub.h
+++ test-tip-latest/arch/x86/include/asm/uv/uv_hub.h
@@ -128,19 +128,19 @@ struct uv_scir_s {
  * They are kept together in a struct to minimize cache misses.
  */
 struct uv_hub_info_s {
-	unsigned long	global_mmr_base;
-	unsigned long	gpa_mask;
-	unsigned long	gnode_upper;
-	unsigned long	lowmem_remap_top;
-	unsigned long	lowmem_remap_base;
-	unsigned short	pnode;
-	unsigned short	pnode_mask;
-	unsigned short	coherency_domain_number;
-	unsigned short	numa_blade_id;
-	unsigned char	blade_processor_id;
-	unsigned char	m_val;
-	unsigned char	n_val;
-	struct uv_scir_s scir;
+	unsigned long		global_mmr_base;
+	unsigned long		gpa_mask;
+	unsigned long		gnode_upper;
+	unsigned long		lowmem_remap_top;
+	unsigned long		lowmem_remap_base;
+	unsigned short		pnode;
+	unsigned short		pnode_mask;
+	unsigned short		coherency_domain_number;
+	unsigned short		numa_blade_id;
+	unsigned char		blade_processor_id;
+	unsigned char		m_val;
+	unsigned char		n_val;
+	struct uv_scir_s	scir;
 };
 
 DECLARE_PER_CPU(struct uv_hub_info_s, __uv_hub_info);
--- test-tip-latest.orig/arch/x86/kernel/genx2apic_uv_x.c
+++ test-tip-latest/arch/x86/kernel/genx2apic_uv_x.c
@@ -371,8 +371,8 @@ static void uv_heartbeat(unsigned long i
 	/* flip heartbeat bit */
 	bits ^= SCIR_CPU_HEARTBEAT;
 
-	/* are we the idle thread? */
-	if (current->pid == 0)
+	/* is this cpu idle? */
+	if (idle_cpu(smp_processor_id()))
 		bits &= ~SCIR_CPU_ACTIVITY;
 	else
 		bits |= SCIR_CPU_ACTIVITY;

Re: [PATCH 1/1] SGI x86 UV: Update SCIR driver to use idle_cpu function

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> Subject: SGI x86 UV: Update SCIR driver to use idle_cpu function
> 
> > Please send the cpu_idle() cleanup patch separately.
> 
> Change UV heartbeat function to use idle_cpu to determine cpu's
> "idleness".  Realign uv_hub definitions.
> 
> Signed-of-by: Mike Travis <travis@sgi.com>
> ---
>  arch/x86/include/asm/uv/uv_hub.h |   26 +++++++++++++-------------
>  arch/x86/kernel/genx2apic_uv_x.c |    4 ++--
>  2 files changed, 15 insertions(+), 15 deletions(-)

applied to tip/x86/uv, thanks Mike!

one small comment:

> +	/* is this cpu idle? */
> +	if (idle_cpu(smp_processor_id()))
>  		bits &= ~SCIR_CPU_ACTIVITY;

were you ever to run an -rt kernel on that hardware, this would 
produce a warning. raw_smp_processor_id() would be more appropriate i 
guess.

	Ingo

[PATCH 1/1] SGI x86 UV: Use raw_smp_processor_id

[Mike Travis]

Subject: SGI x86 UV: Use raw_smp_processor_id

> one small comment:
> 
>> +	/* is this cpu idle? */
>> +	if (idle_cpu(smp_processor_id()))
>>  		bits &= ~SCIR_CPU_ACTIVITY;
> 
> were you ever to run an -rt kernel on that hardware, this would 
> produce a warning. raw_smp_processor_id() would be more appropriate i 
> guess.

Thanks for the heads up!  Yes, -rt is supported.  Might as well avoid that
problem now.

Signed-of-by: Mike Travis <travis@sgi.com>
---
 arch/x86/kernel/genx2apic_uv_x.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- test-tip-latest.orig/arch/x86/kernel/genx2apic_uv_x.c
+++ test-tip-latest/arch/x86/kernel/genx2apic_uv_x.c
@@ -372,7 +372,7 @@ static void uv_heartbeat(unsigned long i
 	bits ^= SCIR_CPU_HEARTBEAT;
 
 	/* is this cpu idle? */
-	if (idle_cpu(smp_processor_id()))
+	if (idle_cpu(raw_smp_processor_id()))
 		bits &= ~SCIR_CPU_ACTIVITY;
 	else
 		bits |= SCIR_CPU_ACTIVITY;

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Andi Kleen]

> Cool, thanks!  I rewrote the patch to use current->pid, removing the
> idle callback.

As a additional measure you could make the timer deferred when in 
idle state.  Then you would only actually do something when 
changing from idle to non idle or keeps being busy, but nothing 
when a CPU stays idle for a longer time.

The frequency wouldn't necessarily be 1 second then though.

-Andi

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Mike Travis]

Andi Kleen wrote:
>> Cool, thanks!  I rewrote the patch to use current->pid, removing the
>> idle callback.
> 
> As a additional measure you could make the timer deferred when in 
> idle state.  Then you would only actually do something when 
> changing from idle to non idle or keeps being busy, but nothing 
> when a CPU stays idle for a longer time.
> 
> The frequency wouldn't necessarily be 1 second then though.
> 
> -Andi

Hi Andi,

Sorry, I didn't see this message until now.  I will look into doing this
and ask the hw group if it's acceptable to drop BMC records when the cpu
is idle.  (I think they may still want them since this might also be
confused with "the cpu is no longer uninterruptible".)

Thanks,
Mike

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> > The idle thread always has pid 0. So if the timer sees 
> > current->pid == 0 it interrupted idle.

that's still hacky as it hardcodes the idle == pid-0 assumption. 
Please use the idle_cpu() function of the scheduler instead.

	Ingo

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> Also, this is only on a UV system, I'm not introducing any overhead 
> for any other x86_64 systems.

true, but it sets a bad example that others might follow - and it's 
not hard to fix anyway.

	Ingo

Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator driver

[Mike Travis]

Pavel Machek wrote:
> On Fri 2008-10-24 04:19:04, Mike Travis wrote:
>> [Ingo - could you let me know what's holding up this driver?
>> Thanks!, Mike]
> 
> I thought that...
> 
>  > +static __init void uv_scir_register_cpu_notifier(void)
>> +{
>> +	hotcpu_notifier(uv_scir_cpu_notify, 0);
>> +	idle_notifier_register(&uv_idle_notifier);
>> +}
> 
> ...Ingo said that idle_notifier_register is deprecated and going away?
> 
> 									Pavel

Is there any other way to get an indication then?  This is also in ia64 code
though not with the formality of a callback -- it's a static function pointer:

arch/ia64/kernel/process.c:
	void (*ia64_mark_idle)(int);

arch/ia64/sn/kernel/setup.c:
	ia64_mark_idle = &snidle;

arch/ia64/sn/kernel/idle.c
	void snidle(int state)
	{
	        if (state) {
	                if (pda->idle_flag == 0) {
	                        /*
	                         * Turn the activity LED off.
        	                 */
        	                set_led_bits(0, LED_CPU_ACTIVITY);
        	        }

	       	        pda->idle_flag = 1;
	        } else {
	                /*
	                 * Turn the activity LED on.
	                 */
	                set_led_bits(LED_CPU_ACTIVITY, LED_CPU_ACTIVITY);

	                pda->idle_flag = 0;
	        }
	}

As I mentioned, I would be happy to use an alternative but I haven't yet found one.
Should I put the above into x86_64?

Thanks,
Mike