Re: [Xen-devel] [PATCH v1 01/13] Export hypervisor symbols

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.09.13 at 17:20, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -102,11 +102,11 @@ $(BASEDIR)/common/symbols-dummy.o:
>  $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
>       $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
>           $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
> -     $(NM) -n $(@D)/.$(@F).0 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).0.S
> +     $(NM) -n $(@D)/.$(@F).0 | $(BASEDIR)/tools/symbols --all-symbols 
> >$(@D)/.$(@F).0.S

For one I can't see what use data symbols have for performance
analysis.

And then I'm opposed to growing the symbol table size
unconditionally for no good reason.

> --- a/xen/include/public/platform.h
> +++ b/xen/include/public/platform.h
> @@ -527,6 +527,26 @@ struct xenpf_core_parking {
>  typedef struct xenpf_core_parking xenpf_core_parking_t;
>  DEFINE_XEN_GUEST_HANDLE(xenpf_core_parking_t);
>  
> +#define XENPF_get_symbols  61
> +
> +#define XENSYMS_SZ         4096

This doesn't appear to belong into the public interface.

> +struct xenpf_symdata {
> +    /*
> +     * offset into Xen's symbol data and symbol number from
> +     * last call. Used only by Xen.
> +     */
> +    uint64_t xen_offset;
> +    uint64_t xen_symnum;

I wonder whether that's really a suitable mechanism.

> +
> +   /*
> +    * Symbols data, formatted similar to /proc/kallsyms:
> +    *   <address> <type> <name>
> +    */
> +    XEN_GUEST_HANDLE(char) buf;

This is too simplistic: Please use a proper structure here, to allow
switching the internal symbol table representation (which I have on
my todo list) without having to mimic old behavior.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel