[PATCH] xen/keyhandler: Rework keyhandler infrastructure

[Andrew Cooper <andrew.cooper3@citrix.com>]

struct keyhandler does not contain much information, and requires a lot
of boilerplate to use.  It is far more convenient to have
register_keyhandler() take each piece of information a parameter,
especially when introducing temporary debugging keyhandlers.

This in turn allows struct keyhandler itself to become private to
keyhandler.c and for the key_table to become more efficient.

key_table doesn't need to contain 256 entries; all keys are ASCII
which limits them to 7 bits of index, rather than 8.  It can also
become a straight array, rather than an array of pointers.  struct
keyhandler itself can become smaller simply via reordering (losing 6
bytes of implicit padding).  The overall effect of this is the
key_table grows in size by 50%, but there are no longer 24-byte
keyhandler structures all over the data section.

All of the key_table entries in keyhandler.c can be initialised at
compile time rather than runtime.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Keir Fraser <keir@xen.org>

---

Note: I have avoided CC'ing all maintainers as this is largely a mechanical
change across the whole codebase.  Most non-mechanical changes are in common
code, but there is a trivial externing change
xen/drivers/passthrough/vtd/extern.h as previously the struct keyhandler was
in a different translation unit to the function it referenced.

Further reduction in the size of the key_table could be achieved by
dropping the first 0x20 entries corresponding to ASCII control
characters, and by hiding the two boolean fields in the low-order bits
of the function pointer, but both of these feel like too much effort
for too little gain.
---
 xen/arch/x86/acpi/cpu_idle.c             |    8 +-
 xen/arch/x86/hvm/irq.c                   |    8 +-
 xen/arch/x86/hvm/svm/vmcb.c              |    8 +-
 xen/arch/x86/hvm/vmx/vmcs.c              |    8 +-
 xen/arch/x86/io_apic.c                   |    7 +-
 xen/arch/x86/irq.c                       |    8 +-
 xen/arch/x86/mm/p2m-ept.c                |    8 +-
 xen/arch/x86/mm/shadow/common.c          |   14 +--
 xen/arch/x86/msi.c                       |    8 +-
 xen/arch/x86/nmi.c                       |   15 +--
 xen/arch/x86/numa.c                      |    8 +-
 xen/arch/x86/time.c                      |    8 +-
 xen/common/event_channel.c               |    8 +-
 xen/common/grant_table.c                 |    9 +-
 xen/common/kexec.c                       |    7 +-
 xen/common/keyhandler.c                  |  199 ++++++++++++------------------
 xen/common/page_alloc.c                  |   16 +--
 xen/common/timer.c                       |    8 +-
 xen/drivers/char/console.c               |   16 +--
 xen/drivers/passthrough/amd/iommu_intr.c |    9 +-
 xen/drivers/passthrough/iommu.c          |    8 +-
 xen/drivers/passthrough/pci.c            |    8 +-
 xen/drivers/passthrough/vtd/extern.h     |    2 +-
 xen/drivers/passthrough/vtd/iommu.c      |    2 +-
 xen/drivers/passthrough/vtd/utils.c      |    8 +-
 xen/include/xen/keyhandler.h             |   49 +++-----
 26 files changed, 124 insertions(+), 333 deletions(-)

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 15fe2e9..d1f99a7 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -334,15 +334,9 @@ static void dump_cx(unsigned char key)
             print_acpi_power(cpu, processor_powers[cpu]);
 }
 
-static struct keyhandler dump_cx_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_cx,
-    .desc = "dump ACPI Cx structures"
-};
-
 static int __init cpu_idle_key_init(void)
 {
-    register_keyhandler('c', &dump_cx_keyhandler);
+    register_keyhandler('c', dump_cx, "dump ACPI Cx structures", 1);
     return 0;
 }
 __initcall(cpu_idle_key_init);
diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
index 50fcf73..990a2ca 100644
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -527,15 +527,9 @@ static void dump_irq_info(unsigned char key)
     rcu_read_unlock(&domlist_read_lock);
 }
 
-static struct keyhandler dump_irq_info_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_irq_info,
-    .desc = "dump HVM irq info"
-};
-
 static int __init dump_irq_info_key_init(void)
 {
-    register_keyhandler('I', &dump_irq_info_keyhandler);
+    register_keyhandler('I', dump_irq_info, "dump HVM irq info", 1);
     return 0;
 }
 __initcall(dump_irq_info_key_init);
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index b5d7165..9ea014f 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -303,15 +303,9 @@ static void vmcb_dump(unsigned char ch)
     printk("**************************************\n");
 }
 
