Re: [PATCH 1 of 2] xenalyze: decode PV_HYPERCALL_V2 records

[David Vrabel <david.vrabel@citrix.com>]

On 03/10/12 11:33, George Dunlap wrote:
> On Mon, Oct 1, 2012 at 6:52 PM, David Vrabel <david.vrabel@citrix.com> wrote:
>> Newer version of Xen produce TRC_PV_HYPERCALL_V2 records instead of
>> the older TRC_PV_HYPERCALL format.  This updated format doesn't
>> included the IP but it does include select hypercall arguments.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> 
> Thanks -- I did a bunch of optimization work on xenalyze late last
> year, so I'm afraid I'm going to be pretty picky about some things to
> avoid losing that effort. :-)

>> +static inline int pv_hypercall_arg_present(const struct record_info *ri, int arg)
>> +{
>> +    return (ri->d[0] >> (20 + 2*arg)) & 0x3;
>> +}
> 
> Try to avoid an integer multiply here; (arg<<1) or (arg+arg) please.

The compiler does this for me.

>> @@ -6523,6 +6537,168 @@ void pv_summary(struct pv_data *pv) {
>>      }
>>  }
>>
>> +uint64_t pv_hypercall_arg(const struct record_info *ri, int arg)
>> +{
>> +    int i, word;
>> +
>> +    for (i = 0, word = 1; i < 6 && word < ri->extra_words; i++) {
>> +        int present = pv_hypercall_arg_present(ri, i);
>> +
>> +        /* Is this the argument we're looking for? */
>> +        if (i == arg) {
>> +            switch (present) {
>> +            case ARG_MISSING:
>> +                return 0;
>> +            case ARG_32BIT:
>> +                return ri->d[word];
>> +            case ARG_64BIT:
>> +                return ((uint64_t)ri->d[word + 1] << 32) | ri->d[word];
>> +            }
>> +        }
>> +
>> +        /* Skip over any words for this argument. */
>> +        word += present;
>> +    }
> 
> 
> Alternately, since pv_hypercall_arg() returns 0 for a non-existent arg
> anyway, couldn't you have an array, initialized to 0, and filled in as
> appropriate?

I've done this.

David