diff mbox series

apic: Report current_count via 'info lapic'

Message ID f6c36298-5e63-f4c6-654c-3b16010ae6da@siemens.com
State New
Headers show
Series apic: Report current_count via 'info lapic' | expand

Commit Message

Jan Kiszka Feb. 6, 2020, 7:50 p.m. UTC 
From: Jan Kiszka <jan.kiszka@siemens.com>

This is helpful when debugging stuck guest timers.

As we need apic_get_current_count for that, and it is really not
emulation specific, move it to apic_common.c and export it.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/intc/apic.c                  | 18 ------------------
 hw/intc/apic_common.c           | 18 ++++++++++++++++++
 include/hw/i386/apic_internal.h |  1 +
 target/i386/helper.c            |  5 +++--
 4 files changed, 22 insertions(+), 20 deletions(-)

Comments

no-reply@patchew.org Feb. 6, 2020, 9 p.m. UTC | #1 
Patchew URL: https://patchew.org/QEMU/f6c36298-5e63-f4c6-654c-3b16010ae6da@siemens.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH] apic: Report current_count via 'info lapic'
Message-id: f6c36298-5e63-f4c6-654c-3b16010ae6da@siemens.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/f6c36298-5e63-f4c6-654c-3b16010ae6da@siemens.com -> patchew/f6c36298-5e63-f4c6-654c-3b16010ae6da@siemens.com
Switched to a new branch 'test'
1f19d5c apic: Report current_count via 'info lapic'

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#62: FILE: hw/intc/apic_common.c:202:
+        if (d >= s->initial_count)
[...]
+        else
[...]

total: 1 errors, 0 warnings, 68 lines checked

Commit 1f19d5c51ce3 (apic: Report current_count via 'info lapic') has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/f6c36298-5e63-f4c6-654c-3b16010ae6da@siemens.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Philippe Mathieu-Daudé Feb. 6, 2020, 10:36 p.m. UTC | #2 
On 2/6/20 8:50 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This is helpful when debugging stuck guest timers.
> 
> As we need apic_get_current_count for that, and it is really not
> emulation specific, move it to apic_common.c and export it.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>   hw/intc/apic.c                  | 18 ------------------
>   hw/intc/apic_common.c           | 18 ++++++++++++++++++
>   include/hw/i386/apic_internal.h |  1 +
>   target/i386/helper.c            |  5 +++--
>   4 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index bd40467965..f2207d0ace 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -615,24 +615,6 @@ int apic_accept_pic_intr(DeviceState *dev)
>       return 0;
>   }
>   
> -static uint32_t apic_get_current_count(APICCommonState *s)
> -{
> -    int64_t d;
> -    uint32_t val;
> -    d = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - s->initial_count_load_time) >>
> -        s->count_shift;
> -    if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
> -        /* periodic */
> -        val = s->initial_count - (d % ((uint64_t)s->initial_count + 1));
> -    } else {
> -        if (d >= s->initial_count)
> -            val = 0;
> -        else
> -            val = s->initial_count - d;
> -    }
> -    return val;
> -}
> -
>   static void apic_timer_update(APICCommonState *s, int64_t current_time)
>   {
>       if (apic_next_timer(s, current_time)) {
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 9ec0f2deb2..6f4e877878 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -189,6 +189,24 @@ bool apic_next_timer(APICCommonState *s, int64_t current_time)
>       return true;
>   }
>   
> +uint32_t apic_get_current_count(APICCommonState *s)
> +{
> +    int64_t d;
> +    uint32_t val;
> +    d = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - s->initial_count_load_time) >>
> +        s->count_shift;
> +    if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
> +        /* periodic */
> +        val = s->initial_count - (d % ((uint64_t)s->initial_count + 1));
> +    } else {
> +        if (d >= s->initial_count)
> +            val = 0;
> +        else
> +            val = s->initial_count - d;

Using QEMU style if () {} else {}:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +    }
> +    return val;
> +}
> +
>   void apic_init_reset(DeviceState *dev)
>   {
>       APICCommonState *s;
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index b04bdd947f..2597000e03 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -211,6 +211,7 @@ void vapic_report_tpr_access(DeviceState *dev, CPUState *cpu, target_ulong ip,
>                                TPRAccess access);
>   
>   int apic_get_ppr(APICCommonState *s);
> +uint32_t apic_get_current_count(APICCommonState *s);
>   
>   static inline void apic_set_bit(uint32_t *tab, int index)
>   {
> diff --git a/target/i386/helper.c b/target/i386/helper.c
> index c3a6e4fabe..e3c3726c29 100644
> --- a/target/i386/helper.c
> +++ b/target/i386/helper.c
> @@ -370,10 +370,11 @@ void x86_cpu_dump_local_apic_state(CPUState *cs, int flags)
>       dump_apic_lvt("LVTTHMR", lvt[APIC_LVT_THERMAL], false);
>       dump_apic_lvt("LVTT", lvt[APIC_LVT_TIMER], true);
>   
> -    qemu_printf("Timer\t DCR=0x%x (divide by %u) initial_count = %u\n",
> +    qemu_printf("Timer\t DCR=0x%x (divide by %u) initial_count = %u"
> +                " current_count = %u\n",
>                   s->divide_conf & APIC_DCR_MASK,
>                   divider_conf(s->divide_conf),
> -                s->initial_count);
> +                s->initial_count, apic_get_current_count(s));
>   
>       qemu_printf("SPIV\t 0x%08x APIC %s, focus=%s, spurious vec %u\n",
>                   s->spurious_vec,
>
Jan Kiszka Feb. 7, 2020, 6:41 a.m. UTC | #3 
On 06.02.20 23:36, Philippe Mathieu-Daudé wrote:
> On 2/6/20 8:50 PM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This is helpful when debugging stuck guest timers.
>>
>> As we need apic_get_current_count for that, and it is really not
>> emulation specific, move it to apic_common.c and export it.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>   hw/intc/apic.c                  | 18 ------------------
>>   hw/intc/apic_common.c           | 18 ++++++++++++++++++
>>   include/hw/i386/apic_internal.h |  1 +
>>   target/i386/helper.c            |  5 +++--
>>   4 files changed, 22 insertions(+), 20 deletions(-)
>>
>> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
>> index bd40467965..f2207d0ace 100644
>> --- a/hw/intc/apic.c
>> +++ b/hw/intc/apic.c
>> @@ -615,24 +615,6 @@ int apic_accept_pic_intr(DeviceState *dev)
>>       return 0;
>>   }
>>   -static uint32_t apic_get_current_count(APICCommonState *s)
>> -{
>> -    int64_t d;
>> -    uint32_t val;
>> -    d = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) -
>> s->initial_count_load_time) >>
>> -        s->count_shift;
>> -    if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
>> -        /* periodic */
>> -        val = s->initial_count - (d % ((uint64_t)s->initial_count + 1));
>> -    } else {
>> -        if (d >= s->initial_count)
>> -            val = 0;
>> -        else
>> -            val = s->initial_count - d;
>> -    }
>> -    return val;
>> -}
>> -
>>   static void apic_timer_update(APICCommonState *s, int64_t current_time)
>>   {
>>       if (apic_next_timer(s, current_time)) {
>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>> index 9ec0f2deb2..6f4e877878 100644
>> --- a/hw/intc/apic_common.c
>> +++ b/hw/intc/apic_common.c
>> @@ -189,6 +189,24 @@ bool apic_next_timer(APICCommonState *s, int64_t
>> current_time)
>>       return true;
>>   }
>>   +uint32_t apic_get_current_count(APICCommonState *s)
>> +{
>> +    int64_t d;
>> +    uint32_t val;
>> +    d = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) -
>> s->initial_count_load_time) >>
>> +        s->count_shift;
>> +    if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
>> +        /* periodic */
>> +        val = s->initial_count - (d % ((uint64_t)s->initial_count + 1));
>> +    } else {
>> +        if (d >= s->initial_count)
>> +            val = 0;
>> +        else
>> +            val = s->initial_count - d;
> 
> Using QEMU style if () {} else {}:

Yeah, that happens when you move old code - will address.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks,
Jan
diff mbox series

Patch

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index bd40467965..f2207d0ace 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -615,24 +615,6 @@  int apic_accept_pic_intr(DeviceState *dev)
     return 0;
 }
 
-static uint32_t apic_get_current_count(APICCommonState *s)
-{
-    int64_t d;
-    uint32_t val;
-    d = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - s->initial_count_load_time) >>
-        s->count_shift;
-    if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
-        /* periodic */
-        val = s->initial_count - (d % ((uint64_t)s->initial_count + 1));
-    } else {
-        if (d >= s->initial_count)
-            val = 0;
-        else
-            val = s->initial_count - d;
-    }
-    return val;
-}
-
 static void apic_timer_update(APICCommonState *s, int64_t current_time)
 {
     if (apic_next_timer(s, current_time)) {
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 9ec0f2deb2..6f4e877878 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -189,6 +189,24 @@  bool apic_next_timer(APICCommonState *s, int64_t current_time)
     return true;
 }
 
+uint32_t apic_get_current_count(APICCommonState *s)
+{
+    int64_t d;
+    uint32_t val;
+    d = (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - s->initial_count_load_time) >>
+        s->count_shift;
+    if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
+        /* periodic */
+        val = s->initial_count - (d % ((uint64_t)s->initial_count + 1));
+    } else {
+        if (d >= s->initial_count)
+            val = 0;
+        else
+            val = s->initial_count - d;
+    }
+    return val;
+}
+
 void apic_init_reset(DeviceState *dev)
 {
     APICCommonState *s;
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index b04bdd947f..2597000e03 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -211,6 +211,7 @@  void vapic_report_tpr_access(DeviceState *dev, CPUState *cpu, target_ulong ip,
                              TPRAccess access);
 
 int apic_get_ppr(APICCommonState *s);
+uint32_t apic_get_current_count(APICCommonState *s);
 
 static inline void apic_set_bit(uint32_t *tab, int index)
 {
diff --git a/target/i386/helper.c b/target/i386/helper.c
index c3a6e4fabe..e3c3726c29 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -370,10 +370,11 @@  void x86_cpu_dump_local_apic_state(CPUState *cs, int flags)
     dump_apic_lvt("LVTTHMR", lvt[APIC_LVT_THERMAL], false);
     dump_apic_lvt("LVTT", lvt[APIC_LVT_TIMER], true);
 
-    qemu_printf("Timer\t DCR=0x%x (divide by %u) initial_count = %u\n",
+    qemu_printf("Timer\t DCR=0x%x (divide by %u) initial_count = %u"
+                " current_count = %u\n",
                 s->divide_conf & APIC_DCR_MASK,
                 divider_conf(s->divide_conf),
-                s->initial_count);
+                s->initial_count, apic_get_current_count(s));
 
     qemu_printf("SPIV\t 0x%08x APIC %s, focus=%s, spurious vec %u\n",
                 s->spurious_vec,