-static struct keyhandler vmcb_dump_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = vmcb_dump,
-    .desc = "dump AMD-V VMCBs"
-};
-
 void __init setup_vmcb_dump(void)
 {
-    register_keyhandler('v', &vmcb_dump_keyhandler);
+    register_keyhandler('v', vmcb_dump, "dump AMD-V VMCBs", 1);
 }
 
 /*
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 08f2078..25594ee 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1858,15 +1858,9 @@ static void vmcs_dump(unsigned char ch)
     printk("**************************************\n");
 }
 
-static struct keyhandler vmcs_dump_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = vmcs_dump,
-    .desc = "dump Intel's VMCS"
-};
-
 void __init setup_vmcs_dump(void)
 {
-    register_keyhandler('v', &vmcs_dump_keyhandler);
+    register_keyhandler('v', vmcs_dump, "dump Intel's VMCS", 1);
 }
 
 
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index b8e37b5..eedd528 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1251,11 +1251,6 @@ static void _print_IO_APIC_keyhandler(unsigned char key)
 {
     __print_IO_APIC(0);
 }
-static struct keyhandler print_IO_APIC_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = _print_IO_APIC_keyhandler,
-    .desc = "print ioapic info"
-};
 
 static void __init enable_IO_APIC(void)
 {
@@ -2054,7 +2049,7 @@ void __init setup_IO_APIC(void)
     print_IO_APIC();
     ioapic_pm_state_alloc();
 
-    register_keyhandler('z', &print_IO_APIC_keyhandler);
+    register_keyhandler('z', _print_IO_APIC_keyhandler, "print ioapic info", 1);
 }
 
 void ioapic_suspend(void)
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index bf2e822..f1397d6 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2316,15 +2316,9 @@ static void dump_irqs(unsigned char key)
     dump_ioapic_irq_info();
 }
 
-static struct keyhandler dump_irqs_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_irqs,
-    .desc = "dump interrupt bindings"
-};
-
 static int __init setup_dump_irqs(void)
 {
-    register_keyhandler('i', &dump_irqs_keyhandler);
+    register_keyhandler('i', dump_irqs, "dump interrupt bindings", 1);
     return 0;
 }
 __initcall(setup_dump_irqs);
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 2f3df91..0ee947f 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -1251,15 +1251,9 @@ static void ept_dump_p2m_table(unsigned char key)
     }
 }
 
-static struct keyhandler ept_p2m_table = {
-    .diagnostic = 0,
-    .u.fn = ept_dump_p2m_table,
-    .desc = "dump ept p2m table"
-};
-
 void setup_ept_dump(void)
 {
-    register_keyhandler('D', &ept_p2m_table);
+    register_keyhandler('D', ept_dump_p2m_table, "dump ept p2m table", 0);
 }
 
 /*
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 0264b91..3759232 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -97,14 +97,9 @@ static void shadow_audit_key(unsigned char key)
            __func__, shadow_audit_enable);
 }
 
-static struct keyhandler shadow_audit_keyhandler = {
-    .u.fn = shadow_audit_key,
-    .desc = "toggle shadow audits"
-};
-
 static int __init shadow_audit_key_init(void)
 {
-    register_keyhandler('O', &shadow_audit_keyhandler);
+    register_keyhandler('O', shadow_audit_key, "toggle shadow audits", 0);
     return 0;
 }
 __initcall(shadow_audit_key_init);
@@ -1409,15 +1404,10 @@ static void shadow_blow_all_tables(unsigned char c)
     rcu_read_unlock(&domlist_read_lock);
 }
 
-static struct keyhandler shadow_blow_all_tables_keyhandler = {
-    .u.fn = shadow_blow_all_tables,
-    .desc = "reset shadow pagetables"
-};
-
 /* Register this function in the Xen console keypress table */
 static __init int shadow_blow_tables_keyhandler_init(void)
 {
-    register_keyhandler('S', &shadow_blow_all_tables_keyhandler);
+    register_keyhandler('S', shadow_blow_all_tables, "reset shadow pagetables", 1);
     return 0;
 }
 __initcall(shadow_blow_tables_keyhandler_init);
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 588305a..630e1b8 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -1588,15 +1588,9 @@ static void dump_msi(unsigned char key)
     }
 }
 
-static struct keyhandler dump_msi_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_msi,
-    .desc = "dump MSI state"
-};
-
 static int __init msi_setup_keyhandler(void)
 {
-    register_keyhandler('M', &dump_msi_keyhandler);
+    register_keyhandler('M', dump_msi, "dump MSI state", 1);
     return 0;
 }
 __initcall(msi_setup_keyhandler);
diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index 2ab97a0..7520454 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -554,11 +554,6 @@ static void do_nmi_trigger(unsigned char key)
     self_nmi();
 }
 
-static struct keyhandler nmi_trigger_keyhandler = {
-    .u.fn = do_nmi_trigger,
-    .desc = "trigger an NMI"
-};
-
 static void do_nmi_stats(unsigned char key)
 {
     int i;
@@ -582,16 +577,10 @@ static void do_nmi_stats(unsigned char key)
         printk("dom0 vcpu0: NMI neither pending nor masked\n");
 }
 
-static struct keyhandler nmi_stats_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = do_nmi_stats,
-    .desc = "NMI statistics"
-};
-
 static __init int register_nmi_trigger(void)
 {
-    register_keyhandler('N', &nmi_trigger_keyhandler);
-    register_keyhandler('n', &nmi_stats_keyhandler);
+    register_keyhandler('N', do_nmi_trigger, "trigger an NMI", 0);
+    register_keyhandler('n', do_nmi_stats, "NMI statistics", 1);
     return 0;
 }
 __initcall(register_nmi_trigger);
diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index 132d694..b953c7f 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -483,15 +483,9 @@ static void dump_numa(unsigned char key)
     rcu_read_unlock(&domlist_read_lock);
 }
 
-static struct keyhandler dump_numa_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_numa,
-    .desc = "dump numa info"
-};
-
 static __init int register_numa_trigger(void)
 {
-    register_keyhandler('u', &dump_numa_keyhandler);
+    register_keyhandler('u', dump_numa, "dump numa info", 1);
     return 0;
 }
 __initcall(register_numa_trigger);
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index bbb7e6c..b334f2f 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -2047,15 +2047,9 @@ static void dump_softtsc(unsigned char key)
             printk("No domains have emulated TSC\n");
 }
 
-static struct keyhandler dump_softtsc_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_softtsc,
-    .desc = "dump softtsc stats"
-};
-
 static int __init setup_dump_softtsc(void)
 {
-    register_keyhandler('s', &dump_softtsc_keyhandler);
+    register_keyhandler('s', dump_softtsc, "dump softtsc stats", 1);
     return 0;
 }
 __initcall(setup_dump_softtsc);
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index bd334f9..5a529a6 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1431,15 +1431,9 @@ static void dump_evtchn_info(unsigned char key)
     rcu_read_unlock(&domlist_read_lock);
 }
 
-static struct keyhandler dump_evtchn_info_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_evtchn_info,
-    .desc = "dump evtchn info"
-};
-
 static int __init dump_evtchn_info_key_init(void)
 {
-    register_keyhandler('e', &dump_evtchn_info_keyhandler);
+    register_keyhandler('e', dump_evtchn_info, "dump evtchn info", 1);
     return 0;
 }
 __initcall(dump_evtchn_info_key_init);
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 23185e7..c92abda 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3492,12 +3492,6 @@ static void gnttab_usage_print_all(unsigned char key)
     printk("%s ] done\n", __FUNCTION__);
 }
 
-static struct keyhandler gnttab_usage_print_all_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = gnttab_usage_print_all,
-    .desc = "print grant table usage"
-};
-
 static int __init gnttab_usage_init(void)
 {
     if ( max_nr_grant_frames )
@@ -3518,7 +3512,8 @@ static int __init gnttab_usage_init(void)
     if ( !max_maptrack_frames )
         max_maptrack_frames = DEFAULT_MAX_MAPTRACK_FRAMES;
 
-    register_keyhandler('g', &gnttab_usage_print_all_keyhandler);
+    register_keyhandler('g', gnttab_usage_print_all,
+                        "print grant table usage", 1);
     return 0;
 }
 __initcall(gnttab_usage_init);
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 7dd2700..2fc2957 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -374,11 +374,6 @@ static void do_crashdump_trigger(unsigned char key)
     printk(" * no crash kernel loaded!\n");
 }
 
-static struct keyhandler crashdump_trigger_keyhandler = {
-    .u.fn = do_crashdump_trigger,
-    .desc = "trigger a crashdump"
-};
-
 static void setup_note(Elf_Note *n, const char *name, int type, int descsz)
 {
     int l = strlen(name) + 1;
@@ -571,7 +566,7 @@ static int __init kexec_init(void)
     if ( ! crash_notes )
         return -ENOMEM;
 
-    register_keyhandler('C', &crashdump_trigger_keyhandler);
+    register_keyhandler('C', do_crashdump_trigger, "trigger a crashdump", 0);
 
     cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);
     register_cpu_notifier(&cpu_nfb);
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 5d21e48..473d554 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -21,12 +21,64 @@
 #include <asm/debugger.h>
 #include <asm/div64.h>
 
-static struct keyhandler *key_table[256];
 static unsigned char keypress_key;
 static bool_t alt_key_handling;
 
 char keyhandler_scratch[1024];
 
+static void do_toggle_alt_key(unsigned char key, struct cpu_user_regs *regs);
+static void show_handlers(unsigned char key);
+static void dump_registers(unsigned char key, struct cpu_user_regs *regs);
+static void dump_hwdom_registers(unsigned char key);
+static void reboot_machine(unsigned char key, struct cpu_user_regs *regs);
+static void dump_domains(unsigned char key);
+static void read_clocks(unsigned char key);
+static void run_all_keyhandlers(unsigned char key, struct cpu_user_regs *regs);
+static void do_debug_key(unsigned char key, struct cpu_user_regs *regs);
+
+static struct keyhandler {
+    union {
+        keyhandler_fn_t fn;
+        irq_keyhandler_fn_t irq_fn;
+    };
+
+    const char *desc;    /* Description for help message.                 */
+    bool_t irq_callback, /* Call in irq context? if not, tasklet context. */
+        diagnostic;      /* Include in 'dump all' handler.                */
+} key_table[128] __read_mostly =
+{
+#define KEYHANDLER(_k, _fn, _desc, _diag)                   \
+    [(_k)] = { .fn = (_fn), .desc = (_desc),                \
+               .irq_callback = 0, .diagnostic = (_diag) }
+#define IRQ_KEYHANDLER(_k, _fn, _desc, _diag)               \
+    [(_k)] = { .irq_fn = (_fn), .desc = (_desc),            \
+               .irq_callback = 1, .diagnostic = (_diag) }
+
+    IRQ_KEYHANDLER('A', do_toggle_alt_key, "toggle alternative key handling", 0),
+    IRQ_KEYHANDLER('d', dump_registers, "dump registers", 1),
+        KEYHANDLER('h', show_handlers, "show this message", 0),
+        KEYHANDLER('q', dump_domains, "dump domain (and guest debug) info", 1),
+        KEYHANDLER('r', dump_runq, "dump run queues", 1),
+    IRQ_KEYHANDLER('R', reboot_machine, "reboot machine", 0),
+        KEYHANDLER('t', read_clocks, "display multi-cpu clock info", 1),
+        KEYHANDLER('0', dump_hwdom_registers, "dump Dom0 registers", 1),
+    IRQ_KEYHANDLER('%', do_debug_key, "trap to xendbg", 0),
+    IRQ_KEYHANDLER('*', run_all_keyhandlers, "print all diagnostics", 0),
+
+#ifdef PERF_COUNTERS
+    KEYHANDLER('p', perfc_printall, "print performance counters", 1),
+    KEYHANDLER('P', perfc_reset, "reset performance counters", 0),
+#endif
+
+#ifdef LOCK_PROFILE
+    KEYHANDLER('l', spinlock_profile_printall, "print lock profile info", 1),
+    KEYHANDLER('L', spinlock_profile_reset, "reset lock profile info", 0),
+#endif
+
+#undef IRQ_KEYHANDLER
+#undef KEYHANDLER
+};
+
 static void keypress_action(unsigned long unused)
 {
     handle_keypress(keypress_key, NULL);
@@ -38,13 +90,13 @@ void handle_keypress(unsigned char key, struct cpu_user_regs *regs)
 {
     struct keyhandler *h;
 
-    if ( (h = key_table[key]) == NULL )
+    if ( key >= ARRAY_SIZE(key_table) || !(h = &key_table[key])->fn )
         return;
 
     if ( !in_irq() || h->irq_callback )
     {
         console_start_log_everything();
-        h->irq_callback ? (*h->u.irq_fn)(key, regs) : (*h->u.fn)(key);
+        h->irq_callback ? h->irq_fn(key, regs) : h->fn(key);
         console_end_log_everything();
     }
     else
@@ -54,27 +106,32 @@ void handle_keypress(unsigned char key, struct cpu_user_regs *regs)
     }
 }
 
-void register_keyhandler(unsigned char key, struct keyhandler *handler)
+void register_keyhandler(unsigned char key, keyhandler_fn_t fn,
+                         const char *desc, bool_t diagnostic)
 {
-    ASSERT(key_table[key] == NULL);
-    key_table[key] = handler;
+    struct keyhandler k = {
+        .fn = fn,
+        .desc = desc,
+        .irq_callback = 0,
+        .diagnostic = diagnostic,
+    };
+
+    BUG_ON(key >= ARRAY_SIZE(key_table)); /* Key in range? */
+    BUG_ON(key_table[key].fn != NULL);    /* Clobbering something else? */
+
+    key_table[key] = k;
 }
 
 static void show_handlers(unsigned char key)
 {
     int i;
     printk("'%c' pressed -> showing installed handlers\n", key);
-    for ( i = 0; i < ARRAY_SIZE(key_table); i++ ) 
-        if ( key_table[i] != NULL ) 
-            printk(" key '%c' (ascii '%02x') => %s\n", 
-                   isprint(i) ? i : ' ', i, key_table[i]->desc);
+    for ( i = 0; i < ARRAY_SIZE(key_table); i++ )
+        if ( key_table[i].fn )
+            printk(" key '%c' (ascii '%02x') => %s\n",
+                   isprint(i) ? i : ' ', i, key_table[i].desc);
 }
 
-static struct keyhandler show_handlers_keyhandler = {
-    .u.fn = show_handlers,
-    .desc = "show this message"
-};
-
 static cpumask_t dump_execstate_mask;
 
 void dump_execstate(struct cpu_user_regs *regs)
@@ -141,13 +198,6 @@ static void dump_registers(unsigned char key, struct cpu_user_regs *regs)
     watchdog_enable();
 }
 
-static struct keyhandler dump_registers_keyhandler = {
-    .irq_callback = 1,
-    .diagnostic = 1,
-    .u.irq_fn = dump_registers,
-    .desc = "dump registers"
-};
-
 static DECLARE_TASKLET(dump_hwdom_tasklet, NULL, 0);
 
 static void dump_hwdom_action(unsigned long arg)
@@ -191,24 +241,12 @@ static void dump_hwdom_registers(unsigned char key)
     }
 }
 
-static struct keyhandler dump_hwdom_registers_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_hwdom_registers,
-    .desc = "dump Dom0 registers"
-};
-
 static void reboot_machine(unsigned char key, struct cpu_user_regs *regs)
 {
     printk("'%c' pressed -> rebooting machine\n", key);
     machine_restart(0);
 }
 
-static struct keyhandler reboot_machine_keyhandler = {
-    .irq_callback = 1,
-    .u.irq_fn = reboot_machine,
-    .desc = "reboot machine"
-};
-
 static void cpuset_print(char *set, int size, const cpumask_t *mask)
 {
     *set++ = '{';
@@ -333,12 +371,6 @@ static void dump_domains(unsigned char key)
 #undef tmpstr
 }
 
-static struct keyhandler dump_domains_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_domains,
-    .desc = "dump domain (and guest debug) info"
-};
-
 static cpumask_t read_clocks_cpumask;
 static DEFINE_PER_CPU(s_time_t, read_clocks_time);
 static DEFINE_PER_CPU(u64, read_cycles_time);
@@ -420,42 +452,6 @@ static void read_clocks(unsigned char key)
            maxdif_cycles, sumdif_cycles/count, count, dif_cycles);
 }
 
-static struct keyhandler read_clocks_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = read_clocks,
-    .desc = "display multi-cpu clock info"
-};
-
-static struct keyhandler dump_runq_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_runq,
-    .desc = "dump run queues"
-};
-
-#ifdef PERF_COUNTERS
-static struct keyhandler perfc_printall_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = perfc_printall,
-    .desc = "print performance counters"
-};
-static struct keyhandler perfc_reset_keyhandler = {
-    .u.fn = perfc_reset,
-    .desc = "reset performance counters"
-};
-#endif
-
-#ifdef LOCK_PROFILE
-static struct keyhandler spinlock_printall_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = spinlock_profile_printall,
-    .desc = "print lock profile info"
-};
-static struct keyhandler spinlock_reset_keyhandler = {
-    .u.fn = spinlock_profile_reset,
-    .desc = "reset lock profile info"
-};
-#endif
-
 static void run_all_nonirq_keyhandlers(unsigned long unused)
 {
     /* Fire all the non-IRQ-context diagnostic keyhandlers */
@@ -467,11 +463,11 @@ static void run_all_nonirq_keyhandlers(unsigned long unused)
     for ( k = 0; k < ARRAY_SIZE(key_table); k++ )
     {
         process_pending_softirqs();
-        h = key_table[k];
-        if ( (h == NULL) || !h->diagnostic || h->irq_callback )
+        h = &key_table[k];
+        if ( !h->fn || !h->diagnostic || h->irq_callback )
             continue;
         printk("[%c: %s]\n", k, h->desc);
-        (*h->u.fn)(k);
+        h->fn(k);
     }
 
     console_end_log_everything();
@@ -492,11 +488,11 @@ static void run_all_keyhandlers(unsigned char key, struct cpu_user_regs *regs)
     /* Fire all the IRQ-context diangostic keyhandlers now */
     for ( k = 0; k < ARRAY_SIZE(key_table); k++ )
     {
-        h = key_table[k];
-        if ( (h == NULL) || !h->diagnostic || !h->irq_callback )
+        h = &key_table[k];
+        if ( !h->fn || !h->diagnostic || !h->irq_callback )
             continue;
         printk("[%c: %s]\n", k, h->desc);
-        (*h->u.irq_fn)(k, regs);
+        h->irq_fn(k, regs);
     }
 
     watchdog_enable();
@@ -505,12 +501,6 @@ static void run_all_keyhandlers(unsigned char key, struct cpu_user_regs *regs)
     tasklet_schedule(&run_all_keyhandlers_tasklet);
 }
 
-static struct keyhandler run_all_keyhandlers_keyhandler = {
-    .irq_callback = 1,
-    .u.irq_fn = run_all_keyhandlers,
-    .desc = "print all diagnostics"
-};
-
 static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
 {
     printk("'%c' pressed -> trapping into debugger\n", key);
@@ -520,12 +510,6 @@ static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
                              bit. */
 }
 
-static struct keyhandler do_debug_key_keyhandler = {
-    .irq_callback = 1,
-    .u.irq_fn = do_debug_key,
-    .desc = "trap to xendbg"
-};
-
 static void do_toggle_alt_key(unsigned char key, struct cpu_user_regs *regs)
 {
     alt_key_handling = !alt_key_handling;
@@ -533,12 +517,6 @@ static void do_toggle_alt_key(unsigned char key, struct cpu_user_regs *regs)
            alt_key_handling ? "alternative" : "normal");
 }
 
-static struct keyhandler toggle_alt_keyhandler = {
-    .irq_callback = 1,
-    .u.irq_fn = do_toggle_alt_key,
-    .desc = "toggle alternative key handling"
-};
-
 void __init initialize_keytable(void)
 {
     if ( num_present_cpus() > 16 )
@@ -547,27 +525,6 @@ void __init initialize_keytable(void)
         printk(XENLOG_INFO "Defaulting to alternative key handling; "
                "send 'A' to switch to normal mode.\n");
     }
-    register_keyhandler('A', &toggle_alt_keyhandler);
-    register_keyhandler('d', &dump_registers_keyhandler);
-    register_keyhandler('h', &show_handlers_keyhandler);
-    register_keyhandler('q', &dump_domains_keyhandler);
-    register_keyhandler('r', &dump_runq_keyhandler);
-    register_keyhandler('R', &reboot_machine_keyhandler);
-    register_keyhandler('t', &read_clocks_keyhandler);
-    register_keyhandler('0', &dump_hwdom_registers_keyhandler);
-    register_keyhandler('%', &do_debug_key_keyhandler);
-    register_keyhandler('*', &run_all_keyhandlers_keyhandler);
-
-#ifdef PERF_COUNTERS
-    register_keyhandler('p', &perfc_printall_keyhandler);
-    register_keyhandler('P', &perfc_reset_keyhandler);
-#endif
-
-#ifdef LOCK_PROFILE
-    register_keyhandler('l', &spinlock_printall_keyhandler);
-    register_keyhandler('L', &spinlock_reset_keyhandler);
-#endif
-
 }
 
 /*
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 74fc1de..2b8810c 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1849,15 +1849,9 @@ static void pagealloc_info(unsigned char key)
     printk("    Dom heap: %lukB free\n", total << (PAGE_SHIFT-10));
 }
 
-static struct keyhandler pagealloc_info_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = pagealloc_info,
-    .desc = "memory info"
-};
-
 static __init int pagealloc_keyhandler_init(void)
 {
-    register_keyhandler('m', &pagealloc_info_keyhandler);
+    register_keyhandler('m', pagealloc_info, "memory info", 1);
     return 0;
 }
 __initcall(pagealloc_keyhandler_init);
@@ -1901,15 +1895,9 @@ static void dump_heap(unsigned char key)
     }
 }
 
-static struct keyhandler dump_heap_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_heap,
-    .desc = "dump heap info"
-};
-
 static __init int register_heap_trigger(void)
 {
-    register_keyhandler('H', &dump_heap_keyhandler);
+    register_keyhandler('H', dump_heap, "dump heap info", 1);
     return 0;
 }
 __initcall(register_heap_trigger);
diff --git a/xen/common/timer.c b/xen/common/timer.c
index f36aebc..29a60a9 100644
--- a/xen/common/timer.c
+++ b/xen/common/timer.c
@@ -540,12 +540,6 @@ static void dump_timerq(unsigned char key)
     }
 }
 
-static struct keyhandler dump_timerq_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_timerq,
-    .desc = "dump timer queues"
-};
-
 static void migrate_timers_from_cpu(unsigned int old_cpu)
 {
     unsigned int new_cpu = cpumask_any(&cpu_online_map);
@@ -639,7 +633,7 @@ void __init timer_init(void)
     cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);
     register_cpu_notifier(&cpu_nfb);
 
-    register_keyhandler('a', &dump_timerq_keyhandler);
+    register_keyhandler('a', dump_timerq, "dump timer queues", 1);
 }
 
 /*
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index fce4cc8..4362400 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -322,11 +322,6 @@ static void dump_console_ring_key(unsigned char key)
     free_xenheap_pages(buf, order);
 }
 
-static struct keyhandler dump_console_ring_keyhandler = {
-    .u.fn = dump_console_ring_key,
-    .desc = "synchronously dump console ring buffer (dmesg)"
-};
-
 /* CTRL-<switch_char> switches input direction between Xen and DOM0. */
 #define switch_code (opt_conswitch[0]-'a'+1)
 static int __read_mostly xen_rx = 1; /* FALSE => input passed to domain 0. */
@@ -833,7 +828,8 @@ void __init console_endboot(void)
     if ( opt_conswitch[1] == 'x' )
         xen_rx = !xen_rx;
 
-    register_keyhandler('w', &dump_console_ring_keyhandler);
+    register_keyhandler('w', dump_console_ring_key,
+                        "synchronously dump console ring buffer (dmesg)", 0);
 
     /* Serial input is directed to DOM0 by default. */
     switch_serial_input();
@@ -1069,11 +1065,6 @@ static void debugtrace_key(unsigned char key)
     debugtrace_toggle();
 }
 
-static struct keyhandler debugtrace_keyhandler = {
-    .u.fn = debugtrace_key,
-    .desc = "toggle debugtrace to console/buffer"
-};
-
 static int __init debugtrace_init(void)
 {
     int order;
@@ -1095,7 +1086,8 @@ static int __init debugtrace_init(void)
 
     debugtrace_bytes = bytes;
 
-    register_keyhandler('T', &debugtrace_keyhandler);
+    register_keyhandler('T', debugtrace_key,
+                        "toggle debugtrace to console/buffer", 0);
 
     return 0;
 }
diff --git a/xen/drivers/passthrough/amd/iommu_intr.c b/xen/drivers/passthrough/amd/iommu_intr.c
index 62e29e9..74c2809 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -36,12 +36,6 @@
 
 static void dump_intremap_tables(unsigned char key);
 
-static struct keyhandler dump_intremap = {
-    .diagnostic = 0,
-    .u.fn = dump_intremap_tables,
-    .desc = "dump IOMMU intremap tables"
-};
-
 static spinlock_t* get_intremap_lock(int seg, int req_id)
 {
     return (amd_iommu_perdev_intremap ?
@@ -269,7 +263,8 @@ int __init amd_iommu_setup_ioapic_remapping(void)
         }
     }
 
-    register_keyhandler('V', &dump_intremap);
+    register_keyhandler('V', &dump_intremap_tables,
+                        "dump IOMMU intremap tables", 0);
 
     return 0;
 }
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index d5137733..d7e4563 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -61,12 +61,6 @@
 PAGE_LIST_HEAD(iommu_pt_cleanup_list);
 static struct tasklet iommu_pt_cleanup_tasklet;
 
-static struct keyhandler iommu_p2m_table = {
-    .diagnostic = 0,
-    .u.fn = iommu_dump_p2m_table,
-    .desc = "dump iommu p2m table"
-};
-
 static void __init parse_iommu_param(char *s)
 {
     char *ss;
@@ -155,7 +149,7 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
     if ( !iommu_enabled )
         return;
 
-    register_keyhandler('o', &iommu_p2m_table);
+    register_keyhandler('o', &iommu_dump_p2m_table, "dump iommu p2m table", 0);
     d->need_iommu = !!iommu_dom0_strict;
     if ( need_iommu(d) && !iommu_use_hap_pt(d) )
     {
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 27b3ca7..62b311b 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1211,15 +1211,9 @@ static void dump_pci_devices(unsigned char ch)
     spin_unlock(&pcidevs_lock);
 }
 
-struct keyhandler dump_pci_devices_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_pci_devices,
-    .desc = "dump PCI devices"
-};
-
 static int __init setup_dump_pcidevs(void)
 {
-    register_keyhandler('Q', &dump_pci_devices_keyhandler);
+    register_keyhandler('Q', dump_pci_devices, "dump PCI devices", 1);
     return 0;
 }
 __initcall(setup_dump_pcidevs);
diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h
index 8acf889..e1e33ac 100644
--- a/xen/drivers/passthrough/vtd/extern.h
+++ b/xen/drivers/passthrough/vtd/extern.h
@@ -29,7 +29,7 @@
 
 void print_iommu_regs(struct acpi_drhd_unit *drhd);
 void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64 gmfn);
-extern struct keyhandler dump_iommu_info_keyhandler;
+void vtd_dump_iommu_info(unsigned char key);
 
 int enable_qinval(struct iommu *iommu);
 void disable_qinval(struct iommu *iommu);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 836aed5..abd0772 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2210,7 +2210,7 @@ int __init intel_vtd_setup(void)
     if ( ret )
         goto error;
 
-    register_keyhandler('V', &dump_iommu_info_keyhandler);
+    register_keyhandler('V', vtd_dump_iommu_info, "dump iommu info", 1);
 
     return 0;
 
diff --git a/xen/drivers/passthrough/vtd/utils.c b/xen/drivers/passthrough/vtd/utils.c
index 44c4ef5..36ed3d6 100644
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -177,7 +177,7 @@ void print_vtd_entries(struct iommu *iommu, int bus, int devfn, u64 gmfn)
     } while ( --level );
 }
 
-static void dump_iommu_info(unsigned char key)
+void vtd_dump_iommu_info(unsigned char key)
 {
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
@@ -291,12 +291,6 @@ static void dump_iommu_info(unsigned char key)
     }
 }
 
-struct keyhandler dump_iommu_info_keyhandler = {
-    .diagnostic = 1,
-    .u.fn = dump_iommu_info,
-    .desc = "dump iommu info"
-};
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/keyhandler.h b/xen/include/xen/keyhandler.h
index d3ab0a9..f7f406d 100644
--- a/xen/include/xen/keyhandler.h
+++ b/xen/include/xen/keyhandler.h
@@ -1,53 +1,32 @@
 /******************************************************************************
  * keyhandler.h
- * 
+ *
  * We keep an array of 'handlers' for each key code between 0 and 255;
- * this is intended to allow very simple debugging routines (toggle 
+ * this is intended to allow very simple debugging routines (toggle
  * debug flag, dump registers, reboot, etc) to be hooked in in a slightly
- * nicer fashion than just editing the serial/keyboard drivers. 
+ * nicer fashion than just editing the serial/keyboard drivers.
  */
 
 #ifndef __XEN_KEYHANDLER_H__
 #define __XEN_KEYHANDLER_H__
 
-typedef void keyhandler_fn_t(
-    unsigned char key);
-typedef void irq_keyhandler_fn_t(
-    unsigned char key, struct cpu_user_regs *regs);
-
-struct keyhandler {
-    /*
-     * If TRUE then u.irq_fn is called in hardirq context with interrupts
-     * disabled. The @regs callback parameter points at the interrupted
-     * register context. 
-     * If FALSE then u.fn is called in softirq context with no locks held and
-     * interrupts enabled.
-     */
-    bool_t irq_callback;
-
-    /*
-     * If TRUE then the keyhandler will be included in the "dump everything"
-     * keyhandler, so must not have any side-effects.
-     */
-    bool_t diagnostic;
-
-    union {
-        keyhandler_fn_t *fn;
-        irq_keyhandler_fn_t *irq_fn;
-    } u;
-
-    /* The string is not copied by register_keyhandler(), so must persist. */
-    char *desc;
-};
+#include <xen/types.h>
+
+typedef void (*keyhandler_fn_t)(unsigned char key);
+typedef void (*irq_keyhandler_fn_t)(unsigned char key,
+                                    struct cpu_user_regs *regs);
 
 /* Initialize keytable with default handlers */
 extern void initialize_keytable(void);
 
 /*
- * Register a callback handler for key @key. The keyhandler structure is not
- * copied, so must persist.
+ * Regiser a callback handler for key @key.  If @diagnostic is set, the
+ * handler will be included in the "dump everything" keyhandler.
  */
-extern void register_keyhandler(unsigned char key, struct keyhandler *handler);
+extern void register_keyhandler(unsigned char key,
+                                keyhandler_fn_t fn,
+                                const char *desc,
+                                bool_t diagnostic);
 
 /* Inject a keypress into the key-handling subsystem. */
 extern void handle_keypress(unsigned char key, struct cpu_user_regs *regs);
-- 
1.7.10.4