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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 09/11/2013 10:12 AM, Jan Beulich wrote:
>
>>>> --- 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.
>> Linux driver needs to know size of the buffer that is passed from
>> the hypervisir. I suppose I can just use PAGE_SIZE.
> Buffer? Passed from the hypervisor?

As it is written now, we pass XENSYMS_SZ worth of (formatted) symbol
information to dom0.

> And no, there's no PAGE_SIZE in the public interface as far as I'm
> aware.
>
>>>> +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.
>> Why do you think this is not suitable?
>>
>> Linux needs to keep track of position in the symbol table while
>> it is walking over the file, otherwise we will need to keep the state
>> in hypervisor which is much less desirable.
> This could be as simple as a "give me the n-th symbol" interface.
> The handler in the hypervisor could cache the last symbol
> together with the associated data (with the assumption that there's
> only ever going to be one iteration in progress), invalidating the
> cache if the coming in index isn't one greater than the last one
> processed. All the caching of course is only necessary if otherwise
> lookup times aren't acceptable.

That would be just having xen_symnum (and caching xen_offset
in the hypervisor).


>>>> +
>>>> +   /*
>>>> +    * 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.
>> I don't think I know what you are referring to here.
> Rather than having a handle to a simply byte array, you ought
> to have a handle to a structure containing address, type, and
> (pointer to/handle of) name.
>

Are you suggesting passing symbols one per hypercall? That's over 4000
hypercalls per one file read. How about requesting N next symbols?

-boris