From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v7 01/19] common/symbols: Export hypervisor symbols to privileged guest Date: Fri, 06 Jun 2014 15:05:16 -0400 Message-ID: <539210EC.9050000@oracle.com> References: <1402076415-26475-1-git-send-email-boris.ostrovsky@oracle.com> <1402076415-26475-2-git-send-email-boris.ostrovsky@oracle.com> <53920101.8090304@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53920101.8090304@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: kevin.tian@intel.com, keir@xen.org, JBeulich@suse.com, jun.nakajima@intel.com, tim@xen.org, dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org, suravee.suthikulpanit@amd.com List-Id: xen-devel@lists.xenproject.org On 06/06/2014 01:57 PM, Andrew Cooper wrote: > On 06/06/14 18:39, Boris Ostrovsky wrote: >> Export Xen's symbols as {
} triplet via new XENPF_get_symbol >> hypercall >> >> Signed-off-by: Boris Ostrovsky >> Reviewed-by: Dietmar Hahn >> Tested-by: Dietmar Hahn >> --- >> xen/arch/x86/platform_hypercall.c | 21 +++++++++++++++ >> xen/common/symbols.c | 54 +++++++++++++++++++++++++++++++++++++++ >> xen/include/public/platform.h | 19 ++++++++++++++ >> xen/include/xen/symbols.h | 3 +++ >> xen/include/xlat.lst | 1 + >> 5 files changed, 98 insertions(+) >> >> diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c >> index 2162811..2c602e7 100644 >> --- a/xen/arch/x86/platform_hypercall.c >> +++ b/xen/arch/x86/platform_hypercall.c >> @@ -23,6 +23,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -601,6 +602,26 @@ ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op) >> } >> break; >> >> + case XENPF_get_symbol: >> + { >> + static char name[KSYM_NAME_LEN + 1]; > There needs to be some concurrency protection surrounding the use of > this static. The symbol mutext in xensyms_read() is not sufficient. > > As it currently stands, two vcpus issuing this hypercall at the time > will collide and both will get junk back. We should be protected by xenpf_lock here. > >> diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h >> index 053b9fa..afe301c 100644 >> --- a/xen/include/public/platform.h >> +++ b/xen/include/public/platform.h >> @@ -527,6 +527,24 @@ struct xenpf_core_parking { >> typedef struct xenpf_core_parking xenpf_core_parking_t; >> DEFINE_XEN_GUEST_HANDLE(xenpf_core_parking_t); >> >> +#define XENPF_get_symbol 61 >> +struct xenpf_symdata { >> + /* IN variables */ >> + uint32_t namelen; /* size of 'name' buffer */ >> + >> + /* IN/OUT variables */ >> + uint32_t symnum; /* IN: Symbol to read */ >> + /* OUT: Next available symbol. If same as IN then */ >> + /* we reached the end */ >> + >> + /* OUT variables */ >> + char type; >> + uint64_t address; >> + XEN_GUEST_HANDLE(char) name; > Please can we avoid introducing non-explicitly-width'd types into the > public ABI. > > If you swap name and type, use XEN_GUEST_HANDLE_64() and promote type to > a uint32, it should be 32/64 bit safe and not need translating. I thought we are going via compat_platform_op for 32-bit guests and thus pass pack(4)'d structures so translating is not needed. Is that not true? (I did test this on 32-bit dom0 and it worked fine, although that alone is probably not a good argument). -boris