Re: [Qemu-devel] [PATCH v4 8/9] ppc/xics: Add xics to the monitor "info pic" command

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6007 bytes --]

On Mon, Sep 19, 2016 at 11:59:36AM +0530, Nikunj A Dadhania wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Useful to debug interrupt problems.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  hmp-commands-info.hx  |  2 ++
>  hw/intc/xics.c        | 38 ++++++++++++++++++++++++++++++++++++++
>  hw/ppc/ppc.c          | 14 ++++++++++++++
>  include/hw/ppc/ppc.h  |  1 +
>  include/hw/ppc/xics.h |  4 +++-
>  monitor.c             |  4 ++++
>  6 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 74446c6..0929cb7 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -203,6 +203,8 @@ ETEXI
>          .mhandler.cmd = sun4m_hmp_info_pic,
>  #elif defined(TARGET_LM32)
>          .mhandler.cmd = lm32_hmp_info_pic,
> +#elif defined(TARGET_PPC)
> +        .mhandler.cmd = ppc_hmp_info_pic,

It's bad enough that the existing ones treat what's really a machine
type dependent thing as an arch dependent thing without adding more.

I need to dust off my patches which try to clean up the whole info pic mess.

>  #else
>          .mhandler.cmd = hmp_info_pic,
>  #endif
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index b15751e..4ac2d00 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -36,6 +36,9 @@
>  #include "hw/ppc/xics.h"
>  #include "qemu/error-report.h"
>  #include "qapi/visitor.h"
> +#include "monitor/monitor.h"
> +
> +static XICSState *g_xics;
>  
>  int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
>  {
> @@ -196,6 +199,9 @@ static void xics_common_initfn(Object *obj)
>      object_property_add(obj, "nr_servers", "int",
>                          xics_prop_get_nr_servers, xics_prop_set_nr_servers,
>                          NULL, NULL, NULL);
> +
> +    /* For exclusive use of monitor command */
> +    g_xics = XICS_COMMON(obj);

I don't think you need this, you should be able to get to the xics via
qdev_get_machine().

>  }
>  
>  static void xics_common_class_init(ObjectClass *oc, void *data)
> @@ -677,6 +683,38 @@ static int ics_simple_dispatch_post_load(void *opaque, int version_id)
>      return 0;
>  }
>  
> +void xics_hmp_info_pic(Monitor *mon, const QDict *qdict)
> +{
> +    ICSState *ics;
> +    uint32_t i;
> +
> +    for (i = 0; i < g_xics->nr_servers; i++) {

This will SEGV on machine types which aren't xics based.

> +        ICPState *icp = &g_xics->ss[i];
> +
> +        if (!icp->output) {
> +            continue;
> +        }
> +        monitor_printf(mon, "CPU %d XIRR=%08x (%p) PP=%02x MFRR=%02x\n",
> +                       i, icp->xirr, icp->xirr_owner,
> +                       icp->pending_priority, icp->mfrr);
> +    }
> +    QLIST_FOREACH(ics, &g_xics->ics, list) {
> +        monitor_printf(mon, "ICS %4x..%4x %p\n",
> +                       ics->offset, ics->offset + ics->nr_irqs - 1, ics);
> +        for (i = 0; i < ics->nr_irqs; i++) {
> +            ICSIRQState *irq = ics->irqs + i;
> +
> +            if (!(irq->flags & XICS_FLAGS_IRQ_MASK)) {
> +                continue;
> +            }
> +            monitor_printf(mon, "  %4x %s %02x %02x\n",
> +                           ics->offset + i,
> +                           (irq->flags & XICS_FLAGS_IRQ_LSI) ? "LSI" : "MSI",
> +                           irq->priority, irq->status);
> +        }
> +    }
> +}
> +
>  static const VMStateDescription vmstate_ics_simple_irq = {
>      .name = "ics/irq",
>      .version_id = 2,
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 8945869..bc73428 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -27,6 +27,7 @@
>  #include "hw/hw.h"
>  #include "hw/ppc/ppc.h"
>  #include "hw/ppc/ppc_e500.h"
> +#include "hw/i386/pc.h"
>  #include "qemu/timer.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/cpus.h"
> @@ -39,6 +40,10 @@
>  #include "kvm_ppc.h"
>  #include "trace.h"
>  
> +#if defined(TARGET_PPC64)
> +#include "hw/ppc/xics.h"
> +#endif
> +
>  //#define PPC_DEBUG_IRQ
>  //#define PPC_DEBUG_TB
>  
> @@ -1376,3 +1381,12 @@ void ppc_cpu_parse_features(const char *cpu_model)
>      cc->parse_features(typename, model_pieces[1], &error_fatal);
>      g_strfreev(model_pieces);
>  }
> +
> +void ppc_hmp_info_pic(Monitor *mon, const QDict *qdict)
> +{
> +    /* Call in turn every PIC around. OpenPIC doesn't have one yet */
> +#ifdef TARGET_PPC64
> +    xics_hmp_info_pic(mon, qdict);
> +#endif
> +    hmp_info_pic(mon, qdict);
> +}
> diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
> index 00c1fb1..69c4b19 100644
> --- a/include/hw/ppc/ppc.h
> +++ b/include/hw/ppc/ppc.h
> @@ -3,6 +3,7 @@
>  
>  #include "target-ppc/cpu-qom.h"
>  
> +void ppc_hmp_info_pic(Monitor *mon, const QDict *qdict);
>  void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level);
>  
>  /* PowerPC hardware exceptions management helpers */
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index f81155c..58bb7c6 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -220,4 +220,6 @@ void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
>  ICSState *xics_find_source(XICSState *icp, int irq);
>  void xics_add_ics(XICSState *xics);
>  
> -#endif /* XICS_H */
> +void xics_hmp_info_pic(Monitor *mon, const QDict *qdict);
> +
> +#endif /* __XICS_H__ */
> diff --git a/monitor.c b/monitor.c
> index 5c00373..e9009de 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -90,6 +90,10 @@
>  #include "hw/s390x/storage-keys.h"
>  #endif
>  
> +#if defined(TARGET_PPC)
> +#include "hw/ppc/ppc.h"
> +#endif
> +
>  /*
>   * Supported types:
>   *

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]