From patchwork Wed Apr 27 03:31:56 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 8952151 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id CA6A79F1D3 for ; Wed, 27 Apr 2016 03:35:28 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 983B32012B for ; Wed, 27 Apr 2016 03:35:27 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4395120125 for ; Wed, 27 Apr 2016 03:35:26 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1avGEC-00082m-2G; Wed, 27 Apr 2016 03:33:28 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1avGEA-00082e-Lg for xen-devel@lists.xenproject.org; Wed, 27 Apr 2016 03:33:26 +0000 Received: from [193.109.254.147] by server-14.bemta-14.messagelabs.com id 97/9C-26345-50330275; Wed, 27 Apr 2016 03:33:25 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrLIsWRWlGSWpSXmKPExsUyZ7p8oC6rsUK 4wdtWVovvWyYzOTB6HP5whSWAMYo1My8pvyKBNeP9+4PMBSfdKh79uc3WwPjZtIuRi0NIoINJ 4smiO4wQzjdGieft69m7GDmBnA2MEq0b9CHsbkaJlXPcuhg5OEQFVCR+HlcGCfMKCEqcnPmEB cK2klh4cBsjiM0ioC2x9+giJpByNgETiTerHEHCIgLKEr2/foOVMwssZ5KYtCITxBYWSJe41b aNBWITUPzE51wQm1PAXuLS799sEPVaEjf+vWSCsOUltr+dwwxiSwgYS7S/vcg2gVFwFpKLZiF pmYWkZQEj8ypGjeLUorLUIl1DC72kosz0jJLcxMwcXUNDE73c1OLixPTUnMSkYr3k/NxNjMCg rWdgYNzBeGS75yFGSQ4mJVHeKHmFcCG+pPyUyozE4oz4otKc1OJDjDIcHEoSvGcMgXKCRanpq RVpmTnA+IFJS3DwKInwihkBpXmLCxJzizPTIVKnGHU5jmzZdJJJiCUvPy9VSpx3LsgMAZCijN I8uBGwWL7EKCslzMvIwMAgxFOQWpSbWYIq/4pRnINRSZj3LsgUnsy8ErhNr4COYAI64vIhWZA jShIRUlINjCY3n1oVM/SoSDM0a++b0q9qsnK3z/yXp3borN2dcen7e7/W7t2TsgQkeGo+FUUs 3XbnaIRS7Pnf3203PnV1DfWd+rJbb2k3k+c0m6KtD0+32VuIsq2dr7Oaic1L/v0Dvp37v1l93 +uw/Jv3bFu7zHm3k+3FWFybyo+EP/wb2ebZKRTSVR5to8RSnJFoqMVcVJwIACyngpDgAgAA X-Env-Sender: konrad.wilk@oracle.com X-Msg-Ref: server-15.tower-27.messagelabs.com!1461728003!37992073!1 X-Originating-IP: [156.151.31.81] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTU2LjE1MS4zMS44MSA9PiAyODgzMzk=\n X-StarScan-Received: X-StarScan-Version: 8.34; banners=-,-,- X-VirusChecked: Checked Received: (qmail 10537 invoked from network); 27 Apr 2016 03:33:24 -0000 Received: from userp1040.oracle.com (HELO userp1040.oracle.com) (156.151.31.81) by server-15.tower-27.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 27 Apr 2016 03:33:24 -0000 Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u3R3XIpM028089 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 27 Apr 2016 03:33:18 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by aserv0022.oracle.com (8.13.8/8.13.8) with ESMTP id u3R3XH5F020650 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 27 Apr 2016 03:33:18 GMT Received: from abhmp0003.oracle.com (abhmp0003.oracle.com [141.146.116.9]) by aserv0122.oracle.com (8.13.8/8.13.8) with ESMTP id u3R3XFDw015012; Wed, 27 Apr 2016 03:33:16 GMT Received: from localhost.localdomain (/209.6.196.81) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 26 Apr 2016 20:32:01 -0700 USER-AGENT: Mutt/1.5.24 (2015-08-30) MIME-Version: 1.0 Message-ID: <20160427033155.GE26540@localhost.localdomain> Date: Tue, 26 Apr 2016 20:31:56 -0700 (PDT) From: Konrad Rzeszutek Wilk To: Jan Beulich References: <1461598514-5440-1-git-send-email-konrad.wilk@oracle.com> <1461598514-5440-17-git-send-email-konrad.wilk@oracle.com> <571F4B9B.5000405@citrix.com> <571F7E1102000078000E5D54@prv-mh.provo.novell.com> <571F6381.3040500@citrix.com> <571F8C2A02000078000E5E0C@prv-mh.provo.novell.com> In-Reply-To: <571F8C2A02000078000E5E0C@prv-mh.provo.novell.com> X-Source-IP: aserv0022.oracle.com [141.146.126.234] Cc: Keir Fraser , andrew.cooper3@citrix.com, Ian Jackson , Tim Deegan , mpohlack@amazon.de, Ross Lagerwall , sasha.levin@oracle.com, xen-devel@lists.xenproject.org Subject: Re: [Xen-devel] [PATCH v9 16/27] x86, xsplice: Print payload's symbol name and payload name in backtraces X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > >> When text_addr is void *, how is this calculation wrong then? > > > > I'm sorry, ignore that. I temporarily forgot how void* arithmetic is > > defined for GCC. > > > > The other two points are still valid and may result in incorrect > > backtraces with > 1 payload loaded. > > Of course. So it would look like this then: From affca85da4d57c466cc3a603afa4d57fea7ed092 Mon Sep 17 00:00:00 2001 From: Ross Lagerwall Date: Fri, 22 Apr 2016 11:16:36 -0400 Subject: [PATCH] x86, xsplice: Print payload's symbol name and payload name in backtraces Naturally the backtrace is presented when an instruction hits an bug_frame or %p is used. The payloads do not support bug_frames yet - however the functions the payloads call could hit an BUG() or WARN(). The traps.c has logic to scan for it this - and eventually it will find the correct bug_frame and the walk the stack using %p to print the backtrace. For %p and symbols to print a string - the 'is_active_kernel_text' is consulted which uses an 'struct virtual_region'. Therefore we register our start->end addresses so that 'is_active_kernel_text' will include our payload address. We also register our symbol lookup table function so that it can scan the list of payloads and retrieve the correct name. Lastly we change vsprintf to take into account s and namebuf. For core code they are the same, but for payloads they are different. This gets us: Xen call trace: [] revert_hook+0x31/0x35 [xen_hello_world] [] xsplice.c#revert_payload+0x86/0xc6 [] check_for_xsplice_work+0x233/0x3cd [] domain.c#continue_idle_domain+0x9/0x1f Which is great if payloads have similar or same symbol names. Signed-off-by: Ross Lagerwall Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Andrew Cooper Reviewed-by: Jan Beulich --- Cc: Ian Jackson Cc: Jan Beulich Cc: Keir Fraser Cc: Tim Deegan v2: Add missing full stop. v3: s/module/payload/ v4: Expand comment and include registration of 'virtual_region' Redo the vsprintf handling of payload name. Drop the ->skip function v6: Add comment explaining the purpose behind the strcmp. Redid per Jan's review. v7: Add Andrew's Review-by Drop the strcmp and just do pointer checks. v9: Do pointer comparison on vsprintf by itself, no need for intermediate payload bool_t Add const in xsplice_symbols_lookup Make 'best' in xsplice_symbols_lookup be unsigned int. Use an RCU list for iterating the applied_list. Define the RCU lock. v10: In xsplice_symbols_lookup use || instead of && when skipping. Also in xsplice_symbols_lookup use ->text_size instead of ->pages. --- --- xen/common/vsprintf.c | 12 +++++++++ xen/common/xsplice.c | 69 ++++++++++++++++++++++++++++++++++++++++++++--- xen/include/xen/xsplice.h | 1 + 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c index 18d2634..70e1edf 100644 --- a/xen/common/vsprintf.c +++ b/xen/common/vsprintf.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -354,6 +355,17 @@ static char *pointer(char *str, char *end, const char **fmt_ptr, str = number(str, end, sym_size, 16, -1, -1, SPECIAL); } + /* + * namebuf contents and s for core hypervisor are same but for xSplice + * payloads they differ (namebuf contains the name of the payload). + */ + if ( namebuf != s ) + { + str = string(str, end, " [", -1, -1, 0); + str = string(str, end, namebuf, -1, -1, 0); + str = string(str, end, "]", -1, -1, 0); + } + return str; } diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c index 85af98a..188b850 100644 --- a/xen/common/xsplice.c +++ b/xen/common/xsplice.c @@ -14,7 +14,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -31,10 +33,9 @@ static LIST_HEAD(payload_list); /* * Patches which have been applied. Need RCU in case we crash (and then - * traps code would iterate via applied_list) when adding entries on the list. - * - * Note: There are no 'rcu_applied_lock' as we don't iterate yet the list. + * traps code would iterate via applied_list) when adding entries onthe list. */ +static DEFINE_RCU_READ_LOCK(rcu_applied_lock); static LIST_HEAD(applied_list); static unsigned int payload_cnt; @@ -56,6 +57,8 @@ struct payload { unsigned int nfuncs; /* Nr of functions to patch. */ const struct xsplice_symbol *symtab; /* All symbols. */ const char *strtab; /* Pointer to .strtab. */ + struct virtual_region region; /* symbol, bug.frame patching and + exception table (x86). */ unsigned int nsyms; /* Nr of entries in .strtab and symbols. */ char name[XEN_XSPLICE_NAME_SIZE]; /* Name of it. */ }; @@ -139,6 +142,55 @@ void *xsplice_symbols_lookup_by_name(const char *symname) return 0; } +static const char *xsplice_symbols_lookup(unsigned long addr, + unsigned long *symbolsize, + unsigned long *offset, + char *namebuf) +{ + const struct payload *data; + unsigned int i, best; + const void *va = (const void *)addr; + const char *n = NULL; + + /* + * Only RCU locking since this list is only ever changed during apply + * or revert context. And in case it dies there we need an safe list. + */ + rcu_read_lock(&rcu_applied_lock); + list_for_each_entry_rcu ( data, &applied_list, applied_list ) + { + if ( va < data->text_addr || + va >= (data->text_addr + data->text_size) ) + continue; + + best = UINT_MAX; + + for ( i = 0; i < data->nsyms; i++ ) + { + if ( data->symtab[i].value <= va && + (best == UINT_MAX || + data->symtab[best].value < data->symtab[i].value) ) + best = i; + } + + if ( best == UINT_MAX ) + break; + + if ( symbolsize ) + *symbolsize = data->symtab[best].size; + if ( offset ) + *offset = va - data->symtab[best].value; + if ( namebuf ) + strlcpy(namebuf, data->name, KSYM_NAME_LEN); + + n = data->symtab[best].name; + break; + } + rcu_read_unlock(&rcu_applied_lock); + + return n; +} + static struct payload *find_payload(const char *name) { struct payload *data, *found = NULL; @@ -375,6 +427,7 @@ static int prepare_payload(struct payload *payload, const struct xsplice_elf_sec *sec; unsigned int i; struct xsplice_patch_func *f; + struct virtual_region *region; sec = xsplice_elf_sec_by_name(elf, ELF_XSPLICE_FUNC); ASSERT(sec); @@ -431,6 +484,13 @@ static int prepare_payload(struct payload *payload, } } + /* Setup the virtual region with proper data. */ + region = &payload->region; + + region->symbols_lookup = xsplice_symbols_lookup; + region->start = payload->text_addr; + region->end = payload->text_addr + payload->text_size; + return 0; } @@ -507,6 +567,7 @@ static int build_symbol_table(struct payload *payload, if ( is_payload_symbol(elf, elf->sym + i) ) { symtab[nsyms].name = strtab + strtab_len; + symtab[nsyms].size = elf->sym[i].sym->st_size; symtab[nsyms].value = (void *)elf->sym[i].sym->st_value; symtab[nsyms].new_symbol = 0; /* May be overwritten below. */ strtab_len += strlcpy(strtab + strtab_len, elf->sym[i].name, @@ -797,6 +858,7 @@ static int apply_payload(struct payload *data) * The applied_list is iterated by the trap code. */ list_add_tail_rcu(&data->applied_list, &applied_list); + register_virtual_region(&data->region); return 0; } @@ -819,6 +881,7 @@ static int revert_payload(struct payload *data) * The applied_list is iterated by the trap code. */ list_del_rcu(&data->applied_list); + unregister_virtual_region(&data->region); return 0; } diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h index 1526752..bb8baee 100644 --- a/xen/include/xen/xsplice.h +++ b/xen/include/xen/xsplice.h @@ -22,6 +22,7 @@ struct xen_sysctl_xsplice_op; struct xsplice_symbol { const char *name; void *value; + unsigned int size; bool_t new_symbol; };