diff mbox series

[v2,1/1] monitor: Support specified vCPU registers

Message ID 20220727005123.1093478-2-pizhenwei@bytedance.com
State New
Headers show
Series monitor: Support specified vCPU registers | expand

Commit Message

zhenwei pi July 27, 2022, 12:51 a.m. UTC
Originally we have to get all the vCPU registers and parse the
specified one. To improve the performance of this usage, allow user
specified vCPU id to query registers.

Run a VM with 16 vCPU, use bcc tool to track the latency of
'hmp_info_registers':
'info registers -a' uses about 3ms;
'info registers 12' uses about 150us.

Cc: Darren Kenny <darren.kenny@oracle.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 hmp-commands-info.hx |  7 ++++---
 monitor/misc.c       | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

Comments

Markus Armbruster Aug. 1, 2022, 1:33 p.m. UTC | #1
zhenwei pi <pizhenwei@bytedance.com> writes:

> Originally we have to get all the vCPU registers and parse the
> specified one. To improve the performance of this usage, allow user
> specified vCPU id to query registers.
>
> Run a VM with 16 vCPU, use bcc tool to track the latency of
> 'hmp_info_registers':
> 'info registers -a' uses about 3ms;
> 'info registers 12' uses about 150us.
>
> Cc: Darren Kenny <darren.kenny@oracle.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  hmp-commands-info.hx |  7 ++++---
>  monitor/misc.c       | 18 ++++++++++++++++++
>  2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 3ffa24bd67..7a00b4ded3 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -100,9 +100,10 @@ ERST
>  
>      {
>          .name       = "registers",
> -        .args_type  = "cpustate_all:-a",
> -        .params     = "[-a]",
> -        .help       = "show the cpu registers (-a: all - show register info for all cpus)",
> +        .args_type  = "cpustate_all:-a,vcpu:i?",
> +        .params     = "[-a|vcpu]",
> +        .help       = "show the cpu registers (-a: all - show register info for all cpus;"
> +                      " vcpu: specific vCPU to query)",

Recommend to document explicitly that it shows the current CPU's
registers when no argument is specified.

>          .cmd        = hmp_info_registers,
>      },
>  
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 3d2312ba8d..8e1d4840f2 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -307,6 +307,7 @@ int monitor_get_cpu_index(Monitor *mon)
>  static void hmp_info_registers(Monitor *mon, const QDict *qdict)
>  {
>      bool all_cpus = qdict_get_try_bool(qdict, "cpustate_all", false);
> +    int vcpu = qdict_get_try_int(qdict, "vcpu", -1);
>      CPUState *cs;
>  
>      if (all_cpus) {
> @@ -314,6 +315,23 @@ static void hmp_info_registers(Monitor *mon, const QDict *qdict)
>              monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index);
>              cpu_dump_state(cs, NULL, CPU_DUMP_FPU);
>          }
> +    } else if (vcpu >= 0) {
> +        CPUState *target_cs = NULL;
> +
> +        CPU_FOREACH(cs) {
> +            if (cs->cpu_index == vcpu) {
> +                target_cs = cs;
> +                break;
> +            }
> +        }

Please use qemu_get_cpu().

> +
> +        if (!target_cs) {
> +            monitor_printf(mon, "CPU#%d not available\n", vcpu);
> +            return;
> +        }
> +
> +        monitor_printf(mon, "\nCPU#%d\n", target_cs->cpu_index);
> +        cpu_dump_state(target_cs, NULL, CPU_DUMP_FPU);

We show the CPU number when the user asked for this number, but ...

>      } else {
>          cs = mon_get_cpu(mon);

           if (!cs) {
               monitor_printf(mon, "No CPU available\n");
               return;
           }

           cpu_dump_state(cs, NULL, CPU_DUMP_FPU);

... we don't show it when the user asked for the current CPU.  It's
arguably more relevant then.

       }
   }

Suggest something like

       } else {
           cs = vcpu >= 0 ? qemu_get_cpu(vcpu) : mon_get_cpu(mon);

           if (!cs) {
               monitor_printf(mon, "CPU#%d not available\n", vcpu);
               return;
           }

           monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index);
           cpu_dump_state(cs, NULL, CPU_DUMP_FPU);
       }
diff mbox series

Patch

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 3ffa24bd67..7a00b4ded3 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -100,9 +100,10 @@  ERST
 
     {
         .name       = "registers",
-        .args_type  = "cpustate_all:-a",
-        .params     = "[-a]",
-        .help       = "show the cpu registers (-a: all - show register info for all cpus)",
+        .args_type  = "cpustate_all:-a,vcpu:i?",
+        .params     = "[-a|vcpu]",
+        .help       = "show the cpu registers (-a: all - show register info for all cpus;"
+                      " vcpu: specific vCPU to query)",
         .cmd        = hmp_info_registers,
     },
 
diff --git a/monitor/misc.c b/monitor/misc.c
index 3d2312ba8d..8e1d4840f2 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -307,6 +307,7 @@  int monitor_get_cpu_index(Monitor *mon)
 static void hmp_info_registers(Monitor *mon, const QDict *qdict)
 {
     bool all_cpus = qdict_get_try_bool(qdict, "cpustate_all", false);
+    int vcpu = qdict_get_try_int(qdict, "vcpu", -1);
     CPUState *cs;
 
     if (all_cpus) {
@@ -314,6 +315,23 @@  static void hmp_info_registers(Monitor *mon, const QDict *qdict)
             monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index);
             cpu_dump_state(cs, NULL, CPU_DUMP_FPU);
         }
+    } else if (vcpu >= 0) {
+        CPUState *target_cs = NULL;
+
+        CPU_FOREACH(cs) {
+            if (cs->cpu_index == vcpu) {
+                target_cs = cs;
+                break;
+            }
+        }
+
+        if (!target_cs) {
+            monitor_printf(mon, "CPU#%d not available\n", vcpu);
+            return;
+        }
+
+        monitor_printf(mon, "\nCPU#%d\n", target_cs->cpu_index);
+        cpu_dump_state(target_cs, NULL, CPU_DUMP_FPU);
     } else {
         cs = mon_get_cpu(mon);