Message ID | 1460000983-28170-16-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 07/04/16 04:49, Konrad Rzeszutek Wilk wrote: > @@ -48,19 +49,23 @@ static void __init swap_ex(void *a, void *b, int size) > } > #endif > > -void __init sort_exception_tables(void) > +void __INIT sort_exception_table(struct exception_table_entry *start, > + struct exception_table_entry *stop) > { > - sort(__start___ex_table, __stop___ex_table - __start___ex_table, > - sizeof(struct exception_table_entry), cmp_ex, swap_ex); > - sort(__start___pre_ex_table, > - __stop___pre_ex_table - __start___pre_ex_table, > + sort(start, stop - start, > sizeof(struct exception_table_entry), cmp_ex, swap_ex); This reminds me that Xen's heapsort implementation is buggy. By shear luck, it does end up in the correct order, but in O(N^2) time. I will submit a separate bugfix patch. > } > > -static inline unsigned long > -search_one_table(const struct exception_table_entry *first, > - const struct exception_table_entry *last, > - unsigned long value) > +void __init sort_exception_tables(void) > +{ > + sort_exception_table(__start___ex_table, __stop___ex_table); > + sort_exception_table(__start___pre_ex_table, __stop___pre_ex_table); > +} > + > +unsigned long > +search_one_extable(const struct exception_table_entry *first, > + const struct exception_table_entry *last, > + unsigned long value) > { > const struct exception_table_entry *mid; > long diff; > @@ -85,7 +90,7 @@ search_exception_table(unsigned long 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 search_one_extable(region->ex, region->ex_end-1, addr); > > return 0; > } > @@ -94,7 +99,7 @@ unsigned long > search_pre_exception_table(struct cpu_user_regs *regs) > { > unsigned long addr = (unsigned long)regs->eip; > - unsigned long fixup = search_one_table( > + unsigned long fixup = search_one_extable( > __start___pre_ex_table, __stop___pre_ex_table-1, addr); > if ( fixup ) > { > diff --git a/xen/arch/x86/test/xen_hello_world_func.c b/xen/arch/x86/test/xen_hello_world_func.c > index 1ad002a..7e239ca 100644 > --- a/xen/arch/x86/test/xen_hello_world_func.c > +++ b/xen/arch/x86/test/xen_hello_world_func.c > @@ -5,9 +5,20 @@ > > #include <xen/types.h> > > +static unsigned long *non_canonical_addr = (unsigned long *)(1UL<<48); I would recommend a more visible hex constant, both for the unlikely case of patching being broken and Xen actually barfing, and because this particular non-canonical address will no longer be non-canonical when 5 level paging appears. how about 0xdead000000000000ULL ? > + > /* Our replacement function for xen_extra_version. */ > const char *xen_hello_world(void) > { > + unsigned long tmp = 0xdeadbeef; No need to initialise. > + int rc; > + /* > + * Any BUG, or WARN_ON will contain symbol and payload name. Furthermore > + * exceptions will be caught and processed properly. > + */ > + rc = __get_user(tmp, non_canonical_addr); > + BUG_ON(rc != -EFAULT); > + > return "Hello World"; > } > > diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c > index 087cb94..31ddd5d 100644 > --- a/xen/common/xsplice.c > +++ b/xen/common/xsplice.c > @@ -525,6 +525,31 @@ static int prepare_payload(struct payload *payload, > sizeof(*region->frame[i].bugs); > } > > +#ifdef CONFIG_X86 > + sec = xsplice_elf_sec_by_name(elf, ".ex_table"); > + if ( sec ) > + { > + struct exception_table_entry *s, *e; > + > + if ( !sec->sec->sh_size || > + (sec->sec->sh_size % sizeof(*region->ex)) ) > + { > + dprintk(XENLOG_DEBUG, XSPLICE "%s: Wrong size of .ex_table (exp:%lu vs %lu)!\n", > + elf->name, sizeof(*region->ex), > + sec->sec->sh_size); > + return -EINVAL; > + } > + > + s = sec->load_addr; > + e = sec->load_addr + sec->sec->sh_size; > + > + sort_exception_table(s ,e); > + > + region->ex = (const struct exception_table_entry *)s; > + region->ex_end = (const struct exception_table_entry *)e; These casts should not be needed at all. > + } > +#endif > + > return 0; > } > > diff --git a/xen/include/asm-x86/uaccess.h b/xen/include/asm-x86/uaccess.h > index 947470d..2c839a9 100644 > --- a/xen/include/asm-x86/uaccess.h > +++ b/xen/include/asm-x86/uaccess.h > @@ -277,5 +277,7 @@ extern struct exception_table_entry __stop___pre_ex_table[]; > > extern unsigned long search_exception_table(unsigned long); > extern void sort_exception_tables(void); > +extern void sort_exception_table(struct exception_table_entry *start, > + struct exception_table_entry *stop); > > #endif /* __X86_UACCESS_H__ */ > diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h > index ca78eae..6113061 100644 > --- a/xen/include/xen/xsplice.h > +++ b/xen/include/xen/xsplice.h > @@ -37,6 +37,15 @@ struct xsplice_patch_func_internal { > } u; > }; > > +/* > + * We use alternative and exception table code - which by default are __init > + * only, however we need them during runtime. These macros allows us to build > + * the image with these functions built-in. (See the #else below). > + */ > +#define __INITCONST > +#define __INITDATA > +#define __INIT > + This isn't very nice, but is probably the best we can do for 4.7 It would be nice to have things like __maybe_init(CONFIG_XSPLICE) , but then negations get hard (and perhaps this needs more thought). No major issues, so with the identified things fixed, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > /* Convenience define for printk. */ > #define XSPLICE "xsplice: " > > @@ -96,6 +105,14 @@ void arch_xsplice_mask(void); > void arch_xsplice_unmask(void); > #else > > +/* > + * If not compiling with xSplice certain functionality should stay as > + * __init. > + */ > +#define __INITCONST __initconst > +#define __INITDATA __initdata > +#define __INIT __init > + > #include <xen/errno.h> /* For -EOPNOTSUPP */ > static inline int xsplice_op(struct xen_sysctl_xsplice_op *op) > {
diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c index 4184ad8..33a917c 100644 --- a/xen/arch/x86/extable.c +++ b/xen/arch/x86/extable.c @@ -7,6 +7,7 @@ #include <xen/spinlock.h> #include <asm/uaccess.h> #include <xen/virtual_region.h> +#include <xen/xsplice.h> #define EX_FIELD(ptr, field) ((unsigned long)&(ptr)->field + (ptr)->field) @@ -20,7 +21,7 @@ static inline unsigned long ex_cont(const struct exception_table_entry *x) return EX_FIELD(x, cont); } -static int __init cmp_ex(const void *a, const void *b) +static int __INIT cmp_ex(const void *a, const void *b) { const struct exception_table_entry *l = a, *r = b; unsigned long lip = ex_addr(l); @@ -35,7 +36,7 @@ static int __init cmp_ex(const void *a, const void *b) } #ifndef swap_ex -static void __init swap_ex(void *a, void *b, int size) +static void __INIT swap_ex(void *a, void *b, int size) { struct exception_table_entry *l = a, *r = b, tmp; long delta = b - a; @@ -48,19 +49,23 @@ static void __init swap_ex(void *a, void *b, int size) } #endif -void __init sort_exception_tables(void) +void __INIT sort_exception_table(struct exception_table_entry *start, + struct exception_table_entry *stop) { - sort(__start___ex_table, __stop___ex_table - __start___ex_table, - sizeof(struct exception_table_entry), cmp_ex, swap_ex); - sort(__start___pre_ex_table, - __stop___pre_ex_table - __start___pre_ex_table, + sort(start, stop - start, sizeof(struct exception_table_entry), cmp_ex, swap_ex); } -static inline unsigned long -search_one_table(const struct exception_table_entry *first, - const struct exception_table_entry *last, - unsigned long value) +void __init sort_exception_tables(void) +{ + sort_exception_table(__start___ex_table, __stop___ex_table); + sort_exception_table(__start___pre_ex_table, __stop___pre_ex_table); +} + +unsigned long +search_one_extable(const struct exception_table_entry *first, + const struct exception_table_entry *last, + unsigned long value) { const struct exception_table_entry *mid; long diff; @@ -85,7 +90,7 @@ search_exception_table(unsigned long 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 search_one_extable(region->ex, region->ex_end-1, addr); return 0; } @@ -94,7 +99,7 @@ unsigned long search_pre_exception_table(struct cpu_user_regs *regs) { unsigned long addr = (unsigned long)regs->eip; - unsigned long fixup = search_one_table( + unsigned long fixup = search_one_extable( __start___pre_ex_table, __stop___pre_ex_table-1, addr); if ( fixup ) { diff --git a/xen/arch/x86/test/xen_hello_world_func.c b/xen/arch/x86/test/xen_hello_world_func.c index 1ad002a..7e239ca 100644 --- a/xen/arch/x86/test/xen_hello_world_func.c +++ b/xen/arch/x86/test/xen_hello_world_func.c @@ -5,9 +5,20 @@ #include <xen/types.h> +static unsigned long *non_canonical_addr = (unsigned long *)(1UL<<48); + /* Our replacement function for xen_extra_version. */ const char *xen_hello_world(void) { + unsigned long tmp = 0xdeadbeef; + int rc; + /* + * Any BUG, or WARN_ON will contain symbol and payload name. Furthermore + * exceptions will be caught and processed properly. + */ + rc = __get_user(tmp, non_canonical_addr); + BUG_ON(rc != -EFAULT); + return "Hello World"; } diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c index 087cb94..31ddd5d 100644 --- a/xen/common/xsplice.c +++ b/xen/common/xsplice.c @@ -525,6 +525,31 @@ static int prepare_payload(struct payload *payload, sizeof(*region->frame[i].bugs); } +#ifdef CONFIG_X86 + sec = xsplice_elf_sec_by_name(elf, ".ex_table"); + if ( sec ) + { + struct exception_table_entry *s, *e; + + if ( !sec->sec->sh_size || + (sec->sec->sh_size % sizeof(*region->ex)) ) + { + dprintk(XENLOG_DEBUG, XSPLICE "%s: Wrong size of .ex_table (exp:%lu vs %lu)!\n", + elf->name, sizeof(*region->ex), + sec->sec->sh_size); + return -EINVAL; + } + + s = sec->load_addr; + e = sec->load_addr + sec->sec->sh_size; + + sort_exception_table(s ,e); + + region->ex = (const struct exception_table_entry *)s; + region->ex_end = (const struct exception_table_entry *)e; + } +#endif + return 0; } diff --git a/xen/include/asm-x86/uaccess.h b/xen/include/asm-x86/uaccess.h index 947470d..2c839a9 100644 --- a/xen/include/asm-x86/uaccess.h +++ b/xen/include/asm-x86/uaccess.h @@ -277,5 +277,7 @@ extern struct exception_table_entry __stop___pre_ex_table[]; extern unsigned long search_exception_table(unsigned long); extern void sort_exception_tables(void); +extern void sort_exception_table(struct exception_table_entry *start, + struct exception_table_entry *stop); #endif /* __X86_UACCESS_H__ */ diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h index ca78eae..6113061 100644 --- a/xen/include/xen/xsplice.h +++ b/xen/include/xen/xsplice.h @@ -37,6 +37,15 @@ struct xsplice_patch_func_internal { } u; }; +/* + * We use alternative and exception table code - which by default are __init + * only, however we need them during runtime. These macros allows us to build + * the image with these functions built-in. (See the #else below). + */ +#define __INITCONST +#define __INITDATA +#define __INIT + /* Convenience define for printk. */ #define XSPLICE "xsplice: " @@ -96,6 +105,14 @@ void arch_xsplice_mask(void); void arch_xsplice_unmask(void); #else +/* + * If not compiling with xSplice certain functionality should stay as + * __init. + */ +#define __INITCONST __initconst +#define __INITDATA __initdata +#define __INIT __init + #include <xen/errno.h> /* For -EOPNOTSUPP */ static inline int xsplice_op(struct xen_sysctl_xsplice_op *op) {