* [PATCH] xen/keyhandler: Rework keyhandler infrastructure
@ 2015-09-14 13:49 Andrew Cooper
2015-09-22 10:35 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2015-09-14 13:49 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] xen/keyhandler: Rework keyhandler infrastructure
2015-09-14 13:49 [PATCH] xen/keyhandler: Rework keyhandler infrastructure Andrew Cooper
@ 2015-09-22 10:35 ` Jan Beulich
2015-09-22 11:10 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2015-09-22 10:35 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Keir Fraser, Xen-devel
>>> On 14.09.15 at 15:49, <andrew.cooper3@citrix.com> wrote:
> 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).
I don't see you losing 6 bytes of padding, I only see you moving them
to the end of the structure.
> 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.
Besides the ugliness on the use site(s), using the low bits of the
function pointer won't work reliably on x86 due to there no being
and alignment guarantees. If anything you could use some of the
high bits, but I agree the benefit would not outweigh the hassle.
> --- 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);
> }
Mind switching this from "Intel" to VMX as you go, considering that
it's not just Intel using that structure?
> +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) }
May I ask to avoid redundant (even if of different kind) brackets?
I.e. [(_k)] can be just [_k] without causing any issues. And I'd also
like to see us move away from underscore prefixed macro parameter
names - underscore prefixed names have a different purpose in the
C standard.
> @@ -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? */
Please keep this an ASSERT() (the first of the two being a BUG_ON()
is fine).
> +
> + key_table[key] = k;
I don't really see the value of the intermediate variable k here.
> @@ -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 )
Since you intend to use h->irq_fn below, please probe that one
instead of h->fn here.
> @@ -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);
Considering the other one in this file is "memory info", just
"heap info" perhaps?
> --- 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);
This type can now also be private to keyhandler.c.
I think I would also like it better if you retained the original
model of these being function types, not pointer-to-function
ones, so that consumers caring could use them to prototype
functions (and you could actually do so yourself for all the
static function declarations at the top of keyhandler.c and in
VT-d code).
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xen/keyhandler: Rework keyhandler infrastructure
2015-09-22 10:35 ` Jan Beulich
@ 2015-09-22 11:10 ` Andrew Cooper
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2015-09-22 11:10 UTC (permalink / raw)
To: Jan Beulich; +Cc: Keir Fraser, Xen-devel
On 22/09/15 11:35, Jan Beulich wrote:
>
>> --- 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);
>> }
> Mind switching this from "Intel" to VMX as you go, considering that
> it's not just Intel using that structure?
I can certainly see about correcting some of the text. I suppose this
is a good time to make it all consistent.
>
>> +
>> + key_table[key] = k;
> I don't really see the value of the intermediate variable k here.
Nor me - I thing it might have been a way of how the patch developed.
>
>> @@ -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);
> Considering the other one in this file is "memory info", just
> "heap info" perhaps?
I will audit all the text in v2.
~Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-09-22 11:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14 13:49 [PATCH] xen/keyhandler: Rework keyhandler infrastructure Andrew Cooper
2015-09-22 10:35 ` Jan Beulich
2015-09-22 11:10 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.