[Qemu-devel] [PATCH v2 2/2] hmp: allow apic-id for "info lapic"

Yi Wang posted 2 patches 8 years, 6 months ago
[Qemu-devel] [PATCH v2 2/2] hmp: allow apic-id for "info lapic"
Posted by Yi Wang 8 years, 6 months ago
Add [apic-id] support for hmp command "info lapic", which is
useful when debugging ipi and so on. Current behavior is not
changed when the parameter isn't specified.

Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Signed-off-by: Yun Liu <liu.yunh@zte.com.cn>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hmp-commands-info.hx  |  7 ++++---
 include/qom/cpu.h     | 10 ++++++++++
 qom/cpu.c             | 11 ++++++++---
 target/i386/monitor.c | 10 +++++++++-
 4 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index d9df238..4ab7fce 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -115,9 +115,10 @@ ETEXI
 #if defined(TARGET_I386)
     {
         .name       = "lapic",
-        .args_type  = "",
-        .params     = "",
-        .help       = "show local apic state",
+        .args_type  = "apic-id:i?",
+        .params     = "[apic-id]",
+        .help       = "show local apic state (apic-id: local apic to read, default is which of current CPU)",
+
         .cmd        = hmp_info_local_apic,
     },
 #endif
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 25eefea..b7ac949 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -755,6 +755,16 @@ CPUState *qemu_get_cpu(int index);
 bool cpu_exists(int64_t id);
 
 /**
+ * cpu_by_arch_id:
+ * @id: Guest-exposed CPU ID of the CPU to obtain.
+ *
+ * Get a CPU with matching @id.
+ *
+ * Returns: The CPU or %NULL if there is no matching CPU.
+ */
+CPUState *cpu_by_arch_id(int64_t id);
+
+/**
  * cpu_throttle_set:
  * @new_throttle_pct: Percent of sleep time. Valid range is 1 to 99.
  *
diff --git a/qom/cpu.c b/qom/cpu.c
index 4f38db0..e6210d5 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -34,7 +34,7 @@
 
 CPUInterruptHandler cpu_interrupt_handler;
 
-bool cpu_exists(int64_t id)
+CPUState *cpu_by_arch_id(int64_t id)
 {
     CPUState *cpu;
 
@@ -42,10 +42,15 @@ bool cpu_exists(int64_t id)
         CPUClass *cc = CPU_GET_CLASS(cpu);
 
         if (cc->get_arch_id(cpu) == id) {
-            return true;
+            return cpu;
         }
     }
-    return false;
+    return NULL;
+}
+
+bool cpu_exists(int64_t id)
+{
+    return !!cpu_by_arch_id(id);
 }
 
 CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 77ead60..fe7d57b 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -632,7 +632,15 @@ const MonitorDef *target_monitor_defs(void)
 
 void hmp_info_local_apic(Monitor *mon, const QDict *qdict)
 {
-    CPUState *cs = mon_get_cpu();
+    CPUState *cs;
+
+    if (qdict_haskey(qdict, "apic-id")) {
+        int id = qdict_get_try_int(qdict, "apic-id", 0);
+        cs = cpu_by_arch_id(id);
+    } else {
+        cs = mon_get_cpu();
+    }
+
 
     if (!cs) {
         monitor_printf(mon, "No CPU available\n");
-- 
1.8.3.1



Re: [Qemu-devel] [PATCH v2 2/2] hmp: allow apic-id for "info lapic"
Posted by Eduardo Habkost 8 years, 6 months ago
On Wed, Jul 26, 2017 at 02:18:37AM -0400, Yi Wang wrote:
> Add [apic-id] support for hmp command "info lapic", which is
> useful when debugging ipi and so on. Current behavior is not
> changed when the parameter isn't specified.
> 
> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> Signed-off-by: Yun Liu <liu.yunh@zte.com.cn>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

I believe your sign-off should come last, or it looks like I
signed-off this whole commit.  And I will only sign it off when
merging the patch on my tree.

I suggest sending cpu_by_arch_id() in a separate patch, to let
maintainers cherry-pick and revert individual patches more
easily.  But there's no need to send v3 just because of that, I
can split it while committing.

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  hmp-commands-info.hx  |  7 ++++---
>  include/qom/cpu.h     | 10 ++++++++++
>  qom/cpu.c             | 11 ++++++++---
>  target/i386/monitor.c | 10 +++++++++-
>  4 files changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index d9df238..4ab7fce 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -115,9 +115,10 @@ ETEXI
>  #if defined(TARGET_I386)
>      {
>          .name       = "lapic",
> -        .args_type  = "",
> -        .params     = "",
> -        .help       = "show local apic state",
> +        .args_type  = "apic-id:i?",
> +        .params     = "[apic-id]",
> +        .help       = "show local apic state (apic-id: local apic to read, default is which of current CPU)",
> +
>          .cmd        = hmp_info_local_apic,
>      },
>  #endif
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 25eefea..b7ac949 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -755,6 +755,16 @@ CPUState *qemu_get_cpu(int index);
>  bool cpu_exists(int64_t id);
>  
>  /**
> + * cpu_by_arch_id:
> + * @id: Guest-exposed CPU ID of the CPU to obtain.
> + *
> + * Get a CPU with matching @id.
> + *
> + * Returns: The CPU or %NULL if there is no matching CPU.
> + */
> +CPUState *cpu_by_arch_id(int64_t id);
> +
> +/**
>   * cpu_throttle_set:
>   * @new_throttle_pct: Percent of sleep time. Valid range is 1 to 99.
>   *
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 4f38db0..e6210d5 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -34,7 +34,7 @@
>  
>  CPUInterruptHandler cpu_interrupt_handler;
>  
> -bool cpu_exists(int64_t id)
> +CPUState *cpu_by_arch_id(int64_t id)
>  {
>      CPUState *cpu;
>  
> @@ -42,10 +42,15 @@ bool cpu_exists(int64_t id)
>          CPUClass *cc = CPU_GET_CLASS(cpu);
>  
>          if (cc->get_arch_id(cpu) == id) {
> -            return true;
> +            return cpu;
>          }
>      }
> -    return false;
> +    return NULL;
> +}
> +
> +bool cpu_exists(int64_t id)
> +{
> +    return !!cpu_by_arch_id(id);
>  }
>  
>  CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 77ead60..fe7d57b 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -632,7 +632,15 @@ const MonitorDef *target_monitor_defs(void)
>  
>  void hmp_info_local_apic(Monitor *mon, const QDict *qdict)
>  {
> -    CPUState *cs = mon_get_cpu();
> +    CPUState *cs;
> +
> +    if (qdict_haskey(qdict, "apic-id")) {
> +        int id = qdict_get_try_int(qdict, "apic-id", 0);
> +        cs = cpu_by_arch_id(id);
> +    } else {
> +        cs = mon_get_cpu();
> +    }
> +
>  
>      if (!cs) {
>          monitor_printf(mon, "No CPU available\n");
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo

[Qemu-devel] [PATCH] cpu: cpu_by_arch_id() helper
Posted by Eduardo Habkost 8 years, 6 months ago
The helper can be used for CPU object lookup using the CPU's
arch-specific ID (the one returned by CPUClass::get_arch_id()).

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
[Yi Wang: Added documentation comments]
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Signed-off-by: Yun Liu <liu.yunh@zte.com.cn>
[ehabkost: extracted cpu_by_arch_id() to a separate patch]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
On Wed, Jul 26, 2017 at 02:36:38PM -0300, Eduardo Habkost wrote:
> I suggest sending cpu_by_arch_id() in a separate patch, to let
> maintainers cherry-pick and revert individual patches more
> easily.  But there's no need to send v3 just because of that, I
> can split it while committing.

This is the patch I am queueing for 2.11 on my x86-next branch.

---
 include/qom/cpu.h | 10 ++++++++++
 qom/cpu.c         | 11 ++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 25eefea..b7ac949 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -755,6 +755,16 @@ CPUState *qemu_get_cpu(int index);
 bool cpu_exists(int64_t id);
 
 /**
+ * cpu_by_arch_id:
+ * @id: Guest-exposed CPU ID of the CPU to obtain.
+ *
+ * Get a CPU with matching @id.
+ *
+ * Returns: The CPU or %NULL if there is no matching CPU.
+ */
+CPUState *cpu_by_arch_id(int64_t id);
+
+/**
  * cpu_throttle_set:
  * @new_throttle_pct: Percent of sleep time. Valid range is 1 to 99.
  *
diff --git a/qom/cpu.c b/qom/cpu.c
index 4f38db0..e6210d5 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -34,7 +34,7 @@
 
 CPUInterruptHandler cpu_interrupt_handler;
 
-bool cpu_exists(int64_t id)
+CPUState *cpu_by_arch_id(int64_t id)
 {
     CPUState *cpu;
 
@@ -42,10 +42,15 @@ bool cpu_exists(int64_t id)
         CPUClass *cc = CPU_GET_CLASS(cpu);
 
         if (cc->get_arch_id(cpu) == id) {
-            return true;
+            return cpu;
         }
     }
-    return false;
+    return NULL;
+}
+
+bool cpu_exists(int64_t id)
+{
+    return !!cpu_by_arch_id(id);
 }
 
 CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
-- 
2.9.4

Re: [Qemu-devel] [PATCH v2 2/2] hmp: allow apic-id for "info lapic"
Posted by Eduardo Habkost 8 years, 6 months ago
On Wed, Jul 26, 2017 at 02:18:37AM -0400, Yi Wang wrote:
> Add [apic-id] support for hmp command "info lapic", which is
> useful when debugging ipi and so on. Current behavior is not
> changed when the parameter isn't specified.
> 
> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> Signed-off-by: Yun Liu <liu.yunh@zte.com.cn>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Queued for 2.11 on x86-next.  Thanks.

-- 
Eduardo