[Qemu-devel] [PATCH v5 0/9] ppc/pnv: interrupt controller (POWER8)

[Qemu-devel] [PATCH v5 0/9] ppc/pnv: interrupt controller (POWER8)

[Cédric Le Goater]

Hello,

Here is a series adding support for the interrupt controller as found
on a POWER8 system. POWER9 uses a different interrupt controller
called XIVE, still to be worked on.

The initial patches are more cleanups of the XICS layer which move the
IRQ 'server' number mapping under the machine handlers.

A new PnvICPState object based on MMIOs, which is specific to PowerNV,
is introduced in XICS. These ICP objects are created for each thread
of a core and linked to the associated PowerPCCPU object.

Finally, to make use of the XICS layer, the PowerNV machine is
extended with a QOM XICSFabric interface and with a global memory
region acting as the Interrupt Management area.


To test, grab a kernel and a rootfs image here :

  https://openpower.xyz/job/openpower-op-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/zImage.epapr
  https://openpower.xyz/job/openpower-op-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/rootfs.cpio.xz

The full patchset is available here :

   https://github.com/legoater/qemu/commits/powernv-ipmi-2.9

Thanks,

C.

Changes since v4:
 - reworked a bit the order of the patches for make check-qtest-ppc64
   to pass. The machine needs to be a XICSFabric before the PnvCore
   use it.
 - allocated the ICPState object from under sPAPRCPUCore and removed
   xics_get_cpu_index_by_dt_id()
 - moved the creation of PnvICPState object before the PowerPCCPU
   object is realized to handle correctly errors.
 - removed the empty ics_get() and ics_resend() handlers. They will be
   added by the PSI HB model

Changes since v3:
 - renamed 'icp' backlink to a more generic name 'intc'
 - removed the array of ICP objects from under the PowerNV machine and
   handled the allocation of the PnvICPState object for each thread
   when the PowerPCCPU object is realized.

Changes since v2:

 - removed the ICS list from the PowerNV machine
 - changed the 'icp' backlink type to be an 'Object'

Changes since v1:

 - introduced PnvICPState to hold the ICP memory region
 - handled pir-to-cpu_index mapping under the machine icp_get handler
 - added multichip support
 - removed ics_eoi handler (came from a bug in PHB3_MSI)
 - kept PSI and OCC model for later, when this part is done.

Cédric Le Goater (9):
  spapr: move the IRQ server number mapping under the machine
  spapr: allocate the ICPState object from under sPAPRCPUCore
  ppc/xics: add a realize() handler to ICPStateClass
  ppc/pnv: add a PnvICPState object
  ppc/pnv: extend the machine with a XICSFabric interface
  ppc/pnv: extend the machine with a InterruptStatsProvider interface
  ppc/pnv: create the ICP object under PnvCore
  ppc/pnv: add a helper to calculate MMIO addresses registers
  ppc/pnv: add memory regions for the ICP registers

 hw/intc/Makefile.objs   |   1 +
 hw/intc/xics.c          |  16 ++--
 hw/intc/xics_pnv.c      | 192 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/intc/xics_spapr.c    |   5 +-
 hw/ppc/pnv.c            | 130 ++++++++++++++++++++++++++++++++
 hw/ppc/pnv_core.c       |  27 ++++++-
 hw/ppc/spapr.c          |  48 ++++--------
 hw/ppc/spapr_cpu_core.c |  18 ++++-
 include/hw/ppc/pnv.h    |  31 +++++++-
 include/hw/ppc/spapr.h  |   2 +-
 include/hw/ppc/xics.h   |  15 +++-
 11 files changed, 428 insertions(+), 57 deletions(-)
 create mode 100644 hw/intc/xics_pnv.c

-- 
2.7.4

[Qemu-devel] [PATCH v5 1/9] spapr: move the IRQ server number mapping under the machine

[Cédric Le Goater]

This is the second step to abstract the IRQ 'server' number of the
XICS layer. Now that the prereq cleanups have been done in the
previous patch, we can move down the 'cpu_dt_id' to 'cpu_index'
mapping in the sPAPR machine handler.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/xics_spapr.c    | 5 ++---
 hw/ppc/spapr.c          | 3 ++-
 hw/ppc/spapr_cpu_core.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 58f100d379cb..f05308b897f2 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -52,9 +52,8 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           target_ulong opcode, target_ulong *args)
 {
-    target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
     target_ulong mfrr = args[1];
-    ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), server);
+    ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
 
     if (!icp) {
         return H_PARAMETER;
@@ -122,7 +121,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     }
 
     nr = rtas_ld(args, 0);
-    server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
+    server = rtas_ld(args, 1);
     priority = rtas_ld(args, 2);
 
     if (!ics_valid_irq(ics, nr) || !xics_icp_get(XICS_FABRIC(spapr), server)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 44c26e4be86c..3358dc9ba638 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3024,9 +3024,10 @@ static void spapr_ics_resend(XICSFabric *dev)
     ics_resend(spapr->ics);
 }
 
-static ICPState *spapr_icp_get(XICSFabric *xi, int server)
+static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
+    int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
 
     return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
 }
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 7db61bd72476..4e1a99591d19 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -64,7 +64,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
 {
     CPUPPCState *env = &cpu->env;
     XICSFabric *xi = XICS_FABRIC(spapr);
-    ICPState *icp = xics_icp_get(xi, CPU(cpu)->cpu_index);
+    ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
 
     /* Set time-base frequency to 512 MHz */
     cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
-- 
2.7.4

[Qemu-devel] [PATCH v5 2/9] spapr: allocate the ICPState object from under sPAPRCPUCore

[Cédric Le Goater]

Today, all the ICPs are created before the CPUs, stored in an array
under the sPAPR machine and linked to the CPU when the core threads
are realized. This modeling brings some complexity when a lookup in
the array is required and it can be simplified by allocating the ICPs
when the CPUs are.

This is the purpose of this proposal which introduces a new 'icp_type'
field under the machine and creates the ICP objects of the right type
(KVM or not) before the PowerPCCPU object are.

This change allows more cleanups : the removal of the icps array under
the sPAPR machine and the removal of the xics_get_cpu_index_by_dt_id()
helper.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/xics.c          | 11 -----------
 hw/ppc/spapr.c          | 47 ++++++++++++++---------------------------------
 hw/ppc/spapr_cpu_core.c | 18 ++++++++++++++----
 include/hw/ppc/spapr.h  |  2 +-
 include/hw/ppc/xics.h   |  2 --
 5 files changed, 29 insertions(+), 51 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 56fe70cd10e9..d4428b41b03a 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -38,17 +38,6 @@
 #include "monitor/monitor.h"
 #include "hw/intc/intc.h"
 
-int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
-{
-    PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
-
-    if (cpu) {
-        return cpu->parent_obj.cpu_index;
-    }
-
-    return -1;
-}
-
 void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3358dc9ba638..e5a6722daed7 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -103,7 +103,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
     XICSFabric *xi = XICS_FABRIC(spapr);
     Error *err = NULL, *local_err = NULL;
     ICSState *ics = NULL;
-    int i;
 
     ics = ICS_SIMPLE(object_new(type_ics));
     object_property_add_child(OBJECT(spapr), "ics", OBJECT(ics), NULL);
@@ -112,34 +111,14 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
     object_property_set_bool(OBJECT(ics), true, "realized", &local_err);
     error_propagate(&err, local_err);
     if (err) {
-        goto error;
+        error_propagate(errp, err);
+        return -1;
     }
 
-    spapr->icps = g_malloc0(nr_servers * sizeof(ICPState));
     spapr->nr_servers = nr_servers;
-
-    for (i = 0; i < nr_servers; i++) {
-        ICPState *icp = &spapr->icps[i];
-
-        object_initialize(icp, sizeof(*icp), type_icp);
-        object_property_add_child(OBJECT(spapr), "icp[*]", OBJECT(icp), NULL);
-        object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xi), NULL);
-        object_property_set_bool(OBJECT(icp), true, "realized", &err);
-        if (err) {
-            goto error;
-        }
-        object_unref(OBJECT(icp));
-    }
-
     spapr->ics = ics;
+    spapr->icp_type = type_icp;
     return 0;
-
-error:
-    error_propagate(errp, err);
-    if (ics) {
-        object_unparent(OBJECT(ics));
-    }
-    return -1;
 }
 
 static int xics_system_init(MachineState *machine,
@@ -1366,9 +1345,10 @@ static int spapr_post_load(void *opaque, int version_id)
     int err = 0;
 
     if (!object_dynamic_cast(OBJECT(spapr->ics), TYPE_ICS_KVM)) {
-        int i;
-        for (i = 0; i < spapr->nr_servers; i++) {
-            icp_resend(&spapr->icps[i]);
+        CPUState *cs;
+        CPU_FOREACH(cs) {
+            PowerPCCPU *cpu = POWERPC_CPU(cs);
+            icp_resend(ICP(cpu->intc));
         }
     }
 
@@ -3026,20 +3006,21 @@ static void spapr_ics_resend(XICSFabric *dev)
 
 static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
 {
-    sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
-    int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
+    PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
 
-    return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
+    return cpu ? ICP(cpu->intc) : NULL;
 }
 
 static void spapr_pic_print_info(InterruptStatsProvider *obj,
                                  Monitor *mon)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
-    int i;
+    CPUState *cs;
+
+    CPU_FOREACH(cs) {
+        PowerPCCPU *cpu = POWERPC_CPU(cs);
 
-    for (i = 0; i < spapr->nr_servers; i++) {
-        icp_pic_print_info(&spapr->icps[i], mon);
+        icp_pic_print_info(ICP(cpu->intc), mon);
     }
 
     ics_pic_print_info(spapr->ics, mon);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 4e1a99591d19..2e689b542de3 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -63,8 +63,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
                            Error **errp)
 {
     CPUPPCState *env = &cpu->env;
-    XICSFabric *xi = XICS_FABRIC(spapr);
-    ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
 
     /* Set time-base frequency to 512 MHz */
     cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
@@ -82,8 +80,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
         }
     }
 
-    xics_cpu_setup(xi, cpu, icp);
-
     qemu_register_reset(spapr_cpu_reset, cpu);
     spapr_cpu_reset(cpu);
 }
@@ -143,18 +139,32 @@ static void spapr_cpu_core_realize_child(Object *child, Error **errp)
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
     CPUState *cs = CPU(child);
     PowerPCCPU *cpu = POWERPC_CPU(cs);
+    Object *obj;
+
+    obj = object_new(spapr->icp_type);
+    object_property_add_child(OBJECT(cpu), "icp", obj, NULL);
+    object_property_add_const_link(obj, "xics", OBJECT(spapr), &error_abort);
+    object_property_set_bool(obj, true, "realized", &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     object_property_set_bool(child, true, "realized", &local_err);
     if (local_err) {
+        object_unparent(obj);
         error_propagate(errp, local_err);
         return;
     }
 
     spapr_cpu_init(spapr, cpu, &local_err);
     if (local_err) {
+        object_unparent(obj);
         error_propagate(errp, local_err);
         return;
     }
+
+    xics_cpu_setup(XICS_FABRIC(spapr), cpu, ICP(obj));
 }
 
 static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 808aac870359..db3d4acb18a6 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -96,7 +96,7 @@ struct sPAPRMachineState {
     MemoryHotplugState hotplug_memory;
 
     uint32_t nr_servers;
-    ICPState *icps;
+    const char *icp_type;
 };
 
 #define H_SUCCESS         0
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 5e0244447fcd..88e0f021b168 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -172,8 +172,6 @@ void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu, ICPState *icp);
 void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu);
 
 /* Internal XICS interfaces */
-int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
-
 void icp_set_cppr(ICPState *icp, uint8_t cppr);
 void icp_set_mfrr(ICPState *icp, uint8_t mfrr);
 uint32_t icp_accept(ICPState *ss);
-- 
2.7.4

[Qemu-devel] [PATCH v5 3/9] ppc/xics: add a realize() handler to ICPStateClass

[Cédric Le Goater]

