Re: [PATCH] apic: Report current_count via 'info lapic'

2020-02-06 Thread Jan Kiszka 
On 06.02.20 23:36, Philippe Mathieu-Daudé wrote:
> On 2/6/20 8:50 PM, Jan Kiszka wrote:
>> From: Jan Kiszka 
>>
>> 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 
>> ---
>>   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é 

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] apic: Report current_count via 'info lapic'

2020-02-06 Thread Philippe Mathieu-Daudé 

On 2/6/20 8:50 PM, Jan Kiszka wrote:

From: Jan Kiszka 

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 
---
  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é 


+}
+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,

Re: [PATCH] apic: Report current_count via 'info lapic'

2020-02-06 Thread no-reply 
Patchew URL: 
https://patchew.org/QEMU/f6c36298-5e63-f4c6-654c-3b16010ae...@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-3b16010ae...@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-3b16010ae...@siemens.com 
-> patchew/f6c36298-5e63-f4c6-654c-3b16010ae...@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-3b16010ae...@siemens.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH] apic: Report current_count via 'info lapic'

2020-02-06 Thread Jan Kiszka 
From: Jan Kiszka 

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 
---
 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;
+}
+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,
-- 
2.16.4