diff mbox

[v4,07/34] arm/x86: Use struct virtual_region to do bug, symbol, and (x86) exception tables

Message ID 20160322201844.GA31231@char.us.oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Konrad Rzeszutek Wilk March 22, 2016, 8:18 p.m. UTC
. snip..
> > +struct virtual_region kernel_text = {
> 
> static
> 
> > +    .list = LIST_HEAD_INIT(kernel_text.list),
> > +    .start = (unsigned long)_stext,
> > +    .end = (unsigned long)_etext,
> > +#ifdef CONFIG_X86
> > +    .ex = (struct exception_table_entry *)__start___ex_table,
> > +    .ex_end = (struct exception_table_entry *)__stop___ex_table,
> > +#endif
> 
> Is this together with ...
> 
> > +/*
> > + * Becomes irrelevant when __init sections are cleared.
> > + */
> > +struct virtual_region kernel_inittext  = {
> > +    .list = LIST_HEAD_INIT(kernel_inittext.list),
> > +    .skip = ignore_if_active,
> > +    .start = (unsigned long)_sinittext,
> > +    .end = (unsigned long)_einittext,
> > +#ifdef CONFIG_X86
> > +    /* Even if they are __init their exception entry still gets stuck here. */
> > +    .ex = (struct exception_table_entry *)__start___ex_table,
> > +    .ex_end = (struct exception_table_entry *)__stop___ex_table,
> > +#endif
> 
> ... this really a good idea? I.e. are there not going to be any
> odd side effects because of that redundancy?

None. If the EIP falls within _stext and _etext then its 'ex' table
will be scanned. If _inittext and _einittext then this one. Both of
them end up scanning the same exact exception table (which is kind
silly) - but there are no side-effect.

It would be good to only have __init related exceptions on the __inittext
(and also ditch the __init exception tables once the boot is completed)
but I am not exactly sure how to automatically make the macros resolve
what sections it should go in. That is a further TODO though.
> 
> Also note that the comment preceding this object is a single line one.
> 
> > +/*
> > + * No 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 MUST make sure all but one CPU
> > + * are running cpu_relax().
> > + *
> > + */
> > +LIST_HEAD(virtual_region_list);
> 
> I wonder whether this wouldn't better be static, with the iterator
> that the various parties need getting put here as an out-of-line
> function (instead of getting open coded in a couple of places).

There are three users of this list:
 * search_exception_table - which ends up calling search_one_table
    if within start->end and if region->ex is defined.
 * do_invalid_trap (x86 and ARM) - where both scan start->end.
 * symbols_lookup - where we scan start->end.

The last two could be unified in a:

struct virtual_region search_virtual_region(unsigned long addr);

And the first one can use this and then check if region->ex is set as well.

[trying it out]

.. snip..
> > @@ -108,13 +127,17 @@ const char *symbols_lookup(unsigned long addr,
> >  {
> >      unsigned long i, low, high, mid;
> >      unsigned long symbol_end = 0;
> > +    symbols_lookup_t symbol_lookup = NULL;
> 
> Pointless initializer.

I believe we need it. That is the contents on the stack can be garbage
and the __is_active_kernel_text won't update symbol_lookup unless
it finds a match. Ah, and also the compiler is unhappy:

symbols.c: In function ‘symbols_lookup’:
symbols.c:136:8: error: ‘symbol_lookup’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     if (symbol_lookup)
        ^
cc1: all warnings being treated as errors

> 
> >      namebuf[KSYM_NAME_LEN] = 0;
> >      namebuf[0] = 0;
> >  
> > -    if (!is_active_kernel_text(addr))
> > +    if (!__is_active_kernel_text(addr, &symbol_lookup))
> >          return NULL;
> >  
> > +    if (symbol_lookup)
> > +        return (symbol_lookup)(addr, symbolsize, offset, namebuf);
> 
> Note that there are few coding style issues here (missing blanks,
> superfluous parentheses).

That file uses a different StyleGuide. The Linux one. Which reminds me
that __is_active_kernel_text needs to adhere to different StyleGuide.

> 
> > --- /dev/null
> > +++ b/xen/include/xen/bug_ex_symbols.h
> > @@ -0,0 +1,74 @@
> > +/*
> > + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
> > + *
> > + */
> > +
> > +#ifndef __BUG_EX_SYMBOL_LIST__
> > +#define __BUG_EX_SYMBOL_LIST__
> > +
> > +#include <xen/config.h>
> > +#include <xen/list.h>
> > +#include <xen/symbols.h>
> > +
> > +#ifdef CONFIG_X86
> > +#include <asm/uaccess.h>
> > +#endif
> 
> Why?

Otherwise the compilation will fail on ARM as they do not have exceptions
(and no asm/uaccess.h file)
> 
> > +#include <asm/bug.h>
> > +
> > +struct virtual_region
> > +{
> > +    struct list_head list;
> > +
> > +#define CHECKING_SYMBOL         (1<<1)
> > +#define CHECKING_BUG_FRAME      (1<<2)
> > +#define CHECKING_EXCEPTION      (1<<3)
> > +    /*
> > +     * Whether to skip this region for particular searches. The flag
> > +     * can be CHECKING_[SYMBOL|BUG_FRAMES|EXCEPTION].
> > +     *
> > +     * If the function returns 1 this region will be skipped.
> > +     */
> > +    bool_t (*skip)(unsigned int flag, unsigned long priv);
> > +
> > +    unsigned long start;        /* Virtual address start. */
> > +    unsigned long end;          /* Virtual address start. */
> > +
> > +    /*
> > +     * If ->skip returns false for CHECKING_SYMBOL we will use
> > +     * 'symbols_lookup' callback to retrieve the name of the
> > +     * addr between start and end. If this is NULL the
> > +     * default lookup mechanism is used (the skip value is
> > +     * ignored).
> > +     */
> > +    symbols_lookup_t symbols_lookup;
> > +
> > +    struct {
> > +        struct bug_frame *bugs; /* The pointer to array of bug frames. */
> > +        ssize_t n_bugs;         /* The number of them. */
> > +    } frame[BUGFRAME_NR];
> > +
> > +#ifdef CONFIG_X86
> > +    struct exception_table_entry *ex;
> > +    struct exception_table_entry *ex_end;
> > +#endif
> 
> The bug frame and exception related data are kind of odd to be
> placed in a structure with this name. Would that not better be
> accessed through ...
> 
> > +    unsigned long priv;         /* To be used by above funcionts if need to. */
> 
> ... this by the interested parties?

This had been re-worked with Andrew's suggestions and yours. Pls see:


From d542ae2b6de421c5402888519d4d11a09abe8d46 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 10 Mar 2016 16:35:50 -0500
Subject: [PATCH] arm/x86: Use struct virtual_region to do bug, symbol, and
 (x86) exception tables lookup.

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 mechansim 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>

---
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
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.
---
---
 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      | 151 +++++++++++++++++++++++++++++++++++++++
 xen/include/xen/symbols.h        |   9 +++
 xen/include/xen/virtual_region.h |  56 +++++++++++++++
 10 files changed, 292 insertions(+), 37 deletions(-)
 create mode 100644 xen/common/virtual_region.c
 create mode 100644 xen/include/xen/virtual_region.h

Comments

Jan Beulich March 23, 2016, 8:19 a.m. UTC | #1
>>> On 22.03.16 at 21:18, <konrad.wilk@oracle.com> wrote:
> It would be good to only have __init related exceptions on the __inittext
> (and also ditch the __init exception tables once the boot is completed)
> but I am not exactly sure how to automatically make the macros resolve
> what sections it should go in. That is a further TODO though.

I'm pretty confident that this can't be reasonably addressed without
the compiler's help, by extending the section attribute to also allow
for templates rather than only fixed names.

>> > @@ -108,13 +127,17 @@ const char *symbols_lookup(unsigned long addr,
>> >  {
>> >      unsigned long i, low, high, mid;
>> >      unsigned long symbol_end = 0;
>> > +    symbols_lookup_t symbol_lookup = NULL;
>> 
>> Pointless initializer.
> 
> I believe we need it. That is the contents on the stack can be garbage
> and the __is_active_kernel_text won't update symbol_lookup unless
> it finds a match. Ah, and also the compiler is unhappy:
> 
> symbols.c: In function ‘symbols_lookup’:
> symbols.c:136:8: error: ‘symbol_lookup’ may be used uninitialized in this 
> function [-Werror=maybe-uninitialized]
>      if (symbol_lookup)
>         ^
> cc1: all warnings being treated as errors

That's unfortunate, since ...

>> >      namebuf[KSYM_NAME_LEN] = 0;
>> >      namebuf[0] = 0;
>> >  
>> > -    if (!is_active_kernel_text(addr))
>> > +    if (!__is_active_kernel_text(addr, &symbol_lookup))
>> >          return NULL;

... this return ensures that ...

>> > +    if (symbol_lookup)
>> > +        return (symbol_lookup)(addr, symbolsize, offset, namebuf);

... this won't be reached with symbol_lookup uninitialized.

>> Note that there are few coding style issues here (missing blanks,
>> superfluous parentheses).
> 
> That file uses a different StyleGuide. The Linux one. Which reminds me
> that __is_active_kernel_text needs to adhere to different StyleGuide.

Linux style would not be using spaces for indentation. The file is
really a bad mixture of styles, but yes, you're right that you don't
violate that pre-existing mixture.

>> > --- /dev/null
>> > +++ b/xen/include/xen/bug_ex_symbols.h
>> > @@ -0,0 +1,74 @@
>> > +/*
>> > + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
>> > + *
>> > + */
>> > +
>> > +#ifndef __BUG_EX_SYMBOL_LIST__
>> > +#define __BUG_EX_SYMBOL_LIST__
>> > +
>> > +#include <xen/config.h>
>> > +#include <xen/list.h>
>> > +#include <xen/symbols.h>
>> > +
>> > +#ifdef CONFIG_X86
>> > +#include <asm/uaccess.h>
>> > +#endif
>> 
>> Why?
> 
> Otherwise the compilation will fail on ARM as they do not have exceptions
> (and no asm/uaccess.h file)

Well, the question was for the #include, not the #ifdef.

> --- 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 !!search_virtual_regions(addr);

search_virtual_regions() doesn't sound like it would be looking for
text addresses only.

> --- /dev/null
> +++ b/xen/common/virtual_region.c
> @@ -0,0 +1,151 @@
> +/*
> + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
> + *
> + */
> +
> +#include <xen/config.h>

No new explicit inclusion of xen/config.h please. We're actually in
the process of getting rid of such elsewhere.

> +#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 compiled = {

I think "compiled" is a bad name, as patch modules are equally
compiled - "core" or "builtin" perhaps?

> +struct virtual_region* search_virtual_regions(unsigned long addr)

Misplaced *. And probably should return a pointer to const.

> +{
> +    struct virtual_region *region;
> +
> +    list_for_each_entry_rcu( region, &virtual_region_list, list )

Where's the rcu_read_lock() use of which the comment preceding
the #define of this mandates?

> +static void __unregister_virtual_region(struct virtual_region *r)

As I think I have said before - no double underscore prefixes on
new identifiers please.

> +{
> +    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 an memory
> +     * barrier - which will flush out the cache lines in other CPUs.

I don't think barriers do any kind of cache flushing on remote CPUs
(not even on the local one).

> +void __init setup_virtual_regions(void)
> +{
> +    ssize_t sz;
> +    unsigned int i;
> +    static const struct bug_frame *const stop_frames[] = {

This (now sitting in an __init function) should then become __initconstrel.

> +        __start_bug_frames,

This element makes the array name bogus.

> +        __stop_bug_frames_0,
> +        __stop_bug_frames_1,
> +        __stop_bug_frames_2,
> +#ifdef CONFIG_X86
> +        __stop_bug_frames_3,
> +#endif
> +        NULL
> +    };
> +
> +    /* N.B. idx != i */

Stale comment?

> +    for ( i = 1; stop_frames[i]; i++ )
> +    {
> +        const struct bug_frame *s;
> +
> +        s = stop_frames[i-1];
> +        sz = stop_frames[i] - s;
> +
> +        compiled.frame[i-1].n_bugs = sz;
> +        compiled.frame[i-1].bugs = s;
> +
> +        compiled_init.frame[i-1].n_bugs = sz;
> +        compiled_init.frame[i-1].bugs = s;
> +    }

Many times "[i - 1]" please.

> +    register_virtual_region(&compiled_init);
> +    register_virtual_region(&compiled);

Is there a particular reason not to do the main region first? Probably
it's benign, but if there is a reason, I think a comment would be
warranted.

> --- 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,

Stray parentheses.

> --- /dev/null
> +++ b/xen/include/xen/virtual_region.h
> @@ -0,0 +1,56 @@
> +/*
> + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
> + *
> + */
> +
> +#ifndef __BUG_EX_SYMBOL_LIST__
> +#define __BUG_EX_SYMBOL_LIST__

No longer in line with the header name, and missing a XEN_ portion.

> +struct virtual_region
> +{
> +    struct list_head list;
> +    unsigned long start;        /* Virtual address start. */
> +    unsigned long end;          /* Virtual address start. */
> +
> +    /*
> +     * If this is NULL the default lookup mechanism is used.
> +     */

Here or elsewhere I'm sure I've made the comment before: This is
a single line comment.

> +    symbols_lookup_t *symbols_lookup;
> +
> +    struct {
> +        const struct bug_frame *bugs; /* The pointer to array of bug 
> frames. */
> +        ssize_t n_bugs;         /* The number of them. */
> +    } frame[BUGFRAME_NR];
> +
> +#ifdef CONFIG_X86
> +    struct exception_table_entry *ex;
> +    struct exception_table_entry *ex_end;
> +#endif

Would there be any harm omitting the #ifdef and leaving the
pointers be NULL on ARM?

> +extern struct virtual_region *search_virtual_regions(unsigned long addr);
> +extern void setup_virtual_regions(void);
> +extern void unregister_init_virtual_region(void);
> +extern int register_virtual_region(struct virtual_region *r);
> +extern void unregister_virtual_region(struct virtual_region *r);

While a matter of taste, I personally would prefer if "extern" was
omitted on function declarations.

Jan
Julien Grall March 23, 2016, 11:17 a.m. UTC | #2
Hi Jan,

On 23/03/16 08:19, Jan Beulich wrote:
>>>> On 22.03.16 at 21:18, <konrad.wilk@oracle.com> wrote:
>> +    symbols_lookup_t *symbols_lookup;
>> +
>> +    struct {
>> +        const struct bug_frame *bugs; /* The pointer to array of bug
>> frames. */
>> +        ssize_t n_bugs;         /* The number of them. */
>> +    } frame[BUGFRAME_NR];
>> +
>> +#ifdef CONFIG_X86
>> +    struct exception_table_entry *ex;
>> +    struct exception_table_entry *ex_end;
>> +#endif
>
> Would there be any harm omitting the #ifdef and leaving the
> pointers be NULL on ARM?

The structure exception_table_entry is only defined for x86 
(asm-x86/uaccess.h).

Regards,
Jan Beulich March 23, 2016, 11:21 a.m. UTC | #3
>>> On 23.03.16 at 12:17, <julien.grall@arm.com> wrote:
> Hi Jan,
> 
> On 23/03/16 08:19, Jan Beulich wrote:
>>>>> On 22.03.16 at 21:18, <konrad.wilk@oracle.com> wrote:
>>> +    symbols_lookup_t *symbols_lookup;
>>> +
>>> +    struct {
>>> +        const struct bug_frame *bugs; /* The pointer to array of bug
>>> frames. */
>>> +        ssize_t n_bugs;         /* The number of them. */
>>> +    } frame[BUGFRAME_NR];
>>> +
>>> +#ifdef CONFIG_X86
>>> +    struct exception_table_entry *ex;
>>> +    struct exception_table_entry *ex_end;
>>> +#endif
>>
>> Would there be any harm omitting the #ifdef and leaving the
>> pointers be NULL on ARM?
> 
> The structure exception_table_entry is only defined for x86 
> (asm-x86/uaccess.h).

But the uses above are fine without the structure ever getting
defined.

Jan
diff mbox

Patch

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 31d2115..97e40bb 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();
+
     /* Setup Hyp vector base */
     WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
 
@@ -1077,27 +1080,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;
+    struct virtual_region *region;
 
-    for ( bug = __start_bug_frames, id = 0; stop_frames[id]; ++bug )
+    region = search_virtual_regions(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..d0f1361 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);
+    struct virtual_region *region = search_virtual_regions(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 1876a28..20cd9b3 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>
@@ -514,6 +515,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. */
@@ -616,6 +620,8 @@  void __init noreturn __start_xen(unsigned long mbi_p)
     smp_prepare_boot_cpu();
     sort_exception_tables();
 
+    setup_virtual_regions();
+
     /* 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 6fbb1cf..f90d6ac 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>
@@ -1132,18 +1133,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;
+    struct virtual_region *region;
 
     DEBUGGER_trap_entry(TRAP_invalid_op, regs);
 
@@ -1160,16 +1155,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 = search_virtual_regions(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 77de27e..e43ec49 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..bba0f2e 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 !!search_virtual_regions(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;
+    struct virtual_region *region;
 
     namebuf[KSYM_NAME_LEN] = 0;
     namebuf[0] = 0;
 
-    if (!is_active_kernel_text(addr))
+    region = search_virtual_regions(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..618a312
--- /dev/null
+++ b/xen/common/virtual_region.c
@@ -0,0 +1,151 @@ 
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include <xen/config.h>
+#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 compiled = {
+    .list = LIST_HEAD_INIT(compiled.list),
+    .start = (unsigned long)_stext,
+    .end = (unsigned long)_etext,
+#ifdef CONFIG_X86
+    .ex = (struct exception_table_entry *)__start___ex_table,
+    .ex_end = (struct exception_table_entry *)__stop___ex_table,
+#endif
+};
+
+/* Becomes irrelevant when __init sections are cleared. */
+static struct virtual_region compiled_init __initdata = {
+    .list = LIST_HEAD_INIT(compiled_init.list),
+    .start = (unsigned long)_sinittext,
+    .end = (unsigned long)_einittext,
+#ifdef CONFIG_X86
+    /* Even if they are __init their exception entry still gets stuck here. */
+    .ex = (struct exception_table_entry *)__start___ex_table,
+    .ex_end = (struct exception_table_entry *)__stop___ex_table,
+#endif
+};
+
+/*
+ * 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);
+
+struct virtual_region* search_virtual_regions(unsigned long addr)
+{
+    struct virtual_region *region;
+
+    list_for_each_entry_rcu( region, &virtual_region_list, list )
+    {
+        if ( addr >= region->start && addr < region->end )
+            return region;
+    }
+
+    return NULL;
+}
+
+int register_virtual_region(struct virtual_region *r)
+{
+    ASSERT(!local_irq_is_enabled());
+
+    list_add_tail_rcu(&r->list, &virtual_region_list);
+
+    return 0;
+}
+
+static void __unregister_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 an memory
+     * barrier - which will flush out the cache lines in other CPUs.
+     *
+     * 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());
+
+    __unregister_virtual_region(r);
+}
+
+void unregister_init_virtual_region(void)
+{
+    BUG_ON(system_state != SYS_STATE_active);
+
+    __unregister_virtual_region(&compiled_init);
+}
+
+void __init setup_virtual_regions(void)
+{
+    ssize_t sz;
+    unsigned int i;
+    static const struct bug_frame *const stop_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
+    };
+
+    /* N.B. idx != i */
+    for ( i = 1; stop_frames[i]; i++ )
+    {
+        const struct bug_frame *s;
+
+        s = stop_frames[i-1];
+        sz = stop_frames[i] - s;
+
+        compiled.frame[i-1].n_bugs = sz;
+        compiled.frame[i-1].bugs = s;
+
+        compiled_init.frame[i-1].n_bugs = sz;
+        compiled_init.frame[i-1].bugs = s;
+    }
+
+    register_virtual_region(&compiled_init);
+    register_virtual_region(&compiled);
+}
+
+/*
+ * 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..fa353f8 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..bf15fe9
--- /dev/null
+++ b/xen/include/xen/virtual_region.h
@@ -0,0 +1,56 @@ 
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#ifndef __BUG_EX_SYMBOL_LIST__
+#define __BUG_EX_SYMBOL_LIST__
+
+#include <xen/config.h>
+#include <xen/list.h>
+#include <xen/symbols.h>
+
+#ifdef CONFIG_X86
+#include <asm/uaccess.h>
+#endif
+#include <asm/bug.h>
+
+struct virtual_region
+{
+    struct list_head list;
+    unsigned long start;        /* Virtual address start. */
+    unsigned long end;          /* Virtual address start. */
+
+    /*
+     * 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. */
+        ssize_t n_bugs;         /* The number of them. */
+    } frame[BUGFRAME_NR];
+
+#ifdef CONFIG_X86
+    struct exception_table_entry *ex;
+    struct exception_table_entry *ex_end;
+#endif
+};
+
+extern struct virtual_region *search_virtual_regions(unsigned long addr);
+extern void setup_virtual_regions(void);
+extern void unregister_init_virtual_region(void);
+extern int register_virtual_region(struct virtual_region *r);
+extern void unregister_virtual_region(struct virtual_region *r);
+
+#endif /* __BUG_EX_SYMBOL_LIST__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */