[Qemu-devel] [PATCH 0/5] ioapic: support TYPE_INTERRUPT_STATS_PROVIDER

[Qemu-devel] [PATCH 0/5] ioapic: support TYPE_INTERRUPT_STATS_PROVIDER

[Peter Xu]

Let ioapic join the big family.  At last, I obsoleted "info ioapic",
kept the interface but telling people to start use "info pic".  We can
fully remove it after some releases.

Please review, thanks.

Peter Xu (5):
  doc: another fix to "info pic"
  ioapic: support "info pic"
  ioapic: some proper indents when dump info
  ioapic: support "info irq"
  hmp: obsolete "info ioapic"

 hmp-commands-info.hx              |  2 +-
 hw/i386/kvm/ioapic.c              | 11 ++-------
 hw/intc/ioapic.c                  | 12 +---------
 hw/intc/ioapic_common.c           | 47 +++++++++++++++++++++++++++++++++++----
 include/hw/i386/ioapic_internal.h |  3 +++
 include/hw/i386/pc.h              |  3 ---
 target/i386/monitor.c             |  8 ++-----
 7 files changed, 52 insertions(+), 34 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 1/5] doc: another fix to "info pic"

[Peter Xu]

Something that commit 254316fa1f ("intc: make HMP 'info irq' and 'info
pic' commands available on all targets", 2016-10-04) forgot to touch up.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hmp-commands-info.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 54c3e5eac6..4ae2df2f86 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -198,7 +198,7 @@ ETEXI
 STEXI
 @item info pic
 @findex info pic
-Show i8259 (PIC) state.
+Show PIC state.
 ETEXI
 
     {
-- 
2.14.3

[Qemu-devel] [PATCH 2/5] ioapic: support "info pic"

[Peter Xu]

People start to use "info pic" for all kinds of irqchip dumps.  Let x86
ioapic join the family.  It dumps the same thing as "info ioapic".

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/intc/ioapic_common.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 3b3d0a7680..c62ba27018 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -24,6 +24,7 @@
 #include "monitor/monitor.h"
 #include "hw/i386/ioapic.h"
 #include "hw/i386/ioapic_internal.h"
+#include "hw/intc/intc.h"
 #include "hw/sysbus.h"
 
 /* ioapic_no count start from 0 to MAX_IOAPICS,
@@ -142,6 +143,15 @@ static void ioapic_common_realize(DeviceState *dev, Error **errp)
     ioapic_no++;
 }
 
+static void ioapic_print_info(InterruptStatsProvider *obj,
+                              Monitor *mon)
+{
+    IOAPICCommonState *s = IOAPIC_COMMON(obj);
+
+    ioapic_dispatch_pre_save(s);
+    ioapic_print_redtbl(mon, s);
+}
+
 static const VMStateDescription vmstate_ioapic_common = {
     .name = "ioapic",
     .version_id = 3,
@@ -161,9 +171,11 @@ static const VMStateDescription vmstate_ioapic_common = {
 static void ioapic_common_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    InterruptStatsProviderClass *ic = INTERRUPT_STATS_PROVIDER_CLASS(klass);
 
     dc->realize = ioapic_common_realize;
     dc->vmsd = &vmstate_ioapic_common;
+    ic->print_info = ioapic_print_info;
 }
 
 static const TypeInfo ioapic_common_type = {
@@ -173,6 +185,10 @@ static const TypeInfo ioapic_common_type = {
     .class_size = sizeof(IOAPICCommonClass),
     .class_init = ioapic_common_class_init,
     .abstract = true,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_INTERRUPT_STATS_PROVIDER },
+        { }
+    },
 };
 
 static void ioapic_common_register_types(void)
-- 
2.14.3

[Qemu-devel] [PATCH 3/5] ioapic: some proper indents when dump info

[Peter Xu]

So that now it looks better when with other irqchips.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/intc/ioapic_common.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index c62ba27018..a02c135b24 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -59,7 +59,7 @@ void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s)
     uint32_t remote_irr = 0;
     int i;
 
-    monitor_printf(mon, "ioapic ver=0x%x id=0x%02x sel=0x%02x",
+    monitor_printf(mon, "ioapic0: ver=0x%x id=0x%02x sel=0x%02x",
                    s->version, s->id, s->ioregsel);
     if (s->ioregsel) {
         monitor_printf(mon, " (redir[%u])\n",
@@ -71,7 +71,7 @@ void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s)
         uint64_t entry = s->ioredtbl[i];
         uint32_t delm = (uint32_t)((entry & IOAPIC_LVT_DELIV_MODE) >>
                                    IOAPIC_LVT_DELIV_MODE_SHIFT);
-        monitor_printf(mon, "pin %-2u 0x%016"PRIx64" dest=%"PRIx64
+        monitor_printf(mon, "  pin %-2u 0x%016"PRIx64" dest=%"PRIx64
                        " vec=%-3"PRIu64" %s %-5s %-6s %-6s %s\n",
                        i, entry,
                        (entry >> IOAPIC_LVT_DEST_SHIFT) &
@@ -86,8 +86,8 @@ void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s)
         remote_irr |= entry & IOAPIC_LVT_TRIGGER_MODE ?
                         (entry & IOAPIC_LVT_REMOTE_IRR ? (1 << i) : 0) : 0;
     }
-    ioapic_irr_dump(mon, "IRR", s->irr);
-    ioapic_irr_dump(mon, "Remote IRR", remote_irr);
+    ioapic_irr_dump(mon, "  IRR", s->irr);
+    ioapic_irr_dump(mon, "  Remote IRR", remote_irr);
 }
 
 void ioapic_reset_common(DeviceState *dev)
-- 
2.14.3

[Qemu-devel] [PATCH 4/5] ioapic: support "info irq"

[Peter Xu]

This include both userspace and in-kernel ioapic.  Note that the numbers
can be inaccurate for kvm-ioapic.  One reason is the same with
kvm-i8259, that when irqfd is used, irqs can be delivered all inside
kernel without our notice.  Meanwhile, kvm-ioapic is specially treated
when irq numbers <ISA_NUM_IRQS, those irqs will be delivered in kernel
too via kvm-i8259 (please refer to kvm_pc_gsi_handler).

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/kvm/ioapic.c              |  2 ++
 hw/intc/ioapic.c                  |  1 +
 hw/intc/ioapic_common.c           | 23 +++++++++++++++++++++++
 include/hw/i386/ioapic_internal.h |  3 +++
 4 files changed, 29 insertions(+)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 98ca480792..0db9a89bd1 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -132,8 +132,10 @@ static void kvm_ioapic_reset(DeviceState *dev)
 static void kvm_ioapic_set_irq(void *opaque, int irq, int level)
 {
     KVMIOAPICState *s = opaque;
+    IOAPICCommonState *common = IOAPIC_COMMON(s);
     int delivered;
 
+    ioapic_stat_update_irq(common, irq, level);
     delivered = kvm_set_irq(kvm_state, s->kvm_gsi_base + irq, level);
     apic_report_irq_delivered(delivered);
 }
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 36139a4db6..1f0c76571e 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -149,6 +149,7 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
      * the cleanest way of doing it but it should work. */
 
     trace_ioapic_set_irq(vector, level);
+    ioapic_stat_update_irq(s, vector, level);
     if (vector == 0) {
         vector = 2;
     }
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index a02c135b24..692dc37bb6 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -35,6 +35,28 @@
  */
 int ioapic_no;
 
+void ioapic_stat_update_irq(IOAPICCommonState *s, int irq, int level)
+{
+    if (level != s->irq_level[irq]) {
+        s->irq_level[irq] = level;
+        if (level == 1) {
+            s->irq_count[irq]++;
+        }
+    }
+}
+
+static bool ioapic_get_statistics(InterruptStatsProvider *obj,
+                                  uint64_t **irq_counts,
+                                  unsigned int *nb_irqs)
+{
+    IOAPICCommonState *s = IOAPIC_COMMON(obj);
+
+    *irq_counts = s->irq_count;
+    *nb_irqs = IOAPIC_NUM_PINS;
+
+    return true;
+}
+
 static void ioapic_irr_dump(Monitor *mon, const char *name, uint32_t bitmap)
 {
     int i;
@@ -176,6 +198,7 @@ static void ioapic_common_class_init(ObjectClass *klass, void *data)
     dc->realize = ioapic_common_realize;
     dc->vmsd = &vmstate_ioapic_common;
     ic->print_info = ioapic_print_info;
+    ic->get_statistics = ioapic_get_statistics;
 }
 
 static const TypeInfo ioapic_common_type = {
diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
index a11d86de46..9848f391bb 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/i386/ioapic_internal.h
@@ -109,10 +109,13 @@ struct IOAPICCommonState {
     uint64_t ioredtbl[IOAPIC_NUM_PINS];
     Notifier machine_done;
     uint8_t version;
+    uint64_t irq_count[IOAPIC_NUM_PINS];
+    int irq_level[IOAPIC_NUM_PINS];
 };
 
 void ioapic_reset_common(DeviceState *dev);
 
 void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s);
+void ioapic_stat_update_irq(IOAPICCommonState *s, int irq, int level);
 
 #endif /* QEMU_IOAPIC_INTERNAL_H */
-- 
2.14.3

[Qemu-devel] [PATCH 5/5] hmp: obsolete "info ioapic"

[Peter Xu]

Let's start to use "info pic" just like other platforms.  For now we
keep the command for a while so that old users can know what is the new
command to use.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/kvm/ioapic.c  |  9 ---------
 hw/intc/ioapic.c      | 11 -----------
 include/hw/i386/pc.h  |  3 ---
 target/i386/monitor.c |  8 ++------
 4 files changed, 2 insertions(+), 29 deletions(-)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 0db9a89bd1..e6cb468910 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -112,15 +112,6 @@ static void kvm_ioapic_put(IOAPICCommonState *s)
     }
 }
 
-void kvm_ioapic_dump_state(Monitor *mon, const QDict *qdict)
-{
-    IOAPICCommonState *s = IOAPIC_COMMON(object_resolve_path("ioapic", NULL));
-
-    assert(s);
-    kvm_ioapic_get(s);
-    ioapic_print_redtbl(mon, s);
-}
-
 static void kvm_ioapic_reset(DeviceState *dev)
 {
     IOAPICCommonState *s = IOAPIC_COMMON(dev);
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 1f0c76571e..aa7f030971 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -235,17 +235,6 @@ void ioapic_eoi_broadcast(int vector)
     }
 }
 
-void ioapic_dump_state(Monitor *mon, const QDict *qdict)
-{
-    int i;
-
-    for (i = 0; i < MAX_IOAPICS; i++) {
-        if (ioapics[i] != 0) {
-            ioapic_print_redtbl(mon, ioapics[i]);
-        }
-    }
-}
-
 static uint64_t
 ioapic_mem_read(void *opaque, hwaddr addr, unsigned int size)
 {
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6f77eb0665..b9f8a79967 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -169,9 +169,6 @@ int pic_get_output(DeviceState *d);
 
 /* ioapic.c */
 
-void kvm_ioapic_dump_state(Monitor *mon, const QDict *qdict);
-void ioapic_dump_state(Monitor *mon, const QDict *qdict);
-
 /* Global System Interrupts */
 
 #define GSI_NUM_PINS IOAPIC_NUM_PINS
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 75e155ffb1..4ff81ae676 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -652,10 +652,6 @@ void hmp_info_local_apic(Monitor *mon, const QDict *qdict)
 
 void hmp_info_io_apic(Monitor *mon, const QDict *qdict)
 {
-    if (kvm_irqchip_in_kernel() &&
-        !kvm_irqchip_is_split()) {
-        kvm_ioapic_dump_state(mon, qdict);
-    } else {
-        ioapic_dump_state(mon, qdict);
-    }
+    monitor_printf(mon, "This command is obsolete and will be "
+                   "removed soon. Please use 'info pic' instead.\n");
 }
-- 
2.14.3

Re: [Qemu-devel] [PATCH 0/5] ioapic: support TYPE_INTERRUPT_STATS_PROVIDER

[Paolo Bonzini]

On 29/12/2017 08:30, Peter Xu wrote:
> Let ioapic join the big family.  At last, I obsoleted "info ioapic",
> kept the interface but telling people to start use "info pic".  We can
> fully remove it after some releases.
> 
> Please review, thanks.
> 
> Peter Xu (5):
>   doc: another fix to "info pic"
>   ioapic: support "info pic"
>   ioapic: some proper indents when dump info
>   ioapic: support "info irq"
>   hmp: obsolete "info ioapic"
> 
>  hmp-commands-info.hx              |  2 +-
>  hw/i386/kvm/ioapic.c              | 11 ++-------
>  hw/intc/ioapic.c                  | 12 +---------
>  hw/intc/ioapic_common.c           | 47 +++++++++++++++++++++++++++++++++++----
>  include/hw/i386/ioapic_internal.h |  3 +++
>  include/hw/i386/pc.h              |  3 ---
>  target/i386/monitor.c             |  8 ++-----
>  7 files changed, 52 insertions(+), 34 deletions(-)
> 

Queued, better late than never.

Paolo

Re: [Qemu-devel] [PATCH 0/5] ioapic: support TYPE_INTERRUPT_STATS_PROVIDER

[Peter Xu]

On Wed, Jun 27, 2018 at 03:11:10PM +0200, Paolo Bonzini wrote:
> On 29/12/2017 08:30, Peter Xu wrote:
> > Let ioapic join the big family.  At last, I obsoleted "info ioapic",
> > kept the interface but telling people to start use "info pic".  We can
> > fully remove it after some releases.
> > 
> > Please review, thanks.
> > 
> > Peter Xu (5):
> >   doc: another fix to "info pic"
> >   ioapic: support "info pic"
> >   ioapic: some proper indents when dump info
> >   ioapic: support "info irq"
> >   hmp: obsolete "info ioapic"
> > 
> >  hmp-commands-info.hx              |  2 +-
> >  hw/i386/kvm/ioapic.c              | 11 ++-------
> >  hw/intc/ioapic.c                  | 12 +---------
> >  hw/intc/ioapic_common.c           | 47 +++++++++++++++++++++++++++++++++++----
> >  include/hw/i386/ioapic_internal.h |  3 +++
> >  include/hw/i386/pc.h              |  3 ---
> >  target/i386/monitor.c             |  8 ++-----
> >  7 files changed, 52 insertions(+), 34 deletions(-)
> > 
> 
> Queued, better late than never.

Thanks, Paolo. :)

-- 
Peter Xu