It will be used by derived classes in PowerNV for customization.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/xics.c        | 5 +++++
 include/hw/ppc/xics.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index d4428b41b03a..292fffecd376 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -337,6 +337,7 @@ static void icp_reset(void *dev)
 static void icp_realize(DeviceState *dev, Error **errp)
 {
     ICPState *icp = ICP(dev);
+    ICPStateClass *icpc = ICP_GET_CLASS(dev);
     Object *obj;
     Error *err = NULL;
 
@@ -349,6 +350,10 @@ static void icp_realize(DeviceState *dev, Error **errp)
 
     icp->xics = XICS_FABRIC(obj);
 
+    if (icpc->realize) {
+        icpc->realize(dev, errp);
+    }
+
     qemu_register_reset(icp_reset, dev);
 }
 
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 88e0f021b168..05bb10a4ce8f 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -60,6 +60,7 @@ typedef struct XICSFabric XICSFabric;
 struct ICPStateClass {
     DeviceClass parent_class;
 
+    void (*realize)(DeviceState *dev, Error **errp);
     void (*pre_save)(ICPState *s);
     int (*post_load)(ICPState *s, int version_id);
     void (*cpu_setup)(ICPState *icp, PowerPCCPU *cpu);
-- 
2.7.4

[Qemu-devel] [PATCH v5 4/9] ppc/pnv: add a PnvICPState object

[Cédric Le Goater]

This provides a new ICPState object for the PowerNV machine (POWER8).
Access to the Interrupt Management area is done though a memory
region. It contains the registers of the Interrupt Control Presenters
of each thread which are used to accept, return, forward interrupts in
the system.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---

 Changes since v1:

 - moved the memory region from PnvCore to a specific PnvICPState object

 hw/intc/Makefile.objs |   1 +
 hw/intc/xics_pnv.c    | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/xics.h |  12 ++++
 3 files changed, 205 insertions(+)
 create mode 100644 hw/intc/xics_pnv.c

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index adedd0da5fd8..78426a7dafcd 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -35,6 +35,7 @@ obj-$(CONFIG_SH4) += sh_intc.o
 obj-$(CONFIG_XICS) += xics.o
 obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
 obj-$(CONFIG_XICS_KVM) += xics_kvm.o
+obj-$(CONFIG_POWERNV) += xics_pnv.o
 obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
 obj-$(CONFIG_S390_FLIC) += s390_flic.o
 obj-$(CONFIG_S390_FLIC_KVM) += s390_flic_kvm.o
diff --git a/hw/intc/xics_pnv.c b/hw/intc/xics_pnv.c
new file mode 100644
index 000000000000..12ae605f10e8
--- /dev/null
+++ b/hw/intc/xics_pnv.c
@@ -0,0 +1,192 @@
+/*
+ * QEMU PowerPC PowerNV Interrupt Control Presenter (ICP) model
+ *
+ * Copyright (c) 2017, IBM Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "hw/ppc/xics.h"
+
+#define ICP_XIRR_POLL    0 /* 1 byte (CPRR) or 4 bytes */
+#define ICP_XIRR         4 /* 1 byte (CPRR) or 4 bytes */
+#define ICP_MFRR        12 /* 1 byte access only */
+
+#define ICP_LINKA       16 /* unused */
+#define ICP_LINKB       20 /* unused */
+#define ICP_LINKC       24 /* unused */
+
+static uint64_t pnv_icp_read(void *opaque, hwaddr addr, unsigned width)
+{
+    ICPState *icp = ICP(opaque);
+    PnvICPState *picp = PNV_ICP(opaque);
+    bool byte0 = (width == 1 && (addr & 0x3) == 0);
+    uint64_t val = 0xffffffff;
+
+    switch (addr & 0xffc) {
+    case ICP_XIRR_POLL:
+        val = icp_ipoll(icp, NULL);
+        if (byte0) {
+            val >>= 24;
+        } else if (width != 4) {
+            goto bad_access;
+        }
+        break;
+    case ICP_XIRR:
+        if (byte0) {
+            val = icp_ipoll(icp, NULL) >> 24;
+        } else if (width == 4) {
+            val = icp_accept(icp);
+        } else {
+            goto bad_access;
+        }
+        break;
+    case ICP_MFRR:
+        if (byte0) {
+            val = icp->mfrr;
+        } else {
+            goto bad_access;
+        }
+        break;
+    case ICP_LINKA:
+        if (width == 4) {
+            val = picp->links[0];
+        } else {
+            goto bad_access;
+        }
+        break;
+    case ICP_LINKB:
+        if (width == 4) {
+            val = picp->links[1];
+        } else {
+            goto bad_access;
+        }
+        break;
+    case ICP_LINKC:
+        if (width == 4) {
+            val = picp->links[2];
+        } else {
+            goto bad_access;
+        }
+        break;
+    default:
+bad_access:
+        qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP access 0x%"
+                      HWADDR_PRIx"/%d\n", addr, width);
+    }
+
+    return val;
+}
+
+static void pnv_icp_write(void *opaque, hwaddr addr, uint64_t val,
+                              unsigned width)
+{
+    ICPState *icp = ICP(opaque);
+    PnvICPState *picp = PNV_ICP(opaque);
+    bool byte0 = (width == 1 && (addr & 0x3) == 0);
+
+    switch (addr & 0xffc) {
+    case ICP_XIRR:
+        if (byte0) {
+            icp_set_cppr(icp, val);
+        } else if (width == 4) {
+            icp_eoi(icp, val);
+        } else {
+            goto bad_access;
+        }
+        break;
+    case ICP_MFRR:
+        if (byte0) {
+            icp_set_mfrr(icp, val);
+        } else {
+            goto bad_access;
+        }
+        break;
+    case ICP_LINKA:
+        if (width == 4) {
+            picp->links[0] = val;
+        } else {
+            goto bad_access;
+        }
+        break;
+    case ICP_LINKB:
+        if (width == 4) {
+            picp->links[1] = val;
+        } else {
+            goto bad_access;
+        }
+        break;
+    case ICP_LINKC:
+        if (width == 4) {
+            picp->links[2] = val;
+        } else {
+            goto bad_access;
+        }
+        break;
+    default:
+bad_access:
+        qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP access 0x%"
+                      HWADDR_PRIx"/%d\n", addr, width);
+    }
+}
+
+static const MemoryRegionOps pnv_icp_ops = {
+    .read = pnv_icp_read,
+    .write = pnv_icp_write,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+};
+
+static void pnv_icp_realize(DeviceState *dev, Error **errp)
+{
+    PnvICPState *icp = PNV_ICP(dev);
+
+    memory_region_init_io(&icp->mmio, OBJECT(dev), &pnv_icp_ops,
+                          icp, "icp-thread", 0x1000);
+}
+
+static void pnv_icp_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ICPStateClass *icpc = ICP_CLASS(klass);
+
+    icpc->realize = pnv_icp_realize;
+    dc->desc = "PowerNV ICP";
+}
+
+static const TypeInfo pnv_icp_info = {
+    .name          = TYPE_PNV_ICP,
+    .parent        = TYPE_ICP,
+    .instance_size = sizeof(PnvICPState),
+    .class_init    = pnv_icp_class_init,
+    .class_size    = sizeof(ICPStateClass),
+};
+
+static void pnv_icp_register_types(void)
+{
+    type_register_static(&pnv_icp_info);
+}
+
+type_init(pnv_icp_register_types)
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 05bb10a4ce8f..e6699c917e24 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -41,10 +41,12 @@
  */
 typedef struct ICPStateClass ICPStateClass;
 typedef struct ICPState ICPState;
+typedef struct PnvICPState PnvICPState;
 typedef struct ICSStateClass ICSStateClass;
 typedef struct ICSState ICSState;
 typedef struct ICSIRQState ICSIRQState;
 typedef struct XICSFabric XICSFabric;
+typedef struct PowerPCCPU PowerPCCPU;
 
 #define TYPE_ICP "icp"
 #define ICP(obj) OBJECT_CHECK(ICPState, (obj), TYPE_ICP)
@@ -52,6 +54,9 @@ typedef struct XICSFabric XICSFabric;
 #define TYPE_KVM_ICP "icp-kvm"
 #define KVM_ICP(obj) OBJECT_CHECK(ICPState, (obj), TYPE_KVM_ICP)
 
+#define TYPE_PNV_ICP "pnv-icp"
+#define PNV_ICP(obj) OBJECT_CHECK(PnvICPState, (obj), TYPE_PNV_ICP)
+
 #define ICP_CLASS(klass) \
      OBJECT_CLASS_CHECK(ICPStateClass, (klass), TYPE_ICP)
 #define ICP_GET_CLASS(obj) \
@@ -81,6 +86,13 @@ struct ICPState {
     XICSFabric *xics;
 };
 
+struct PnvICPState {
+    ICPState parent_obj;
+
+    MemoryRegion mmio;
+    uint32_t links[3];
+};
+
 #define TYPE_ICS_BASE "ics-base"
 #define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_BASE)
 
-- 
2.7.4

[Qemu-devel] [PATCH v5 5/9] ppc/pnv: extend the machine with a XICSFabric interface

[Cédric Le Goater]

A XICSFabric QOM interface is used by the XICS layer to manipulate the
ICP and ICS objects. Let's define the associated handlers for the
PowerNV machine. All handlers should be defined even if there is no
ICS under the PowerNV machine yet.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 Changes since v4:

 - removed the empty ics_get() and ics_resend() handlers

 hw/ppc/pnv.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 3fa722af82e6..2d7aa5dcfbca 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -34,6 +34,7 @@
 #include "qemu/cutils.h"
 #include "qapi/visitor.h"
 
+#include "hw/ppc/xics.h"
 #include "hw/ppc/pnv_xscom.h"
 
 #include "hw/isa/isa.h"
@@ -737,6 +738,29 @@ static const TypeInfo pnv_chip_info = {
     .abstract      = true,
 };
 
+static PowerPCCPU *ppc_get_vcpu_by_pir(int pir)
+{
+    CPUState *cs;
+
+    CPU_FOREACH(cs) {
+        PowerPCCPU *cpu = POWERPC_CPU(cs);
+        CPUPPCState *env = &cpu->env;
+
+        if (env->spr_cb[SPR_PIR].default_value == pir) {
+            return cpu;
+        }
+    }
+
+    return NULL;
+}
+
+static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
+{
+    PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir);
+
+    return cpu ? ICP(cpu->intc) : NULL;
+}
+
 static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
                               void *opaque, Error **errp)
 {
@@ -787,6 +811,7 @@ static void powernv_machine_class_props_init(ObjectClass *oc)
 static void powernv_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
 
     mc->desc = "IBM PowerNV (Non-Virtualized)";
     mc->init = ppc_powernv_init;
@@ -797,6 +822,7 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
     mc->no_parallel = 1;
     mc->default_boot_order = NULL;
     mc->default_ram_size = 1 * G_BYTE;
+    xic->icp_get = pnv_icp_get;
 
     powernv_machine_class_props_init(oc);
 }
@@ -807,6 +833,10 @@ static const TypeInfo powernv_machine_info = {
     .instance_size = sizeof(PnvMachineState),
     .instance_init = powernv_machine_initfn,
     .class_init    = powernv_machine_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_XICS_FABRIC },
+        { },
+    },
 };
 
 static void powernv_machine_register_types(void)
-- 
2.7.4

[Qemu-devel] [PATCH v5 6/9] ppc/pnv: extend the machine with a InterruptStatsProvider interface

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 2d7aa5dcfbca..1c0672cbeb40 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -33,6 +33,8 @@
 #include "exec/address-spaces.h"
 #include "qemu/cutils.h"
 #include "qapi/visitor.h"
+#include "monitor/monitor.h"
+#include "hw/intc/intc.h"
 
 #include "hw/ppc/xics.h"
 #include "hw/ppc/pnv_xscom.h"
@@ -761,6 +763,18 @@ static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
     return cpu ? ICP(cpu->intc) : NULL;
 }
 
+static void pnv_pic_print_info(InterruptStatsProvider *obj,
+                               Monitor *mon)
+{
+    CPUState *cs;
+
+    CPU_FOREACH(cs) {
+        PowerPCCPU *cpu = POWERPC_CPU(cs);
+
+        icp_pic_print_info(ICP(cpu->intc), mon);
+    }
+}
+
 static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
                               void *opaque, Error **errp)
 {
@@ -812,6 +826,7 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
     XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
+    InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
 
     mc->desc = "IBM PowerNV (Non-Virtualized)";
     mc->init = ppc_powernv_init;
@@ -823,6 +838,7 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
     mc->default_boot_order = NULL;
     mc->default_ram_size = 1 * G_BYTE;
     xic->icp_get = pnv_icp_get;
+    ispc->print_info = pnv_pic_print_info;
 
     powernv_machine_class_props_init(oc);
 }
@@ -835,6 +851,7 @@ static const TypeInfo powernv_machine_info = {
     .class_init    = powernv_machine_class_init,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_XICS_FABRIC },
+        { TYPE_INTERRUPT_STATS_PROVIDER },
         { },
     },
 };
-- 
2.7.4

[Qemu-devel] [PATCH v5 7/9] ppc/pnv: create the ICP object under PnvCore

[Cédric Le Goater]

Each thread of a core is linked to an ICP. This allocates a PnvICPState
object before the PowerPCCPU object is realized and lets the XICSFabric
do the store under the 'intc' backlink when xics_cpu_setup() is
called.

This modeling removes the need of maintaining an array of ICP objects
under the PowerNV machine and also simplifies the XICSFabric icp_get()
handler.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---

 Changes since v4:

 - moved the creation of PnvICPState object before the PowerPCCPU
   object is realized to handle correctly errors.
 
 Changes since v3:
 
 - removed the array of ICP objects from under the PowerNV machine and
   handled the allocation of the PnvICPState object for each thread
   when the PowerPCCPU object is realized.

 hw/ppc/pnv.c      |  2 ++
 hw/ppc/pnv_core.c | 27 +++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 1c0672cbeb40..ab079ac60af5 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -694,6 +694,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
         object_property_set_int(OBJECT(pnv_core),
                                 pcc->core_pir(chip, core_hwid),
                                 "pir", &error_fatal);
+        object_property_add_const_link(OBJECT(pnv_core), "xics",
+                                       qdev_get_machine(), &error_fatal);
         object_property_set_bool(OBJECT(pnv_core), true, "realized",
                                  &error_fatal);
         object_unref(OBJECT(pnv_core));
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index d79d530b4881..1b7ec70f033d 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -25,6 +25,7 @@
 #include "hw/ppc/pnv.h"
 #include "hw/ppc/pnv_core.h"
 #include "hw/ppc/pnv_xscom.h"
+#include "hw/ppc/xics.h"
 
 static void powernv_cpu_reset(void *opaque)
 {
@@ -110,23 +111,37 @@ static const MemoryRegionOps pnv_core_xscom_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
-static void pnv_core_realize_child(Object *child, Error **errp)
+static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp)
 {
     Error *local_err = NULL;
     CPUState *cs = CPU(child);
     PowerPCCPU *cpu = POWERPC_CPU(cs);
+    Object *obj;
+
+    obj = object_new(TYPE_PNV_ICP);
+    object_property_add_child(OBJECT(cpu), "icp", obj, NULL);
+    object_property_add_const_link(obj, "xics", OBJECT(xi), &error_abort);
+    object_property_set_bool(obj, true, "realized", &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
 
     object_property_set_bool(child, true, "realized", &local_err);
     if (local_err) {
+        object_unparent(obj);
         error_propagate(errp, local_err);
         return;
     }
 
     powernv_cpu_init(cpu, &local_err);
     if (local_err) {
+        object_unparent(obj);
         error_propagate(errp, local_err);
         return;
     }
+
+    xics_cpu_setup(xi, cpu, ICP(obj));
 }
 
 static void pnv_core_realize(DeviceState *dev, Error **errp)
@@ -140,6 +155,14 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
     void *obj;
     int i, j;
     char name[32];
+    Object *xi;
+
+    xi = object_property_get_link(OBJECT(dev), "xics", &local_err);
+    if (!xi) {
+        error_setg(errp, "%s: required link 'xics' not found: %s",
+                   __func__, error_get_pretty(local_err));
+        return;
+    }
 
     pc->threads = g_malloc0(size * cc->nr_threads);
     for (i = 0; i < cc->nr_threads; i++) {
@@ -160,7 +183,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
     for (j = 0; j < cc->nr_threads; j++) {
         obj = pc->threads + j * size;
 
-        pnv_core_realize_child(obj, &local_err);
+        pnv_core_realize_child(obj, XICS_FABRIC(xi), &local_err);
         if (local_err) {
             goto err;
         }
-- 
2.7.4

[Qemu-devel] [PATCH v5 8/9] ppc/pnv: add a helper to calculate MMIO addresses registers

[Cédric Le Goater]

Some controllers (ICP, PSI) have a base register address which is
calculated using the chip id.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 include/hw/ppc/pnv.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index df98a72006e4..5693ba181d24 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -91,14 +91,24 @@ typedef struct PnvChipClass {
     OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER9)
 
 /*
- * This generates a HW chip id depending on an index:
+ * This generates a HW chip id depending on an index, as found on a
+ * two socket system with dual chip modules :
  *
  *    0x0, 0x1, 0x10, 0x11
  *
  * 4 chips should be the maximum
+ *
+ * TODO: use a machine property to define the chip ids
  */
 #define PNV_CHIP_HWID(i) ((((i) & 0x3e) << 3) | ((i) & 0x1))
 
+/*
+ * Converts back a HW chip id to an index. This is useful to calculate
+ * the MMIO addresses of some controllers which depend on the chip id.
+ */
+#define PNV_CHIP_INDEX(chip)                                    \
+    (((chip)->chip_id >> 2) * 2 + ((chip)->chip_id & 0x3))
+
 #define TYPE_POWERNV_MACHINE       MACHINE_TYPE_NAME("powernv")
 #define POWERNV_MACHINE(obj) \
     OBJECT_CHECK(PnvMachineState, (obj), TYPE_POWERNV_MACHINE)
-- 
2.7.4

[Qemu-devel] [PATCH v5 9/9] ppc/pnv: add memory regions for the ICP registers

[Cédric Le Goater]

This provides to a PowerNV chip (POWER8) access to the Interrupt
Management area, which contains the registers of the Interrupt Control
Presenters of each thread. These are used to accept, return, forward
interrupts in the system.

This area is modeled with a per-chip container memory region holding
all the ICP registers. Each thread of a chip is then associated with
its ICP registers using a memory subregion indexed by its PIR number
in the overall region.

The device tree is populated accordingly.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 Changes since v1:

 - added multichip support
 - adapted to use PnvICPState object

 hw/ppc/pnv.c         | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/pnv.h | 19 ++++++++++++
 2 files changed, 100 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index ab079ac60af5..903bbac03fcc 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -218,6 +218,43 @@ static void powernv_create_core_node(PnvChip *chip, PnvCore *pc, void *fdt)
                        servers_prop, sizeof(servers_prop))));
 }
 
+static void powernv_populate_icp(PnvChip *chip, void *fdt, uint32_t pir,
+                                 uint32_t nr_threads)
+{
+    uint64_t addr = PNV_ICP_BASE(chip) | (pir << 12);
+    char *name;
+    const char compat[] = "IBM,power8-icp\0IBM,ppc-xicp";
+    uint32_t irange[2], i, rsize;
+    uint64_t *reg;
+    int offset;
+
+    irange[0] = cpu_to_be32(pir);
+    irange[1] = cpu_to_be32(nr_threads);
+
+    rsize = sizeof(uint64_t) * 2 * nr_threads;
+    reg = g_malloc(rsize);
+    for (i = 0; i < nr_threads; i++) {
+        reg[i * 2] = cpu_to_be64(addr | ((pir + i) * 0x1000));
+        reg[i * 2 + 1] = cpu_to_be64(0x1000);
+    }
+
+    name = g_strdup_printf("interrupt-controller@%"PRIX64, addr);
+    offset = fdt_add_subnode(fdt, 0, name);
+    _FDT(offset);
+    g_free(name);
+
+    _FDT((fdt_setprop(fdt, offset, "compatible", compat, sizeof(compat))));
+    _FDT((fdt_setprop(fdt, offset, "reg", reg, rsize)));
+    _FDT((fdt_setprop_string(fdt, offset, "device_type",
+                              "PowerPC-External-Interrupt-Presentation")));
+    _FDT((fdt_setprop(fdt, offset, "interrupt-controller", NULL, 0)));
+    _FDT((fdt_setprop(fdt, offset, "ibm,interrupt-server-ranges",
+                       irange, sizeof(irange))));
+    _FDT((fdt_setprop_cell(fdt, offset, "#interrupt-cells", 1)));
+    _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0)));
+    g_free(reg);
+}
+
 static void powernv_populate_chip(PnvChip *chip, void *fdt)
 {
     PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
@@ -231,6 +268,10 @@ static void powernv_populate_chip(PnvChip *chip, void *fdt)
         PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
 
         powernv_create_core_node(chip, pnv_core, fdt);
+
+        /* Interrupt Control Presenters (ICP). One per core. */
+        powernv_populate_icp(chip, fdt, pnv_core->pir,
+                             CPU_CORE(pnv_core)->nr_threads);
     }
 
     if (chip->ram_size) {
@@ -643,6 +684,38 @@ static void pnv_chip_init(Object *obj)
     object_property_add_child(obj, "lpc", OBJECT(&chip->lpc), NULL);
 }
 
+static void pnv_chip_icp_realize(PnvChip *chip, Error **errp)
+{
+    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
+    char *typename = pnv_core_typename(pcc->cpu_model);
+    size_t typesize = object_type_get_instance_size(typename);
+    int i, j;
+    char *name;
+    XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
+
+    name = g_strdup_printf("icp-%x", chip->chip_id);
+    memory_region_init(&chip->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
+    sysbus_init_mmio(SYS_BUS_DEVICE(chip), &chip->icp_mmio);
+    g_free(name);
+
+    sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
+
+    /* Map the ICP registers for each thread */
+    for (i = 0; i < chip->nr_cores; i++) {
+        PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
+        int core_hwid = CPU_CORE(pnv_core)->core_id;
+
+        for (j = 0; j < CPU_CORE(pnv_core)->nr_threads; j++) {
+            uint32_t pir = pcc->core_pir(chip, core_hwid) + j;
+            PnvICPState *icp = PNV_ICP(xics_icp_get(xi, pir));
+
+            memory_region_add_subregion(&chip->icp_mmio, pir << 12, &icp->mmio);
+        }
+    }
+
+    g_free(typename);
+}
+
 static void pnv_chip_realize(DeviceState *dev, Error **errp)
 {
     PnvChip *chip = PNV_CHIP(dev);
@@ -713,6 +786,14 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
     object_property_set_bool(OBJECT(&chip->lpc), true, "realized",
                              &error_fatal);
     pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, &chip->lpc.xscom_regs);
+
+    /* Interrupt Management Area. This is the memory region holding
+     * all the Interrupt Control Presenter (ICP) registers */
+    pnv_chip_icp_realize(chip, &error);
+    if (error) {
+        error_propagate(errp, error);
+        return;
+    }
 }
 
 static Property pnv_chip_properties[] = {
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 5693ba181d24..96231c1ff708 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -54,6 +54,7 @@ typedef struct PnvChip {
     MemoryRegion xscom_mmio;
     MemoryRegion xscom;
     AddressSpace xscom_as;
+    MemoryRegion icp_mmio;
 
     PnvLpcController lpc;
 } PnvChip;
@@ -136,4 +137,22 @@ typedef struct PnvMachineState {
 #define PNV_XSCOM_BASE(chip)                                            \
     (chip->xscom_base + ((uint64_t)(chip)->chip_id) * PNV_XSCOM_SIZE)
 
+/*
+ * XSCOM 0x20109CA defines the ICP BAR:
+ *
+ * 0:29   : bits 14 to 43 of address to define 1 MB region.
+ * 30     : 1 to enable ICP to receive loads/stores against its BAR region
+ * 31:63  : Constant 0
+ *
+ * Usually defined as :
+ *
+ *      0xffffe00200000000 -> 0x0003ffff80000000
+ *      0xffffe00600000000 -> 0x0003ffff80100000
+ *      0xffffe02200000000 -> 0x0003ffff80800000
+ *      0xffffe02600000000 -> 0x0003ffff80900000
+ */
+#define PNV_ICP_SIZE         0x0000000000100000ull
+#define PNV_ICP_BASE(chip)                                              \
+    (0x0003ffff80000000ull + (uint64_t) PNV_CHIP_INDEX(chip) * PNV_ICP_SIZE)
+
 #endif /* _PPC_PNV_H */
-- 
2.7.4

Re: [Qemu-devel] [PATCH v5 0/9] ppc/pnv: interrupt controller (POWER8)

[David Gibson]

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

On Mon, Apr 03, 2017 at 09:45:56AM +0200, Cédric Le Goater wrote:
> Hello,
> 
> Here is a series adding support for the interrupt controller as found
> on a POWER8 system. POWER9 uses a different interrupt controller
> called XIVE, still to be worked on.
> 
> The initial patches are more cleanups of the XICS layer which move the
> IRQ 'server' number mapping under the machine handlers.
> 
> A new PnvICPState object based on MMIOs, which is specific to PowerNV,
> is introduced in XICS. These ICP objects are created for each thread
> of a core and linked to the associated PowerPCCPU object.
> 
> Finally, to make use of the XICS layer, the PowerNV machine is
> extended with a QOM XICSFabric interface and with a global memory
> region acting as the Interrupt Management area.
> 
> 
> To test, grab a kernel and a rootfs image here :
> 
>   https://openpower.xyz/job/openpower-op-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/zImage.epapr
>   https://openpower.xyz/job/openpower-op-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/rootfs.cpio.xz
> 
> The full patchset is available here :
> 
>    https://github.com/legoater/qemu/commits/powernv-ipmi-2.9
> 
> Thanks,

I've applied the series to my ppc-for-2.10 branch.  A couple of things
to think about for follow up cleanups:

1) The spapr->nr_servers field has very little remaining use; can you
   go a bit further and remove it entirely?

2) At the moment you're creating (and realizing) the ICP objects just
before you realize the core objects.  I'm not entirely sure if
creating new objects at realize time is QOMishly correct.  Maybe do
some more enquiries to see if it is or not, and if not update.

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

