From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v7 01/19] common/symbols: Export hypervisor
 symbols to privileged guest
Date: Fri, 6 Jun 2014 18:57:21 +0100
Message-ID: <53920101.8090304@citrix.com>
References: <1402076415-26475-1-git-send-email-boris.ostrovsky@oracle.com>
	<1402076415-26475-2-git-send-email-boris.ostrovsky@oracle.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1402076415-26475-2-git-send-email-boris.ostrovsky@oracle.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>
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/14 18:39, Boris Ostrovsky wrote:
> Export Xen's symbols as {<address><type><name>} triplet via new XENPF_get_symbol
> hypercall
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Reviewed-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
> Tested-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
> ---
>  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 <xen/cpu.h>
>  #include <xen/pmstat.h>
>  #include <xen/irq.h>
> +#include <xen/symbols.h>
>  #include <asm/current.h>
>  #include <public/platform.h>
>  #include <acpi/cpufreq/processor_perf.h>
> @@ -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.

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

~Andrew