Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Xen: Devel

[PATCH 07 of 10] xenalyze: decode PV_HYPERCALL_V2 records

 

 

Xen devel RSS feed   Index | Next | Previous | View Threaded


david.vrabel at citrix

May 31, 2012, 4:16 AM

Post #1 of 3 (49 views)
Permalink
[PATCH 07 of 10] xenalyze: decode PV_HYPERCALL_V2 records

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 [at] citrix>

diff --git a/pv.h b/pv.h
new file mode 100644
--- /dev/null
+++ b/pv.h
@@ -0,0 +1,41 @@
+/*
+ * PV event decoding.
+ *
+ * Copyright (C) 2012 Citrix Systems R&D Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#ifndef __PV_H
+
+#include "analyze.h"
+#include "trace.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define ARG_MISSING 0x0
+#define ARG_32BIT 0x1
+#define ARG_64BIT 0x2
+
+#define MMU_UPDATE_PREEMPTED (~(~0U>>1))
+
+static inline uint32_t pv_hypercall_op(const struct record_info *ri)
+{
+ return ri->d[0] & ~TRC_PV_HYPERCALL_V2_ARG_MASK;
+}
+
+static inline int pv_hypercall_arg_present(const struct record_info *ri, int arg)
+{
+ return (ri->d[0] >> (20 + 2*arg)) & 0x3;
+}
+
+uint64_t pv_hypercall_arg(const struct record_info *ri, int i);
+
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
+#endif
diff --git a/xenalyze.c b/xenalyze.c
--- a/xenalyze.c
+++ b/xenalyze.c
@@ -32,6 +32,7 @@
#include "trace.h"
#include "analyze.h"
#include "mread.h"
+#include "pv.h"
#include <errno.h>
#include <strings.h>
#include <string.h>
@@ -1480,6 +1481,7 @@ enum {
PV_GDT_LDT_MAPPING_FAULT,
PV_PTWR_EMULATION,
PV_PTWR_EMULATION_PAE,
+ PV_HYPERCALL_V2 = 13,
PV_MAX
};

@@ -6518,6 +6520,96 @@ 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]) | ri->d[word];
+ }
+ }
+
+ /* Skip over any words for this argument. */
+ word += present;
+ }
+
+ return 0;
+}
+
+static const char *grant_table_op_cmd_to_str(uint32_t cmd)
+{
+ const char *cmd_str[] = {
+ "map_grant_ref", "unmap_grant_ref", "setup_table", "dump_table",
+ "transfer", "copy", "query_size", "unmap_and_replace",
+ "set_version", "get_status_frames", "get_version", "swap_grant_ref",
+ };
+ static char buf[32];
+
+ if (cmd <= 11)
+ return cmd_str[cmd];
+
+ snprintf(buf, sizeof(buf), "unknown (%d)", cmd);
+ return buf;
+}
+
+void pv_hypercall_v2_process(struct record_info *ri, struct pv_data *pv)
+{
+ int op = pv_hypercall_op(ri);
+
+ if(opt.summary_info) {
+ if(op < PV_HYPERCALL_MAX)
+ pv->hypercall_count[op]++;
+ }
+
+ if(opt.dump_all) {
+ if(op < HYPERCALL_MAX)
+ printf(" %s hypercall %2x (%s)",
+ ri->dump_header, op, hypercall_name[op]);
+ else
+ printf(" %s hypercall %2x",
+ ri->dump_header, op);
+ switch(op) {
+ case HYPERCALL_mmu_update:
+ {
+ uint32_t count = pv_hypercall_arg(ri, 1);
+ printf(" %d updates%s", count & ~MMU_UPDATE_PREEMPTED,
+ (count & MMU_UPDATE_PREEMPTED) ? " (preempted)" : "");
+ }
+ break;
+ case HYPERCALL_multicall:
+ {
+ uint32_t calls = pv_hypercall_arg(ri, 1);
+ printf(" %d calls", calls);
+ }
+ break;
+ case HYPERCALL_grant_table_op:
+ {
+ uint32_t cmd = pv_hypercall_arg(ri, 0);
+ uint32_t count = pv_hypercall_arg(ri, 2);
+ printf(" %s %d ops", grant_table_op_cmd_to_str(cmd), count);
+ }
+ break;
+ case HYPERCALL_mmuext_op:
+ {
+ uint32_t count = pv_hypercall_arg(ri, 1);
+ printf(" %d ops", count);
+ }
+ break;
+ }
+ printf("\n");
+ }
+}
+
void pv_process(struct pcpu_info *p)
{
struct record_info *ri = &p->ri;
@@ -6550,9 +6642,9 @@ void pv_process(struct pcpu_info *p)
case PV_PTWR_EMULATION_PAE:
pv_ptwr_emulation_process(ri, pv);
break;
- case PV_PAGE_FAULT:
- //pv_pf_process(ri, pv);
- //break;
+ case PV_HYPERCALL_V2:
+ pv_hypercall_v2_process(ri, pv);
+ break;
default:
pv_generic_process(ri, pv);
break;

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


george.dunlap at eu

Jun 7, 2012, 4:35 AM

Post #2 of 3 (45 views)
Permalink
Re: [PATCH 07 of 10] xenalyze: decode PV_HYPERCALL_V2 records [In reply to]

On 31/05/12 12:16, David Vrabel 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 [at] citrix>
>
> diff --git a/pv.h b/pv.h
> new file mode 100644
> --- /dev/null
> +++ b/pv.h
Why does this need its own file?
> +static const char *grant_table_op_cmd_to_str(uint32_t cmd)
Hmm -- this is a different style to the other lists of this type. I
guess I like having it in a function.
> +{
> + const char *cmd_str[] = {
> + "map_grant_ref", "unmap_grant_ref", "setup_table", "dump_table",
> + "transfer", "copy", "query_size", "unmap_and_replace",
> + "set_version", "get_status_frames", "get_version", "swap_grant_ref",
> + };
I'm a bit wary of having stuff just in a big list like this -- it seems
like it makes it harder to double-check that you've gotten the right
match-up. I'd prefer it look like hvm_event_handler_name[], where the
number is annotated with a comment from time to time.
> + static char buf[32];
> +
> + if (cmd<= 11)
> + return cmd_str[cmd];
Instead of hardcoding the number of elements, could you use some
calculation involving sizeof() to get that automatically? In any case,
it should be "cmd < N", rather than "cmd <= N-1" (where N is the number
of elements in the array).
> + switch(op) {
> + case HYPERCALL_mmu_update:
> + {
I'm not a fan of indenting a brace within a case statement -- I think
this is emacs' default C mode, but I prefer it the other way. (Not
sure which config option sets this, though.)

Other than that, looks good.

-George


_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


david.vrabel at citrix

Jun 7, 2012, 8:20 AM

Post #3 of 3 (45 views)
Permalink
Re: [PATCH 07 of 10] xenalyze: decode PV_HYPERCALL_V2 records [In reply to]

On 07/06/12 12:35, George Dunlap wrote:
> On 31/05/12 12:16, David Vrabel 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 [at] citrix>
>>
>> diff --git a/pv.h b/pv.h
>> new file mode 100644
>> --- /dev/null
>> +++ b/pv.h
> Why does this need its own file?

I would like to see Xenalyze split into more, smaller files each with
related functionality. This is a start.

>> +static const char *grant_table_op_cmd_to_str(uint32_t cmd)
> Hmm -- this is a different style to the other lists of this type. I
> guess I like having it in a function.
>> +{
>> + const char *cmd_str[] = {
>> + "map_grant_ref", "unmap_grant_ref", "setup_table", "dump_table",
>> + "transfer", "copy", "query_size", "unmap_and_replace",
>> + "set_version", "get_status_frames", "get_version", "swap_grant_ref",
>> + };
> I'm a bit wary of having stuff just in a big list like this -- it seems
> like it makes it harder to double-check that you've gotten the right
> match-up. I'd prefer it look like hvm_event_handler_name[], where the
> number is annotated with a comment from time to time.

Ok.

>> + static char buf[32];
>> +
>> + if (cmd<= 11)
>> + return cmd_str[cmd];
> Instead of hardcoding the number of elements, could you use some
> calculation involving sizeof() to get that automatically? In any case,
> it should be "cmd < N", rather than "cmd <= N-1" (where N is the number
> of elements in the array).

Ok.

>> + switch(op) {
>> + case HYPERCALL_mmu_update:
>> + {
> I'm not a fan of indenting a brace within a case statement -- I think
> this is emacs' default C mode, but I prefer it the other way. (Not
> sure which config option sets this, though.)

Ok.

Also, this also doesn't add the event name so (null) is printed in the
summary. I'll fix this up as well.

David

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel

Xen devel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.