All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup
@ 2016-06-28 19:05 Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 1/8] ppc/xics: Rename existing xics to xics_spapr Nikunj A Dadhania
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-28 19:05 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, clg, benh, nikunj

sPAPR xics related changes required for powernv platform. This brings
infrastructure to get the xics native mode for powernv. Tested pseries guests
in KVM and TCG mode.

Changelog v1:
 * Change XICS to XICS_SPAPR and KVM_XICS to XICS_KVM_SPAPR
 * Added xics_ to function get_cpu_index_by_dt_id as this is a global symbol
 * Dropped server parameter from  icp_check_ipi
 * Send HW_ERROR when ics is NULL
 * Remove redundant parameters in trace routines
 * Use type ICS_SIMPLE, ICS_BASE and ICS_KVM
 * Dropped xics-native and info pic patches for this version

ToDo:
 + Use ICPNative and XICSNative in "native" implementation
 + xics_spapr_alloc - getting rid of that
 + xirr_owner - how to reassign after migration

Benjamin Herrenschmidt (8):
  ppc/xics: Rename existing xics to xics_spapr
  ppc/xics: Move SPAPR specific code to a separate file
  ppc/xics: Implement H_IPOLL using an accessor
  ppc/xics: Replace "icp" with "xics" in most places
  ppc/xics: Make the ICSState a list
  ppc/xics: An ICS with offset 0 is assumed to be uninitialized
  ppc/xics: Use a helper to add a new ICS
  ppc/xics: Split ICS into ics-base and ics class

 default-configs/ppc64-softmmu.mak |   1 +
 hw/intc/Makefile.objs             |   1 +
 hw/intc/trace-events              |  14 +-
 hw/intc/xics.c                    | 724 +++++++++++---------------------------
 hw/intc/xics_kvm.c                |  92 +++--
 hw/intc/xics_spapr.c              | 460 ++++++++++++++++++++++++
 hw/ppc/spapr.c                    |  19 +-
 hw/ppc/spapr_cpu_core.c           |   4 +-
 hw/ppc/spapr_events.c             |   8 +-
 hw/ppc/spapr_pci.c                |  12 +-
 hw/ppc/spapr_vio.c                |   2 +-
 include/hw/pci-host/spapr.h       |   2 +-
 include/hw/ppc/spapr.h            |   2 +-
 include/hw/ppc/spapr_vio.h        |   2 +-
 include/hw/ppc/xics.h             |  79 +++--
 15 files changed, 803 insertions(+), 619 deletions(-)
 create mode 100644 hw/intc/xics_spapr.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 1/8] ppc/xics: Rename existing xics to xics_spapr
  2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
@ 2016-06-28 19:05 ` Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 2/8] ppc/xics: Move SPAPR specific code to a separate file Nikunj A Dadhania
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-28 19:05 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, clg, benh, nikunj

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

The common class doesn't change, the KVM one is sPAPR specific. Rename
variables and functions to xics_spapr.

Retain the type name as "xics" to preserve migration for existing sPAPR
guests.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/intc/xics.c        | 33 +++++++++++++++++----------------
 hw/intc/xics_kvm.c    | 14 +++++++-------
 hw/ppc/spapr.c        |  7 ++++---
 hw/ppc/spapr_events.c |  2 +-
 hw/ppc/spapr_pci.c    | 10 +++++-----
 hw/ppc/spapr_vio.c    |  2 +-
 include/hw/ppc/xics.h | 30 ++++++++++++++++++------------
 7 files changed, 53 insertions(+), 45 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 2e83d41..bf5eb56 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -718,7 +718,8 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum)
     return -1;
 }
 
-int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi, Error **errp)
+int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
+                     Error **errp)
 {
     ICSState *ics = &icp->ics[src];
     int irq;
@@ -749,8 +750,8 @@ int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi, Error **errp)
  * Allocate block of consecutive IRQs, and return the number of the first IRQ in the block.
  * If align==true, aligns the first IRQ number to num.
  */
-int xics_alloc_block(XICSState *icp, int src, int num, bool lsi, bool align,
-                     Error **errp)
+int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
+                           bool align, Error **errp)
 {
     int i, first = -1;
     ICSState *ics = &icp->ics[src];
@@ -799,7 +800,7 @@ static void ics_free(ICSState *ics, int srcno, int num)
     }
 }
 
-void xics_free(XICSState *icp, int irq, int num)
+void xics_spapr_free(XICSState *icp, int irq, int num)
 {
     int src = xics_find_source(icp, irq);
 
@@ -1018,9 +1019,9 @@ static void xics_set_nr_servers(XICSState *icp, uint32_t nr_servers,
     }
 }
 
-static void xics_realize(DeviceState *dev, Error **errp)
+static void xics_spapr_realize(DeviceState *dev, Error **errp)
 {
-    XICSState *icp = XICS(dev);
+    XICSState *icp = XICS_SPAPR(dev);
     Error *error = NULL;
     int i;
 
@@ -1057,38 +1058,38 @@ static void xics_realize(DeviceState *dev, Error **errp)
     }
 }
 
-static void xics_initfn(Object *obj)
+static void xics_spapr_initfn(Object *obj)
 {
-    XICSState *xics = XICS(obj);
+    XICSState *xics = XICS_SPAPR(obj);
 
     xics->ics = ICS(object_new(TYPE_ICS));
     object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
     xics->ics->icp = xics;
 }
 
-static void xics_class_init(ObjectClass *oc, void *data)
+static void xics_spapr_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
-    XICSStateClass *xsc = XICS_CLASS(oc);
+    XICSStateClass *xsc = XICS_SPAPR_CLASS(oc);
 
-    dc->realize = xics_realize;
+    dc->realize = xics_spapr_realize;
     xsc->set_nr_irqs = xics_set_nr_irqs;
     xsc->set_nr_servers = xics_set_nr_servers;
 }
 
-static const TypeInfo xics_info = {
-    .name          = TYPE_XICS,
+static const TypeInfo xics_spapr_info = {
+    .name          = TYPE_XICS_SPAPR,
     .parent        = TYPE_XICS_COMMON,
     .instance_size = sizeof(XICSState),
     .class_size = sizeof(XICSStateClass),
-    .class_init    = xics_class_init,
-    .instance_init = xics_initfn,
+    .class_init    = xics_spapr_class_init,
+    .instance_init = xics_spapr_initfn,
 };
 
 static void xics_register_types(void)
 {
     type_register_static(&xics_common_info);
-    type_register_static(&xics_info);
+    type_register_static(&xics_spapr_info);
     type_register_static(&ics_info);
     type_register_static(&icp_info);
 }
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index b17d6a9..a533e3d 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -145,7 +145,7 @@ static const TypeInfo icp_kvm_info = {
  */
 static void ics_get_kvm_state(ICSState *ics)
 {
-    KVMXICSState *icpkvm = KVM_XICS(ics->icp);
+    KVMXICSState *icpkvm = XICS_SPAPR_KVM(ics->icp);
     uint64_t state;
     struct kvm_device_attr attr = {
         .flags = 0,
@@ -204,7 +204,7 @@ static void ics_get_kvm_state(ICSState *ics)
 
 static int ics_set_kvm_state(ICSState *ics, int version_id)
 {
-    KVMXICSState *icpkvm = KVM_XICS(ics->icp);
+    KVMXICSState *icpkvm = XICS_SPAPR_KVM(ics->icp);
     uint64_t state;
     struct kvm_device_attr attr = {
         .flags = 0,
@@ -328,7 +328,7 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
 {
     CPUState *cs;
     ICPState *ss;
-    KVMXICSState *icpkvm = KVM_XICS(icp);
+    KVMXICSState *icpkvm = XICS_SPAPR_KVM(icp);
 
     cs = CPU(cpu);
     ss = &icp->ss[cs->cpu_index];
@@ -394,7 +394,7 @@ static void rtas_dummy(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
 static void xics_kvm_realize(DeviceState *dev, Error **errp)
 {
-    KVMXICSState *icpkvm = KVM_XICS(dev);
+    KVMXICSState *icpkvm = XICS_SPAPR_KVM(dev);
     XICSState *icp = XICS_COMMON(dev);
     int i, rc;
     Error *error = NULL;
@@ -495,8 +495,8 @@ static void xics_kvm_class_init(ObjectClass *oc, void *data)
     xsc->set_nr_servers = xics_kvm_set_nr_servers;
 }
 
-static const TypeInfo xics_kvm_info = {
-    .name          = TYPE_KVM_XICS,
+static const TypeInfo xics_spapr_kvm_info = {
+    .name          = TYPE_XICS_SPAPR_KVM,
     .parent        = TYPE_XICS_COMMON,
     .instance_size = sizeof(KVMXICSState),
     .class_init    = xics_kvm_class_init,
@@ -505,7 +505,7 @@ static const TypeInfo xics_kvm_info = {
 
 static void xics_kvm_register_types(void)
 {
-    type_register_static(&xics_kvm_info);
+    type_register_static(&xics_spapr_kvm_info);
     type_register_static(&ics_kvm_info);
     type_register_static(&icp_kvm_info);
 }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0b6bb9c..a8d497c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -122,7 +122,8 @@ static XICSState *xics_system_init(MachineState *machine,
         Error *err = NULL;
 
         if (machine_kernel_irqchip_allowed(machine)) {
-            icp = try_create_xics(TYPE_KVM_XICS, nr_servers, nr_irqs, &err);
+            icp = try_create_xics(TYPE_XICS_SPAPR_KVM, nr_servers, nr_irqs,
+                                  &err);
         }
         if (machine_kernel_irqchip_required(machine) && !icp) {
             error_reportf_err(err,
@@ -133,7 +134,7 @@ static XICSState *xics_system_init(MachineState *machine,
     }
 
     if (!icp) {
-        icp = try_create_xics(TYPE_XICS, nr_servers, nr_irqs, errp);
+        icp = try_create_xics(TYPE_XICS_SPAPR, nr_servers, nr_irqs, errp);
     }
 
     return icp;
@@ -1781,7 +1782,7 @@ static void ppc_spapr_init(MachineState *machine)
     /* Set up Interrupt Controller before we create the VCPUs */
     spapr->icp = xics_system_init(machine,
                                   DIV_ROUND_UP(max_cpus * smt, smp_threads),
-                                  XICS_IRQS, &error_fatal);
+                                  XICS_IRQS_SPAPR, &error_fatal);
 
     if (smc->dr_lmb_enabled) {
         spapr_validate_node_memory(machine, &error_fatal);
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index af80992..0585f8a 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -603,7 +603,7 @@ out_no_events:
 void spapr_events_init(sPAPRMachineState *spapr)
 {
     QTAILQ_INIT(&spapr->pending_events);
-    spapr->check_exception_irq = xics_alloc(spapr->icp, 0, 0, false,
+    spapr->check_exception_irq = xics_spapr_alloc(spapr->icp, 0, 0, false,
                                             &error_fatal);
     spapr->epow_notifier.notify = spapr_powerdown_req;
     qemu_register_powerdown_notifier(&spapr->epow_notifier);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 9f28fb3..451651d 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -322,7 +322,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
             return;
         }
 
-        xics_free(spapr->icp, msi->first_irq, msi->num);
+        xics_spapr_free(spapr->icp, msi->first_irq, msi->num);
         if (msi_present(pdev)) {
             spapr_msi_setmsg(pdev, 0, false, 0, 0);
         }
@@ -360,7 +360,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     }
 
     /* Allocate MSIs */
-    irq = xics_alloc_block(spapr->icp, 0, req_num, false,
+    irq = xics_spapr_alloc_block(spapr->icp, 0, req_num, false,
                            ret_intr_type == RTAS_TYPE_MSI, &err);
     if (err) {
         error_reportf_err(err, "Can't allocate MSIs for device %x: ",
@@ -371,7 +371,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
     /* Release previous MSIs */
     if (msi) {
-        xics_free(spapr->icp, msi->first_irq, msi->num);
+        xics_spapr_free(spapr->icp, msi->first_irq, msi->num);
         g_hash_table_remove(phb->msi, &config_addr);
     }
 
@@ -1442,7 +1442,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         uint32_t irq;
         Error *local_err = NULL;
 
-        irq = xics_alloc_block(spapr->icp, 0, 1, true, false, &local_err);
+        irq = xics_spapr_alloc_block(spapr->icp, 0, 1, true, false, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             error_prepend(errp, "can't allocate LSIs: ");
@@ -1801,7 +1801,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
     _FDT(fdt_setprop(fdt, bus_off, "ranges", &ranges, sizeof_ranges));
     _FDT(fdt_setprop(fdt, bus_off, "reg", &bus_reg, sizeof(bus_reg)));
     _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pci-config-space-type", 0x1));
-    _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", XICS_IRQS));
+    _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", XICS_IRQS_SPAPR));
 
     /* Build the interrupt-map, this must matches what is done
      * in pci_spapr_map_irq
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index ae40db8..7ffd23e 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -463,7 +463,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp)
         dev->qdev.id = id;
     }
 
-    dev->irq = xics_alloc(spapr->icp, 0, dev->irq, false, &local_err);
+    dev->irq = xics_spapr_alloc(spapr->icp, 0, dev->irq, false, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 6925677..c946770 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -32,20 +32,25 @@
 #define TYPE_XICS_COMMON "xics-common"
 #define XICS_COMMON(obj) OBJECT_CHECK(XICSState, (obj), TYPE_XICS_COMMON)
 
-#define TYPE_XICS "xics"
-#define XICS(obj) OBJECT_CHECK(XICSState, (obj), TYPE_XICS)
+/*
+ * Retain xics as the type name to be compatible for migration. Rest all the
+ * functions, class and variables are renamed as xics_spapr.
+ */
+#define TYPE_XICS_SPAPR "xics"
+#define XICS_SPAPR(obj) OBJECT_CHECK(XICSState, (obj), TYPE_XICS_SPAPR)
 
-#define TYPE_KVM_XICS "xics-kvm"
-#define KVM_XICS(obj) OBJECT_CHECK(KVMXICSState, (obj), TYPE_KVM_XICS)
+#define TYPE_XICS_SPAPR_KVM "xics-spapr-kvm"
+#define XICS_SPAPR_KVM(obj) \
+     OBJECT_CHECK(KVMXICSState, (obj), TYPE_XICS_SPAPR_KVM)
 
 #define XICS_COMMON_CLASS(klass) \
      OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS_COMMON)
-#define XICS_CLASS(klass) \
-     OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS)
+#define XICS_SPAPR_CLASS(klass) \
+     OBJECT_CLASS_CHECK(XICSStateClass, (klass), TYPE_XICS_SPAPR)
 #define XICS_COMMON_GET_CLASS(obj) \
      OBJECT_GET_CLASS(XICSStateClass, (obj), TYPE_XICS_COMMON)
-#define XICS_GET_CLASS(obj) \
-     OBJECT_GET_CLASS(XICSStateClass, (obj), TYPE_XICS)
+#define XICS_SPAPR_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(XICSStateClass, (obj), TYPE_XICS_SPAPR)
 
 #define XICS_IPI        0x2
 #define XICS_BUID       0x1
@@ -157,13 +162,14 @@ struct ICSIRQState {
     uint8_t flags;
 };
 
-#define XICS_IRQS               1024
+#define XICS_IRQS_SPAPR               1024
 
 qemu_irq xics_get_qirq(XICSState *icp, int irq);
-int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi, Error **errp);
-int xics_alloc_block(XICSState *icp, int src, int num, bool lsi, bool align,
+int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
                      Error **errp);
-void xics_free(XICSState *icp, int irq, int num);
+int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
+                           bool align, Error **errp);
+void xics_spapr_free(XICSState *icp, int irq, int num);
 
 void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
 void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 2/8] ppc/xics: Move SPAPR specific code to a separate file
  2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 1/8] ppc/xics: Rename existing xics to xics_spapr Nikunj A Dadhania
@ 2016-06-28 19:05 ` Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 3/8] ppc/xics: Implement H_IPOLL using an accessor Nikunj A Dadhania
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-28 19:05 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, clg, benh, nikunj

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Leave the core ICP/ICS logic in xics.c and move the top level
class wrapper, hypercall and RTAS handlers to xics_spapr.c

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[add cpu.h in xics_spapr.c, move set_nr_irqs and set_nr_servers to
 xics_spapr.c]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 default-configs/ppc64-softmmu.mak |   1 +
 hw/intc/Makefile.objs             |   1 +
 hw/intc/xics.c                    | 418 +-----------------------------------
 hw/intc/xics_spapr.c              | 432 ++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/xics.h             |  21 ++
 5 files changed, 464 insertions(+), 409 deletions(-)
 create mode 100644 hw/intc/xics_spapr.c

diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index bb71b23..c4be59f 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -49,6 +49,7 @@ CONFIG_ETSEC=y
 CONFIG_LIBDECNUMBER=y
 # For pSeries
 CONFIG_XICS=$(CONFIG_PSERIES)
+CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
 CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
 # For PReP
 CONFIG_MC146818RTC=y
diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index c7bbf88..530df2e 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -30,6 +30,7 @@ obj-$(CONFIG_OPENPIC_KVM) += openpic_kvm.o
 obj-$(CONFIG_RASPI) += bcm2835_ic.o bcm2836_control.o
 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_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
 obj-$(CONFIG_S390_FLIC) += s390_flic.o
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index bf5eb56..f01af08 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -32,12 +32,11 @@
 #include "hw/hw.h"
 #include "trace.h"
 #include "qemu/timer.h"
-#include "hw/ppc/spapr.h"
 #include "hw/ppc/xics.h"
 #include "qemu/error-report.h"
 #include "qapi/visitor.h"
 
-static int get_cpu_index_by_dt_id(int cpu_dt_id)
+int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
 {
     PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
 
@@ -242,7 +241,7 @@ static void icp_resend(XICSState *icp, int server)
     ics_resend(icp->ics);
 }
 
-static void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
+void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
 {
     ICPState *ss = icp->ss + server;
     uint8_t old_cppr;
@@ -266,7 +265,7 @@ static void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
     }
 }
 
-static void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
+void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
 {
     ICPState *ss = icp->ss + server;
 
@@ -276,7 +275,7 @@ static void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
     }
 }
 
-static uint32_t icp_accept(ICPState *ss)
+uint32_t icp_accept(ICPState *ss)
 {
     uint32_t xirr = ss->xirr;
 
@@ -289,7 +288,7 @@ static uint32_t icp_accept(ICPState *ss)
     return xirr;
 }
 
-static void icp_eoi(XICSState *icp, int server, uint32_t xirr)
+void icp_eoi(XICSState *icp, int server, uint32_t xirr)
 {
     ICPState *ss = icp->ss + server;
 
@@ -390,12 +389,6 @@ static const TypeInfo icp_info = {
 /*
  * ICS: Source layer
  */
-static int ics_valid_irq(ICSState *ics, uint32_t nr)
-{
-    return (nr >= ics->offset)
-        && (nr < (ics->offset + ics->nr_irqs));
-}
-
 static void resend_msi(ICSState *ics, int srcno)
 {
     ICSIRQState *irq = ics->irqs + srcno;
@@ -480,8 +473,8 @@ static void write_xive_lsi(ICSState *ics, int srcno)
     resend_lsi(ics, srcno);
 }
 
-static void ics_write_xive(ICSState *ics, int nr, int server,
-                           uint8_t priority, uint8_t saved_priority)
+void ics_write_xive(ICSState *ics, int nr, int server,
+                    uint8_t priority, uint8_t saved_priority)
 {
     int srcno = nr - ics->offset;
     ICSIRQState *irq = ics->irqs + srcno;
@@ -658,7 +651,7 @@ static const TypeInfo ics_info = {
 /*
  * Exported functions
  */
-static int xics_find_source(XICSState *icp, int irq)
+int xics_find_source(XICSState *icp, int irq)
 {
     int sources = 1;
     int src;
@@ -686,7 +679,7 @@ qemu_irq xics_get_qirq(XICSState *icp, int irq)
     return NULL;
 }
 
-static void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
+void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
 {
     assert(!(ics->irqs[srcno].flags & XICS_FLAGS_IRQ_MASK));
 
@@ -694,402 +687,9 @@ static void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
         lsi ? XICS_FLAGS_IRQ_LSI : XICS_FLAGS_IRQ_MSI;
 }
 
-#define ICS_IRQ_FREE(ics, srcno)   \
-    (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
-
-static int ics_find_free_block(ICSState *ics, int num, int alignnum)
-{
-    int first, i;
-
-    for (first = 0; first < ics->nr_irqs; first += alignnum) {
-        if (num > (ics->nr_irqs - first)) {
-            return -1;
-        }
-        for (i = first; i < first + num; ++i) {
-            if (!ICS_IRQ_FREE(ics, i)) {
-                break;
-            }
-        }
-        if (i == (first + num)) {
-            return first;
-        }
-    }
-
-    return -1;
-}
-
-int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
-                     Error **errp)
-{
-    ICSState *ics = &icp->ics[src];
-    int irq;
-
-    if (irq_hint) {
-        assert(src == xics_find_source(icp, irq_hint));
-        if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
-            error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
-            return -1;
-        }
-        irq = irq_hint;
-    } else {
-        irq = ics_find_free_block(ics, 1, 1);
-        if (irq < 0) {
-            error_setg(errp, "can't allocate IRQ: no IRQ left");
-            return -1;
-        }
-        irq += ics->offset;
-    }
-
-    ics_set_irq_type(ics, irq - ics->offset, lsi);
-    trace_xics_alloc(src, irq);
-
-    return irq;
-}
-
-/*
- * Allocate block of consecutive IRQs, and return the number of the first IRQ in the block.
- * If align==true, aligns the first IRQ number to num.
- */
-int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
-                           bool align, Error **errp)
-{
-    int i, first = -1;
-    ICSState *ics = &icp->ics[src];
-
-    assert(src == 0);
-    /*
-     * MSIMesage::data is used for storing VIRQ so
-     * it has to be aligned to num to support multiple
-     * MSI vectors. MSI-X is not affected by this.
-     * The hint is used for the first IRQ, the rest should
-     * be allocated continuously.
-     */
-    if (align) {
-        assert((num == 1) || (num == 2) || (num == 4) ||
-               (num == 8) || (num == 16) || (num == 32));
-        first = ics_find_free_block(ics, num, num);
-    } else {
-        first = ics_find_free_block(ics, num, 1);
-    }
-    if (first < 0) {
-        error_setg(errp, "can't find a free %d-IRQ block", num);
-        return -1;
-    }
-
-    if (first >= 0) {
-        for (i = first; i < first + num; ++i) {
-            ics_set_irq_type(ics, i, lsi);
-        }
-    }
-    first += ics->offset;
-
-    trace_xics_alloc_block(src, first, num, lsi, align);
-
-    return first;
-}
-
-static void ics_free(ICSState *ics, int srcno, int num)
-{
-    int i;
-
-    for (i = srcno; i < srcno + num; ++i) {
-        if (ICS_IRQ_FREE(ics, i)) {
-            trace_xics_ics_free_warn(ics - ics->icp->ics, i + ics->offset);
-        }
-        memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
-    }
-}
-
-void xics_spapr_free(XICSState *icp, int irq, int num)
-{
-    int src = xics_find_source(icp, irq);
-
-    if (src >= 0) {
-        ICSState *ics = &icp->ics[src];
-
-        /* FIXME: implement multiple sources */
-        assert(src == 0);
-
-        trace_xics_ics_free(ics - icp->ics, irq, num);
-        ics_free(ics, irq - ics->offset, num);
-    }
-}
-
-/*
- * Guest interfaces
- */
-
-static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                           target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    target_ulong cppr = args[0];
-
-    icp_set_cppr(spapr->icp, cs->cpu_index, cppr);
-    return H_SUCCESS;
-}
-
-static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                          target_ulong opcode, target_ulong *args)
-{
-    target_ulong server = get_cpu_index_by_dt_id(args[0]);
-    target_ulong mfrr = args[1];
-
-    if (server >= spapr->icp->nr_servers) {
-        return H_PARAMETER;
-    }
-
-    icp_set_mfrr(spapr->icp, server, mfrr);
-    return H_SUCCESS;
-}
-
-static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                           target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    uint32_t xirr = icp_accept(spapr->icp->ss + cs->cpu_index);
-
-    args[0] = xirr;
-    return H_SUCCESS;
-}
-
-static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                             target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
-    uint32_t xirr = icp_accept(ss);
-
-    args[0] = xirr;
-    args[1] = cpu_get_host_ticks();
-    return H_SUCCESS;
-}
-
-static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                          target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    target_ulong xirr = args[0];
-
-    icp_eoi(spapr->icp, cs->cpu_index, xirr);
-    return H_SUCCESS;
-}
-
-static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                            target_ulong opcode, target_ulong *args)
-{
-    CPUState *cs = CPU(cpu);
-    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
-
-    args[0] = ss->xirr;
-    args[1] = ss->mfrr;
-
-    return H_SUCCESS;
-}
-
-static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                          uint32_t token,
-                          uint32_t nargs, target_ulong args,
-                          uint32_t nret, target_ulong rets)
-{
-    ICSState *ics = spapr->icp->ics;
-    uint32_t nr, server, priority;
-
-    if ((nargs != 3) || (nret != 1)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    nr = rtas_ld(args, 0);
-    server = get_cpu_index_by_dt_id(rtas_ld(args, 1));
-    priority = rtas_ld(args, 2);
-
-    if (!ics_valid_irq(ics, nr) || (server >= ics->icp->nr_servers)
-        || (priority > 0xff)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    ics_write_xive(ics, nr, server, priority, priority);
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-}
-
-static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                          uint32_t token,
-                          uint32_t nargs, target_ulong args,
-                          uint32_t nret, target_ulong rets)
-{
-    ICSState *ics = spapr->icp->ics;
-    uint32_t nr;
-
-    if ((nargs != 1) || (nret != 3)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    nr = rtas_ld(args, 0);
-
-    if (!ics_valid_irq(ics, nr)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-    rtas_st(rets, 1, ics->irqs[nr - ics->offset].server);
-    rtas_st(rets, 2, ics->irqs[nr - ics->offset].priority);
-}
-
-static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                         uint32_t token,
-                         uint32_t nargs, target_ulong args,
-                         uint32_t nret, target_ulong rets)
-{
-    ICSState *ics = spapr->icp->ics;
-    uint32_t nr;
-
-    if ((nargs != 1) || (nret != 1)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    nr = rtas_ld(args, 0);
-
-    if (!ics_valid_irq(ics, nr)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server, 0xff,
-                   ics->irqs[nr - ics->offset].priority);
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-}
-
-static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
-                        uint32_t token,
-                        uint32_t nargs, target_ulong args,
-                        uint32_t nret, target_ulong rets)
-{
-    ICSState *ics = spapr->icp->ics;
-    uint32_t nr;
-
-    if ((nargs != 1) || (nret != 1)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    nr = rtas_ld(args, 0);
-
-    if (!ics_valid_irq(ics, nr)) {
-        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
-        return;
-    }
-
-    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server,
-                   ics->irqs[nr - ics->offset].saved_priority,
-                   ics->irqs[nr - ics->offset].saved_priority);
-
-    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-}
-
-/*
- * XICS
- */
-
-static void xics_set_nr_irqs(XICSState *icp, uint32_t nr_irqs, Error **errp)
-{
-    icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
-}
-
-static void xics_set_nr_servers(XICSState *icp, uint32_t nr_servers,
-                                Error **errp)
-{
-    int i;
-
-    icp->nr_servers = nr_servers;
-
-    icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState));
-    for (i = 0; i < icp->nr_servers; i++) {
-        char buffer[32];
-        object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_ICP);
-        snprintf(buffer, sizeof(buffer), "icp[%d]", i);
-        object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]),
-                                  errp);
-    }
-}
-
-static void xics_spapr_realize(DeviceState *dev, Error **errp)
-{
-    XICSState *icp = XICS_SPAPR(dev);
-    Error *error = NULL;
-    int i;
-
-    if (!icp->nr_servers) {
-        error_setg(errp, "Number of servers needs to be greater 0");
-        return;
-    }
-
-    /* Registration of global state belongs into realize */
-    spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive);
-    spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive);
-    spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off);
-    spapr_rtas_register(RTAS_IBM_INT_ON, "ibm,int-on", rtas_int_on);
-
-    spapr_register_hypercall(H_CPPR, h_cppr);
-    spapr_register_hypercall(H_IPI, h_ipi);
-    spapr_register_hypercall(H_XIRR, h_xirr);
-    spapr_register_hypercall(H_XIRR_X, h_xirr_x);
-    spapr_register_hypercall(H_EOI, h_eoi);
-    spapr_register_hypercall(H_IPOLL, h_ipoll);
-
-    object_property_set_bool(OBJECT(icp->ics), true, "realized", &error);
-    if (error) {
-        error_propagate(errp, error);
-        return;
-    }
-
-    for (i = 0; i < icp->nr_servers; i++) {
-        object_property_set_bool(OBJECT(&icp->ss[i]), true, "realized", &error);
-        if (error) {
-            error_propagate(errp, error);
-            return;
-        }
-    }
-}
-
-static void xics_spapr_initfn(Object *obj)
-{
-    XICSState *xics = XICS_SPAPR(obj);
-
-    xics->ics = ICS(object_new(TYPE_ICS));
-    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
-    xics->ics->icp = xics;
-}
-
-static void xics_spapr_class_init(ObjectClass *oc, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(oc);
-    XICSStateClass *xsc = XICS_SPAPR_CLASS(oc);
-
-    dc->realize = xics_spapr_realize;
-    xsc->set_nr_irqs = xics_set_nr_irqs;
-    xsc->set_nr_servers = xics_set_nr_servers;
-}
-
-static const TypeInfo xics_spapr_info = {
-    .name          = TYPE_XICS_SPAPR,
-    .parent        = TYPE_XICS_COMMON,
-    .instance_size = sizeof(XICSState),
-    .class_size = sizeof(XICSStateClass),
-    .class_init    = xics_spapr_class_init,
-    .instance_init = xics_spapr_initfn,
-};
-
 static void xics_register_types(void)
 {
     type_register_static(&xics_common_info);
-    type_register_static(&xics_spapr_info);
     type_register_static(&ics_info);
     type_register_static(&icp_info);
 }
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
new file mode 100644
index 0000000..89190ef
--- /dev/null
+++ b/hw/intc/xics_spapr.c
@@ -0,0 +1,432 @@
+/*
+ * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System Emulator
+ *
+ * PAPR Virtualized Interrupt System, aka ICS/ICP aka xics
+ *
+ * Copyright (c) 2010,2011 David Gibson, IBM Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/hw.h"
+#include "trace.h"
+#include "qemu/timer.h"
+#include "hw/ppc/spapr.h"
+#include "hw/ppc/xics.h"
+#include "qapi/visitor.h"
+#include "qapi/error.h"
+
+/*
+ * Guest interfaces
+ */
+
+static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    target_ulong cppr = args[0];
+
+    icp_set_cppr(spapr->icp, cs->cpu_index, cppr);
+    return H_SUCCESS;
+}
+
+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];
+
+    if (server >= spapr->icp->nr_servers) {
+        return H_PARAMETER;
+    }
+
+    icp_set_mfrr(spapr->icp, server, mfrr);
+    return H_SUCCESS;
+}
+
+static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                           target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    uint32_t xirr = icp_accept(spapr->icp->ss + cs->cpu_index);
+
+    args[0] = xirr;
+    return H_SUCCESS;
+}
+
+static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                             target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
+    uint32_t xirr = icp_accept(ss);
+
+    args[0] = xirr;
+    args[1] = cpu_get_host_ticks();
+    return H_SUCCESS;
+}
+
+static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                          target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    target_ulong xirr = args[0];
+
+    icp_eoi(spapr->icp, cs->cpu_index, xirr);
+    return H_SUCCESS;
+}
+
+static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                            target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs = CPU(cpu);
+    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
+
+    args[0] = ss->xirr;
+    args[1] = ss->mfrr;
+
+    return H_SUCCESS;
+}
+
+static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                          uint32_t token,
+                          uint32_t nargs, target_ulong args,
+                          uint32_t nret, target_ulong rets)
+{
+    ICSState *ics = spapr->icp->ics;
+    uint32_t nr, server, priority;
+
+    if ((nargs != 3) || (nret != 1)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    nr = rtas_ld(args, 0);
+    server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
+    priority = rtas_ld(args, 2);
+
+    if (!ics_valid_irq(ics, nr) || (server >= ics->icp->nr_servers)
+        || (priority > 0xff)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    ics_write_xive(ics, nr, server, priority, priority);
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                          uint32_t token,
+                          uint32_t nargs, target_ulong args,
+                          uint32_t nret, target_ulong rets)
+{
+    ICSState *ics = spapr->icp->ics;
+    uint32_t nr;
+
+    if ((nargs != 1) || (nret != 3)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    nr = rtas_ld(args, 0);
+
+    if (!ics_valid_irq(ics, nr)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+    rtas_st(rets, 1, ics->irqs[nr - ics->offset].server);
+    rtas_st(rets, 2, ics->irqs[nr - ics->offset].priority);
+}
+
+static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                         uint32_t token,
+                         uint32_t nargs, target_ulong args,
+                         uint32_t nret, target_ulong rets)
+{
+    ICSState *ics = spapr->icp->ics;
+    uint32_t nr;
+
+    if ((nargs != 1) || (nret != 1)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    nr = rtas_ld(args, 0);
+
+    if (!ics_valid_irq(ics, nr)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server, 0xff,
+                   ics->irqs[nr - ics->offset].priority);
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                        uint32_t token,
+                        uint32_t nargs, target_ulong args,
+                        uint32_t nret, target_ulong rets)
+{
+    ICSState *ics = spapr->icp->ics;
+    uint32_t nr;
+
+    if ((nargs != 1) || (nret != 1)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    nr = rtas_ld(args, 0);
+
+    if (!ics_valid_irq(ics, nr)) {
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
+
+    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server,
+                   ics->irqs[nr - ics->offset].saved_priority,
+                   ics->irqs[nr - ics->offset].saved_priority);
+
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void xics_spapr_set_nr_irqs(XICSState *icp, uint32_t nr_irqs,
+                                   Error **errp)
+{
+    icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
+}
+
+static void xics_spapr_set_nr_servers(XICSState *icp, uint32_t nr_servers,
+                                      Error **errp)
+{
+    int i;
+
+    icp->nr_servers = nr_servers;
+
+    icp->ss = g_malloc0(icp->nr_servers * sizeof(ICPState));
+    for (i = 0; i < icp->nr_servers; i++) {
+        char buffer[32];
+        object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_ICP);
+        snprintf(buffer, sizeof(buffer), "icp[%d]", i);
+        object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]),
+                                  errp);
+    }
+}
+
+static void xics_spapr_realize(DeviceState *dev, Error **errp)
+{
+    XICSState *icp = XICS_SPAPR(dev);
+    Error *error = NULL;
+    int i;
+
+    if (!icp->nr_servers) {
+        error_setg(errp, "Number of servers needs to be greater 0");
+        return;
+    }
+
+    /* Registration of global state belongs into realize */
+    spapr_rtas_register(RTAS_IBM_SET_XIVE, "ibm,set-xive", rtas_set_xive);
+    spapr_rtas_register(RTAS_IBM_GET_XIVE, "ibm,get-xive", rtas_get_xive);
+    spapr_rtas_register(RTAS_IBM_INT_OFF, "ibm,int-off", rtas_int_off);
+    spapr_rtas_register(RTAS_IBM_INT_ON, "ibm,int-on", rtas_int_on);
+
+    spapr_register_hypercall(H_CPPR, h_cppr);
+    spapr_register_hypercall(H_IPI, h_ipi);
+    spapr_register_hypercall(H_XIRR, h_xirr);
+    spapr_register_hypercall(H_XIRR_X, h_xirr_x);
+    spapr_register_hypercall(H_EOI, h_eoi);
+    spapr_register_hypercall(H_IPOLL, h_ipoll);
+
+    object_property_set_bool(OBJECT(icp->ics), true, "realized", &error);
+    if (error) {
+        error_propagate(errp, error);
+        return;
+    }
+
+    for (i = 0; i < icp->nr_servers; i++) {
+        object_property_set_bool(OBJECT(&icp->ss[i]), true, "realized", &error);
+        if (error) {
+            error_propagate(errp, error);
+            return;
+        }
+    }
+}
+
+static void xics_spapr_initfn(Object *obj)
+{
+    XICSState *xics = XICS_SPAPR(obj);
+
+    xics->ics = ICS(object_new(TYPE_ICS));
+    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
+    xics->ics->icp = xics;
+}
+
+static void xics_spapr_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    XICSStateClass *xsc = XICS_SPAPR_CLASS(oc);
+
+    dc->realize = xics_spapr_realize;
+    xsc->set_nr_irqs = xics_spapr_set_nr_irqs;
+    xsc->set_nr_servers = xics_spapr_set_nr_servers;
+}
+
+static const TypeInfo xics_spapr_info = {
+    .name          = TYPE_XICS_SPAPR,
+    .parent        = TYPE_XICS_COMMON,
+    .instance_size = sizeof(XICSState),
+    .class_size = sizeof(XICSStateClass),
+    .class_init    = xics_spapr_class_init,
+    .instance_init = xics_spapr_initfn,
+};
+
+#define ICS_IRQ_FREE(ics, srcno)   \
+    (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
+
+static int ics_find_free_block(ICSState *ics, int num, int alignnum)
+{
+    int first, i;
+
+    for (first = 0; first < ics->nr_irqs; first += alignnum) {
+        if (num > (ics->nr_irqs - first)) {
+            return -1;
+        }
+        for (i = first; i < first + num; ++i) {
+            if (!ICS_IRQ_FREE(ics, i)) {
+                break;
+            }
+        }
+        if (i == (first + num)) {
+            return first;
+        }
+    }
+
+    return -1;
+}
+
+int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
+                     Error **errp)
+{
+    ICSState *ics = &icp->ics[src];
+    int irq;
+
+    if (irq_hint) {
+        assert(src == xics_find_source(icp, irq_hint));
+        if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
+            error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
+            return -1;
+        }
+        irq = irq_hint;
+    } else {
+        irq = ics_find_free_block(ics, 1, 1);
+        if (irq < 0) {
+            error_setg(errp, "can't allocate IRQ: no IRQ left");
+            return -1;
+        }
+        irq += ics->offset;
+    }
+
+    ics_set_irq_type(ics, irq - ics->offset, lsi);
+    trace_xics_alloc(src, irq);
+
+    return irq;
+}
+
+/*
+ * Allocate block of consecutive IRQs, and return the number of the first IRQ in
+ * the block. If align==true, aligns the first IRQ number to num.
+ */
+int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
+                           bool align, Error **errp)
+{
+    int i, first = -1;
+    ICSState *ics = &icp->ics[src];
+
+    assert(src == 0);
+    /*
+     * MSIMesage::data is used for storing VIRQ so
+     * it has to be aligned to num to support multiple
+     * MSI vectors. MSI-X is not affected by this.
+     * The hint is used for the first IRQ, the rest should
+     * be allocated continuously.
+     */
+    if (align) {
+        assert((num == 1) || (num == 2) || (num == 4) ||
+               (num == 8) || (num == 16) || (num == 32));
+        first = ics_find_free_block(ics, num, num);
+    } else {
+        first = ics_find_free_block(ics, num, 1);
+    }
+    if (first < 0) {
+        error_setg(errp, "can't find a free %d-IRQ block", num);
+        return -1;
+    }
+
+    if (first >= 0) {
+        for (i = first; i < first + num; ++i) {
+            ics_set_irq_type(ics, i, lsi);
+        }
+    }
+    first += ics->offset;
+
+    trace_xics_alloc_block(src, first, num, lsi, align);
+
+    return first;
+}
+
+static void ics_free(ICSState *ics, int srcno, int num)
+{
+    int i;
+
+    for (i = srcno; i < srcno + num; ++i) {
+        if (ICS_IRQ_FREE(ics, i)) {
+            trace_xics_ics_free_warn(ics - ics->icp->ics, i + ics->offset);
+        }
+        memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
+    }
+}
+
+void xics_spapr_free(XICSState *icp, int irq, int num)
+{
+    int src = xics_find_source(icp, irq);
+
+    if (src >= 0) {
+        ICSState *ics = &icp->ics[src];
+
+        /* FIXME: implement multiple sources */
+        assert(src == 0);
+
+        trace_xics_ics_free(ics - icp->ics, irq, num);
+        ics_free(ics, irq - ics->offset, num);
+    }
+}
+
+static void xics_spapr_register_types(void)
+{
+    type_register_static(&xics_spapr_info);
+}
+
+type_init(xics_spapr_register_types)
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index c946770..3349006 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -146,6 +146,12 @@ struct ICSState {
     XICSState *icp;
 };
 
+static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
+{
+    return (nr >= ics->offset)
+        && (nr < (ics->offset + ics->nr_irqs));
+}
+
 struct ICSIRQState {
     uint32_t server;
     uint8_t priority;
@@ -174,4 +180,19 @@ void xics_spapr_free(XICSState *icp, int irq, int num);
 void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
 void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu);
 
+/* Internal XICS interfaces */
+int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
+
+void icp_set_cppr(XICSState *icp, int server, uint8_t cppr);
+void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr);
+uint32_t icp_accept(ICPState *ss);
+void icp_eoi(XICSState *icp, int server, uint32_t xirr);
+
+void ics_write_xive(ICSState *ics, int nr, int server,
+                    uint8_t priority, uint8_t saved_priority);
+
+void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
+
+int xics_find_source(XICSState *icp, int irq);
+
 #endif /* __XICS_H__ */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 3/8] ppc/xics: Implement H_IPOLL using an accessor
  2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 1/8] ppc/xics: Rename existing xics to xics_spapr Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 2/8] ppc/xics: Move SPAPR specific code to a separate file Nikunj A Dadhania
@ 2016-06-28 19:05 ` Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 4/8] ppc/xics: Replace "icp" with "xics" in most places Nikunj A Dadhania
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-28 19:05 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, clg, benh, nikunj

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

None of the other presenter functions directly mucks with the
internal state, so don't do it there either.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/xics.c        | 8 ++++++++
 hw/intc/xics_spapr.c  | 7 ++++---
 include/hw/ppc/xics.h | 1 +
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index f01af08..f43f98a 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -288,6 +288,14 @@ uint32_t icp_accept(ICPState *ss)
     return xirr;
 }
 
+uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr)
+{
+    if (mfrr) {
+        *mfrr = ss->mfrr;
+    }
+    return ss->xirr;
+}
+
 void icp_eoi(XICSState *icp, int server, uint32_t xirr)
 {
     ICPState *ss = icp->ss + server;
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 89190ef..94571dd 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -99,10 +99,11 @@ static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                             target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
-    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
+    uint32_t mfrr;
+    uint32_t xirr = icp_ipoll(spapr->icp->ss + cs->cpu_index, &mfrr);
 
-    args[0] = ss->xirr;
-    args[1] = ss->mfrr;
+    args[0] = xirr;
+    args[1] = mfrr;
 
     return H_SUCCESS;
 }
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 3349006..7445a14 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -186,6 +186,7 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
 void icp_set_cppr(XICSState *icp, int server, uint8_t cppr);
 void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr);
 uint32_t icp_accept(ICPState *ss);
+uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr);
 void icp_eoi(XICSState *icp, int server, uint32_t xirr);
 
 void ics_write_xive(ICSState *ics, int nr, int server,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 4/8] ppc/xics: Replace "icp" with "xics" in most places
  2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
                   ` (2 preceding siblings ...)
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 3/8] ppc/xics: Implement H_IPOLL using an accessor Nikunj A Dadhania
@ 2016-06-28 19:05 ` Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 5/8] ppc/xics: Make the ICSState a list Nikunj A Dadhania
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-28 19:05 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, clg, benh, nikunj

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

The "ICP" is a different object than the "XICS". For historical reasons,
we have a number of places where we name a variable "icp" while it contains
a XICSState pointer. There *is* an ICPState structure too so this makes
the code really confusing.

This is a mechanical replacement of all those instances to use the name
"xics" instead. There should be no functional change.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[spapr_cpu_init has been moved to spapr_cpu_core.c, change there]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/xics.c              | 120 ++++++++++++++++++++++----------------------
 hw/intc/xics_kvm.c          |  57 +++++++++++----------
 hw/intc/xics_spapr.c        |  73 ++++++++++++++-------------
 hw/ppc/spapr.c              |  20 ++++----
 hw/ppc/spapr_cpu_core.c     |   4 +-
 hw/ppc/spapr_events.c       |   8 +--
 hw/ppc/spapr_pci.c          |  11 ++--
 hw/ppc/spapr_vio.c          |   2 +-
 include/hw/pci-host/spapr.h |   2 +-
 include/hw/ppc/spapr.h      |   2 +-
 include/hw/ppc/spapr_vio.h  |   2 +-
 include/hw/ppc/xics.h       |   2 +-
 12 files changed, 154 insertions(+), 149 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index f43f98a..cd48f42 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -47,31 +47,31 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
     return -1;
 }
 
-void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu)
+void xics_cpu_destroy(XICSState *xics, PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
-    ICPState *ss = &icp->ss[cs->cpu_index];
+    ICPState *ss = &xics->ss[cs->cpu_index];
 
-    assert(cs->cpu_index < icp->nr_servers);
+    assert(cs->cpu_index < xics->nr_servers);
     assert(cs == ss->cs);
 
     ss->output = NULL;
     ss->cs = NULL;
 }
 
-void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
+void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
-    ICPState *ss = &icp->ss[cs->cpu_index];
-    XICSStateClass *info = XICS_COMMON_GET_CLASS(icp);
+    ICPState *ss = &xics->ss[cs->cpu_index];
+    XICSStateClass *info = XICS_COMMON_GET_CLASS(xics);
 
-    assert(cs->cpu_index < icp->nr_servers);
+    assert(cs->cpu_index < xics->nr_servers);
 
     ss->cs = cs;
 
     if (info->cpu_setup) {
-        info->cpu_setup(icp, cpu);
+        info->cpu_setup(xics, cpu);
     }
 
     switch (PPC_INPUT(env)) {
@@ -95,21 +95,21 @@ void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
  */
 static void xics_common_reset(DeviceState *d)
 {
-    XICSState *icp = XICS_COMMON(d);
+    XICSState *xics = XICS_COMMON(d);
     int i;
 
-    for (i = 0; i < icp->nr_servers; i++) {
-        device_reset(DEVICE(&icp->ss[i]));
+    for (i = 0; i < xics->nr_servers; i++) {
+        device_reset(DEVICE(&xics->ss[i]));
     }
 
-    device_reset(DEVICE(icp->ics));
+    device_reset(DEVICE(xics->ics));
 }
 
 static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
                                   void *opaque, Error **errp)
 {
-    XICSState *icp = XICS_COMMON(obj);
-    int64_t value = icp->nr_irqs;
+    XICSState *xics = XICS_COMMON(obj);
+    int64_t value = xics->nr_irqs;
 
     visit_type_int(v, name, &value, errp);
 }
@@ -117,8 +117,8 @@ static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
 static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name,
                                   void *opaque, Error **errp)
 {
-    XICSState *icp = XICS_COMMON(obj);
-    XICSStateClass *info = XICS_COMMON_GET_CLASS(icp);
+    XICSState *xics = XICS_COMMON(obj);
+    XICSStateClass *info = XICS_COMMON_GET_CLASS(xics);
     Error *error = NULL;
     int64_t value;
 
@@ -127,23 +127,23 @@ static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name,
         error_propagate(errp, error);
         return;
     }
-    if (icp->nr_irqs) {
+    if (xics->nr_irqs) {
         error_setg(errp, "Number of interrupts is already set to %u",
-                   icp->nr_irqs);
+                   xics->nr_irqs);
         return;
     }
 
     assert(info->set_nr_irqs);
-    assert(icp->ics);
-    info->set_nr_irqs(icp, value, errp);
+    assert(xics->ics);
+    info->set_nr_irqs(xics, value, errp);
 }
 
 static void xics_prop_get_nr_servers(Object *obj, Visitor *v,
                                      const char *name, void *opaque,
                                      Error **errp)
 {
-    XICSState *icp = XICS_COMMON(obj);
-    int64_t value = icp->nr_servers;
+    XICSState *xics = XICS_COMMON(obj);
+    int64_t value = xics->nr_servers;
 
     visit_type_int(v, name, &value, errp);
 }
@@ -152,8 +152,8 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor *v,
                                      const char *name, void *opaque,
                                      Error **errp)
 {
-    XICSState *icp = XICS_COMMON(obj);
-    XICSStateClass *info = XICS_COMMON_GET_CLASS(icp);
+    XICSState *xics = XICS_COMMON(obj);
+    XICSStateClass *info = XICS_COMMON_GET_CLASS(xics);
     Error *error = NULL;
     int64_t value;
 
@@ -162,14 +162,14 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor *v,
         error_propagate(errp, error);
         return;
     }
-    if (icp->nr_servers) {
+    if (xics->nr_servers) {
         error_setg(errp, "Number of servers is already set to %u",
-                   icp->nr_servers);
+                   xics->nr_servers);
         return;
     }
 
     assert(info->set_nr_servers);
-    info->set_nr_servers(icp, value, errp);
+    info->set_nr_servers(xics, value, errp);
 }
 
 static void xics_common_initfn(Object *obj)
@@ -212,9 +212,9 @@ static void ics_reject(ICSState *ics, int nr);
 static void ics_resend(ICSState *ics);
 static void ics_eoi(ICSState *ics, int nr);
 
-static void icp_check_ipi(XICSState *icp, int server)
+static void icp_check_ipi(XICSState *xics, int server)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     if (XISR(ss) && (ss->pending_priority <= ss->mfrr)) {
         return;
@@ -223,7 +223,7 @@ static void icp_check_ipi(XICSState *icp, int server)
     trace_xics_icp_check_ipi(server, ss->mfrr);
 
     if (XISR(ss)) {
-        ics_reject(icp->ics, XISR(ss));
+        ics_reject(xics->ics, XISR(ss));
     }
 
     ss->xirr = (ss->xirr & ~XISR_MASK) | XICS_IPI;
@@ -231,19 +231,19 @@ static void icp_check_ipi(XICSState *icp, int server)
     qemu_irq_raise(ss->output);
 }
 
-static void icp_resend(XICSState *icp, int server)
+static void icp_resend(XICSState *xics, int server)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     if (ss->mfrr < CPPR(ss)) {
-        icp_check_ipi(icp, server);
+        icp_check_ipi(xics, server);
     }
-    ics_resend(icp->ics);
+    ics_resend(xics->ics);
 }
 
-void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
+void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
     uint8_t old_cppr;
     uint32_t old_xisr;
 
@@ -256,22 +256,22 @@ void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
             ss->xirr &= ~XISR_MASK; /* Clear XISR */
             ss->pending_priority = 0xff;
             qemu_irq_lower(ss->output);
-            ics_reject(icp->ics, old_xisr);
+            ics_reject(xics->ics, old_xisr);
         }
     } else {
         if (!XISR(ss)) {
-            icp_resend(icp, server);
+            icp_resend(xics, server);
         }
     }
 }
 
-void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
+void icp_set_mfrr(XICSState *xics, int server, uint8_t mfrr)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     ss->mfrr = mfrr;
     if (mfrr < CPPR(ss)) {
-        icp_check_ipi(icp, server);
+        icp_check_ipi(xics, server);
     }
 }
 
@@ -296,31 +296,31 @@ uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr)
     return ss->xirr;
 }
 
-void icp_eoi(XICSState *icp, int server, uint32_t xirr)
+void icp_eoi(XICSState *xics, int server, uint32_t xirr)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     /* Send EOI -> ICS */
     ss->xirr = (ss->xirr & ~CPPR_MASK) | (xirr & CPPR_MASK);
     trace_xics_icp_eoi(server, xirr, ss->xirr);
-    ics_eoi(icp->ics, xirr & XISR_MASK);
+    ics_eoi(xics->ics, xirr & XISR_MASK);
     if (!XISR(ss)) {
-        icp_resend(icp, server);
+        icp_resend(xics, server);
     }
 }
 
-static void icp_irq(XICSState *icp, int server, int nr, uint8_t priority)
+static void icp_irq(XICSState *xics, int server, int nr, uint8_t priority)
 {
-    ICPState *ss = icp->ss + server;
+    ICPState *ss = xics->ss + server;
 
     trace_xics_icp_irq(server, nr, priority);
 
     if ((priority >= CPPR(ss))
         || (XISR(ss) && (ss->pending_priority <= priority))) {
-        ics_reject(icp->ics, nr);
+        ics_reject(xics->ics, nr);
     } else {
         if (XISR(ss)) {
-            ics_reject(icp->ics, XISR(ss));
+            ics_reject(xics->ics, XISR(ss));
         }
         ss->xirr = (ss->xirr & ~XISR_MASK) | (nr & XISR_MASK);
         ss->pending_priority = priority;
@@ -405,7 +405,7 @@ static void resend_msi(ICSState *ics, int srcno)
     if (irq->status & XICS_STATUS_REJECTED) {
         irq->status &= ~XICS_STATUS_REJECTED;
         if (irq->priority != 0xff) {
-            icp_irq(ics->icp, irq->server, srcno + ics->offset,
+            icp_irq(ics->xics, irq->server, srcno + ics->offset,
                     irq->priority);
         }
     }
@@ -419,7 +419,7 @@ static void resend_lsi(ICSState *ics, int srcno)
         && (irq->status & XICS_STATUS_ASSERTED)
         && !(irq->status & XICS_STATUS_SENT)) {
         irq->status |= XICS_STATUS_SENT;
-        icp_irq(ics->icp, irq->server, srcno + ics->offset, irq->priority);
+        icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
     }
 }
 
@@ -434,7 +434,7 @@ static void set_irq_msi(ICSState *ics, int srcno, int val)
             irq->status |= XICS_STATUS_MASKED_PENDING;
             trace_xics_masked_pending();
         } else  {
-            icp_irq(ics->icp, irq->server, srcno + ics->offset, irq->priority);
+            icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
         }
     }
 }
@@ -473,7 +473,7 @@ static void write_xive_msi(ICSState *ics, int srcno)
     }
 
     irq->status &= ~XICS_STATUS_MASKED_PENDING;
-    icp_irq(ics->icp, irq->server, srcno + ics->offset, irq->priority);
+    icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
 }
 
 static void write_xive_lsi(ICSState *ics, int srcno)
@@ -558,8 +558,8 @@ static int ics_post_load(ICSState *ics, int version_id)
 {
     int i;
 
-    for (i = 0; i < ics->icp->nr_servers; i++) {
-        icp_resend(ics->icp, i);
+    for (i = 0; i < ics->xics->nr_servers; i++) {
+        icp_resend(ics->xics, i);
     }
 
     return 0;
@@ -659,14 +659,14 @@ static const TypeInfo ics_info = {
 /*
  * Exported functions
  */
-int xics_find_source(XICSState *icp, int irq)
+int xics_find_source(XICSState *xics, int irq)
 {
     int sources = 1;
     int src;
 
     /* FIXME: implement multiple sources */
     for (src = 0; src < sources; ++src) {
-        ICSState *ics = &icp->ics[src];
+        ICSState *ics = &xics->ics[src];
         if (ics_valid_irq(ics, irq)) {
             return src;
         }
@@ -675,12 +675,12 @@ int xics_find_source(XICSState *icp, int irq)
     return -1;
 }
 
-qemu_irq xics_get_qirq(XICSState *icp, int irq)
+qemu_irq xics_get_qirq(XICSState *xics, int irq)
 {
-    int src = xics_find_source(icp, irq);
+    int src = xics_find_source(xics, irq);
 
     if (src >= 0) {
-        ICSState *ics = &icp->ics[src];
+        ICSState *ics = &xics->ics[src];
         return ics->qirqs[irq - ics->offset];
     }
 
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index a533e3d..edbd62f 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -145,7 +145,7 @@ static const TypeInfo icp_kvm_info = {
  */
 static void ics_get_kvm_state(ICSState *ics)
 {
-    KVMXICSState *icpkvm = XICS_SPAPR_KVM(ics->icp);
+    KVMXICSState *xicskvm = XICS_SPAPR_KVM(ics->xics);
     uint64_t state;
     struct kvm_device_attr attr = {
         .flags = 0,
@@ -160,7 +160,7 @@ static void ics_get_kvm_state(ICSState *ics)
 
         attr.attr = i + ics->offset;
 
-        ret = ioctl(icpkvm->kernel_xics_fd, KVM_GET_DEVICE_ATTR, &attr);
+        ret = ioctl(xicskvm->kernel_xics_fd, KVM_GET_DEVICE_ATTR, &attr);
         if (ret != 0) {
             error_report("Unable to retrieve KVM interrupt controller state"
                     " for IRQ %d: %s", i + ics->offset, strerror(errno));
@@ -204,7 +204,7 @@ static void ics_get_kvm_state(ICSState *ics)
 
 static int ics_set_kvm_state(ICSState *ics, int version_id)
 {
-    KVMXICSState *icpkvm = XICS_SPAPR_KVM(ics->icp);
+    KVMXICSState *xicskvm = XICS_SPAPR_KVM(ics->xics);
     uint64_t state;
     struct kvm_device_attr attr = {
         .flags = 0,
@@ -238,7 +238,7 @@ static int ics_set_kvm_state(ICSState *ics, int version_id)
             }
         }
 
-        ret = ioctl(icpkvm->kernel_xics_fd, KVM_SET_DEVICE_ATTR, &attr);
+        ret = ioctl(xicskvm->kernel_xics_fd, KVM_SET_DEVICE_ATTR, &attr);
         if (ret != 0) {
             error_report("Unable to restore KVM interrupt controller state"
                     " for IRQs %d: %s", i + ics->offset, strerror(errno));
@@ -324,17 +324,17 @@ static const TypeInfo ics_kvm_info = {
 /*
  * XICS-KVM
  */
-static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
+static void xics_kvm_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
 {
     CPUState *cs;
     ICPState *ss;
-    KVMXICSState *icpkvm = XICS_SPAPR_KVM(icp);
+    KVMXICSState *xicskvm = XICS_SPAPR_KVM(xics);
 
     cs = CPU(cpu);
-    ss = &icp->ss[cs->cpu_index];
+    ss = &xics->ss[cs->cpu_index];
 
-    assert(cs->cpu_index < icp->nr_servers);
-    if (icpkvm->kernel_xics_fd == -1) {
+    assert(cs->cpu_index < xics->nr_servers);
+    if (xicskvm->kernel_xics_fd == -1) {
         abort();
     }
 
@@ -347,11 +347,12 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
         return;
     }
 
-    if (icpkvm->kernel_xics_fd != -1) {
+    if (xicskvm->kernel_xics_fd != -1) {
         int ret;
 
         ret = kvm_vcpu_enable_cap(cs, KVM_CAP_IRQ_XICS, 0,
-                                  icpkvm->kernel_xics_fd, kvm_arch_vcpu_id(cs));
+                                  xicskvm->kernel_xics_fd,
+                                  kvm_arch_vcpu_id(cs));
         if (ret < 0) {
             error_report("Unable to connect CPU%ld to kernel XICS: %s",
                     kvm_arch_vcpu_id(cs), strerror(errno));
@@ -361,24 +362,25 @@ static void xics_kvm_cpu_setup(XICSState *icp, PowerPCCPU *cpu)
     }
 }
 
-static void xics_kvm_set_nr_irqs(XICSState *icp, uint32_t nr_irqs, Error **errp)
+static void xics_kvm_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
+                                 Error **errp)
 {
-    icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
+    xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
 }
 
-static void xics_kvm_set_nr_servers(XICSState *icp, uint32_t nr_servers,
+static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers,
                                     Error **errp)
 {
     int i;
 
-    icp->nr_servers = nr_servers;
+    xics->nr_servers = nr_servers;
 
-    icp->ss = g_malloc0(icp->nr_servers*sizeof(ICPState));
-    for (i = 0; i < icp->nr_servers; i++) {
+    xics->ss = g_malloc0(xics->nr_servers * sizeof(ICPState));
+    for (i = 0; i < xics->nr_servers; i++) {
         char buffer[32];
-        object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_KVM_ICP);
+        object_initialize(&xics->ss[i], sizeof(xics->ss[i]), TYPE_KVM_ICP);
         snprintf(buffer, sizeof(buffer), "icp[%d]", i);
-        object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]),
+        object_property_add_child(OBJECT(xics), buffer, OBJECT(&xics->ss[i]),
                                   errp);
     }
 }
@@ -394,8 +396,8 @@ static void rtas_dummy(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
 static void xics_kvm_realize(DeviceState *dev, Error **errp)
 {
-    KVMXICSState *icpkvm = XICS_SPAPR_KVM(dev);
-    XICSState *icp = XICS_COMMON(dev);
+    KVMXICSState *xicskvm = XICS_SPAPR_KVM(dev);
+    XICSState *xics = XICS_COMMON(dev);
     int i, rc;
     Error *error = NULL;
     struct kvm_create_device xics_create_device = {
@@ -445,17 +447,18 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp)
         goto fail;
     }
 
-    icpkvm->kernel_xics_fd = xics_create_device.fd;
+    xicskvm->kernel_xics_fd = xics_create_device.fd;
 
-    object_property_set_bool(OBJECT(icp->ics), true, "realized", &error);
+    object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
     if (error) {
         error_propagate(errp, error);
         goto fail;
     }
 
-    assert(icp->nr_servers);
-    for (i = 0; i < icp->nr_servers; i++) {
-        object_property_set_bool(OBJECT(&icp->ss[i]), true, "realized", &error);
+    assert(xics->nr_servers);
+    for (i = 0; i < xics->nr_servers; i++) {
+        object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized",
+                                 &error);
         if (error) {
             error_propagate(errp, error);
             goto fail;
@@ -481,7 +484,7 @@ static void xics_kvm_initfn(Object *obj)
 
     xics->ics = ICS(object_new(TYPE_KVM_ICS));
     object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
-    xics->ics->icp = xics;
+    xics->ics->xics = xics;
 }
 
 static void xics_kvm_class_init(ObjectClass *oc, void *data)
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 94571dd..618826d 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -45,7 +45,7 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     CPUState *cs = CPU(cpu);
     target_ulong cppr = args[0];
 
-    icp_set_cppr(spapr->icp, cs->cpu_index, cppr);
+    icp_set_cppr(spapr->xics, cs->cpu_index, cppr);
     return H_SUCCESS;
 }
 
@@ -55,11 +55,11 @@ static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
     target_ulong mfrr = args[1];
 
-    if (server >= spapr->icp->nr_servers) {
+    if (server >= spapr->xics->nr_servers) {
         return H_PARAMETER;
     }
 
-    icp_set_mfrr(spapr->icp, server, mfrr);
+    icp_set_mfrr(spapr->xics, server, mfrr);
     return H_SUCCESS;
 }
 
@@ -67,7 +67,7 @@ static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
-    uint32_t xirr = icp_accept(spapr->icp->ss + cs->cpu_index);
+    uint32_t xirr = icp_accept(spapr->xics->ss + cs->cpu_index);
 
     args[0] = xirr;
     return H_SUCCESS;
@@ -77,7 +77,7 @@ static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                              target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
-    ICPState *ss = &spapr->icp->ss[cs->cpu_index];
+    ICPState *ss = &spapr->xics->ss[cs->cpu_index];
     uint32_t xirr = icp_accept(ss);
 
     args[0] = xirr;
@@ -91,7 +91,7 @@ static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     CPUState *cs = CPU(cpu);
     target_ulong xirr = args[0];
 
-    icp_eoi(spapr->icp, cs->cpu_index, xirr);
+    icp_eoi(spapr->xics, cs->cpu_index, xirr);
     return H_SUCCESS;
 }
 
@@ -100,7 +100,7 @@ static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 {
     CPUState *cs = CPU(cpu);
     uint32_t mfrr;
-    uint32_t xirr = icp_ipoll(spapr->icp->ss + cs->cpu_index, &mfrr);
+    uint32_t xirr = icp_ipoll(spapr->xics->ss + cs->cpu_index, &mfrr);
 
     args[0] = xirr;
     args[1] = mfrr;
@@ -113,7 +113,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           uint32_t nargs, target_ulong args,
                           uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->icp->ics;
+    ICSState *ics = spapr->xics->ics;
     uint32_t nr, server, priority;
 
     if ((nargs != 3) || (nret != 1)) {
@@ -125,7 +125,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
     priority = rtas_ld(args, 2);
 
-    if (!ics_valid_irq(ics, nr) || (server >= ics->icp->nr_servers)
+    if (!ics_valid_irq(ics, nr) || (server >= ics->xics->nr_servers)
         || (priority > 0xff)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
@@ -141,7 +141,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           uint32_t nargs, target_ulong args,
                           uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->icp->ics;
+    ICSState *ics = spapr->xics->ics;
     uint32_t nr;
 
     if ((nargs != 1) || (nret != 3)) {
@@ -166,7 +166,7 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                          uint32_t nargs, target_ulong args,
                          uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->icp->ics;
+    ICSState *ics = spapr->xics->ics;
     uint32_t nr;
 
     if ((nargs != 1) || (nret != 1)) {
@@ -192,7 +192,7 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                         uint32_t nargs, target_ulong args,
                         uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->icp->ics;
+    ICSState *ics = spapr->xics->ics;
     uint32_t nr;
 
     if ((nargs != 1) || (nret != 1)) {
@@ -214,36 +214,36 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
 
-static void xics_spapr_set_nr_irqs(XICSState *icp, uint32_t nr_irqs,
+static void xics_spapr_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
                                    Error **errp)
 {
-    icp->nr_irqs = icp->ics->nr_irqs = nr_irqs;
+    xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
 }
 
-static void xics_spapr_set_nr_servers(XICSState *icp, uint32_t nr_servers,
+static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers,
                                       Error **errp)
 {
     int i;
 
-    icp->nr_servers = nr_servers;
+    xics->nr_servers = nr_servers;
 
-    icp->ss = g_malloc0(icp->nr_servers * sizeof(ICPState));
-    for (i = 0; i < icp->nr_servers; i++) {
+    xics->ss = g_malloc0(xics->nr_servers * sizeof(ICPState));
+    for (i = 0; i < xics->nr_servers; i++) {
         char buffer[32];
-        object_initialize(&icp->ss[i], sizeof(icp->ss[i]), TYPE_ICP);
+        object_initialize(&xics->ss[i], sizeof(xics->ss[i]), TYPE_ICP);
         snprintf(buffer, sizeof(buffer), "icp[%d]", i);
-        object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss[i]),
+        object_property_add_child(OBJECT(xics), buffer, OBJECT(&xics->ss[i]),
                                   errp);
     }
 }
 
 static void xics_spapr_realize(DeviceState *dev, Error **errp)
 {
-    XICSState *icp = XICS_SPAPR(dev);
+    XICSState *xics = XICS_SPAPR(dev);
     Error *error = NULL;
     int i;
 
-    if (!icp->nr_servers) {
+    if (!xics->nr_servers) {
         error_setg(errp, "Number of servers needs to be greater 0");
         return;
     }
@@ -261,14 +261,15 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp)
     spapr_register_hypercall(H_EOI, h_eoi);
     spapr_register_hypercall(H_IPOLL, h_ipoll);
 
-    object_property_set_bool(OBJECT(icp->ics), true, "realized", &error);
+    object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
     if (error) {
         error_propagate(errp, error);
         return;
     }
 
-    for (i = 0; i < icp->nr_servers; i++) {
-        object_property_set_bool(OBJECT(&icp->ss[i]), true, "realized", &error);
+    for (i = 0; i < xics->nr_servers; i++) {
+        object_property_set_bool(OBJECT(&xics->ss[i]), true, "realized",
+                                 &error);
         if (error) {
             error_propagate(errp, error);
             return;
@@ -282,7 +283,7 @@ static void xics_spapr_initfn(Object *obj)
 
     xics->ics = ICS(object_new(TYPE_ICS));
     object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
-    xics->ics->icp = xics;
+    xics->ics->xics = xics;
 }
 
 static void xics_spapr_class_init(ObjectClass *oc, void *data)
@@ -328,14 +329,14 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum)
     return -1;
 }
 
-int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
+int xics_spapr_alloc(XICSState *xics, int src, int irq_hint, bool lsi,
                      Error **errp)
 {
-    ICSState *ics = &icp->ics[src];
+    ICSState *ics = &xics->ics[src];
     int irq;
 
     if (irq_hint) {
-        assert(src == xics_find_source(icp, irq_hint));
+        assert(src == xics_find_source(xics, irq_hint));
         if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
             error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
             return -1;
@@ -360,11 +361,11 @@ int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
  * Allocate block of consecutive IRQs, and return the number of the first IRQ in
  * the block. If align==true, aligns the first IRQ number to num.
  */
-int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
+int xics_spapr_alloc_block(XICSState *xics, int src, int num, bool lsi,
                            bool align, Error **errp)
 {
     int i, first = -1;
-    ICSState *ics = &icp->ics[src];
+    ICSState *ics = &xics->ics[src];
 
     assert(src == 0);
     /*
@@ -404,23 +405,23 @@ static void ics_free(ICSState *ics, int srcno, int num)
 
     for (i = srcno; i < srcno + num; ++i) {
         if (ICS_IRQ_FREE(ics, i)) {
-            trace_xics_ics_free_warn(ics - ics->icp->ics, i + ics->offset);
+            trace_xics_ics_free_warn(ics - ics->xics->ics, i + ics->offset);
         }
         memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
     }
 }
 
-void xics_spapr_free(XICSState *icp, int irq, int num)
+void xics_spapr_free(XICSState *xics, int irq, int num)
 {
-    int src = xics_find_source(icp, irq);
+    int src = xics_find_source(xics, irq);
 
     if (src >= 0) {
-        ICSState *ics = &icp->ics[src];
+        ICSState *ics = &xics->ics[src];
 
         /* FIXME: implement multiple sources */
         assert(src == 0);
 
-        trace_xics_ics_free(ics - icp->ics, irq, num);
+        trace_xics_ics_free(ics - xics->ics, irq, num);
         ics_free(ics, irq - ics->offset, num);
     }
 }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a8d497c..d0454e4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -116,16 +116,16 @@ static XICSState *try_create_xics(const char *type, int nr_servers,
 static XICSState *xics_system_init(MachineState *machine,
                                    int nr_servers, int nr_irqs, Error **errp)
 {
-    XICSState *icp = NULL;
+    XICSState *xics = NULL;
 
     if (kvm_enabled()) {
         Error *err = NULL;
 
         if (machine_kernel_irqchip_allowed(machine)) {
-            icp = try_create_xics(TYPE_XICS_SPAPR_KVM, nr_servers, nr_irqs,
-                                  &err);
+            xics = try_create_xics(TYPE_XICS_SPAPR_KVM, nr_servers, nr_irqs,
+                                   &err);
         }
-        if (machine_kernel_irqchip_required(machine) && !icp) {
+        if (machine_kernel_irqchip_required(machine) && !xics) {
             error_reportf_err(err,
                               "kernel_irqchip requested but unavailable: ");
         } else {
@@ -133,11 +133,11 @@ static XICSState *xics_system_init(MachineState *machine,
         }
     }
 
-    if (!icp) {
-        icp = try_create_xics(TYPE_XICS_SPAPR, nr_servers, nr_irqs, errp);
+    if (!xics) {
+        xics = try_create_xics(TYPE_XICS_SPAPR, nr_servers, nr_irqs, errp);
     }
 
-    return icp;
+    return xics;
 }
 
 static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
@@ -1780,9 +1780,9 @@ static void ppc_spapr_init(MachineState *machine)
     load_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FW_OVERHEAD;
 
     /* Set up Interrupt Controller before we create the VCPUs */
-    spapr->icp = xics_system_init(machine,
-                                  DIV_ROUND_UP(max_cpus * smt, smp_threads),
-                                  XICS_IRQS_SPAPR, &error_fatal);
+    spapr->xics = xics_system_init(machine,
+                                   DIV_ROUND_UP(max_cpus * smt, smp_threads),
+                                   XICS_IRQS_SPAPR, &error_fatal);
 
     if (smc->dr_lmb_enabled) {
         spapr_validate_node_memory(machine, &error_fatal);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 3a5da09..a57635a 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -42,7 +42,7 @@ static void spapr_cpu_destroy(PowerPCCPU *cpu)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 
-    xics_cpu_destroy(spapr->icp, cpu);
+    xics_cpu_destroy(spapr->xics, cpu);
     qemu_unregister_reset(spapr_cpu_reset, cpu);
 }
 
@@ -76,7 +76,7 @@ void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
         }
     }
 
-    xics_cpu_setup(spapr->icp, cpu);
+    xics_cpu_setup(spapr->xics, cpu);
 
     qemu_register_reset(spapr_cpu_reset, cpu);
     spapr_cpu_reset(cpu);
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 0585f8a..b0668b3 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -386,7 +386,7 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
 
     rtas_event_log_queue(RTAS_LOG_TYPE_EPOW, new_epow, true);
 
-    qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
+    qemu_irq_pulse(xics_get_qirq(spapr->xics, spapr->check_exception_irq));
 }
 
 static void spapr_hotplug_set_signalled(uint32_t drc_index)
@@ -468,7 +468,7 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
 
     rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true);
 
-    qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
+    qemu_irq_pulse(xics_get_qirq(spapr->xics, spapr->check_exception_irq));
 }
 
 void spapr_hotplug_req_add_by_index(sPAPRDRConnector *drc)
@@ -551,7 +551,7 @@ static void check_exception(PowerPCCPU *cpu, sPAPRMachineState *spapr,
      * interrupts.
      */
     if (rtas_event_log_contains(mask, true)) {
-        qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
+        qemu_irq_pulse(xics_get_qirq(spapr->xics, spapr->check_exception_irq));
     }
 
     return;
@@ -603,7 +603,7 @@ out_no_events:
 void spapr_events_init(sPAPRMachineState *spapr)
 {
     QTAILQ_INIT(&spapr->pending_events);
-    spapr->check_exception_irq = xics_spapr_alloc(spapr->icp, 0, 0, false,
+    spapr->check_exception_irq = xics_spapr_alloc(spapr->xics, 0, 0, false,
                                             &error_fatal);
     spapr->epow_notifier.notify = spapr_powerdown_req;
     qemu_register_powerdown_notifier(&spapr->epow_notifier);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 451651d..8c1e6b1 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -322,7 +322,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
             return;
         }
 
-        xics_spapr_free(spapr->icp, msi->first_irq, msi->num);
+        xics_spapr_free(spapr->xics, msi->first_irq, msi->num);
         if (msi_present(pdev)) {
             spapr_msi_setmsg(pdev, 0, false, 0, 0);
         }
@@ -360,7 +360,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     }
 
     /* Allocate MSIs */
-    irq = xics_spapr_alloc_block(spapr->icp, 0, req_num, false,
+    irq = xics_spapr_alloc_block(spapr->xics, 0, req_num, false,
                            ret_intr_type == RTAS_TYPE_MSI, &err);
     if (err) {
         error_reportf_err(err, "Can't allocate MSIs for device %x: ",
@@ -371,7 +371,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
     /* Release previous MSIs */
     if (msi) {
-        xics_spapr_free(spapr->icp, msi->first_irq, msi->num);
+        xics_spapr_free(spapr->xics, msi->first_irq, msi->num);
         g_hash_table_remove(phb->msi, &config_addr);
     }
 
@@ -733,7 +733,7 @@ static void spapr_msi_write(void *opaque, hwaddr addr,
 
     trace_spapr_pci_msi_write(addr, data, irq);
 
-    qemu_irq_pulse(xics_get_qirq(spapr->icp, irq));
+    qemu_irq_pulse(xics_get_qirq(spapr->xics, irq));
 }
 
 static const MemoryRegionOps spapr_msi_ops = {
@@ -1442,7 +1442,8 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         uint32_t irq;
         Error *local_err = NULL;
 
-        irq = xics_spapr_alloc_block(spapr->icp, 0, 1, true, false, &local_err);
+        irq = xics_spapr_alloc_block(spapr->xics, 0, 1, true, false,
+                                     &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             error_prepend(errp, "can't allocate LSIs: ");
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 7ffd23e..f93244d 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -463,7 +463,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp)
         dev->qdev.id = id;
     }
 
-    dev->irq = xics_spapr_alloc(spapr->icp, 0, dev->irq, false, &local_err);
+    dev->irq = xics_spapr_alloc(spapr->xics, 0, dev->irq, false, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 7848366..288b89c 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -93,7 +93,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 
-    return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);
+    return xics_get_qirq(spapr->xics, phb->lsi_table[pin].irq);
 }
 
 PCIHostState *spapr_create_phb(sPAPRMachineState *spapr, int index);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index e1f8274..4932555 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -52,7 +52,7 @@ struct sPAPRMachineState {
     struct VIOsPAPRBus *vio_bus;
     QLIST_HEAD(, sPAPRPHBState) phbs;
     struct sPAPRNVRAM *nvram;
-    XICSState *icp;
+    XICSState *xics;
     DeviceState *rtc;
 
     void *htab;
diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
index 5f8b042..bdb5d2f 100644
--- a/include/hw/ppc/spapr_vio.h
+++ b/include/hw/ppc/spapr_vio.h
@@ -90,7 +90,7 @@ static inline qemu_irq spapr_vio_qirq(VIOsPAPRDevice *dev)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 
-    return xics_get_qirq(spapr->icp, dev->irq);
+    return xics_get_qirq(spapr->xics, dev->irq);
 }
 
 static inline bool spapr_vio_dma_valid(VIOsPAPRDevice *dev, uint64_t taddr,
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 7445a14..6189a3b 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -143,7 +143,7 @@ struct ICSState {
     uint32_t offset;
     qemu_irq *qirqs;
     ICSIRQState *irqs;
-    XICSState *icp;
+    XICSState *xics;
 };
 
 static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 5/8] ppc/xics: Make the ICSState a list
  2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
                   ` (3 preceding siblings ...)
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 4/8] ppc/xics: Replace "icp" with "xics" in most places Nikunj A Dadhania
@ 2016-06-28 19:05 ` Nikunj A Dadhania
  2016-06-29  3:37   ` David Gibson
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 6/8] ppc/xics: An ICS with offset 0 is assumed to be uninitialized Nikunj A Dadhania
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-28 19:05 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, clg, benh, nikunj

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Instead of an array of fixed sized blocks, use a list, as we will need
to have sources with variable number of interrupts. SPAPR only uses
a single entry. Native will create more. If performance becomes an
issue we can add some hashed lookup but for now this will do fine.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[ move the initialization of list to xics_common_initfn ]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/intc/trace-events  |  4 +--
 hw/intc/xics.c        | 83 ++++++++++++++++++++++++++++--------------------
 hw/intc/xics_kvm.c    | 27 +++++++++++-----
 hw/intc/xics_spapr.c  | 88 +++++++++++++++++++++++++++++++++------------------
 hw/ppc/spapr_events.c |  2 +-
 hw/ppc/spapr_pci.c    |  5 ++-
 hw/ppc/spapr_vio.c    |  2 +-
 include/hw/ppc/xics.h | 13 ++++----
 8 files changed, 138 insertions(+), 86 deletions(-)

diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 376dd18..5f0f783 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -56,8 +56,8 @@ xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
 xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
 xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
 xics_ics_eoi(int nr) "ics_eoi: irq %#x"
-xics_alloc(int src, int irq) "source#%d, irq %d"
-xics_alloc_block(int src, int first, int num, bool lsi, int align) "source#%d, first irq %d, %d irqs, lsi=%d, alignnum %d"
+xics_alloc(int irq) "irq %d"
+xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d irqs, lsi=%d, alignnum %d"
 xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
 xics_ics_free_warn(int src, int irq) "Source#%d, irq %d is already free"
 
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index cd48f42..5148bdf 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -96,13 +96,16 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
 static void xics_common_reset(DeviceState *d)
 {
     XICSState *xics = XICS_COMMON(d);
+    ICSState *ics;
     int i;
 
     for (i = 0; i < xics->nr_servers; i++) {
         device_reset(DEVICE(&xics->ss[i]));
     }
 
-    device_reset(DEVICE(xics->ics));
+    QLIST_FOREACH(ics, &xics->ics, list) {
+        device_reset(DEVICE(ics));
+    }
 }
 
 static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
@@ -134,7 +137,6 @@ static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name,
     }
 
     assert(info->set_nr_irqs);
-    assert(xics->ics);
     info->set_nr_irqs(xics, value, errp);
 }
 
@@ -174,6 +176,9 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor *v,
 
 static void xics_common_initfn(Object *obj)
 {
+    XICSState *xics = XICS_COMMON(obj);
+
+    QLIST_INIT(&xics->ics);
     object_property_add(obj, "nr_irqs", "int",
                         xics_prop_get_nr_irqs, xics_prop_set_nr_irqs,
                         NULL, NULL, NULL);
@@ -212,33 +217,35 @@ static void ics_reject(ICSState *ics, int nr);
 static void ics_resend(ICSState *ics);
 static void ics_eoi(ICSState *ics, int nr);
 
-static void icp_check_ipi(XICSState *xics, int server)
+static void icp_check_ipi(ICPState *ss)
 {
-    ICPState *ss = xics->ss + server;
-
     if (XISR(ss) && (ss->pending_priority <= ss->mfrr)) {
         return;
     }
 
-    trace_xics_icp_check_ipi(server, ss->mfrr);
+    trace_xics_icp_check_ipi(ss->cs->cpu_index, ss->mfrr);
 
-    if (XISR(ss)) {
-        ics_reject(xics->ics, XISR(ss));
+    if (XISR(ss) && ss->xirr_owner) {
+        ics_reject(ss->xirr_owner, XISR(ss));
     }
 
     ss->xirr = (ss->xirr & ~XISR_MASK) | XICS_IPI;
     ss->pending_priority = ss->mfrr;
+    ss->xirr_owner = NULL;
     qemu_irq_raise(ss->output);
 }
 
 static void icp_resend(XICSState *xics, int server)
 {
     ICPState *ss = xics->ss + server;
+    ICSState *ics;
 
     if (ss->mfrr < CPPR(ss)) {
-        icp_check_ipi(xics, server);
+        icp_check_ipi(ss);
+    }
+    QLIST_FOREACH(ics, &xics->ics, list) {
+        ics_resend(ics);
     }
-    ics_resend(xics->ics);
 }
 
 void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
@@ -256,7 +263,10 @@ void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
             ss->xirr &= ~XISR_MASK; /* Clear XISR */
             ss->pending_priority = 0xff;
             qemu_irq_lower(ss->output);
-            ics_reject(xics->ics, old_xisr);
+            if (ss->xirr_owner) {
+                ics_reject(ss->xirr_owner, old_xisr);
+                ss->xirr_owner = NULL;
+            }
         }
     } else {
         if (!XISR(ss)) {
@@ -271,7 +281,7 @@ void icp_set_mfrr(XICSState *xics, int server, uint8_t mfrr)
 
     ss->mfrr = mfrr;
     if (mfrr < CPPR(ss)) {
-        icp_check_ipi(xics, server);
+        icp_check_ipi(ss);
     }
 }
 
@@ -282,6 +292,7 @@ uint32_t icp_accept(ICPState *ss)
     qemu_irq_lower(ss->output);
     ss->xirr = ss->pending_priority << 24;
     ss->pending_priority = 0xff;
+    ss->xirr_owner = NULL;
 
     trace_xics_icp_accept(xirr, ss->xirr);
 
@@ -299,30 +310,40 @@ uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr)
 void icp_eoi(XICSState *xics, int server, uint32_t xirr)
 {
     ICPState *ss = xics->ss + server;
+    ICSState *ics;
+    uint32_t irq;
 
     /* Send EOI -> ICS */
     ss->xirr = (ss->xirr & ~CPPR_MASK) | (xirr & CPPR_MASK);
     trace_xics_icp_eoi(server, xirr, ss->xirr);
-    ics_eoi(xics->ics, xirr & XISR_MASK);
+    irq = xirr & XISR_MASK;
+    QLIST_FOREACH(ics, &xics->ics, list) {
+        if (ics_valid_irq(ics, irq)) {
+            ics_eoi(ics, irq);
+        }
+    }
     if (!XISR(ss)) {
         icp_resend(xics, server);
     }
 }
 
-static void icp_irq(XICSState *xics, int server, int nr, uint8_t priority)
+static void icp_irq(ICSState *ics, int server, int nr, uint8_t priority)
 {
+    XICSState *xics = ics->xics;
     ICPState *ss = xics->ss + server;
 
     trace_xics_icp_irq(server, nr, priority);
 
     if ((priority >= CPPR(ss))
         || (XISR(ss) && (ss->pending_priority <= priority))) {
-        ics_reject(xics->ics, nr);
+        ics_reject(ics, nr);
     } else {
-        if (XISR(ss)) {
-            ics_reject(xics->ics, XISR(ss));
+        if (XISR(ss) && ss->xirr_owner) {
+            ics_reject(ss->xirr_owner, XISR(ss));
+            ss->xirr_owner = NULL;
         }
         ss->xirr = (ss->xirr & ~XISR_MASK) | (nr & XISR_MASK);
+        ss->xirr_owner = ics;
         ss->pending_priority = priority;
         trace_xics_icp_raise(ss->xirr, ss->pending_priority);
         qemu_irq_raise(ss->output);
@@ -405,8 +426,7 @@ static void resend_msi(ICSState *ics, int srcno)
     if (irq->status & XICS_STATUS_REJECTED) {
         irq->status &= ~XICS_STATUS_REJECTED;
         if (irq->priority != 0xff) {
-            icp_irq(ics->xics, irq->server, srcno + ics->offset,
-                    irq->priority);
+            icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
         }
     }
 }
@@ -419,7 +439,7 @@ static void resend_lsi(ICSState *ics, int srcno)
         && (irq->status & XICS_STATUS_ASSERTED)
         && !(irq->status & XICS_STATUS_SENT)) {
         irq->status |= XICS_STATUS_SENT;
-        icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
+        icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
     }
 }
 
@@ -434,7 +454,7 @@ static void set_irq_msi(ICSState *ics, int srcno, int val)
             irq->status |= XICS_STATUS_MASKED_PENDING;
             trace_xics_masked_pending();
         } else  {
-            icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
+            icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
         }
     }
 }
@@ -473,7 +493,7 @@ static void write_xive_msi(ICSState *ics, int srcno)
     }
 
     irq->status &= ~XICS_STATUS_MASKED_PENDING;
-    icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
+    icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
 }
 
 static void write_xive_lsi(ICSState *ics, int srcno)
@@ -659,28 +679,23 @@ static const TypeInfo ics_info = {
 /*
  * Exported functions
  */
-int xics_find_source(XICSState *xics, int irq)
+ICSState *xics_find_source(XICSState *xics, int irq)
 {
-    int sources = 1;
-    int src;
+    ICSState *ics;
 
-    /* FIXME: implement multiple sources */
-    for (src = 0; src < sources; ++src) {
-        ICSState *ics = &xics->ics[src];
+    QLIST_FOREACH(ics, &xics->ics, list) {
         if (ics_valid_irq(ics, irq)) {
-            return src;
+            return ics;
         }
     }
-
-    return -1;
+    return NULL;
 }
 
 qemu_irq xics_get_qirq(XICSState *xics, int irq)
 {
-    int src = xics_find_source(xics, irq);
+    ICSState *ics = xics_find_source(xics, irq);
 
-    if (src >= 0) {
-        ICSState *ics = &xics->ics[src];
+    if (ics) {
         return ics->qirqs[irq - ics->offset];
     }
 
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index edbd62f..04fa7cb 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -365,7 +365,13 @@ static void xics_kvm_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
 static void xics_kvm_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
                                  Error **errp)
 {
-    xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
+    ICSState *ics = QLIST_FIRST(&xics->ics);
+
+    /* This needs to be deprecated ... */
+    xics->nr_irqs = nr_irqs;
+    if (ics) {
+        ics->nr_irqs = nr_irqs;
+    }
 }
 
 static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers,
@@ -398,6 +404,7 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp)
 {
     KVMXICSState *xicskvm = XICS_SPAPR_KVM(dev);
     XICSState *xics = XICS_COMMON(dev);
+    ICSState *ics;
     int i, rc;
     Error *error = NULL;
     struct kvm_create_device xics_create_device = {
@@ -449,10 +456,12 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp)
 
     xicskvm->kernel_xics_fd = xics_create_device.fd;
 
-    object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
-    if (error) {
-        error_propagate(errp, error);
-        goto fail;
+    QLIST_FOREACH(ics, &xics->ics, list) {
+        object_property_set_bool(OBJECT(ics), true, "realized", &error);
+        if (error) {
+            error_propagate(errp, error);
+            goto fail;
+        }
     }
 
     assert(xics->nr_servers);
@@ -481,10 +490,12 @@ fail:
 static void xics_kvm_initfn(Object *obj)
 {
     XICSState *xics = XICS_COMMON(obj);
+    ICSState *ics;
 
-    xics->ics = ICS(object_new(TYPE_KVM_ICS));
-    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
-    xics->ics->xics = xics;
+    ics = ICS(object_new(TYPE_KVM_ICS));
+    object_property_add_child(obj, "ics", OBJECT(ics), NULL);
+    ics->xics = xics;
+    QLIST_INSERT_HEAD(&xics->ics, ics, list);
 }
 
 static void xics_kvm_class_init(ObjectClass *oc, void *data)
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 618826d..0b0845d 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -113,13 +113,17 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           uint32_t nargs, target_ulong args,
                           uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->xics->ics;
+    ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
     uint32_t nr, server, priority;
 
     if ((nargs != 3) || (nret != 1)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
     }
+    if (!ics) {
+        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+        return;
+    }
 
     nr = rtas_ld(args, 0);
     server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
@@ -141,13 +145,17 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           uint32_t nargs, target_ulong args,
                           uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->xics->ics;
+    ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
     uint32_t nr;
 
     if ((nargs != 1) || (nret != 3)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
     }
+    if (!ics) {
+        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+        return;
+    }
 
     nr = rtas_ld(args, 0);
 
@@ -166,13 +174,17 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                          uint32_t nargs, target_ulong args,
                          uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->xics->ics;
+    ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
     uint32_t nr;
 
     if ((nargs != 1) || (nret != 1)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
     }
+    if (!ics) {
+        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+        return;
+    }
 
     nr = rtas_ld(args, 0);
 
@@ -192,13 +204,17 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                         uint32_t nargs, target_ulong args,
                         uint32_t nret, target_ulong rets)
 {
-    ICSState *ics = spapr->xics->ics;
+    ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
     uint32_t nr;
 
     if ((nargs != 1) || (nret != 1)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
         return;
     }
+    if (!ics) {
+        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+        return;
+    }
 
     nr = rtas_ld(args, 0);
 
@@ -217,7 +233,13 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 static void xics_spapr_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
                                    Error **errp)
 {
-    xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
+    ICSState *ics = QLIST_FIRST(&xics->ics);
+
+    /* This needs to be deprecated ... */
+    xics->nr_irqs = nr_irqs;
+    if (ics) {
+        ics->nr_irqs = nr_irqs;
+    }
 }
 
 static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers,
@@ -240,6 +262,7 @@ static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers,
 static void xics_spapr_realize(DeviceState *dev, Error **errp)
 {
     XICSState *xics = XICS_SPAPR(dev);
+    ICSState *ics;
     Error *error = NULL;
     int i;
 
@@ -261,10 +284,12 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp)
     spapr_register_hypercall(H_EOI, h_eoi);
     spapr_register_hypercall(H_IPOLL, h_ipoll);
 
-    object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
-    if (error) {
-        error_propagate(errp, error);
-        return;
+    QLIST_FOREACH(ics, &xics->ics, list) {
+        object_property_set_bool(OBJECT(ics), true, "realized", &error);
+        if (error) {
+            error_propagate(errp, error);
+            return;
+        }
     }
 
     for (i = 0; i < xics->nr_servers; i++) {
@@ -280,10 +305,12 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp)
 static void xics_spapr_initfn(Object *obj)
 {
     XICSState *xics = XICS_SPAPR(obj);
+    ICSState *ics;
 
-    xics->ics = ICS(object_new(TYPE_ICS));
-    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
-    xics->ics->xics = xics;
+    ics = ICS(object_new(TYPE_ICS));
+    object_property_add_child(obj, "ics", OBJECT(ics), NULL);
+    ics->xics = xics;
+    QLIST_INSERT_HEAD(&xics->ics, ics, list);
 }
 
 static void xics_spapr_class_init(ObjectClass *oc, void *data)
@@ -329,14 +356,15 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum)
     return -1;
 }
 
-int xics_spapr_alloc(XICSState *xics, int src, int irq_hint, bool lsi,
-                     Error **errp)
+int xics_spapr_alloc(XICSState *xics, int irq_hint, bool lsi, Error **errp)
 {
-    ICSState *ics = &xics->ics[src];
+    ICSState *ics = QLIST_FIRST(&xics->ics);
     int irq;
 
+    if (!ics) {
+        return -1;
+    }
     if (irq_hint) {
-        assert(src == xics_find_source(xics, irq_hint));
         if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
             error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
             return -1;
@@ -352,7 +380,7 @@ int xics_spapr_alloc(XICSState *xics, int src, int irq_hint, bool lsi,
     }
 
     ics_set_irq_type(ics, irq - ics->offset, lsi);
-    trace_xics_alloc(src, irq);
+    trace_xics_alloc(irq);
 
     return irq;
 }
@@ -361,13 +389,16 @@ int xics_spapr_alloc(XICSState *xics, int src, int irq_hint, bool lsi,
  * Allocate block of consecutive IRQs, and return the number of the first IRQ in
  * the block. If align==true, aligns the first IRQ number to num.
  */
-int xics_spapr_alloc_block(XICSState *xics, int src, int num, bool lsi,
-                           bool align, Error **errp)
+int xics_spapr_alloc_block(XICSState *xics, int num, bool lsi, bool align,
+                           Error **errp)
 {
+    ICSState *ics = QLIST_FIRST(&xics->ics);
     int i, first = -1;
-    ICSState *ics = &xics->ics[src];
 
-    assert(src == 0);
+    if (!ics) {
+        return -1;
+    }
+
     /*
      * MSIMesage::data is used for storing VIRQ so
      * it has to be aligned to num to support multiple
@@ -394,7 +425,7 @@ int xics_spapr_alloc_block(XICSState *xics, int src, int num, bool lsi,
     }
     first += ics->offset;
 
-    trace_xics_alloc_block(src, first, num, lsi, align);
+    trace_xics_alloc_block(first, num, lsi, align);
 
     return first;
 }
@@ -405,7 +436,7 @@ static void ics_free(ICSState *ics, int srcno, int num)
 
     for (i = srcno; i < srcno + num; ++i) {
         if (ICS_IRQ_FREE(ics, i)) {
-            trace_xics_ics_free_warn(ics - ics->xics->ics, i + ics->offset);
+            trace_xics_ics_free_warn(0, i + ics->offset);
         }
         memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
     }
@@ -413,15 +444,10 @@ static void ics_free(ICSState *ics, int srcno, int num)
 
 void xics_spapr_free(XICSState *xics, int irq, int num)
 {
-    int src = xics_find_source(xics, irq);
-
-    if (src >= 0) {
-        ICSState *ics = &xics->ics[src];
-
-        /* FIXME: implement multiple sources */
-        assert(src == 0);
+    ICSState *ics = xics_find_source(xics, irq);
 
-        trace_xics_ics_free(ics - xics->ics, irq, num);
+    if (ics) {
+        trace_xics_ics_free(0, irq, num);
         ics_free(ics, irq - ics->offset, num);
     }
 }
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index b0668b3..adf8da4 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -603,7 +603,7 @@ out_no_events:
 void spapr_events_init(sPAPRMachineState *spapr)
 {
     QTAILQ_INIT(&spapr->pending_events);
-    spapr->check_exception_irq = xics_spapr_alloc(spapr->xics, 0, 0, false,
+    spapr->check_exception_irq = xics_spapr_alloc(spapr->xics, 0, false,
                                             &error_fatal);
     spapr->epow_notifier.notify = spapr_powerdown_req;
     qemu_register_powerdown_notifier(&spapr->epow_notifier);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 8c1e6b1..3d08c0a 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -360,7 +360,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     }
 
     /* Allocate MSIs */
-    irq = xics_spapr_alloc_block(spapr->xics, 0, req_num, false,
+    irq = xics_spapr_alloc_block(spapr->xics, req_num, false,
                            ret_intr_type == RTAS_TYPE_MSI, &err);
     if (err) {
         error_reportf_err(err, "Can't allocate MSIs for device %x: ",
@@ -1442,8 +1442,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         uint32_t irq;
         Error *local_err = NULL;
 
-        irq = xics_spapr_alloc_block(spapr->xics, 0, 1, true, false,
-                                     &local_err);
+        irq = xics_spapr_alloc_block(spapr->xics, 1, true, false, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             error_prepend(errp, "can't allocate LSIs: ");
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index f93244d..22360af 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -463,7 +463,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp)
         dev->qdev.id = id;
     }
 
-    dev->irq = xics_spapr_alloc(spapr->xics, 0, dev->irq, false, &local_err);
+    dev->irq = xics_spapr_alloc(spapr->xics, dev->irq, false, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 6189a3b..6ad3057 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -84,7 +84,7 @@ struct XICSState {
     uint32_t nr_servers;
     uint32_t nr_irqs;
     ICPState *ss;
-    ICSState *ics;
+    QLIST_HEAD(, ICSState) ics;
 };
 
 #define TYPE_ICP "icp"
@@ -110,6 +110,7 @@ struct ICPState {
     DeviceState parent_obj;
     /*< public >*/
     CPUState *cs;
+    ICSState *xirr_owner;
     uint32_t xirr;
     uint8_t pending_priority;
     uint8_t mfrr;
@@ -144,6 +145,7 @@ struct ICSState {
     qemu_irq *qirqs;
     ICSIRQState *irqs;
     XICSState *xics;
+    QLIST_ENTRY(ICSState) list;
 };
 
 static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
@@ -171,10 +173,9 @@ struct ICSIRQState {
 #define XICS_IRQS_SPAPR               1024
 
 qemu_irq xics_get_qirq(XICSState *icp, int irq);
-int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
-                     Error **errp);
-int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
-                           bool align, Error **errp);
+int xics_spapr_alloc(XICSState *icp, int irq_hint, bool lsi, Error **errp);
+int xics_spapr_alloc_block(XICSState *icp, int num, bool lsi, bool align,
+                           Error **errp);
 void xics_spapr_free(XICSState *icp, int irq, int num);
 
 void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
@@ -194,6 +195,6 @@ void ics_write_xive(ICSState *ics, int nr, int server,
 
 void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
 
-int xics_find_source(XICSState *icp, int irq);
+ICSState *xics_find_source(XICSState *icp, int irq);
 
 #endif /* __XICS_H__ */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 6/8] ppc/xics: An ICS with offset 0 is assumed to be uninitialized
  2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
                   ` (4 preceding siblings ...)
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 5/8] ppc/xics: Make the ICSState a list Nikunj A Dadhania
@ 2016-06-28 19:05 ` Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 7/8] ppc/xics: Use a helper to add a new ICS Nikunj A Dadhania
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-28 19:05 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, clg, benh, nikunj

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

This will make life easier for dealing with dynamically configured
ICSes such as PHB3

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 include/hw/ppc/xics.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 6ad3057..8c22daf 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -150,7 +150,7 @@ struct ICSState {
 
 static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
 {
-    return (nr >= ics->offset)
+    return (ics->offset != 0) && (nr >= ics->offset)
         && (nr < (ics->offset + ics->nr_irqs));
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 7/8] ppc/xics: Use a helper to add a new ICS
  2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
                   ` (5 preceding siblings ...)
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 6/8] ppc/xics: An ICS with offset 0 is assumed to be uninitialized Nikunj A Dadhania
@ 2016-06-28 19:05 ` Nikunj A Dadhania
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class Nikunj A Dadhania
  2016-06-29  3:40 ` [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup David Gibson
  8 siblings, 0 replies; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-28 19:05 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, clg, benh, nikunj

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[Move object allocation and adding child to the helper]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/intc/xics.c        | 10 ++++++++++
 hw/intc/xics_spapr.c  |  6 +-----
 include/hw/ppc/xics.h |  1 +
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 5148bdf..bbdba84 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -108,6 +108,16 @@ static void xics_common_reset(DeviceState *d)
     }
 }
 
+void xics_add_ics(XICSState *xics)
+{
+    ICSState *ics;
+
+    ics = ICS(object_new(TYPE_ICS));
+    object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
+    ics->xics = xics;
+    QLIST_INSERT_HEAD(&xics->ics, ics, list);
+}
+
 static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
                                   void *opaque, Error **errp)
 {
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 0b0845d..270f20e 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -305,12 +305,8 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp)
 static void xics_spapr_initfn(Object *obj)
 {
     XICSState *xics = XICS_SPAPR(obj);
-    ICSState *ics;
 
-    ics = ICS(object_new(TYPE_ICS));
-    object_property_add_child(obj, "ics", OBJECT(ics), NULL);
-    ics->xics = xics;
-    QLIST_INSERT_HEAD(&xics->ics, ics, list);
+    xics_add_ics(xics);
 }
 
 static void xics_spapr_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 8c22daf..8433bf9 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -196,5 +196,6 @@ void ics_write_xive(ICSState *ics, int nr, int server,
 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__ */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class
  2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
                   ` (6 preceding siblings ...)
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 7/8] ppc/xics: Use a helper to add a new ICS Nikunj A Dadhania
@ 2016-06-28 19:05 ` Nikunj A Dadhania
  2016-06-30  8:24   ` Cédric Le Goater
  2016-06-29  3:40 ` [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup David Gibson
  8 siblings, 1 reply; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-28 19:05 UTC (permalink / raw)
  To: qemu-ppc, david; +Cc: qemu-devel, clg, benh, nikunj

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

The existing implementation remains same and ics-base is introduced. The
type name "ics" is retained, and all the related functions renamed as
ics_simple_*

This will allow different implementations for the source controllers
such as the MSI support of PHB3 on Power8 which uses in-memory state
tables for example.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/intc/trace-events  |  10 ++--
 hw/intc/xics.c        | 146 +++++++++++++++++++++++++++++++-------------------
 hw/intc/xics_kvm.c    |  10 ++--
 hw/intc/xics_spapr.c  |  28 +++++-----
 include/hw/ppc/xics.h |  23 +++++---
 5 files changed, 132 insertions(+), 85 deletions(-)

diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 5f0f783..e5e7ec7 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -50,12 +50,12 @@ xics_icp_accept(uint32_t old_xirr, uint32_t new_xirr) "icp_accept: XIRR %#"PRIx3
 xics_icp_eoi(int server, uint32_t xirr, uint32_t new_xirr) "icp_eoi: server %d given XIRR %#"PRIx32" new XIRR %#"PRIx32
 xics_icp_irq(int server, int nr, uint8_t priority) "cpu %d trying to deliver irq %#"PRIx32" priority %#x"
 xics_icp_raise(uint32_t xirr, uint8_t pending_priority) "raising IRQ new XIRR=%#x new pending priority=%#x"
-xics_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq %#x]"
+xics_ics_simple_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq %#x]"
 xics_masked_pending(void) "set_irq_msi: masked pending"
-xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
-xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
-xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
-xics_ics_eoi(int nr) "ics_eoi: irq %#x"
+xics_ics_simple_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
+xics_ics_simple_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
+xics_ics_simple_reject(int nr, int srcno) "reject irq %#x [src %d]"
+xics_ics_simple_eoi(int nr) "ics_eoi: irq %#x"
 xics_alloc(int irq) "irq %d"
 xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d irqs, lsi=%d, alignnum %d"
 xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index bbdba84..39928d9 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -112,7 +112,7 @@ void xics_add_ics(XICSState *xics)
 {
     ICSState *ics;
 
-    ics = ICS(object_new(TYPE_ICS));
+    ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE));
     object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
     ics->xics = xics;
     QLIST_INSERT_HEAD(&xics->ics, ics, list);
@@ -223,9 +223,32 @@ static const TypeInfo xics_common_info = {
 #define XISR(ss)   (((ss)->xirr) & XISR_MASK)
 #define CPPR(ss)   (((ss)->xirr) >> 24)
 
-static void ics_reject(ICSState *ics, int nr);
-static void ics_resend(ICSState *ics);
-static void ics_eoi(ICSState *ics, int nr);
+static void ics_reject(ICSState *ics, uint32_t nr)
+{
+    ICSStateClass *k = ICS_GET_CLASS(ics);
+
+    if (k->reject) {
+        k->reject(ics, nr);
+    }
+}
+
+static void ics_resend(ICSState *ics)
+{
+    ICSStateClass *k = ICS_GET_CLASS(ics);
+
+    if (k->resend) {
+        k->resend(ics);
+    }
+}
+
+static void ics_eoi(ICSState *ics, int nr)
+{
+    ICSStateClass *k = ICS_GET_CLASS(ics);
+
+    if (k->eoi) {
+        k->eoi(ics, nr);
+    }
+}
 
 static void icp_check_ipi(ICPState *ss)
 {
@@ -428,7 +451,7 @@ static const TypeInfo icp_info = {
 /*
  * ICS: Source layer
  */
-static void resend_msi(ICSState *ics, int srcno)
+static void ics_simple_resend_msi(ICSState *ics, int srcno)
 {
     ICSIRQState *irq = ics->irqs + srcno;
 
@@ -441,7 +464,7 @@ static void resend_msi(ICSState *ics, int srcno)
     }
 }
 
-static void resend_lsi(ICSState *ics, int srcno)
+static void ics_simple_resend_lsi(ICSState *ics, int srcno)
 {
     ICSIRQState *irq = ics->irqs + srcno;
 
@@ -453,11 +476,11 @@ static void resend_lsi(ICSState *ics, int srcno)
     }
 }
 
-static void set_irq_msi(ICSState *ics, int srcno, int val)
+static void ics_simple_set_irq_msi(ICSState *ics, int srcno, int val)
 {
     ICSIRQState *irq = ics->irqs + srcno;
 
-    trace_xics_set_irq_msi(srcno, srcno + ics->offset);
+    trace_xics_ics_simple_set_irq_msi(srcno, srcno + ics->offset);
 
     if (val) {
         if (irq->priority == 0xff) {
@@ -469,31 +492,31 @@ static void set_irq_msi(ICSState *ics, int srcno, int val)
     }
 }
 
-static void set_irq_lsi(ICSState *ics, int srcno, int val)
+static void ics_simple_set_irq_lsi(ICSState *ics, int srcno, int val)
 {
     ICSIRQState *irq = ics->irqs + srcno;
 
-    trace_xics_set_irq_lsi(srcno, srcno + ics->offset);
+    trace_xics_ics_simple_set_irq_lsi(srcno, srcno + ics->offset);
     if (val) {
         irq->status |= XICS_STATUS_ASSERTED;
     } else {
         irq->status &= ~XICS_STATUS_ASSERTED;
     }
-    resend_lsi(ics, srcno);
+    ics_simple_resend_lsi(ics, srcno);
 }
 
-static void ics_set_irq(void *opaque, int srcno, int val)
+static void ics_simple_set_irq(void *opaque, int srcno, int val)
 {
     ICSState *ics = (ICSState *)opaque;
 
     if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
-        set_irq_lsi(ics, srcno, val);
+        ics_simple_set_irq_lsi(ics, srcno, val);
     } else {
-        set_irq_msi(ics, srcno, val);
+        ics_simple_set_irq_msi(ics, srcno, val);
     }
 }
 
-static void write_xive_msi(ICSState *ics, int srcno)
+static void ics_simple_write_xive_msi(ICSState *ics, int srcno)
 {
     ICSIRQState *irq = ics->irqs + srcno;
 
@@ -506,68 +529,68 @@ static void write_xive_msi(ICSState *ics, int srcno)
     icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
 }
 
-static void write_xive_lsi(ICSState *ics, int srcno)
+static void ics_simple_write_xive_lsi(ICSState *ics, int srcno)
 {
-    resend_lsi(ics, srcno);
+    ics_simple_resend_lsi(ics, srcno);
 }
 
-void ics_write_xive(ICSState *ics, int nr, int server,
-                    uint8_t priority, uint8_t saved_priority)
+void ics_simple_write_xive(ICSState *ics, int srcno, int server,
+                           uint8_t priority, uint8_t saved_priority)
 {
-    int srcno = nr - ics->offset;
     ICSIRQState *irq = ics->irqs + srcno;
 
     irq->server = server;
     irq->priority = priority;
     irq->saved_priority = saved_priority;
 
-    trace_xics_ics_write_xive(nr, srcno, server, priority);
+    trace_xics_ics_simple_write_xive(ics->offset + srcno, srcno, server,
+                                     priority);
 
     if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
-        write_xive_lsi(ics, srcno);
+        ics_simple_write_xive_lsi(ics, srcno);
     } else {
-        write_xive_msi(ics, srcno);
+        ics_simple_write_xive_msi(ics, srcno);
     }
 }
 
-static void ics_reject(ICSState *ics, int nr)
+static void ics_simple_reject(ICSState *ics, uint32_t nr)
 {
     ICSIRQState *irq = ics->irqs + nr - ics->offset;
 
-    trace_xics_ics_reject(nr, nr - ics->offset);
+    trace_xics_ics_simple_reject(nr, nr - ics->offset);
     irq->status |= XICS_STATUS_REJECTED; /* Irrelevant but harmless for LSI */
     irq->status &= ~XICS_STATUS_SENT; /* Irrelevant but harmless for MSI */
 }
 
-static void ics_resend(ICSState *ics)
+static void ics_simple_resend(ICSState *ics)
 {
     int i;
 
     for (i = 0; i < ics->nr_irqs; i++) {
         /* FIXME: filter by server#? */
         if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
-            resend_lsi(ics, i);
+            ics_simple_resend_lsi(ics, i);
         } else {
-            resend_msi(ics, i);
+            ics_simple_resend_msi(ics, i);
         }
     }
 }
 
-static void ics_eoi(ICSState *ics, int nr)
+static void ics_simple_eoi(ICSState *ics, uint32_t nr)
 {
     int srcno = nr - ics->offset;
     ICSIRQState *irq = ics->irqs + srcno;
 
-    trace_xics_ics_eoi(nr);
+    trace_xics_ics_simple_eoi(nr);
 
     if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
         irq->status &= ~XICS_STATUS_SENT;
     }
 }
 
-static void ics_reset(DeviceState *dev)
+static void ics_simple_reset(DeviceState *dev)
 {
-    ICSState *ics = ICS(dev);
+    ICSState *ics = ICS_SIMPLE(dev);
     int i;
     uint8_t flags[ics->nr_irqs];
 
@@ -584,7 +607,7 @@ static void ics_reset(DeviceState *dev)
     }
 }
 
-static int ics_post_load(ICSState *ics, int version_id)
+static int ics_simple_post_load(ICSState *ics, int version_id)
 {
     int i;
 
@@ -595,7 +618,7 @@ static int ics_post_load(ICSState *ics, int version_id)
     return 0;
 }
 
-static void ics_dispatch_pre_save(void *opaque)
+static void ics_simple_dispatch_pre_save(void *opaque)
 {
     ICSState *ics = opaque;
     ICSStateClass *info = ICS_GET_CLASS(ics);
@@ -605,7 +628,7 @@ static void ics_dispatch_pre_save(void *opaque)
     }
 }
 
-static int ics_dispatch_post_load(void *opaque, int version_id)
+static int ics_simple_dispatch_post_load(void *opaque, int version_id)
 {
     ICSState *ics = opaque;
     ICSStateClass *info = ICS_GET_CLASS(ics);
@@ -617,7 +640,7 @@ static int ics_dispatch_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static const VMStateDescription vmstate_ics_irq = {
+static const VMStateDescription vmstate_ics_simple_irq = {
     .name = "ics/irq",
     .version_id = 2,
     .minimum_version_id = 1,
@@ -631,59 +654,71 @@ static const VMStateDescription vmstate_ics_irq = {
     },
 };
 
-static const VMStateDescription vmstate_ics = {
+static const VMStateDescription vmstate_ics_simple = {
     .name = "ics",
     .version_id = 1,
     .minimum_version_id = 1,
-    .pre_save = ics_dispatch_pre_save,
-    .post_load = ics_dispatch_post_load,
+    .pre_save = ics_simple_dispatch_pre_save,
+    .post_load = ics_simple_dispatch_post_load,
     .fields = (VMStateField[]) {
         /* Sanity check */
         VMSTATE_UINT32_EQUAL(nr_irqs, ICSState),
 
         VMSTATE_STRUCT_VARRAY_POINTER_UINT32(irqs, ICSState, nr_irqs,
-                                             vmstate_ics_irq, ICSIRQState),
+                                             vmstate_ics_simple_irq,
+                                             ICSIRQState),
         VMSTATE_END_OF_LIST()
     },
 };
 
-static void ics_initfn(Object *obj)
+static void ics_simple_initfn(Object *obj)
 {
-    ICSState *ics = ICS(obj);
+    ICSState *ics = ICS_SIMPLE(obj);
 
     ics->offset = XICS_IRQ_BASE;
 }
 
-static void ics_realize(DeviceState *dev, Error **errp)
+static void ics_simple_realize(DeviceState *dev, Error **errp)
 {
-    ICSState *ics = ICS(dev);
+    ICSState *ics = ICS_SIMPLE(dev);
 
     if (!ics->nr_irqs) {
         error_setg(errp, "Number of interrupts needs to be greater 0");
         return;
     }
     ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
-    ics->qirqs = qemu_allocate_irqs(ics_set_irq, ics, ics->nr_irqs);
+    ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
 }
 
-static void ics_class_init(ObjectClass *klass, void *data)
+static void ics_simple_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     ICSStateClass *isc = ICS_CLASS(klass);
 
-    dc->realize = ics_realize;
-    dc->vmsd = &vmstate_ics;
-    dc->reset = ics_reset;
-    isc->post_load = ics_post_load;
+    dc->realize = ics_simple_realize;
+    dc->vmsd = &vmstate_ics_simple;
+    dc->reset = ics_simple_reset;
+    isc->post_load = ics_simple_post_load;
+    isc->reject = ics_simple_reject;
+    isc->resend = ics_simple_resend;
+    isc->eoi = ics_simple_eoi;
 }
 
-static const TypeInfo ics_info = {
-    .name = TYPE_ICS,
+static const TypeInfo ics_simple_info = {
+    .name = TYPE_ICS_SIMPLE,
+    .parent = TYPE_ICS_BASE,
+    .instance_size = sizeof(ICSState),
+    .class_init = ics_simple_class_init,
+    .class_size = sizeof(ICSStateClass),
+    .instance_init = ics_simple_initfn,
+};
+
+static const TypeInfo ics_base_info = {
+    .name = TYPE_ICS_BASE,
     .parent = TYPE_DEVICE,
+    .abstract = true,
     .instance_size = sizeof(ICSState),
-    .class_init = ics_class_init,
     .class_size = sizeof(ICSStateClass),
-    .instance_init = ics_initfn,
 };
 
 /*
@@ -723,7 +758,8 @@ void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
 static void xics_register_types(void)
 {
     type_register_static(&xics_common_info);
-    type_register_static(&ics_info);
+    type_register_static(&ics_simple_info);
+    type_register_static(&ics_base_info);
     type_register_static(&icp_info);
 }
 
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index 04fa7cb..89862df 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -272,7 +272,7 @@ static void ics_kvm_set_irq(void *opaque, int srcno, int val)
 
 static void ics_kvm_reset(DeviceState *dev)
 {
-    ICSState *ics = ICS(dev);
+    ICSState *ics = ICS_SIMPLE(dev);
     int i;
     uint8_t flags[ics->nr_irqs];
 
@@ -293,7 +293,7 @@ static void ics_kvm_reset(DeviceState *dev)
 
 static void ics_kvm_realize(DeviceState *dev, Error **errp)
 {
-    ICSState *ics = ICS(dev);
+    ICSState *ics = ICS_SIMPLE(dev);
 
     if (!ics->nr_irqs) {
         error_setg(errp, "Number of interrupts needs to be greater 0");
@@ -315,8 +315,8 @@ static void ics_kvm_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ics_kvm_info = {
-    .name = TYPE_KVM_ICS,
-    .parent = TYPE_ICS,
+    .name = TYPE_ICS_KVM,
+    .parent = TYPE_ICS_SIMPLE,
     .instance_size = sizeof(ICSState),
     .class_init = ics_kvm_class_init,
 };
@@ -492,7 +492,7 @@ static void xics_kvm_initfn(Object *obj)
     XICSState *xics = XICS_COMMON(obj);
     ICSState *ics;
 
-    ics = ICS(object_new(TYPE_KVM_ICS));
+    ics = ICS_SIMPLE(object_new(TYPE_ICS_KVM));
     object_property_add_child(obj, "ics", OBJECT(ics), NULL);
     ics->xics = xics;
     QLIST_INSERT_HEAD(&xics->ics, ics, list);
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 270f20e..4dd1399 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -114,7 +114,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           uint32_t nret, target_ulong rets)
 {
     ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
-    uint32_t nr, server, priority;
+    uint32_t nr, srcno, server, priority;
 
     if ((nargs != 3) || (nret != 1)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
@@ -135,7 +135,8 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         return;
     }
 
-    ics_write_xive(ics, nr, server, priority, priority);
+    srcno = nr - ics->offset;
+    ics_simple_write_xive(ics, srcno, server, priority, priority);
 
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
@@ -146,7 +147,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           uint32_t nret, target_ulong rets)
 {
     ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
-    uint32_t nr;
+    uint32_t nr, srcno;
 
     if ((nargs != 1) || (nret != 3)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
@@ -165,8 +166,9 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     }
 
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-    rtas_st(rets, 1, ics->irqs[nr - ics->offset].server);
-    rtas_st(rets, 2, ics->irqs[nr - ics->offset].priority);
+    srcno = nr - ics->offset;
+    rtas_st(rets, 1, ics->irqs[srcno].server);
+    rtas_st(rets, 2, ics->irqs[srcno].priority);
 }
 
 static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
@@ -175,7 +177,7 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                          uint32_t nret, target_ulong rets)
 {
     ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
-    uint32_t nr;
+    uint32_t nr, srcno;
 
     if ((nargs != 1) || (nret != 1)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
@@ -193,8 +195,9 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         return;
     }
 
-    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server, 0xff,
-                   ics->irqs[nr - ics->offset].priority);
+    srcno = nr - ics->offset;
+    ics_simple_write_xive(ics, srcno, ics->irqs[srcno].server, 0xff,
+                          ics->irqs[srcno].priority);
 
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
@@ -205,7 +208,7 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                         uint32_t nret, target_ulong rets)
 {
     ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
-    uint32_t nr;
+    uint32_t nr, srcno;
 
     if ((nargs != 1) || (nret != 1)) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
@@ -223,9 +226,10 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         return;
     }
 
-    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server,
-                   ics->irqs[nr - ics->offset].saved_priority,
-                   ics->irqs[nr - ics->offset].saved_priority);
+    srcno = nr - ics->offset;
+    ics_simple_write_xive(ics, srcno, ics->irqs[srcno].server,
+                          ics->irqs[srcno].saved_priority,
+                          ics->irqs[srcno].saved_priority);
 
     rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 8433bf9..bc10c16 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -118,22 +118,29 @@ struct ICPState {
     bool cap_irq_xics_enabled;
 };
 
-#define TYPE_ICS "ics"
-#define ICS(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS)
+#define TYPE_ICS_BASE "ics-base"
+#define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE)
 
-#define TYPE_KVM_ICS "icskvm"
-#define KVM_ICS(obj) OBJECT_CHECK(ICSState, (obj), TYPE_KVM_ICS)
+/* Retain ics for sPAPR for migration from existing sPAPR guests */
+#define TYPE_ICS_SIMPLE "ics"
+#define ICS_SIMPLE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE)
+
+#define TYPE_ICS_KVM "icskvm"
+#define ICS_KVM(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_KVM)
 
 #define ICS_CLASS(klass) \
-     OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS)
+     OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS_SIMPLE)
 #define ICS_GET_CLASS(obj) \
-     OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS)
+     OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS_SIMPLE)
 
 struct ICSStateClass {
     DeviceClass parent_class;
 
     void (*pre_save)(ICSState *s);
     int (*post_load)(ICSState *s, int version_id);
+    void (*reject)(ICSState *s, uint32_t irq);
+    void (*resend)(ICSState *s);
+    void (*eoi)(ICSState *s, uint32_t irq);
 };
 
 struct ICSState {
@@ -190,8 +197,8 @@ uint32_t icp_accept(ICPState *ss);
 uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr);
 void icp_eoi(XICSState *icp, int server, uint32_t xirr);
 
-void ics_write_xive(ICSState *ics, int nr, int server,
-                    uint8_t priority, uint8_t saved_priority);
+void ics_simple_write_xive(ICSState *ics, int nr, int server,
+                           uint8_t priority, uint8_t saved_priority);
 
 void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 5/8] ppc/xics: Make the ICSState a list
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 5/8] ppc/xics: Make the ICSState a list Nikunj A Dadhania
@ 2016-06-29  3:37   ` David Gibson
  2016-06-29  4:05     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2016-06-29  3:37 UTC (permalink / raw)
  To: Nikunj A Dadhania; +Cc: qemu-ppc, qemu-devel, clg, benh

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

On Wed, Jun 29, 2016 at 12:35:16AM +0530, Nikunj A Dadhania wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Instead of an array of fixed sized blocks, use a list, as we will need
> to have sources with variable number of interrupts. SPAPR only uses
> a single entry. Native will create more. If performance becomes an
> issue we can add some hashed lookup but for now this will do fine.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> [ move the initialization of list to xics_common_initfn ]
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

AFAICT xirr_owner will be lost on migration, which will break things.
That will need to be transferred on migration, somehow.  If it can be
recalculated from existing data in post_load() that would be ideal,
otherwise we'll have to devise a wire encoding for it.

> ---
>  hw/intc/trace-events  |  4 +--
>  hw/intc/xics.c        | 83 ++++++++++++++++++++++++++++--------------------
>  hw/intc/xics_kvm.c    | 27 +++++++++++-----
>  hw/intc/xics_spapr.c  | 88 +++++++++++++++++++++++++++++++++------------------
>  hw/ppc/spapr_events.c |  2 +-
>  hw/ppc/spapr_pci.c    |  5 ++-
>  hw/ppc/spapr_vio.c    |  2 +-
>  include/hw/ppc/xics.h | 13 ++++----
>  8 files changed, 138 insertions(+), 86 deletions(-)
> 
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index 376dd18..5f0f783 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -56,8 +56,8 @@ xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
>  xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
>  xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
>  xics_ics_eoi(int nr) "ics_eoi: irq %#x"
> -xics_alloc(int src, int irq) "source#%d, irq %d"
> -xics_alloc_block(int src, int first, int num, bool lsi, int align) "source#%d, first irq %d, %d irqs, lsi=%d, alignnum %d"
> +xics_alloc(int irq) "irq %d"
> +xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d irqs, lsi=%d, alignnum %d"
>  xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
>  xics_ics_free_warn(int src, int irq) "Source#%d, irq %d is already free"
>  
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index cd48f42..5148bdf 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -96,13 +96,16 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
>  static void xics_common_reset(DeviceState *d)
>  {
>      XICSState *xics = XICS_COMMON(d);
> +    ICSState *ics;
>      int i;
>  
>      for (i = 0; i < xics->nr_servers; i++) {
>          device_reset(DEVICE(&xics->ss[i]));
>      }
>  
> -    device_reset(DEVICE(xics->ics));
> +    QLIST_FOREACH(ics, &xics->ics, list) {
> +        device_reset(DEVICE(ics));
> +    }
>  }
>  
>  static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
> @@ -134,7 +137,6 @@ static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name,
>      }
>  
>      assert(info->set_nr_irqs);
> -    assert(xics->ics);
>      info->set_nr_irqs(xics, value, errp);
>  }
>  
> @@ -174,6 +176,9 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor *v,
>  
>  static void xics_common_initfn(Object *obj)
>  {
> +    XICSState *xics = XICS_COMMON(obj);
> +
> +    QLIST_INIT(&xics->ics);
>      object_property_add(obj, "nr_irqs", "int",
>                          xics_prop_get_nr_irqs, xics_prop_set_nr_irqs,
>                          NULL, NULL, NULL);
> @@ -212,33 +217,35 @@ static void ics_reject(ICSState *ics, int nr);
>  static void ics_resend(ICSState *ics);
>  static void ics_eoi(ICSState *ics, int nr);
>  
> -static void icp_check_ipi(XICSState *xics, int server)
> +static void icp_check_ipi(ICPState *ss)
>  {
> -    ICPState *ss = xics->ss + server;
> -
>      if (XISR(ss) && (ss->pending_priority <= ss->mfrr)) {
>          return;
>      }
>  
> -    trace_xics_icp_check_ipi(server, ss->mfrr);
> +    trace_xics_icp_check_ipi(ss->cs->cpu_index, ss->mfrr);
>  
> -    if (XISR(ss)) {
> -        ics_reject(xics->ics, XISR(ss));
> +    if (XISR(ss) && ss->xirr_owner) {
> +        ics_reject(ss->xirr_owner, XISR(ss));
>      }
>  
>      ss->xirr = (ss->xirr & ~XISR_MASK) | XICS_IPI;
>      ss->pending_priority = ss->mfrr;
> +    ss->xirr_owner = NULL;
>      qemu_irq_raise(ss->output);
>  }
>  
>  static void icp_resend(XICSState *xics, int server)
>  {
>      ICPState *ss = xics->ss + server;
> +    ICSState *ics;
>  
>      if (ss->mfrr < CPPR(ss)) {
> -        icp_check_ipi(xics, server);
> +        icp_check_ipi(ss);
> +    }
> +    QLIST_FOREACH(ics, &xics->ics, list) {
> +        ics_resend(ics);
>      }
> -    ics_resend(xics->ics);
>  }
>  
>  void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
> @@ -256,7 +263,10 @@ void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
>              ss->xirr &= ~XISR_MASK; /* Clear XISR */
>              ss->pending_priority = 0xff;
>              qemu_irq_lower(ss->output);
> -            ics_reject(xics->ics, old_xisr);
> +            if (ss->xirr_owner) {
> +                ics_reject(ss->xirr_owner, old_xisr);
> +                ss->xirr_owner = NULL;
> +            }
>          }
>      } else {
>          if (!XISR(ss)) {
> @@ -271,7 +281,7 @@ void icp_set_mfrr(XICSState *xics, int server, uint8_t mfrr)
>  
>      ss->mfrr = mfrr;
>      if (mfrr < CPPR(ss)) {
> -        icp_check_ipi(xics, server);
> +        icp_check_ipi(ss);
>      }
>  }
>  
> @@ -282,6 +292,7 @@ uint32_t icp_accept(ICPState *ss)
>      qemu_irq_lower(ss->output);
>      ss->xirr = ss->pending_priority << 24;
>      ss->pending_priority = 0xff;
> +    ss->xirr_owner = NULL;
>  
>      trace_xics_icp_accept(xirr, ss->xirr);
>  
> @@ -299,30 +310,40 @@ uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr)
>  void icp_eoi(XICSState *xics, int server, uint32_t xirr)
>  {
>      ICPState *ss = xics->ss + server;
> +    ICSState *ics;
> +    uint32_t irq;
>  
>      /* Send EOI -> ICS */
>      ss->xirr = (ss->xirr & ~CPPR_MASK) | (xirr & CPPR_MASK);
>      trace_xics_icp_eoi(server, xirr, ss->xirr);
> -    ics_eoi(xics->ics, xirr & XISR_MASK);
> +    irq = xirr & XISR_MASK;
> +    QLIST_FOREACH(ics, &xics->ics, list) {
> +        if (ics_valid_irq(ics, irq)) {
> +            ics_eoi(ics, irq);
> +        }
> +    }
>      if (!XISR(ss)) {
>          icp_resend(xics, server);
>      }
>  }
>  
> -static void icp_irq(XICSState *xics, int server, int nr, uint8_t priority)
> +static void icp_irq(ICSState *ics, int server, int nr, uint8_t priority)
>  {
> +    XICSState *xics = ics->xics;
>      ICPState *ss = xics->ss + server;
>  
>      trace_xics_icp_irq(server, nr, priority);
>  
>      if ((priority >= CPPR(ss))
>          || (XISR(ss) && (ss->pending_priority <= priority))) {
> -        ics_reject(xics->ics, nr);
> +        ics_reject(ics, nr);
>      } else {
> -        if (XISR(ss)) {
> -            ics_reject(xics->ics, XISR(ss));
> +        if (XISR(ss) && ss->xirr_owner) {
> +            ics_reject(ss->xirr_owner, XISR(ss));
> +            ss->xirr_owner = NULL;
>          }
>          ss->xirr = (ss->xirr & ~XISR_MASK) | (nr & XISR_MASK);
> +        ss->xirr_owner = ics;
>          ss->pending_priority = priority;
>          trace_xics_icp_raise(ss->xirr, ss->pending_priority);
>          qemu_irq_raise(ss->output);
> @@ -405,8 +426,7 @@ static void resend_msi(ICSState *ics, int srcno)
>      if (irq->status & XICS_STATUS_REJECTED) {
>          irq->status &= ~XICS_STATUS_REJECTED;
>          if (irq->priority != 0xff) {
> -            icp_irq(ics->xics, irq->server, srcno + ics->offset,
> -                    irq->priority);
> +            icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
>          }
>      }
>  }
> @@ -419,7 +439,7 @@ static void resend_lsi(ICSState *ics, int srcno)
>          && (irq->status & XICS_STATUS_ASSERTED)
>          && !(irq->status & XICS_STATUS_SENT)) {
>          irq->status |= XICS_STATUS_SENT;
> -        icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
> +        icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
>      }
>  }
>  
> @@ -434,7 +454,7 @@ static void set_irq_msi(ICSState *ics, int srcno, int val)
>              irq->status |= XICS_STATUS_MASKED_PENDING;
>              trace_xics_masked_pending();
>          } else  {
> -            icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
> +            icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
>          }
>      }
>  }
> @@ -473,7 +493,7 @@ static void write_xive_msi(ICSState *ics, int srcno)
>      }
>  
>      irq->status &= ~XICS_STATUS_MASKED_PENDING;
> -    icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
> +    icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
>  }
>  
>  static void write_xive_lsi(ICSState *ics, int srcno)
> @@ -659,28 +679,23 @@ static const TypeInfo ics_info = {
>  /*
>   * Exported functions
>   */
> -int xics_find_source(XICSState *xics, int irq)
> +ICSState *xics_find_source(XICSState *xics, int irq)
>  {
> -    int sources = 1;
> -    int src;
> +    ICSState *ics;
>  
> -    /* FIXME: implement multiple sources */
> -    for (src = 0; src < sources; ++src) {
> -        ICSState *ics = &xics->ics[src];
> +    QLIST_FOREACH(ics, &xics->ics, list) {
>          if (ics_valid_irq(ics, irq)) {
> -            return src;
> +            return ics;
>          }
>      }
> -
> -    return -1;
> +    return NULL;
>  }
>  
>  qemu_irq xics_get_qirq(XICSState *xics, int irq)
>  {
> -    int src = xics_find_source(xics, irq);
> +    ICSState *ics = xics_find_source(xics, irq);
>  
> -    if (src >= 0) {
> -        ICSState *ics = &xics->ics[src];
> +    if (ics) {
>          return ics->qirqs[irq - ics->offset];
>      }
>  
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index edbd62f..04fa7cb 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -365,7 +365,13 @@ static void xics_kvm_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
>  static void xics_kvm_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
>                                   Error **errp)
>  {
> -    xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
> +    ICSState *ics = QLIST_FIRST(&xics->ics);
> +
> +    /* This needs to be deprecated ... */
> +    xics->nr_irqs = nr_irqs;
> +    if (ics) {
> +        ics->nr_irqs = nr_irqs;
> +    }
>  }
>  
>  static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers,
> @@ -398,6 +404,7 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp)
>  {
>      KVMXICSState *xicskvm = XICS_SPAPR_KVM(dev);
>      XICSState *xics = XICS_COMMON(dev);
> +    ICSState *ics;
>      int i, rc;
>      Error *error = NULL;
>      struct kvm_create_device xics_create_device = {
> @@ -449,10 +456,12 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp)
>  
>      xicskvm->kernel_xics_fd = xics_create_device.fd;
>  
> -    object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
> -    if (error) {
> -        error_propagate(errp, error);
> -        goto fail;
> +    QLIST_FOREACH(ics, &xics->ics, list) {
> +        object_property_set_bool(OBJECT(ics), true, "realized", &error);
> +        if (error) {
> +            error_propagate(errp, error);
> +            goto fail;
> +        }
>      }
>  
>      assert(xics->nr_servers);
> @@ -481,10 +490,12 @@ fail:
>  static void xics_kvm_initfn(Object *obj)
>  {
>      XICSState *xics = XICS_COMMON(obj);
> +    ICSState *ics;
>  
> -    xics->ics = ICS(object_new(TYPE_KVM_ICS));
> -    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
> -    xics->ics->xics = xics;
> +    ics = ICS(object_new(TYPE_KVM_ICS));
> +    object_property_add_child(obj, "ics", OBJECT(ics), NULL);
> +    ics->xics = xics;
> +    QLIST_INSERT_HEAD(&xics->ics, ics, list);
>  }
>  
>  static void xics_kvm_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 618826d..0b0845d 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -113,13 +113,17 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                            uint32_t nargs, target_ulong args,
>                            uint32_t nret, target_ulong rets)
>  {
> -    ICSState *ics = spapr->xics->ics;
> +    ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
>      uint32_t nr, server, priority;
>  
>      if ((nargs != 3) || (nret != 1)) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
>      }
> +    if (!ics) {
> +        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> +        return;
> +    }
>  
>      nr = rtas_ld(args, 0);
>      server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
> @@ -141,13 +145,17 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                            uint32_t nargs, target_ulong args,
>                            uint32_t nret, target_ulong rets)
>  {
> -    ICSState *ics = spapr->xics->ics;
> +    ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
>      uint32_t nr;
>  
>      if ((nargs != 1) || (nret != 3)) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
>      }
> +    if (!ics) {
> +        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> +        return;
> +    }
>  
>      nr = rtas_ld(args, 0);
>  
> @@ -166,13 +174,17 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                           uint32_t nargs, target_ulong args,
>                           uint32_t nret, target_ulong rets)
>  {
> -    ICSState *ics = spapr->xics->ics;
> +    ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
>      uint32_t nr;
>  
>      if ((nargs != 1) || (nret != 1)) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
>      }
> +    if (!ics) {
> +        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> +        return;
> +    }
>  
>      nr = rtas_ld(args, 0);
>  
> @@ -192,13 +204,17 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                          uint32_t nargs, target_ulong args,
>                          uint32_t nret, target_ulong rets)
>  {
> -    ICSState *ics = spapr->xics->ics;
> +    ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
>      uint32_t nr;
>  
>      if ((nargs != 1) || (nret != 1)) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>          return;
>      }
> +    if (!ics) {
> +        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> +        return;
> +    }
>  
>      nr = rtas_ld(args, 0);
>  
> @@ -217,7 +233,13 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>  static void xics_spapr_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
>                                     Error **errp)
>  {
> -    xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
> +    ICSState *ics = QLIST_FIRST(&xics->ics);
> +
> +    /* This needs to be deprecated ... */
> +    xics->nr_irqs = nr_irqs;
> +    if (ics) {
> +        ics->nr_irqs = nr_irqs;
> +    }
>  }
>  
>  static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers,
> @@ -240,6 +262,7 @@ static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers,
>  static void xics_spapr_realize(DeviceState *dev, Error **errp)
>  {
>      XICSState *xics = XICS_SPAPR(dev);
> +    ICSState *ics;
>      Error *error = NULL;
>      int i;
>  
> @@ -261,10 +284,12 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp)
>      spapr_register_hypercall(H_EOI, h_eoi);
>      spapr_register_hypercall(H_IPOLL, h_ipoll);
>  
> -    object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
> -    if (error) {
> -        error_propagate(errp, error);
> -        return;
> +    QLIST_FOREACH(ics, &xics->ics, list) {
> +        object_property_set_bool(OBJECT(ics), true, "realized", &error);
> +        if (error) {
> +            error_propagate(errp, error);
> +            return;
> +        }
>      }
>  
>      for (i = 0; i < xics->nr_servers; i++) {
> @@ -280,10 +305,12 @@ static void xics_spapr_realize(DeviceState *dev, Error **errp)
>  static void xics_spapr_initfn(Object *obj)
>  {
>      XICSState *xics = XICS_SPAPR(obj);
> +    ICSState *ics;
>  
> -    xics->ics = ICS(object_new(TYPE_ICS));
> -    object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
> -    xics->ics->xics = xics;
> +    ics = ICS(object_new(TYPE_ICS));
> +    object_property_add_child(obj, "ics", OBJECT(ics), NULL);
> +    ics->xics = xics;
> +    QLIST_INSERT_HEAD(&xics->ics, ics, list);
>  }
>  
>  static void xics_spapr_class_init(ObjectClass *oc, void *data)
> @@ -329,14 +356,15 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum)
>      return -1;
>  }
>  
> -int xics_spapr_alloc(XICSState *xics, int src, int irq_hint, bool lsi,
> -                     Error **errp)
> +int xics_spapr_alloc(XICSState *xics, int irq_hint, bool lsi, Error **errp)
>  {
> -    ICSState *ics = &xics->ics[src];
> +    ICSState *ics = QLIST_FIRST(&xics->ics);
>      int irq;
>  
> +    if (!ics) {
> +        return -1;
> +    }
>      if (irq_hint) {
> -        assert(src == xics_find_source(xics, irq_hint));
>          if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
>              error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
>              return -1;
> @@ -352,7 +380,7 @@ int xics_spapr_alloc(XICSState *xics, int src, int irq_hint, bool lsi,
>      }
>  
>      ics_set_irq_type(ics, irq - ics->offset, lsi);
> -    trace_xics_alloc(src, irq);
> +    trace_xics_alloc(irq);
>  
>      return irq;
>  }
> @@ -361,13 +389,16 @@ int xics_spapr_alloc(XICSState *xics, int src, int irq_hint, bool lsi,
>   * Allocate block of consecutive IRQs, and return the number of the first IRQ in
>   * the block. If align==true, aligns the first IRQ number to num.
>   */
> -int xics_spapr_alloc_block(XICSState *xics, int src, int num, bool lsi,
> -                           bool align, Error **errp)
> +int xics_spapr_alloc_block(XICSState *xics, int num, bool lsi, bool align,
> +                           Error **errp)
>  {
> +    ICSState *ics = QLIST_FIRST(&xics->ics);
>      int i, first = -1;
> -    ICSState *ics = &xics->ics[src];
>  
> -    assert(src == 0);
> +    if (!ics) {
> +        return -1;
> +    }
> +
>      /*
>       * MSIMesage::data is used for storing VIRQ so
>       * it has to be aligned to num to support multiple
> @@ -394,7 +425,7 @@ int xics_spapr_alloc_block(XICSState *xics, int src, int num, bool lsi,
>      }
>      first += ics->offset;
>  
> -    trace_xics_alloc_block(src, first, num, lsi, align);
> +    trace_xics_alloc_block(first, num, lsi, align);
>  
>      return first;
>  }
> @@ -405,7 +436,7 @@ static void ics_free(ICSState *ics, int srcno, int num)
>  
>      for (i = srcno; i < srcno + num; ++i) {
>          if (ICS_IRQ_FREE(ics, i)) {
> -            trace_xics_ics_free_warn(ics - ics->xics->ics, i + ics->offset);
> +            trace_xics_ics_free_warn(0, i + ics->offset);
>          }
>          memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
>      }
> @@ -413,15 +444,10 @@ static void ics_free(ICSState *ics, int srcno, int num)
>  
>  void xics_spapr_free(XICSState *xics, int irq, int num)
>  {
> -    int src = xics_find_source(xics, irq);
> -
> -    if (src >= 0) {
> -        ICSState *ics = &xics->ics[src];
> -
> -        /* FIXME: implement multiple sources */
> -        assert(src == 0);
> +    ICSState *ics = xics_find_source(xics, irq);
>  
> -        trace_xics_ics_free(ics - xics->ics, irq, num);
> +    if (ics) {
> +        trace_xics_ics_free(0, irq, num);
>          ics_free(ics, irq - ics->offset, num);
>      }
>  }
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index b0668b3..adf8da4 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -603,7 +603,7 @@ out_no_events:
>  void spapr_events_init(sPAPRMachineState *spapr)
>  {
>      QTAILQ_INIT(&spapr->pending_events);
> -    spapr->check_exception_irq = xics_spapr_alloc(spapr->xics, 0, 0, false,
> +    spapr->check_exception_irq = xics_spapr_alloc(spapr->xics, 0, false,
>                                              &error_fatal);
>      spapr->epow_notifier.notify = spapr_powerdown_req;
>      qemu_register_powerdown_notifier(&spapr->epow_notifier);
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 8c1e6b1..3d08c0a 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -360,7 +360,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>      }
>  
>      /* Allocate MSIs */
> -    irq = xics_spapr_alloc_block(spapr->xics, 0, req_num, false,
> +    irq = xics_spapr_alloc_block(spapr->xics, req_num, false,
>                             ret_intr_type == RTAS_TYPE_MSI, &err);
>      if (err) {
>          error_reportf_err(err, "Can't allocate MSIs for device %x: ",
> @@ -1442,8 +1442,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>          uint32_t irq;
>          Error *local_err = NULL;
>  
> -        irq = xics_spapr_alloc_block(spapr->xics, 0, 1, true, false,
> -                                     &local_err);
> +        irq = xics_spapr_alloc_block(spapr->xics, 1, true, false, &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
>              error_prepend(errp, "can't allocate LSIs: ");
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index f93244d..22360af 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -463,7 +463,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp)
>          dev->qdev.id = id;
>      }
>  
> -    dev->irq = xics_spapr_alloc(spapr->xics, 0, dev->irq, false, &local_err);
> +    dev->irq = xics_spapr_alloc(spapr->xics, dev->irq, false, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 6189a3b..6ad3057 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -84,7 +84,7 @@ struct XICSState {
>      uint32_t nr_servers;
>      uint32_t nr_irqs;
>      ICPState *ss;
> -    ICSState *ics;
> +    QLIST_HEAD(, ICSState) ics;
>  };
>  
>  #define TYPE_ICP "icp"
> @@ -110,6 +110,7 @@ struct ICPState {
>      DeviceState parent_obj;
>      /*< public >*/
>      CPUState *cs;
> +    ICSState *xirr_owner;
>      uint32_t xirr;
>      uint8_t pending_priority;
>      uint8_t mfrr;
> @@ -144,6 +145,7 @@ struct ICSState {
>      qemu_irq *qirqs;
>      ICSIRQState *irqs;
>      XICSState *xics;
> +    QLIST_ENTRY(ICSState) list;
>  };
>  
>  static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
> @@ -171,10 +173,9 @@ struct ICSIRQState {
>  #define XICS_IRQS_SPAPR               1024
>  
>  qemu_irq xics_get_qirq(XICSState *icp, int irq);
> -int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
> -                     Error **errp);
> -int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
> -                           bool align, Error **errp);
> +int xics_spapr_alloc(XICSState *icp, int irq_hint, bool lsi, Error **errp);
> +int xics_spapr_alloc_block(XICSState *icp, int num, bool lsi, bool align,
> +                           Error **errp);
>  void xics_spapr_free(XICSState *icp, int irq, int num);
>  
>  void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
> @@ -194,6 +195,6 @@ void ics_write_xive(ICSState *ics, int nr, int server,
>  
>  void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
>  
> -int xics_find_source(XICSState *icp, int irq);
> +ICSState *xics_find_source(XICSState *icp, int irq);
>  
>  #endif /* __XICS_H__ */

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup
  2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
                   ` (7 preceding siblings ...)
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class Nikunj A Dadhania
@ 2016-06-29  3:40 ` David Gibson
  8 siblings, 0 replies; 14+ messages in thread
From: David Gibson @ 2016-06-29  3:40 UTC (permalink / raw)
  To: Nikunj A Dadhania; +Cc: qemu-ppc, qemu-devel, clg, benh

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

On Wed, Jun 29, 2016 at 12:35:11AM +0530, Nikunj A Dadhania wrote:
> sPAPR xics related changes required for powernv platform. This brings
> infrastructure to get the xics native mode for powernv. Tested pseries guests
> in KVM and TCG mode.
> 
> Changelog v1:
>  * Change XICS to XICS_SPAPR and KVM_XICS to XICS_KVM_SPAPR
>  * Added xics_ to function get_cpu_index_by_dt_id as this is a global symbol
>  * Dropped server parameter from  icp_check_ipi
>  * Send HW_ERROR when ics is NULL
>  * Remove redundant parameters in trace routines
>  * Use type ICS_SIMPLE, ICS_BASE and ICS_KVM
>  * Dropped xics-native and info pic patches for this version
> 
> ToDo:
>  + Use ICPNative and XICSNative in "native" implementation
>  + xics_spapr_alloc - getting rid of that
>  + xirr_owner - how to reassign after migration
> 
> Benjamin Herrenschmidt (8):
>   ppc/xics: Rename existing xics to xics_spapr
>   ppc/xics: Move SPAPR specific code to a separate file
>   ppc/xics: Implement H_IPOLL using an accessor
>   ppc/xics: Replace "icp" with "xics" in most places
>   ppc/xics: Make the ICSState a list
>   ppc/xics: An ICS with offset 0 is assumed to be uninitialized
>   ppc/xics: Use a helper to add a new ICS
>   ppc/xics: Split ICS into ics-base and ics class
> 
>  default-configs/ppc64-softmmu.mak |   1 +
>  hw/intc/Makefile.objs             |   1 +
>  hw/intc/trace-events              |  14 +-
>  hw/intc/xics.c                    | 724 +++++++++++---------------------------
>  hw/intc/xics_kvm.c                |  92 +++--
>  hw/intc/xics_spapr.c              | 460 ++++++++++++++++++++++++
>  hw/ppc/spapr.c                    |  19 +-
>  hw/ppc/spapr_cpu_core.c           |   4 +-
>  hw/ppc/spapr_events.c             |   8 +-
>  hw/ppc/spapr_pci.c                |  12 +-
>  hw/ppc/spapr_vio.c                |   2 +-
>  include/hw/pci-host/spapr.h       |   2 +-
>  include/hw/ppc/spapr.h            |   2 +-
>  include/hw/ppc/spapr_vio.h        |   2 +-
>  include/hw/ppc/xics.h             |  79 +++--
>  15 files changed, 803 insertions(+), 619 deletions(-)
>  create mode 100644 hw/intc/xics_spapr.c

I've put 1-4/8 into ppc-for-2.7.  5/8, unfortunately will break
migration and 6-8/8 don't make much sense without 5/8, so I've left
them for now.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 5/8] ppc/xics: Make the ICSState a list
  2016-06-29  3:37   ` David Gibson
@ 2016-06-29  4:05     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2016-06-29  4:05 UTC (permalink / raw)
  To: David Gibson, Nikunj A Dadhania; +Cc: qemu-ppc, qemu-devel, clg

On Wed, 2016-06-29 at 13:37 +1000, David Gibson wrote:
> AFAICT xirr_owner will be lost on migration, which will break things.
> That will need to be transferred on migration, somehow.  If it can be
> recalculated from existing data in post_load() that would be ideal,
> otherwise we'll have to devise a wire encoding for it.

It should be possible to get it back from the interrupt number by
walking the list of ICS yes.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class
  2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class Nikunj A Dadhania
@ 2016-06-30  8:24   ` Cédric Le Goater
  2016-06-30  9:54     ` Nikunj A Dadhania
  0 siblings, 1 reply; 14+ messages in thread
From: Cédric Le Goater @ 2016-06-30  8:24 UTC (permalink / raw)
  To: Nikunj A Dadhania, qemu-ppc, david; +Cc: qemu-devel, benh

On 06/28/2016 09:05 PM, Nikunj A Dadhania wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> The existing implementation remains same and ics-base is introduced. The
> type name "ics" is retained, and all the related functions renamed as
> ics_simple_*
> 
> This will allow different implementations for the source controllers
> such as the MSI support of PHB3 on Power8 which uses in-memory state
> tables for example.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  hw/intc/trace-events  |  10 ++--
>  hw/intc/xics.c        | 146 +++++++++++++++++++++++++++++++-------------------
>  hw/intc/xics_kvm.c    |  10 ++--
>  hw/intc/xics_spapr.c  |  28 +++++-----
>  include/hw/ppc/xics.h |  23 +++++---
>  5 files changed, 132 insertions(+), 85 deletions(-)
> 
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index 5f0f783..e5e7ec7 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -50,12 +50,12 @@ xics_icp_accept(uint32_t old_xirr, uint32_t new_xirr) "icp_accept: XIRR %#"PRIx3
>  xics_icp_eoi(int server, uint32_t xirr, uint32_t new_xirr) "icp_eoi: server %d given XIRR %#"PRIx32" new XIRR %#"PRIx32
>  xics_icp_irq(int server, int nr, uint8_t priority) "cpu %d trying to deliver irq %#"PRIx32" priority %#x"
>  xics_icp_raise(uint32_t xirr, uint8_t pending_priority) "raising IRQ new XIRR=%#x new pending priority=%#x"
> -xics_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq %#x]"
> +xics_ics_simple_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq %#x]"
>  xics_masked_pending(void) "set_irq_msi: masked pending"
> -xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
> -xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
> -xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
> -xics_ics_eoi(int nr) "ics_eoi: irq %#x"
> +xics_ics_simple_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
> +xics_ics_simple_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
> +xics_ics_simple_reject(int nr, int srcno) "reject irq %#x [src %d]"
> +xics_ics_simple_eoi(int nr) "ics_eoi: irq %#x"
>  xics_alloc(int irq) "irq %d"
>  xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d irqs, lsi=%d, alignnum %d"
>  xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index bbdba84..39928d9 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -112,7 +112,7 @@ void xics_add_ics(XICSState *xics)
>  {
>      ICSState *ics;
>  
> -    ics = ICS(object_new(TYPE_ICS));
> +    ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE));

Should not that be ICS_BASE() ? 


>      object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
>      ics->xics = xics;
>      QLIST_INSERT_HEAD(&xics->ics, ics, list);
> @@ -223,9 +223,32 @@ static const TypeInfo xics_common_info = {
>  #define XISR(ss)   (((ss)->xirr) & XISR_MASK)
>  #define CPPR(ss)   (((ss)->xirr) >> 24)
>  
> -static void ics_reject(ICSState *ics, int nr);
> -static void ics_resend(ICSState *ics);
> -static void ics_eoi(ICSState *ics, int nr);
> +static void ics_reject(ICSState *ics, uint32_t nr)
> +{
> +    ICSStateClass *k = ICS_GET_CLASS(ics);
> +
> +    if (k->reject) {
> +        k->reject(ics, nr);
> +    }
> +}
> +
> +static void ics_resend(ICSState *ics)
> +{
> +    ICSStateClass *k = ICS_GET_CLASS(ics);
> +
> +    if (k->resend) {
> +        k->resend(ics);
> +    }
> +}
> +
> +static void ics_eoi(ICSState *ics, int nr)
> +{
> +    ICSStateClass *k = ICS_GET_CLASS(ics);
> +
> +    if (k->eoi) {
> +        k->eoi(ics, nr);
> +    }
> +}
>  
>  static void icp_check_ipi(ICPState *ss)
>  {
> @@ -428,7 +451,7 @@ static const TypeInfo icp_info = {
>  /*
>   * ICS: Source layer
>   */
> -static void resend_msi(ICSState *ics, int srcno)
> +static void ics_simple_resend_msi(ICSState *ics, int srcno)
>  {
>      ICSIRQState *irq = ics->irqs + srcno;
>  
> @@ -441,7 +464,7 @@ static void resend_msi(ICSState *ics, int srcno)
>      }
>  }
>  
> -static void resend_lsi(ICSState *ics, int srcno)
> +static void ics_simple_resend_lsi(ICSState *ics, int srcno)
>  {
>      ICSIRQState *irq = ics->irqs + srcno;
>  
> @@ -453,11 +476,11 @@ static void resend_lsi(ICSState *ics, int srcno)
>      }
>  }
>  
> -static void set_irq_msi(ICSState *ics, int srcno, int val)
> +static void ics_simple_set_irq_msi(ICSState *ics, int srcno, int val)
>  {
>      ICSIRQState *irq = ics->irqs + srcno;
>  
> -    trace_xics_set_irq_msi(srcno, srcno + ics->offset);
> +    trace_xics_ics_simple_set_irq_msi(srcno, srcno + ics->offset);
>  
>      if (val) {
>          if (irq->priority == 0xff) {
> @@ -469,31 +492,31 @@ static void set_irq_msi(ICSState *ics, int srcno, int val)
>      }
>  }
>  
> -static void set_irq_lsi(ICSState *ics, int srcno, int val)
> +static void ics_simple_set_irq_lsi(ICSState *ics, int srcno, int val)
>  {
>      ICSIRQState *irq = ics->irqs + srcno;
>  
> -    trace_xics_set_irq_lsi(srcno, srcno + ics->offset);
> +    trace_xics_ics_simple_set_irq_lsi(srcno, srcno + ics->offset);
>      if (val) {
>          irq->status |= XICS_STATUS_ASSERTED;
>      } else {
>          irq->status &= ~XICS_STATUS_ASSERTED;
>      }
> -    resend_lsi(ics, srcno);
> +    ics_simple_resend_lsi(ics, srcno);
>  }
>  
> -static void ics_set_irq(void *opaque, int srcno, int val)
> +static void ics_simple_set_irq(void *opaque, int srcno, int val)
>  {
>      ICSState *ics = (ICSState *)opaque;
>  
>      if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
> -        set_irq_lsi(ics, srcno, val);
> +        ics_simple_set_irq_lsi(ics, srcno, val);
>      } else {
> -        set_irq_msi(ics, srcno, val);
> +        ics_simple_set_irq_msi(ics, srcno, val);
>      }
>  }
>  
> -static void write_xive_msi(ICSState *ics, int srcno)
> +static void ics_simple_write_xive_msi(ICSState *ics, int srcno)
>  {
>      ICSIRQState *irq = ics->irqs + srcno;
>  
> @@ -506,68 +529,68 @@ static void write_xive_msi(ICSState *ics, int srcno)
>      icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
>  }
>  
> -static void write_xive_lsi(ICSState *ics, int srcno)
> +static void ics_simple_write_xive_lsi(ICSState *ics, int srcno)
>  {
> -    resend_lsi(ics, srcno);
> +    ics_simple_resend_lsi(ics, srcno);
>  }
>  
> -void ics_write_xive(ICSState *ics, int nr, int server,
> -                    uint8_t priority, uint8_t saved_priority)
> +void ics_simple_write_xive(ICSState *ics, int srcno, int server,
> +                           uint8_t priority, uint8_t saved_priority)
>  {
> -    int srcno = nr - ics->offset;
>      ICSIRQState *irq = ics->irqs + srcno;
>  
>      irq->server = server;
>      irq->priority = priority;
>      irq->saved_priority = saved_priority;
>  
> -    trace_xics_ics_write_xive(nr, srcno, server, priority);
> +    trace_xics_ics_simple_write_xive(ics->offset + srcno, srcno, server,
> +                                     priority);
>  
>      if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
> -        write_xive_lsi(ics, srcno);
> +        ics_simple_write_xive_lsi(ics, srcno);
>      } else {
> -        write_xive_msi(ics, srcno);
> +        ics_simple_write_xive_msi(ics, srcno);
>      }
>  }
>  
> -static void ics_reject(ICSState *ics, int nr)
> +static void ics_simple_reject(ICSState *ics, uint32_t nr)
>  {
>      ICSIRQState *irq = ics->irqs + nr - ics->offset;
>  
> -    trace_xics_ics_reject(nr, nr - ics->offset);
> +    trace_xics_ics_simple_reject(nr, nr - ics->offset);
>      irq->status |= XICS_STATUS_REJECTED; /* Irrelevant but harmless for LSI */
>      irq->status &= ~XICS_STATUS_SENT; /* Irrelevant but harmless for MSI */
>  }
>  
> -static void ics_resend(ICSState *ics)
> +static void ics_simple_resend(ICSState *ics)
>  {
>      int i;
>  
>      for (i = 0; i < ics->nr_irqs; i++) {
>          /* FIXME: filter by server#? */
>          if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
> -            resend_lsi(ics, i);
> +            ics_simple_resend_lsi(ics, i);
>          } else {
> -            resend_msi(ics, i);
> +            ics_simple_resend_msi(ics, i);
>          }
>      }
>  }
>  
> -static void ics_eoi(ICSState *ics, int nr)
> +static void ics_simple_eoi(ICSState *ics, uint32_t nr)
>  {
>      int srcno = nr - ics->offset;
>      ICSIRQState *irq = ics->irqs + srcno;
>  
> -    trace_xics_ics_eoi(nr);
> +    trace_xics_ics_simple_eoi(nr);
>  
>      if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
>          irq->status &= ~XICS_STATUS_SENT;
>      }
>  }
>  
> -static void ics_reset(DeviceState *dev)
> +static void ics_simple_reset(DeviceState *dev)
>  {
> -    ICSState *ics = ICS(dev);
> +    ICSState *ics = ICS_SIMPLE(dev);

and here ? 

>      int i;
>      uint8_t flags[ics->nr_irqs];
>  
> @@ -584,7 +607,7 @@ static void ics_reset(DeviceState *dev)
>      }
>  }
>  
> -static int ics_post_load(ICSState *ics, int version_id)
> +static int ics_simple_post_load(ICSState *ics, int version_id)
>  {
>      int i;
>  
> @@ -595,7 +618,7 @@ static int ics_post_load(ICSState *ics, int version_id)
>      return 0;
>  }
>  
> -static void ics_dispatch_pre_save(void *opaque)
> +static void ics_simple_dispatch_pre_save(void *opaque)
>  {
>      ICSState *ics = opaque;
>      ICSStateClass *info = ICS_GET_CLASS(ics);
> @@ -605,7 +628,7 @@ static void ics_dispatch_pre_save(void *opaque)
>      }
>  }
>  
> -static int ics_dispatch_post_load(void *opaque, int version_id)
> +static int ics_simple_dispatch_post_load(void *opaque, int version_id)
>  {
>      ICSState *ics = opaque;
>      ICSStateClass *info = ICS_GET_CLASS(ics);
> @@ -617,7 +640,7 @@ static int ics_dispatch_post_load(void *opaque, int version_id)
>      return 0;
>  }
>  
> -static const VMStateDescription vmstate_ics_irq = {
> +static const VMStateDescription vmstate_ics_simple_irq = {
>      .name = "ics/irq",
>      .version_id = 2,
>      .minimum_version_id = 1,
> @@ -631,59 +654,71 @@ static const VMStateDescription vmstate_ics_irq = {
>      },
>  };
>  
> -static const VMStateDescription vmstate_ics = {
> +static const VMStateDescription vmstate_ics_simple = {
>      .name = "ics",
>      .version_id = 1,
>      .minimum_version_id = 1,
> -    .pre_save = ics_dispatch_pre_save,
> -    .post_load = ics_dispatch_post_load,
> +    .pre_save = ics_simple_dispatch_pre_save,
> +    .post_load = ics_simple_dispatch_post_load,
>      .fields = (VMStateField[]) {
>          /* Sanity check */
>          VMSTATE_UINT32_EQUAL(nr_irqs, ICSState),
>  
>          VMSTATE_STRUCT_VARRAY_POINTER_UINT32(irqs, ICSState, nr_irqs,
> -                                             vmstate_ics_irq, ICSIRQState),
> +                                             vmstate_ics_simple_irq,
> +                                             ICSIRQState),
>          VMSTATE_END_OF_LIST()
>      },
>  };
>  
> -static void ics_initfn(Object *obj)
> +static void ics_simple_initfn(Object *obj)
>  {
> -    ICSState *ics = ICS(obj);
> +    ICSState *ics = ICS_SIMPLE(obj);

and here ? 

>      ics->offset = XICS_IRQ_BASE;
>  }
>  
> -static void ics_realize(DeviceState *dev, Error **errp)
> +static void ics_simple_realize(DeviceState *dev, Error **errp)
>  {
> -    ICSState *ics = ICS(dev);
> +    ICSState *ics = ICS_SIMPLE(dev);

same.

>  
>      if (!ics->nr_irqs) {
>          error_setg(errp, "Number of interrupts needs to be greater 0");
>          return;
>      }
>      ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
> -    ics->qirqs = qemu_allocate_irqs(ics_set_irq, ics, ics->nr_irqs);
> +    ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
>  }
>  
> -static void ics_class_init(ObjectClass *klass, void *data)
> +static void ics_simple_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      ICSStateClass *isc = ICS_CLASS(klass);
>  
> -    dc->realize = ics_realize;
> -    dc->vmsd = &vmstate_ics;
> -    dc->reset = ics_reset;
> -    isc->post_load = ics_post_load;
> +    dc->realize = ics_simple_realize;
> +    dc->vmsd = &vmstate_ics_simple;
> +    dc->reset = ics_simple_reset;
> +    isc->post_load = ics_simple_post_load;
> +    isc->reject = ics_simple_reject;
> +    isc->resend = ics_simple_resend;
> +    isc->eoi = ics_simple_eoi;
>  }
>  
> -static const TypeInfo ics_info = {
> -    .name = TYPE_ICS,
> +static const TypeInfo ics_simple_info = {
> +    .name = TYPE_ICS_SIMPLE,
> +    .parent = TYPE_ICS_BASE,
> +    .instance_size = sizeof(ICSState),
> +    .class_init = ics_simple_class_init,
> +    .class_size = sizeof(ICSStateClass),
> +    .instance_init = ics_simple_initfn,
> +};
> +
> +static const TypeInfo ics_base_info = {
> +    .name = TYPE_ICS_BASE,
>      .parent = TYPE_DEVICE,
> +    .abstract = true,
>      .instance_size = sizeof(ICSState),
> -    .class_init = ics_class_init,
>      .class_size = sizeof(ICSStateClass),
> -    .instance_init = ics_initfn,
>  };
>  
>  /*
> @@ -723,7 +758,8 @@ void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
>  static void xics_register_types(void)
>  {
>      type_register_static(&xics_common_info);
> -    type_register_static(&ics_info);
> +    type_register_static(&ics_simple_info);
> +    type_register_static(&ics_base_info);
>      type_register_static(&icp_info);
>  }
>  
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 04fa7cb..89862df 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -272,7 +272,7 @@ static void ics_kvm_set_irq(void *opaque, int srcno, int val)
>  
>  static void ics_kvm_reset(DeviceState *dev)
>  {
> -    ICSState *ics = ICS(dev);
> +    ICSState *ics = ICS_SIMPLE(dev);
>      int i;
>      uint8_t flags[ics->nr_irqs];
>  
> @@ -293,7 +293,7 @@ static void ics_kvm_reset(DeviceState *dev)
>  
>  static void ics_kvm_realize(DeviceState *dev, Error **errp)
>  {
> -    ICSState *ics = ICS(dev);
> +    ICSState *ics = ICS_SIMPLE(dev);
>  
>      if (!ics->nr_irqs) {
>          error_setg(errp, "Number of interrupts needs to be greater 0");
> @@ -315,8 +315,8 @@ static void ics_kvm_class_init(ObjectClass *klass, void *data)
>  }
>  
>  static const TypeInfo ics_kvm_info = {
> -    .name = TYPE_KVM_ICS,
> -    .parent = TYPE_ICS,
> +    .name = TYPE_ICS_KVM,
> +    .parent = TYPE_ICS_SIMPLE,
>      .instance_size = sizeof(ICSState),
>      .class_init = ics_kvm_class_init,
>  };
> @@ -492,7 +492,7 @@ static void xics_kvm_initfn(Object *obj)
>      XICSState *xics = XICS_COMMON(obj);
>      ICSState *ics;
>  
> -    ics = ICS(object_new(TYPE_KVM_ICS));
> +    ics = ICS_SIMPLE(object_new(TYPE_ICS_KVM));
>      object_property_add_child(obj, "ics", OBJECT(ics), NULL);
>      ics->xics = xics;
>      QLIST_INSERT_HEAD(&xics->ics, ics, list);
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 270f20e..4dd1399 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -114,7 +114,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                            uint32_t nret, target_ulong rets)
>  {
>      ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
> -    uint32_t nr, server, priority;
> +    uint32_t nr, srcno, server, priority;
>  
>      if ((nargs != 3) || (nret != 1)) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> @@ -135,7 +135,8 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>          return;
>      }
>  
> -    ics_write_xive(ics, nr, server, priority, priority);
> +    srcno = nr - ics->offset;
> +    ics_simple_write_xive(ics, srcno, server, priority, priority);
>  
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }
> @@ -146,7 +147,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                            uint32_t nret, target_ulong rets)
>  {
>      ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
> -    uint32_t nr;
> +    uint32_t nr, srcno;
>  
>      if ((nargs != 1) || (nret != 3)) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> @@ -165,8 +166,9 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>      }
>  
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> -    rtas_st(rets, 1, ics->irqs[nr - ics->offset].server);
> -    rtas_st(rets, 2, ics->irqs[nr - ics->offset].priority);
> +    srcno = nr - ics->offset;
> +    rtas_st(rets, 1, ics->irqs[srcno].server);
> +    rtas_st(rets, 2, ics->irqs[srcno].priority);
>  }
>  
>  static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> @@ -175,7 +177,7 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                           uint32_t nret, target_ulong rets)
>  {
>      ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
> -    uint32_t nr;
> +    uint32_t nr, srcno;
>  
>      if ((nargs != 1) || (nret != 1)) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> @@ -193,8 +195,9 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>          return;
>      }
>  
> -    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server, 0xff,
> -                   ics->irqs[nr - ics->offset].priority);
> +    srcno = nr - ics->offset;
> +    ics_simple_write_xive(ics, srcno, ics->irqs[srcno].server, 0xff,
> +                          ics->irqs[srcno].priority);
>  
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }
> @@ -205,7 +208,7 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                          uint32_t nret, target_ulong rets)
>  {
>      ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
> -    uint32_t nr;
> +    uint32_t nr, srcno;
>  
>      if ((nargs != 1) || (nret != 1)) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> @@ -223,9 +226,10 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>          return;
>      }
>  
> -    ics_write_xive(ics, nr, ics->irqs[nr - ics->offset].server,
> -                   ics->irqs[nr - ics->offset].saved_priority,
> -                   ics->irqs[nr - ics->offset].saved_priority);
> +    srcno = nr - ics->offset;
> +    ics_simple_write_xive(ics, srcno, ics->irqs[srcno].server,
> +                          ics->irqs[srcno].saved_priority,
> +                          ics->irqs[srcno].saved_priority);
>  
>      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 8433bf9..bc10c16 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -118,22 +118,29 @@ struct ICPState {
>      bool cap_irq_xics_enabled;
>  };
>  
> -#define TYPE_ICS "ics"
> -#define ICS(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS)
> +#define TYPE_ICS_BASE "ics-base"
> +#define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE)

should not that be TYPE_ICS_BASE ? 

> -#define TYPE_KVM_ICS "icskvm"
> -#define KVM_ICS(obj) OBJECT_CHECK(ICSState, (obj), TYPE_KVM_ICS)
> +/* Retain ics for sPAPR for migration from existing sPAPR guests */
> +#define TYPE_ICS_SIMPLE "ics"
> +#define ICS_SIMPLE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE)
> +
> +#define TYPE_ICS_KVM "icskvm"
> +#define ICS_KVM(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_KVM)
>  
>  #define ICS_CLASS(klass) \
> -     OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS)
> +     OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS_SIMPLE)

and here  ? 


>  #define ICS_GET_CLASS(obj) \
> -     OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS)
> +     OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS_SIMPLE)

and here  ? 

but I might be confusing all these ICS :)

Thanks,

C.

>  struct ICSStateClass {
>      DeviceClass parent_class;
>  
>      void (*pre_save)(ICSState *s);
>      int (*post_load)(ICSState *s, int version_id);
> +    void (*reject)(ICSState *s, uint32_t irq);
> +    void (*resend)(ICSState *s);
> +    void (*eoi)(ICSState *s, uint32_t irq);
>  };
>  
>  struct ICSState {
> @@ -190,8 +197,8 @@ uint32_t icp_accept(ICPState *ss);
>  uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr);
>  void icp_eoi(XICSState *icp, int server, uint32_t xirr);
>  
> -void ics_write_xive(ICSState *ics, int nr, int server,
> -                    uint8_t priority, uint8_t saved_priority);
> +void ics_simple_write_xive(ICSState *ics, int nr, int server,
> +                           uint8_t priority, uint8_t saved_priority);
>  
>  void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
>  
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class
  2016-06-30  8:24   ` Cédric Le Goater
@ 2016-06-30  9:54     ` Nikunj A Dadhania
  0 siblings, 0 replies; 14+ messages in thread
From: Nikunj A Dadhania @ 2016-06-30  9:54 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc, david; +Cc: qemu-devel, benh

Cédric Le Goater <clg@kaod.org> writes:

> On 06/28/2016 09:05 PM, Nikunj A Dadhania wrote:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> 
>> The existing implementation remains same and ics-base is introduced. The
>> type name "ics" is retained, and all the related functions renamed as
>> ics_simple_*
>> 
>> This will allow different implementations for the source controllers
>> such as the MSI support of PHB3 on Power8 which uses in-memory state
>> tables for example.
>> 
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index bbdba84..39928d9 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -112,7 +112,7 @@ void xics_add_ics(XICSState *xics)
>>  {
>>      ICSState *ics;
>>  
>> -    ics = ICS(object_new(TYPE_ICS));
>> +    ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE));
>
> Should not that be ICS_BASE() ? 

No, we renamed ICS => ICS_SIMPLE (retaining the typename "ics" for
migration compatibility). ICS_SIMPLE is a child of ICS_BASE

>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 8433bf9..bc10c16 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -118,22 +118,29 @@ struct ICPState {
>>      bool cap_irq_xics_enabled;
>>  };
>>  
>> -#define TYPE_ICS "ics"
>> -#define ICS(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS)
>> +#define TYPE_ICS_BASE "ics-base"
>> +#define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE)
>
> should not that be TYPE_ICS_BASE ?

Oops, you are right.

>>  #define ICS_CLASS(klass) \
>> -     OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS)
>> +     OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS_SIMPLE)
>
> and here  ? 
>
>
>>  #define ICS_GET_CLASS(obj) \
>> -     OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS)
>> +     OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS_SIMPLE)
>
> and here  ? 
>
> but I might be confusing all these ICS :)

Earlier there were two
1) TYPE_ICS
2) TYPE_ICS_KVM (parent: TYPE_ICS)

Now:
1) TYPE_ICS_BASE - new
2) TYPE_ICS_SIMPLE - was known as TYPE_ICS
3) TYPE_ICS_KVM - same as before (parent: TYPE_ICS_SIMPLE)

Regards
Nikunj

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-06-30  9:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-28 19:05 [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup Nikunj A Dadhania
2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 1/8] ppc/xics: Rename existing xics to xics_spapr Nikunj A Dadhania
2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 2/8] ppc/xics: Move SPAPR specific code to a separate file Nikunj A Dadhania
2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 3/8] ppc/xics: Implement H_IPOLL using an accessor Nikunj A Dadhania
2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 4/8] ppc/xics: Replace "icp" with "xics" in most places Nikunj A Dadhania
2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 5/8] ppc/xics: Make the ICSState a list Nikunj A Dadhania
2016-06-29  3:37   ` David Gibson
2016-06-29  4:05     ` Benjamin Herrenschmidt
2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 6/8] ppc/xics: An ICS with offset 0 is assumed to be uninitialized Nikunj A Dadhania
2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 7/8] ppc/xics: Use a helper to add a new ICS Nikunj A Dadhania
2016-06-28 19:05 ` [Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class Nikunj A Dadhania
2016-06-30  8:24   ` Cédric Le Goater
2016-06-30  9:54     ` Nikunj A Dadhania
2016-06-29  3:40 ` [Qemu-devel] [PATCH v2 0/8] sPAPR xics rework/cleanup David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.