Re: [Qemu-devel] [PATCH v5 0/9] ppc/pnv: interrupt controller (POWER8)

[Cédric Le Goater]

On 04/05/2017 07:51 AM, David Gibson wrote:
> On Mon, Apr 03, 2017 at 09:45:56AM +0200, Cédric Le Goater wrote:
>> Hello,
>>
>> Here is a series adding support for the interrupt controller as found
>> on a POWER8 system. POWER9 uses a different interrupt controller
>> called XIVE, still to be worked on.
>>
>> The initial patches are more cleanups of the XICS layer which move the
>> IRQ 'server' number mapping under the machine handlers.
>>
>> A new PnvICPState object based on MMIOs, which is specific to PowerNV,
>> is introduced in XICS. These ICP objects are created for each thread
>> of a core and linked to the associated PowerPCCPU object.
>>
>> Finally, to make use of the XICS layer, the PowerNV machine is
>> extended with a QOM XICSFabric interface and with a global memory
>> region acting as the Interrupt Management area.
>>
>>
>> To test, grab a kernel and a rootfs image here :
>>
>>   https://openpower.xyz/job/openpower-op-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/zImage.epapr
>>   https://openpower.xyz/job/openpower-op-build/distro=ubuntu,target=palmetto/lastSuccessfulBuild/artifact/images/rootfs.cpio.xz
>>
>> The full patchset is available here :
>>
>>    https://github.com/legoater/qemu/commits/powernv-ipmi-2.9
>>
>> Thanks,
> 
> I've applied the series to my ppc-for-2.10 branch.  A couple of things
> to think about for follow up cleanups:
> 
> 1) The spapr->nr_servers field has very little remaining use; can you
>    go a bit further and remove it entirely?

yes. I have a patch for it already, reshuffling the xics_system_init
routine again to make it look a little better : 

	https://github.com/legoater/qemu/commit/94e90b4b660d521ad4806a5496dd89f351995810
 
> 2) At the moment you're creating (and realizing) the ICP objects 

yes. This is because the number of ICPState objects depends 
on 'nr_threads' which is only known at realize time.

We could also define an array of icps in the core:

	PnvICPState icps[MAX_THREADS]

initialize them in an init routine and only realize a subset 
depending on 'nr_threads'. 

This is possible because the size of TYPE_PNV_ICP and TYPE_ICP
are the same. 

But, the current method is safer I think. 

> just  before you realize the core objects. 

yes. this is to not complete the core realization if the ICP 
creation fails. Else the core would be fully realized before 
the ICP, which feels incorrect as the ICP is an attribute 
of the core now. 

> I'm not entirely sure if
> creating new objects at realize time is QOMishly correct.  Maybe do
> some more enquiries to see if it is or not, and if not update.

Yes.

Thanks,

C.