[PATCH v8 05/25] arm/x86: Use struct virtual_region to do bug, symbol, and (x86) exception tables lookup.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

During execution of the hypervisor we have two regions of
executable code - stext -> _etext, and _sinittext -> _einitext.

The later is not needed after bootup.

We also have various built-in macros and functions to search
in between those two swaths depending on the state of the system.

That is either for bug_frames, exceptions (x86) or symbol
names for the instruction.

With xSplice in the picture - we need a mechanism for new payloads
to searched as well for all of this.

Originally we had extra 'if (xsplice)...' but that gets
a bit tiring and does not hook up nicely.

This 'struct virtual_region' and virtual_region_list provide a
mechanism to search for the bug_frames, exception table,
and symbol names entries without having various calls in
other sub-components in the system.

Code which wishes to participate in bug_frames and exception table
entries search has to only use two public APIs:
 - register_virtual_region
 - unregister_virtual_region

to let the core code know.

If the ->lookup_symbol is not then the default internal symbol lookup
mechanism is used.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com> [ARM]

---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v4: New patch.
v5:
 - Rename to virtual_region.
 - Ditch the 'skip' function.
 - Remove the _stext.
 - Use RCU lists.
 - Add a search function.
 - Remove extern, add rcu_read_lock. remove __ from name.
v6: s/search_for_text/find_text_region/.
 - Drop the uaccess.h need. Make setup_virtual_regions accept two parameters.
   Remove #ifdef.
 - Constify struct exception_table_entry.
 - Remove some newlines.
 - Change header file guard #define to proper name.
---
---
 xen/arch/arm/setup.c             |   4 ++
 xen/arch/arm/traps.c             |  39 +++++++----
 xen/arch/x86/extable.c           |  12 +++-
 xen/arch/x86/setup.c             |   6 ++
 xen/arch/x86/traps.c             |  40 ++++++-----
 xen/common/Makefile              |   1 +
 xen/common/symbols.c             |  11 ++-
 xen/common/virtual_region.c      | 148 +++++++++++++++++++++++++++++++++++++++
 xen/include/xen/symbols.h        |   9 +++
 xen/include/xen/virtual_region.h |  47 +++++++++++++
 10 files changed, 280 insertions(+), 37 deletions(-)
 create mode 100644 xen/common/virtual_region.c
 create mode 100644 xen/include/xen/virtual_region.h

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 6d205a9..09ff1ea 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -34,6 +34,7 @@
 #include <xen/keyhandler.h>
 #include <xen/cpu.h>
 #include <xen/pfn.h>
+#include <xen/virtual_region.h>
 #include <xen/vmap.h>
 #include <xen/libfdt/libfdt.h>
 #include <xen/acpi.h>
@@ -860,6 +861,9 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     system_state = SYS_STATE_active;
 
+    /* Must be done past setting system_state. */
+    unregister_init_virtual_region();
+
     domain_unpause_by_systemcontroller(dom0);
 
     /* Switch on to the dynamically allocated stack for the idle vcpu
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 1516abd..4c423de 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -31,6 +31,7 @@
 #include <xen/softirq.h>
 #include <xen/domain_page.h>
 #include <xen/perfc.h>
+#include <xen/virtual_region.h>
 #include <public/sched.h>
 #include <public/xen.h>
 #include <asm/debugger.h>
@@ -101,6 +102,8 @@ integer_param("debug_stack_lines", debug_stack_lines);
 
 void init_traps(void)
 {
+    setup_virtual_regions(NULL, NULL);
+
     /* Setup Hyp vector base */
     WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
 
@@ -1116,27 +1119,33 @@ void do_unexpected_trap(const char *msg, struct cpu_user_regs *regs)
 
 int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc)
 {
-    const struct bug_frame *bug;
+    const struct bug_frame *bug = NULL;
     const char *prefix = "", *filename, *predicate;
     unsigned long fixup;
-    int id, lineno;
-    static const struct bug_frame *const stop_frames[] = {
-        __stop_bug_frames_0,
-        __stop_bug_frames_1,
-        __stop_bug_frames_2,
-        NULL
-    };
+    int id = -1, lineno;
+    const struct virtual_region *region;
 
-    for ( bug = __start_bug_frames, id = 0; stop_frames[id]; ++bug )
+    region = find_text_region(pc);
+    if ( region )
     {
-        while ( unlikely(bug == stop_frames[id]) )
-            ++id;
+        for ( id = 0; id < BUGFRAME_NR; id++ )
+        {
+            const struct bug_frame *b;
+            unsigned int i;
 
-        if ( ((vaddr_t)bug_loc(bug)) == pc )
-            break;
+            for ( i = 0, b = region->frame[id].bugs;
+                  i < region->frame[id].n_bugs; b++, i++ )
+            {
+                if ( ((vaddr_t)bug_loc(b)) == pc )
+                {
+                    bug = b;
+                    goto found;
+                }
+            }
+        }
     }
-
-    if ( !stop_frames[id] )
+ found:
+    if ( !bug )
         return -ENOENT;
 
     /* WARN, BUG or ASSERT: decode the filename pointer and line number. */
diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c
index 89b5bcb..2a06cca 100644
--- a/xen/arch/x86/extable.c
+++ b/xen/arch/x86/extable.c
@@ -1,10 +1,12 @@
 
-#include <xen/config.h>
 #include <xen/init.h>
+#include <xen/list.h>
 #include <xen/perfc.h>
+#include <xen/rcupdate.h>
 #include <xen/sort.h>
 #include <xen/spinlock.h>
 #include <asm/uaccess.h>
+#include <xen/virtual_region.h>
 
 #define EX_FIELD(ptr, field) ((unsigned long)&(ptr)->field + (ptr)->field)
 
@@ -80,8 +82,12 @@ search_one_table(const struct exception_table_entry *first,
 unsigned long
 search_exception_table(unsigned long addr)
 {
-    return search_one_table(
-        __start___ex_table, __stop___ex_table-1, addr);
+    const struct virtual_region *region = find_text_region(addr);
+
+    if ( region && region->ex )
+        return search_one_table(region->ex, region->ex_end - 1, addr);
+
+    return 0;
 }
 
 unsigned long
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 3696c31..22dc148 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -26,6 +26,7 @@
 #include <xen/pfn.h>
 #include <xen/nodemask.h>
 #include <xen/tmem_xen.h>
+#include <xen/virtual_region.h>
 #include <xen/watchdog.h>
 #include <public/version.h>
 #include <compat/platform.h>
@@ -515,6 +516,9 @@ static void noinline init_done(void)
 
     system_state = SYS_STATE_active;
 
+    /* MUST be done prior to removing .init data. */
+    unregister_init_virtual_region();
+
     domain_unpause_by_systemcontroller(hardware_domain);
 
     /* Zero the .init code and data. */
@@ -617,6 +621,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     smp_prepare_boot_cpu();
     sort_exception_tables();
 
+    setup_virtual_regions(__start___ex_table, __stop___ex_table);
+
     /* Full exception support from here on in. */
 
     loader = (mbi->flags & MBI_LOADERNAME)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index c6c9227..f73f7f3 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -48,6 +48,7 @@
 #include <xen/kexec.h>
 #include <xen/trace.h>
 #include <xen/paging.h>
+#include <xen/virtual_region.h>
 #include <xen/watchdog.h>
 #include <asm/system.h>
 #include <asm/io.h>
@@ -1229,18 +1230,12 @@ static int emulate_forced_invalid_op(struct cpu_user_regs *regs)
 
 void do_invalid_op(struct cpu_user_regs *regs)
 {
-    const struct bug_frame *bug;
+    const struct bug_frame *bug = NULL;
     u8 bug_insn[2];
     const char *prefix = "", *filename, *predicate, *eip = (char *)regs->eip;
     unsigned long fixup;
-    int id, lineno;
-    static const struct bug_frame *const stop_frames[] = {
-        __stop_bug_frames_0,
-        __stop_bug_frames_1,
-        __stop_bug_frames_2,
-        __stop_bug_frames_3,
-        NULL
-    };
+    int id = -1, lineno;
+    const struct virtual_region *region;
 
     DEBUGGER_trap_entry(TRAP_invalid_op, regs);
 
@@ -1257,16 +1252,29 @@ void do_invalid_op(struct cpu_user_regs *regs)
          memcmp(bug_insn, "\xf\xb", sizeof(bug_insn)) )
         goto die;
 
-    for ( bug = __start_bug_frames, id = 0; stop_frames[id]; ++bug )
+    region = find_text_region(regs->eip);
+    if ( region )
     {
-        while ( unlikely(bug == stop_frames[id]) )
-            ++id;
-        if ( bug_loc(bug) == eip )
-            break;
+        for ( id = 0; id < BUGFRAME_NR; id++ )
+        {
+            const struct bug_frame *b;
+            unsigned int i;
+
+            for ( i = 0, b = region->frame[id].bugs;
+                  i < region->frame[id].n_bugs; b++, i++ )
+            {
+                if ( bug_loc(b) == eip )
+                {
+                    bug = b;
+                    goto found;
+                }
+            }
+        }
     }
-    if ( !stop_frames[id] )
-        goto die;
 
+ found:
+    if ( !bug )
+        goto die;
     eip += sizeof(bug_insn);
     if ( id == BUGFRAME_run_fn )
     {
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 910ac69..1e4bc70 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -51,6 +51,7 @@ obj-y += time.o
 obj-y += timer.o
 obj-y += trace.o
 obj-y += version.o
+obj-y += virtual_region.o
 obj-y += vm_event.o
 obj-y += vmap.o
 obj-y += vsprintf.o
diff --git a/xen/common/symbols.c b/xen/common/symbols.c
index a59c59d..b18ddcd1 100644
--- a/xen/common/symbols.c
+++ b/xen/common/symbols.c
@@ -17,6 +17,7 @@
 #include <xen/lib.h>
 #include <xen/string.h>
 #include <xen/spinlock.h>
+#include <xen/virtual_region.h>
 #include <public/platform.h>
 #include <xen/guest_access.h>
 
@@ -97,8 +98,7 @@ static unsigned int get_symbol_offset(unsigned long pos)
 
 bool_t is_active_kernel_text(unsigned long addr)
 {
-    return (is_kernel_text(addr) ||
-            (system_state < SYS_STATE_active && is_kernel_inittext(addr)));
+    return !!find_text_region(addr);
 }
 
 const char *symbols_lookup(unsigned long addr,
@@ -108,13 +108,18 @@ const char *symbols_lookup(unsigned long addr,
 {
     unsigned long i, low, high, mid;
     unsigned long symbol_end = 0;
+    const struct virtual_region *region;
 
     namebuf[KSYM_NAME_LEN] = 0;
     namebuf[0] = 0;
 
-    if (!is_active_kernel_text(addr))
+    region = find_text_region(addr);
+    if (!region)
         return NULL;
 
+    if (region->symbols_lookup)
+        return region->symbols_lookup(addr, symbolsize, offset, namebuf);
+
         /* do a binary search on the sorted symbols_addresses array */
     low = 0;
     high = symbols_num_syms;
diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c
new file mode 100644
index 0000000..82487e6
--- /dev/null
+++ b/xen/common/virtual_region.c
@@ -0,0 +1,148 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include <xen/init.h>
+#include <xen/kernel.h>
+#include <xen/rcupdate.h>
+#include <xen/spinlock.h>
+#include <xen/virtual_region.h>
+
+static struct virtual_region core = {
+    .list = LIST_HEAD_INIT(core.list),
+    .start = (unsigned long)_stext,
+    .end = (unsigned long)_etext,
+};
+
+/* Becomes irrelevant when __init sections are cleared. */
+static struct virtual_region core_init __initdata = {
+    .list = LIST_HEAD_INIT(core_init.list),
+    .start = (unsigned long)_sinittext,
+    .end = (unsigned long)_einittext,
+};
+
+/*
+ * RCU locking. Additions are done either at startup (when there is only
+ * one CPU) or when all CPUs are running without IRQs.
+ *
+ * Deletions are big tricky. We do it when xSplicing (all CPUs running
+ * without IRQs) or during bootup (when clearing the init).
+ *
+ * Hence we use list_del_rcu (which sports an memory fence) and a spinlock
+ * on deletion.
+ *
+ * All readers of virtual_region_list MUST use list list_for_each_entry_rcu.
+ *
+ */
+static LIST_HEAD(virtual_region_list);
+static DEFINE_SPINLOCK(virtual_region_lock);
+static DEFINE_RCU_READ_LOCK(rcu_virtual_region_lock);
+
+const struct virtual_region *find_text_region(unsigned long addr)
+{
+    const struct virtual_region *region;
+
+    rcu_read_lock(&rcu_virtual_region_lock);
+    list_for_each_entry_rcu( region, &virtual_region_list, list )
+    {
+        if ( addr >= region->start && addr < region->end )
+        {
+            rcu_read_unlock(&rcu_virtual_region_lock);
+            return region;
+        }
+    }
+    rcu_read_unlock(&rcu_virtual_region_lock);
+
+    return NULL;
+}
+
+void register_virtual_region(struct virtual_region *r)
+{
+    ASSERT(!local_irq_is_enabled());
+
+    list_add_tail_rcu(&r->list, &virtual_region_list);
+}
+
+static void remove_virtual_region(struct virtual_region *r)
+{
+    unsigned long flags;
+
+    spin_lock_irqsave(&virtual_region_lock, flags);
+    list_del_rcu(&r->list);
+    spin_unlock_irqrestore(&virtual_region_lock, flags);
+    /*
+     * We do not need to invoke call_rcu.
+     *
+     * This is due to the fact that on the deletion we have made sure
+     * to use spinlocks (to guard against somebody else calling
+     * unregister_virtual_region) and list_deletion spiced with
+     * memory barrier.
+     *
+     * That protects us from corrupting the list as the readers all
+     * use list_for_each_entry_rcu which is safe against concurrent
+     * deletions.
+     */
+}
+
+void unregister_virtual_region(struct virtual_region *r)
+{
+    /* Expected to be called from xSplice - which has IRQs disabled. */
+    ASSERT(!local_irq_is_enabled());
+
+    remove_virtual_region(r);
+}
+
+void __init unregister_init_virtual_region(void)
+{
+    BUG_ON(system_state != SYS_STATE_active);
+
+    remove_virtual_region(&core_init);
+}
+
+void __init setup_virtual_regions(const struct exception_table_entry *start,
+                                  const struct exception_table_entry *end)
+{
+    size_t sz;
+    unsigned int i;
+    static const struct bug_frame *const __initconstrel bug_frames[] = {
+        __start_bug_frames,
+        __stop_bug_frames_0,
+        __stop_bug_frames_1,
+        __stop_bug_frames_2,
+#ifdef CONFIG_X86
+        __stop_bug_frames_3,
+#endif
+        NULL
+    };
+
+    for ( i = 1; bug_frames[i]; i++ )
+    {
+        const struct bug_frame *s;
+
+        s = bug_frames[i - 1];
+        sz = bug_frames[i] - s;
+
+        core.frame[i - 1].n_bugs = sz;
+        core.frame[i - 1].bugs = s;
+
+        core_init.frame[i - 1].n_bugs = sz;
+        core_init.frame[i - 1].bugs = s;
+    }
+
+    core_init.ex = core.ex = start;
+    core_init.ex_end = core.ex_end = end;
+
+    register_virtual_region(&core_init);
+    register_virtual_region(&core);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h
index 1fa0537..f58e611 100644
--- a/xen/include/xen/symbols.h
+++ b/xen/include/xen/symbols.h
@@ -5,6 +5,15 @@
 
 #define KSYM_NAME_LEN 127
 
+/*
+ * Typedef for the callback functions that symbols_lookup
+ * can call if virtual_region_list has an callback for it.
+ */
+typedef const char *symbols_lookup_t(unsigned long addr,
+                                     unsigned long *symbolsize,
+                                     unsigned long *offset,
+                                     char *namebuf);
+
 /* Lookup an address. */
 const char *symbols_lookup(unsigned long addr,
                            unsigned long *symbolsize,
diff --git a/xen/include/xen/virtual_region.h b/xen/include/xen/virtual_region.h
new file mode 100644
index 0000000..31826d1
--- /dev/null
+++ b/xen/include/xen/virtual_region.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#ifndef __XEN_VIRTUAL_REGION__
+#define __XEN_VIRTUAL_REGION__
+
+#include <xen/list.h>
+#include <xen/symbols.h>
+
+struct virtual_region
+{
+    struct list_head list;
+    unsigned long start;        /* Virtual address start. */
+    unsigned long end;          /* Virtual address end. */
+
+    /* If this is NULL the default lookup mechanism is used. */
+    symbols_lookup_t *symbols_lookup;
+
+    struct {
+        const struct bug_frame *bugs; /* The pointer to array of bug frames. */
+        size_t n_bugs;          /* The number of them. */
+    } frame[BUGFRAME_NR];
+
+    const struct exception_table_entry *ex;
+    const struct exception_table_entry *ex_end;
+};
+
+const struct virtual_region *find_text_region(unsigned long addr);
+void setup_virtual_regions(const struct exception_table_entry *start,
+                           const struct exception_table_entry *end);
+void unregister_init_virtual_region(void);
+void register_virtual_region(struct virtual_region *r);
+void unregister_virtual_region(struct virtual_region *r);
+
+#endif /* __XEN_VIRTUAL_REGION__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.5.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel