xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: jun.nakajima@intel.com, George.Dunlap@eu.citrix.com,
	jacob.shin@amd.com, eddie.dong@intel.com,
	dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org,
	JBeulich@suse.com, suravee.suthikulpanit@amd.com
Subject: Re: [PATCH v2 01/13] Export hypervisor symbols
Date: Mon, 23 Sep 2013 16:06:46 -0400	[thread overview]
Message-ID: <52409F56.9090208@oracle.com> (raw)
In-Reply-To: <20130923194249.GA3019@phenom.dumpdata.com>

On 09/23/2013 03:42 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 20, 2013 at 05:42:00AM -0400, Boris Ostrovsky wrote:
>
> I think the title needs a prefix of say: "xen/platform_op:" or such.
>
>> Export Xen's symbols in format similar to Linux' /proc/kallsyms.
> Which is?
>
> I see in Linux:
>
> 000000000000bd58 D xen_cr3
> ...
> 000000000000e978 d soft_lockup_hrtimer_cnt
> ..
> ffffffff8101cef0 T eager_fpu_init
> ffffffff8101d010 t ptrace_triggered
>
> I know that the first column is the EIP (thought some of them seem to be
> based on some offset). The last one is pretty obvious too.
>
> But the middle one:
> konrad@phenom:~$ cat /proc/kallsyms  | awk '{print $2}' | sort | uniq
> b
> B
> d
> D
> r
> R
> t
> T
> V
> W
>
> Could you explain what those mean? And are they part of this?
> Or does Xen not carry those?

These are symbol types, described in 'man nm'. For Xen we should only 
see 't' and 'T' which denote local or global text symbol.

So the format would be
<address> <type> <symbol name>

In any case, this is not a comlpetely correct commit message, it really 
describes to v1 implementation. With v2 Xen will return these three 
values in a structure, which has nothing to do with *format* of how 
these symbols are presented. I'll re-phrase it.

(And there other remnants of v1 in this patch that I just noticed, such 
as changes to xen/tools/symbols.c which are no longer needed)

-boris


>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>>   xen/arch/x86/platform_hypercall.c        |  9 +++++
>>   xen/arch/x86/x86_64/platform_hypercall.c |  2 +-
>>   xen/common/symbols-dummy.c               |  1 +
>>   xen/common/symbols.c                     | 58 ++++++++++++++++++++++++++++++--
>>   xen/include/public/platform.h            | 22 ++++++++++++
>>   xen/include/xen/symbols.h                |  4 +++
>>   xen/tools/symbols.c                      |  4 +++
>>   7 files changed, 97 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
>> index 7175a82..39376fe 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>
>> @@ -597,6 +598,14 @@ ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
>>       }
>>       break;
>>   
>> +    case XENPF_get_symbols:
>> +    {
>> +        ret = xensyms_read(&op->u.symdata);
>> +        if ( ret >= 0 && __copy_field_to_guest(u_xenpf_op, op, u.symdata) )
>> +            ret = -EFAULT;
>> +    }
>> +    break;
>> +
>>       default:
>>           ret = -ENOSYS;
>>           break;
>> diff --git a/xen/arch/x86/x86_64/platform_hypercall.c b/xen/arch/x86/x86_64/platform_hypercall.c
>> index aa2ad54..9ef705a 100644
>> --- a/xen/arch/x86/x86_64/platform_hypercall.c
>> +++ b/xen/arch/x86/x86_64/platform_hypercall.c
>> @@ -35,7 +35,7 @@ CHECK_pf_pcpu_version;
>>   #undef xen_pf_pcpu_version
>>   
>>   #define xenpf_enter_acpi_sleep compat_pf_enter_acpi_sleep
>> -
>> +#define xenpf_symdata	compat_pf_symdata
>>   #define COMPAT
>>   #define _XEN_GUEST_HANDLE(t) XEN_GUEST_HANDLE(t)
>>   #define _XEN_GUEST_HANDLE_PARAM(t) XEN_GUEST_HANDLE_PARAM(t)
>> diff --git a/xen/common/symbols-dummy.c b/xen/common/symbols-dummy.c
>> index 5090c3b..52a86c7 100644
>> --- a/xen/common/symbols-dummy.c
>> +++ b/xen/common/symbols-dummy.c
>> @@ -12,6 +12,7 @@ const unsigned int symbols_offsets[1];
>>   const unsigned long symbols_addresses[1];
>>   #endif
>>   const unsigned int symbols_num_syms;
>> +const unsigned long symbols_names_bytes;
>>   const u8 symbols_names[1];
>>   
>>   const u8 symbols_token_table[1];
>> diff --git a/xen/common/symbols.c b/xen/common/symbols.c
>> index 83b2b58..e74a585 100644
>> --- a/xen/common/symbols.c
>> +++ b/xen/common/symbols.c
>> @@ -17,6 +17,8 @@
>>   #include <xen/lib.h>
>>   #include <xen/string.h>
>>   #include <xen/spinlock.h>
>> +#include <public/platform.h>
>> +#include <xen/guest_access.h>
>>   
>>   #ifdef SYMBOLS_ORIGIN
>>   extern const unsigned int symbols_offsets[1];
>> @@ -26,6 +28,7 @@ extern const unsigned long symbols_addresses[];
>>   #define symbols_address(n) symbols_addresses[n]
>>   #endif
>>   extern const unsigned int symbols_num_syms;
>> +extern const unsigned long symbols_names_bytes;
>>   extern const u8 symbols_names[];
>>   
>>   extern const u8 symbols_token_table[];
>> @@ -35,7 +38,8 @@ extern const unsigned int symbols_markers[];
>>   
>>   /* expand a compressed symbol data into the resulting uncompressed string,
>>      given the offset to where the symbol is in the compressed stream */
>> -static unsigned int symbols_expand_symbol(unsigned int off, char *result)
>> +static unsigned int symbols_expand_symbol(unsigned int off, char *result,
>> +                                          int maxlen)
>>   {
>>       int len, skipped_first = 0;
>>       const u8 *tptr, *data;
>> @@ -49,6 +53,9 @@ static unsigned int symbols_expand_symbol(unsigned int off, char *result)
>>        * the compressed stream */
>>       off += len + 1;
>>   
>> +    if (maxlen < len)
>> +        len = maxlen;
>> +
>>       /* for every byte on the compressed symbol data, copy the table
>>          entry for that byte */
>>       while(len) {
>> @@ -129,7 +136,7 @@ const char *symbols_lookup(unsigned long addr,
>>           --low;
>>   
>>           /* Grab name */
>> -    symbols_expand_symbol(get_symbol_offset(low), namebuf);
>> +    symbols_expand_symbol(get_symbol_offset(low), namebuf, sizeof(namebuf));
>>   
>>       /* Search for next non-aliased symbol */
>>       for (i = low + 1; i < symbols_num_syms; i++) {
>> @@ -174,3 +181,50 @@ void __print_symbol(const char *fmt, unsigned long address)
>>   
>>       spin_unlock_irqrestore(&lock, flags);
>>   }
>> +
>> +/*
>> + * Get symbol type information. This is encoded as a single char at the
>> + * beginning of the symbol name.
>> + */
>> +static char symbols_get_symbol_type(unsigned int off)
>> +{
>> +    /*
>> +     * Get just the first code, look it up in the token table,
>> +     * and return the first char from this token.
>> +     */
>> +    return symbols_token_table[symbols_token_index[symbols_names[off + 1]]];
>> +}
>> +
>> +/*
>> + * Symbols are most likely accessed sequentially so we remember position from
>> + * previous read. This can help us avoid extra call to get_symbol_offset().
>> + */
>> +static uint64_t next_symbol, next_offset;
>> +static DEFINE_SPINLOCK(symbols_mutex);
>> +
>> +int xensyms_read(struct xenpf_symdata *symdata)
>> +{
>> +    if ( symdata->xen_symnum > symbols_num_syms )
>> +        return -EINVAL;
>> +    else if ( symdata->xen_symnum == symbols_num_syms )
>> +        return 0;
>> +
>> +    spin_lock(&symbols_mutex);
>> +
>> +    if ( symdata->xen_symnum == 0 )
>> +        next_offset = next_symbol = 0;
>> +    else if ( next_symbol != symdata->xen_symnum )
>> +        /* Non-sequential access */
>> +        next_offset = get_symbol_offset(symdata->xen_symnum);
>> +
>> +    symdata->type = symbols_get_symbol_type(next_offset);
>> +    next_offset = symbols_expand_symbol(next_offset, symdata->name,
>> +        sizeof(symdata->name));
>> +    symdata->address = symbols_offsets[symdata->xen_symnum] + SYMBOLS_ORIGIN;
>> +
>> +    next_symbol = symdata->xen_symnum + 1;
>> +
>> +    spin_unlock(&symbols_mutex);
>> +
>> +    return strlen(symdata->name);
>> +}
>> diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
>> index 4341f54..870e14b 100644
>> --- a/xen/include/public/platform.h
>> +++ b/xen/include/public/platform.h
>> @@ -527,6 +527,27 @@ 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
>> +
>> +struct xenpf_symdata {
>> +    /* IN variables */
>> +    uint64_t xen_symnum;
>> +
>> +    /* OUT variables */
>> +    uint64_t address;
>> +    uint64_t type;
>> +    /*
>> +     * KSYM_NAME_LEN is 128 bytes. However, we cannot be larger than pad in
>> +     * xen_platform_op below (which is 128 bytes as well). Since the largest
>> +     * symbol is around 50 bytes it's probably more trouble than it's worth
>> +     * to try to deal with symbols that are close to 128 bytes in length.
>> +     */
>> +#define XEN_SYMS_MAX_LEN (128 - 3 * 8)
>> +    char name[XEN_SYMS_MAX_LEN];
>> +};
>> +typedef struct xenpf_symdata xenpf_symdata_t;
>> +DEFINE_XEN_GUEST_HANDLE(xenpf_symdata_t);
>> +
>>   /*
>>    * ` enum neg_errnoval
>>    * ` HYPERVISOR_platform_op(const struct xen_platform_op*);
>> @@ -553,6 +574,7 @@ struct xen_platform_op {
>>           struct xenpf_cpu_hotadd        cpu_add;
>>           struct xenpf_mem_hotadd        mem_add;
>>           struct xenpf_core_parking      core_parking;
>> +        struct xenpf_symdata           symdata;
>>           uint8_t                        pad[128];
>>       } u;
>>   };
>> diff --git a/xen/include/xen/symbols.h b/xen/include/xen/symbols.h
>> index 37cf6bf..c8df28f 100644
>> --- a/xen/include/xen/symbols.h
>> +++ b/xen/include/xen/symbols.h
>> @@ -2,6 +2,8 @@
>>   #define _XEN_SYMBOLS_H
>>   
>>   #include <xen/types.h>
>> +#include <public/xen.h>
>> +#include <public/platform.h>
>>   
>>   #define KSYM_NAME_LEN 127
>>   
>> @@ -34,4 +36,6 @@ do {						\
>>   	__print_symbol(fmt, addr);		\
>>   } while(0)
>>   
>> +extern int xensyms_read(struct xenpf_symdata *symdata);
>> +
>>   #endif /*_XEN_SYMBOLS_H*/
>> diff --git a/xen/tools/symbols.c b/xen/tools/symbols.c
>> index f39c906..818204d 100644
>> --- a/xen/tools/symbols.c
>> +++ b/xen/tools/symbols.c
>> @@ -272,6 +272,10 @@ static void write_src(void)
>>   	}
>>   	printf("\n");
>>   
>> +	output_label("symbols_names_bytes");
>> +	printf("\t.long\t%d\n", off);
>> +	printf("\n");
>> +
>>   	output_label("symbols_markers");
>>   	for (i = 0; i < ((table_cnt + 255) >> 8); i++)
>>   		printf("\t.long\t%d\n", markers[i]);
>> -- 
>> 1.8.1.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

  reply	other threads:[~2013-09-23 20:06 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-20  9:41 [PATCH v2 00/13] x86/PMU: Xen PMU PV support Boris Ostrovsky
2013-09-20  9:42 ` [PATCH v2 01/13] Export hypervisor symbols Boris Ostrovsky
2013-09-23 19:42   ` Konrad Rzeszutek Wilk
2013-09-23 20:06     ` Boris Ostrovsky [this message]
2013-09-24 17:40       ` Konrad Rzeszutek Wilk
2013-09-25 13:15   ` Jan Beulich
2013-09-25 14:03     ` Boris Ostrovsky
2013-09-25 14:53       ` Jan Beulich
2013-09-20  9:42 ` [PATCH v2 02/13] Set VCPU's is_running flag closer to when the VCPU is dispatched Boris Ostrovsky
2013-09-25 13:42   ` Jan Beulich
2013-09-25 14:08     ` Keir Fraser
2013-09-20  9:42 ` [PATCH v2 03/13] x86/PMU: Stop AMD counters when called from vpmu_save_force() Boris Ostrovsky
2013-09-20  9:42 ` [PATCH v2 04/13] x86/VPMU: Minor VPMU cleanup Boris Ostrovsky
2013-09-20  9:42 ` [PATCH v2 05/13] intel/VPMU: Clean up Intel VPMU code Boris Ostrovsky
2013-09-23 11:42   ` Dietmar Hahn
2013-09-23 19:46   ` Konrad Rzeszutek Wilk
2013-09-25 13:55   ` Jan Beulich
2013-09-25 14:39     ` Boris Ostrovsky
2013-09-25 14:57       ` Jan Beulich
2013-09-25 15:37         ` Boris Ostrovsky
2013-09-20  9:42 ` [PATCH v2 06/13] x86/PMU: Add public xenpmu.h Boris Ostrovsky
2013-09-23 13:04   ` Dietmar Hahn
2013-09-23 13:16     ` Jan Beulich
2013-09-23 14:00       ` Boris Ostrovsky
2013-09-23 13:45     ` Boris Ostrovsky
2013-09-25 14:04   ` Jan Beulich
2013-09-25 15:59     ` Boris Ostrovsky
2013-09-25 16:08       ` Jan Beulich
2013-09-30 13:25     ` Boris Ostrovsky
2013-09-30 13:30       ` Jan Beulich
2013-09-30 13:55         ` Boris Ostrovsky
2013-09-20  9:42 ` [PATCH v2 07/13] x86/PMU: Make vpmu not HVM-specific Boris Ostrovsky
2013-09-25 14:05   ` Jan Beulich
2013-09-25 14:49     ` Boris Ostrovsky
2013-09-25 14:57       ` Jan Beulich
2013-09-20  9:42 ` [PATCH v2 08/13] x86/PMU: Interface for setting PMU mode and flags Boris Ostrovsky
2013-09-25 14:11   ` Jan Beulich
2013-09-25 14:55     ` Boris Ostrovsky
2013-09-20  9:42 ` [PATCH v2 09/13] x86/PMU: Initialize PMU for PV guests Boris Ostrovsky
2013-09-20  9:42 ` [PATCH v2 10/13] x86/PMU: Add support for PMU registes handling on " Boris Ostrovsky
2013-09-23 13:50   ` Dietmar Hahn
2013-09-25 14:23   ` Jan Beulich
2013-09-25 15:03     ` Boris Ostrovsky
2013-09-20  9:42 ` [PATCH v2 11/13] x86/PMU: Handle PMU interrupts for " Boris Ostrovsky
2013-09-25 14:33   ` Jan Beulich
2013-09-25 14:40     ` Andrew Cooper
2013-09-25 15:52       ` Boris Ostrovsky
2013-09-25 15:19     ` Boris Ostrovsky
2013-09-25 15:25       ` Jan Beulich
2013-09-20  9:42 ` [PATCH v2 12/13] x86/PMU: Save VPMU state for PV guests during context switch Boris Ostrovsky
2013-09-20  9:42 ` [PATCH v2 13/13] x86/PMU: Move vpmu files up from hvm directory Boris Ostrovsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52409F56.9090208@oracle.com \
    --to=boris.ostrovsky@oracle.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=dietmar.hahn@ts.fujitsu.com \
    --cc=eddie.dong@intel.com \
    --cc=jacob.shin@amd.com \
    --cc=jun.nakajima@intel.com \
    --cc=konrad.wilk@oracle.com \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).