[Qemu-devel] [PATCH v2 00/13] spapr: Add support for PHB hotplug

[Qemu-devel] [PATCH v2 00/13] spapr: Add support for PHB hotplug

[Greg Kurz]

This allows to hotplug/unplug PHBs. I could successfully test:
- hotplug/unplug with e1000 device to validate LSIs
- hotplug/unplug with virtio-net device to validate MSIs
- some simple migration scenarios

Changes in v2:
- rebased on current ppc-for-4.0
- added some preliminary cleanup
- call unrealize from realize error path
- advertise PHB hotplug in last patch
- reworked phandle related code
- sync LSIs to KVM

Please comment.

--
Greg

---

Greg Kurz (5):
      spapr: Rename xics to intc in interrupt controller agnostic code
      spapr_pci: add PHB unrealize
      spapr_irq: Expose the phandle of the interrupt controller
      spapr_irq: Allow synchronization of a single irq state to KVM
      spapr: add hotplug hooks for PHB hotplug

Michael Roth (6):
      spapr: create DR connectors for PHBs
      spapr_events: add support for phb hotplug events
      qdev: pass an Object * to qbus_set_hotplug_handler()
      spapr_pci: provide node start offset via spapr_populate_pci_dt()
      spapr_pci: add ibm, my-drc-index property for PHB hotplug
      spapr: enable PHB hotplug for default pseries machine type

Nathan Fontenot (1):
      spapr: populate PHB DRC entries for root DT node

Thomas Huth (1):
      ppc: Move spapr-related prototypes from xics.h into a seperate header file


 hw/acpi/pcihp.c               |    2 -
 hw/acpi/piix4.c               |    2 -
 hw/char/virtio-serial-bus.c   |    2 -
 hw/core/bus.c                 |   11 +--
 hw/intc/spapr_xive.c          |   34 ++++++++-
 hw/intc/xics_kvm.c            |   68 ++++++++++--------
 hw/intc/xics_spapr.c          |   29 +++++++-
 hw/pci/pcie.c                 |    2 -
 hw/pci/shpc.c                 |    2 -
 hw/ppc/spapr.c                |  156 ++++++++++++++++++++++++++++++++++++++++-
 hw/ppc/spapr_drc.c            |   18 +++++
 hw/ppc/spapr_events.c         |    5 +
 hw/ppc/spapr_irq.c            |   45 ++++++++++++
 hw/ppc/spapr_pci.c            |  122 +++++++++++++++++++++++++-------
 hw/ppc/spapr_vio.c            |    2 -
 hw/s390x/css-bridge.c         |    2 -
 hw/s390x/s390-pci-bus.c       |    6 +-
 hw/scsi/virtio-scsi.c         |    2 -
 hw/scsi/vmw_pvscsi.c          |    2 -
 hw/usb/dev-smartcard-reader.c |    2 -
 include/hw/pci-host/spapr.h   |    8 ++
 include/hw/ppc/spapr.h        |    4 +
 include/hw/ppc/spapr_drc.h    |    8 ++
 include/hw/ppc/spapr_irq.h    |    3 +
 include/hw/ppc/spapr_xive.h   |    2 +
 include/hw/ppc/xics.h         |    9 +-
 include/hw/ppc/xics_spapr.h   |   39 ++++++++++
 include/hw/qdev-core.h        |    3 -
 28 files changed, 492 insertions(+), 98 deletions(-)
 create mode 100644 include/hw/ppc/xics_spapr.h

[Qemu-devel] [PATCH v2 01/13] ppc: Move spapr-related prototypes from xics.h into a seperate header file

[Greg Kurz]

From: Thomas Huth <thuth@redhat.com>

When compiling with Clang in -std=gnu99 mode, there is a warning/error:

  CC      ppc64-softmmu/hw/intc/xics_spapr.o
In file included from /home/thuth/devel/qemu/hw/intc/xics_spapr.c:34:
/home/thuth/devel/qemu/include/hw/ppc/xics.h:203:34: error: redefinition of typedef 'sPAPRMachineState' is a C11 feature
      [-Werror,-Wtypedef-redefinition]
typedef struct sPAPRMachineState sPAPRMachineState;
                                 ^
/home/thuth/devel/qemu/include/hw/ppc/spapr_irq.h:25:34: note: previous definition is here
typedef struct sPAPRMachineState sPAPRMachineState;
                                 ^

We have to remove the duplicated typedef here and include "spapr.h" instead.
But "spapr.h" should not be included for the pnv machine files. So move
the spapr-related prototypes into a new file called "xics_spapr.h" instead.

Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/xics_kvm.c          |    1 +
 hw/intc/xics_spapr.c        |    1 +
 hw/ppc/spapr_irq.c          |    1 +
 include/hw/ppc/xics.h       |    7 -------
 include/hw/ppc/xics_spapr.h |   37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 40 insertions(+), 7 deletions(-)
 create mode 100644 include/hw/ppc/xics_spapr.h

diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index ac94594b1919..dff13300504c 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -34,6 +34,7 @@
 #include "sysemu/kvm.h"
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/xics.h"
+#include "hw/ppc/xics_spapr.h"
 #include "kvm_ppc.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 9c1a90d7094b..de6cc15b6474 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -32,6 +32,7 @@
 #include "qemu/timer.h"
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/xics.h"
+#include "hw/ppc/xics_spapr.h"
 #include "hw/ppc/fdt.h"
 #include "qapi/visitor.h"
 
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 5fce72fe0f6c..1da7a32348fc 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -14,6 +14,7 @@
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_xive.h"
 #include "hw/ppc/xics.h"
+#include "hw/ppc/xics_spapr.h"
 #include "sysemu/kvm.h"
 
 #include "trace.h"
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 07508cbd217e..fad786e8b22d 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -200,13 +200,6 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon);
 void ics_resend(ICSState *ics);
 void icp_resend(ICPState *ss);
 
-typedef struct sPAPRMachineState sPAPRMachineState;
-
-void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
-                   uint32_t phandle);
-int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
-void xics_spapr_init(sPAPRMachineState *spapr);
-
 Object *icp_create(Object *cpu, const char *type, XICSFabric *xi,
                    Error **errp);
 
diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
new file mode 100644
index 000000000000..b1ab27d022cf
--- /dev/null
+++ b/include/hw/ppc/xics_spapr.h
@@ -0,0 +1,37 @@
+/*
+ * 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.
+ */
+
+#ifndef XICS_SPAPR_H
+#define XICS_SPAPR_H
+
+#include "hw/ppc/spapr.h"
+
+void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
+                   uint32_t phandle);
+int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
+void xics_spapr_init(sPAPRMachineState *spapr);
+
+#endif /* XICS_SPAPR_H */

[Qemu-devel] [PATCH v2 02/13] spapr: Rename xics to intc in interrupt controller agnostic code

[Greg Kurz]

All this code is used with both the XICS and XIVE interrupt controllers.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c              |    6 +++---
 hw/ppc/spapr_events.c       |    2 +-
 hw/ppc/spapr_pci.c          |    6 +++---
 hw/ppc/spapr_vio.c          |    2 +-
 include/hw/pci-host/spapr.h |    2 +-
 include/hw/ppc/spapr.h      |    2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 83081defde4e..26f8e55cc25e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -96,7 +96,7 @@
 
 #define MIN_RMA_SLOF            128UL
 
-#define PHANDLE_XICP            0x00001111
+#define PHANDLE_INTC            0x00001111
 
 /* These two functions implement the VCPU id numbering: one to compute them
  * all and one to identify thread 0 of a VCORE. Any change to the first one
@@ -1276,7 +1276,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
 
     /* /interrupt controller */
     spapr->irq->dt_populate(spapr, spapr_max_server_number(spapr), fdt,
-                          PHANDLE_XICP);
+                          PHANDLE_INTC);
 
     ret = spapr_populate_memory(spapr, fdt);
     if (ret < 0) {
@@ -1296,7 +1296,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
     }
 
     QLIST_FOREACH(phb, &spapr->phbs, list) {
-        ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt,
+        ret = spapr_populate_pci_dt(phb, PHANDLE_INTC, fdt,
                                     spapr->irq->nr_msis);
         if (ret < 0) {
             error_report("couldn't setup PCI devices in fdt");
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 32719a1b72d0..d4e75211954d 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -282,7 +282,7 @@ void spapr_dt_events(sPAPRMachineState *spapr, void *fdt)
             continue;
         }
 
-        spapr_dt_xics_irq(interrupts, source->irq, false);
+        spapr_dt_intc_irq(interrupts, source->irq, false);
 
         _FDT(node_offset = fdt_add_subnode(fdt, event_sources, source_name));
         _FDT(fdt_setprop(fdt, node_offset, "interrupts", interrupts,
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index b74f2632ecc6..24f95a400623 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2066,7 +2066,7 @@ static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
 
 }
 
-int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
+int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
                           uint32_t nr_msis)
 {
     int bus_off, i, j, ret;
@@ -2164,8 +2164,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
             irqmap[1] = 0;
             irqmap[2] = 0;
             irqmap[3] = cpu_to_be32(j+1);
-            irqmap[4] = cpu_to_be32(xics_phandle);
-            spapr_dt_xics_irq(&irqmap[5], phb->lsi_table[lsi_num].irq, true);
+            irqmap[4] = cpu_to_be32(intc_phandle);
+            spapr_dt_intc_irq(&irqmap[5], phb->lsi_table[lsi_num].irq, true);
         }
     }
     /* Write interrupt map */
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 7e8a9ad09337..cfd9849d8dfb 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -158,7 +158,7 @@ static int vio_make_devnode(VIOsPAPRDevice *dev,
     if (dev->irq) {
         uint32_t ints_prop[2];
 
-        spapr_dt_xics_irq(ints_prop, dev->irq, false);
+        spapr_dt_intc_irq(ints_prop, dev->irq, false);
         ret = fdt_setprop(fdt, node_off, "interrupts", ints_prop,
                           sizeof(ints_prop));
         if (ret < 0) {
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 4eb3a2ce3eb8..e0e683c32469 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -113,7 +113,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
     return spapr_qirq(spapr, phb->lsi_table[pin].irq);
 }
 
-int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
+int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
                           uint32_t nr_msis);
 
 void spapr_pci_rtas_init(void);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 9e01a5a12e4a..7193ce094689 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -682,7 +682,7 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr);
  * "interrupt-controller" node has its "#interrupt-cells" property set to 2 (ie,
  * VIO devices, RTAS event sources and PHBs).
  */
-static inline void spapr_dt_xics_irq(uint32_t *intspec, int irq, bool is_lsi)
+static inline void spapr_dt_intc_irq(uint32_t *intspec, int irq, bool is_lsi)
 {
     intspec[0] = cpu_to_be32(irq);
     intspec[1] = is_lsi ? cpu_to_be32(1) : 0;

[Qemu-devel] [PATCH v2 03/13] spapr_pci: add PHB unrealize

[Greg Kurz]

To support PHB hotplug we need to clean up lingering references,
memory, child properties, etc. prior to the PHB object being
finalized. Generally this will be called as a result of calling
object_unparent() on the PHB object, which in turn would normally
be called as the result of an unplug() operation.

When the PHB is finalized, child objects will be unparented in
turn, and finalized if the PHB was the only reference holder. so
we don't bother to explicitly unparent child objects of the PHB
(spapr_iommu, spapr_drc, etc).

The formula that gives the number of DMA windows is moved to an
inline function in the hw/pci-host/spapr.h header because it
will have other users.

The unrealize function is able to cope with partially realized PHBs.
It is hence used to implement proper rollback on the realize error
path.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
v2: - implement rollback with unrealize function
---
 hw/ppc/spapr_pci.c          |   73 ++++++++++++++++++++++++++++++++++++++++---
 include/hw/pci-host/spapr.h |    4 ++
 2 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 24f95a400623..0d680b71b5d6 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1559,6 +1559,64 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
     }
 }
 
+static void spapr_phb_finalizefn(Object *obj)
+{
+    sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(obj);
+
+    g_free(sphb->dtbusname);
+    sphb->dtbusname = NULL;
+}
+
+static void spapr_phb_unrealize(DeviceState *dev, Error **errp)
+{
+    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+    SysBusDevice *s = SYS_BUS_DEVICE(dev);
+    PCIHostState *phb = PCI_HOST_BRIDGE(s);
+    sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(phb);
+    sPAPRTCETable *tcet;
+    int i;
+    const unsigned windows_supported = spapr_phb_windows_supported(sphb);
+
+    if (sphb->msi) {
+        g_hash_table_unref(sphb->msi);
+        sphb->msi = NULL;
+    }
+
+    /*
+     * Remove IO/MMIO subregions and aliases, rest should get cleaned
+     * via PHB's unrealize->object_finalize
+     */
+    for (i = windows_supported - 1; i >= 0; i--) {
+        tcet = spapr_tce_find_by_liobn(sphb->dma_liobn[i]);
+        if (tcet) {
+            memory_region_del_subregion(&sphb->iommu_root,
+                                        spapr_tce_get_iommu(tcet));
+        }
+    }
+
+    for (i = PCI_NUM_PINS - 1; i >= 0; i--) {
+        if (sphb->lsi_table[i].irq) {
+            spapr_irq_free(spapr, sphb->lsi_table[i].irq, 1);
+            sphb->lsi_table[i].irq = 0;
+        }
+    }
+
+    QLIST_REMOVE(sphb, list);
+
+    memory_region_del_subregion(&sphb->iommu_root, &sphb->msiwindow);
+
+    address_space_destroy(&sphb->iommu_as);
+
+    qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
+    pci_unregister_root_bus(phb->bus);
+
+    memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
+    if (sphb->mem64_win_pciaddr != (hwaddr)-1) {
+        memory_region_del_subregion(get_system_memory(), &sphb->mem64window);
+    }
+    memory_region_del_subregion(get_system_memory(), &sphb->mem32window);
+}
+
 static void spapr_phb_realize(DeviceState *dev, Error **errp)
 {
     /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user
@@ -1576,8 +1634,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     PCIBus *bus;
     uint64_t msi_window_size = 4096;
     sPAPRTCETable *tcet;
-    const unsigned windows_supported =
-        sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
+    const unsigned windows_supported = spapr_phb_windows_supported(sphb);
 
     if (!spapr) {
         error_setg(errp, TYPE_SPAPR_PCI_HOST_BRIDGE " needs a pseries machine");
@@ -1734,14 +1791,14 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
             if (local_err) {
                 error_propagate_prepend(errp, local_err,
                                         "can't allocate LSIs: ");
-                return;
+                goto unrealize;
             }
         }
 
         spapr_irq_claim(spapr, irq, true, &local_err);
         if (local_err) {
             error_propagate_prepend(errp, local_err, "can't allocate LSIs: ");
-            return;
+            goto unrealize;
         }
 
         sphb->lsi_table[i].irq = irq;
@@ -1761,13 +1818,17 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         if (!tcet) {
             error_setg(errp, "Creating window#%d failed for %s",
                        i, sphb->dtbusname);
-            return;
+            goto unrealize;
         }
         memory_region_add_subregion(&sphb->iommu_root, 0,
                                     spapr_tce_get_iommu(tcet));
     }
 
     sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
+    return;
+
+unrealize:
+    spapr_phb_unrealize(dev, NULL);
 }
 
 static int spapr_phb_children_reset(Object *child, void *opaque)
@@ -1966,6 +2027,7 @@ static void spapr_phb_class_init(ObjectClass *klass, void *data)
 
     hc->root_bus_path = spapr_phb_root_bus_path;
     dc->realize = spapr_phb_realize;
+    dc->unrealize = spapr_phb_unrealize;
     dc->props = spapr_phb_properties;
     dc->reset = spapr_phb_reset;
     dc->vmsd = &vmstate_spapr_pci;
@@ -1981,6 +2043,7 @@ static const TypeInfo spapr_phb_info = {
     .name          = TYPE_SPAPR_PCI_HOST_BRIDGE,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(sPAPRPHBState),
+    .instance_finalize = spapr_phb_finalizefn,
     .class_init    = spapr_phb_class_init,
     .interfaces    = (InterfaceInfo[]) {
         { TYPE_HOTPLUG_HANDLER },
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index e0e683c32469..16f42d0c0254 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -164,4 +164,8 @@ static inline void spapr_phb_vfio_reset(DeviceState *qdev)
 
 void spapr_phb_dma_reset(sPAPRPHBState *sphb);
 
+static inline unsigned spapr_phb_windows_supported(sPAPRPHBState *sphb)
+{
+    return sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
+}
 #endif /* PCI_HOST_SPAPR_H */

[Qemu-devel] [PATCH v2 04/13] spapr: create DR connectors for PHBs

[Greg Kurz]

From: Michael Roth <mdroth@linux.vnet.ibm.com>

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c             |   13 +++++++++++++
 hw/ppc/spapr_drc.c         |   17 +++++++++++++++++
 include/hw/ppc/spapr.h     |    1 +
 include/hw/ppc/spapr_drc.h |    8 ++++++++
 4 files changed, 39 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 26f8e55cc25e..9eeb6792e261 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2797,6 +2797,19 @@ static void spapr_machine_init(MachineState *machine)
     /* We always have at least the nvram device on VIO */
     spapr_create_nvram(spapr);
 
+    /*
+     * Setup hotplug / dynamic-reconfiguration connectors. top-level
+     * connectors (described in root DT node's "ibm,drc-types" property)
+     * are pre-initialized here. additional child connectors (such as
+     * connectors for a PHBs PCI slots) are added as needed during their
+     * parent's realization.
+     */
+    if (smc->dr_phb_enabled) {
+        for (i = 0; i < SPAPR_MAX_PHBS; i++) {
+            spapr_dr_connector_new(OBJECT(machine), TYPE_SPAPR_DRC_PHB, i);
+        }
+    }
+
     /* Set up PCI */
     spapr_pci_rtas_init();
 
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 2edb7d1e9c8c..189ee681062a 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -696,6 +696,15 @@ static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
     drck->release = spapr_lmb_release;
 }
 
+static void spapr_drc_phb_class_init(ObjectClass *k, void *data)
+{
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
+
+    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB;
+    drck->typename = "PHB";
+    drck->drc_name_prefix = "PHB ";
+}
+
 static const TypeInfo spapr_dr_connector_info = {
     .name          = TYPE_SPAPR_DR_CONNECTOR,
     .parent        = TYPE_DEVICE,
@@ -739,6 +748,13 @@ static const TypeInfo spapr_drc_lmb_info = {
     .class_init    = spapr_drc_lmb_class_init,
 };
 
+static const TypeInfo spapr_drc_phb_info = {
+    .name          = TYPE_SPAPR_DRC_PHB,
+    .parent        = TYPE_SPAPR_DRC_LOGICAL,
+    .instance_size = sizeof(sPAPRDRConnector),
+    .class_init    = spapr_drc_phb_class_init,
+};
+
 /* helper functions for external users */
 
 sPAPRDRConnector *spapr_drc_by_index(uint32_t index)
@@ -1189,6 +1205,7 @@ static void spapr_drc_register_types(void)
     type_register_static(&spapr_drc_cpu_info);
     type_register_static(&spapr_drc_pci_info);
     type_register_static(&spapr_drc_lmb_info);
+    type_register_static(&spapr_drc_phb_info);
 
     spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
                         rtas_set_indicator);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 7193ce094689..4eb80c3d888c 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -103,6 +103,7 @@ struct sPAPRMachineClass {
 
     /*< public >*/
     bool dr_lmb_enabled;       /* enable dynamic-reconfig/hotplug of LMBs */
+    bool dr_phb_enabled;       /* enable dynamic-reconfig/hotplug of PHBs */
     bool update_dt_enabled;    /* enable KVMPPC_H_UPDATE_DT */
     bool use_ohci_by_default;  /* use USB-OHCI instead of XHCI */
     bool pre_2_10_has_unused_icps;
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index f6ff32e7e2f2..56bba36ad4da 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -70,6 +70,14 @@
 #define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
                                         TYPE_SPAPR_DRC_LMB)
 
+#define TYPE_SPAPR_DRC_PHB "spapr-drc-phb"
+#define SPAPR_DRC_PHB_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHB)
+#define SPAPR_DRC_PHB_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_PHB)
+#define SPAPR_DRC_PHB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                        TYPE_SPAPR_DRC_PHB)
+
 /*
  * Various hotplug types managed by sPAPRDRConnector
  *

[Qemu-devel] [PATCH v2 05/13] spapr: populate PHB DRC entries for root DT node

[Greg Kurz]

From: Nathan Fontenot <nfont@linux.vnet.ibm.com>

This add entries to the root OF node to advertise our PHBs as being
DR-capable in accordance with PAPR specification.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9eeb6792e261..ef8984286587 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1350,6 +1350,14 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
         exit(1);
     }
 
+    if (smc->dr_phb_enabled) {
+        ret = spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_PHB);
+        if (ret < 0) {
+            error_report("Couldn't set up PHB DR device tree properties");
+            exit(1);
+        }
+    }
+
     return fdt;
 }
 

[Qemu-devel] [PATCH v2 06/13] spapr_events: add support for phb hotplug events

[Greg Kurz]

From: Michael Roth <mdroth@linux.vnet.ibm.com>

Extend the existing EPOW event format we use for PCI
devices to emit PHB plug/unplug events.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_events.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index d4e75211954d..6cd640b700dd 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -526,6 +526,9 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
     case SPAPR_DR_CONNECTOR_TYPE_CPU:
         hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_CPU;
         break;
+    case SPAPR_DR_CONNECTOR_TYPE_PHB:
+        hp->hotplug_type = RTAS_LOG_V6_HP_TYPE_PHB;
+        break;
     default:
         /* we shouldn't be signaling hotplug events for resources
          * that don't support them

[Qemu-devel] [PATCH v2 07/13] qdev: pass an Object * to qbus_set_hotplug_handler()

[Greg Kurz]

From: Michael Roth <mdroth@linux.vnet.ibm.com>

Certain devices types, like memory/CPU, are now being handled using a
hotplug interface provided by a top-level MachineClass. Hotpluggable
host bridges are another such device where it makes sense to use a
machine-level hotplug handler. However, unlike those devices,
host-bridges have a parent bus (the main system bus), and devices with
a parent bus use a different mechanism for registering their hotplug
handlers: qbus_set_hotplug_handler(). This interface currently expects
a handler to be a subclass of DeviceClass, but this is not the case
for MachineClass, which derives directly from ObjectClass.

Internally, the interface only requires an ObjectClass, so expose that
in qbus_set_hotplug_handler().

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/acpi/pcihp.c               |    2 +-
 hw/acpi/piix4.c               |    2 +-
 hw/char/virtio-serial-bus.c   |    2 +-
 hw/core/bus.c                 |   11 ++---------
 hw/pci/pcie.c                 |    2 +-
 hw/pci/shpc.c                 |    2 +-
 hw/ppc/spapr_pci.c            |    2 +-
 hw/s390x/css-bridge.c         |    2 +-
 hw/s390x/s390-pci-bus.c       |    6 +++---
 hw/scsi/virtio-scsi.c         |    2 +-
 hw/scsi/vmw_pvscsi.c          |    2 +-
 hw/usb/dev-smartcard-reader.c |    2 +-
 include/hw/qdev-core.h        |    3 +--
 13 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 7bc7a723407b..942918132376 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -251,7 +251,7 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
             object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
             PCIBus *sec = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
 
-            qbus_set_hotplug_handler(BUS(sec), DEVICE(hotplug_dev),
+            qbus_set_hotplug_handler(BUS(sec), OBJECT(hotplug_dev),
                                      &error_abort);
             /* We don't have to overwrite any other hotplug handler yet */
             assert(QLIST_EMPTY(&sec->child));
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 88f9a9ec0912..df8c0db909ce 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -536,7 +536,7 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
 
     piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
                                    pci_get_bus(dev), s);
-    qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), DEVICE(s), &error_abort);
+    qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), OBJECT(s), &error_abort);
 
     piix4_pm_add_propeties(s);
 }
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 04e3ebe3526a..e4310c78f2dc 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1052,7 +1052,7 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
     /* Spawn a new virtio-serial bus on which the ports will ride as devices */
     qbus_create_inplace(&vser->bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS,
                         dev, vdev->bus_name);
-    qbus_set_hotplug_handler(BUS(&vser->bus), DEVICE(vser), errp);
+    qbus_set_hotplug_handler(BUS(&vser->bus), OBJECT(vser), errp);
     vser->bus.vser = vser;
     QTAILQ_INIT(&vser->ports);
 
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 4651f244864c..e09843f6abea 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -22,22 +22,15 @@
 #include "hw/qdev.h"
 #include "qapi/error.h"
 
-static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler,
-                                              Error **errp)
+void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
 {
-
     object_property_set_link(OBJECT(bus), OBJECT(handler),
                              QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
 }
 
-void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error **errp)
-{
-    qbus_set_hotplug_handler_internal(bus, OBJECT(handler), errp);
-}
-
 void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
 {
-    qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp);
+    qbus_set_hotplug_handler(bus, OBJECT(bus), errp);
 }
 
 int qbus_walk_children(BusState *bus,
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 2d3d8a047b83..a2287c3eb997 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -534,7 +534,7 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
     dev->exp.hpev_notified = false;
 
     qbus_set_hotplug_handler(BUS(pci_bridge_get_sec_bus(PCI_BRIDGE(dev))),
-                             DEVICE(dev), NULL);
+                             OBJECT(dev), NULL);
 }
 
 void pcie_cap_slot_reset(PCIDevice *dev)
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 45053b39b92c..52ccdc5ae3b9 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -648,7 +648,7 @@ int shpc_init(PCIDevice *d, PCIBus *sec_bus, MemoryRegion *bar,
     shpc_cap_update_dword(d);
     memory_region_add_subregion(bar, offset, &shpc->mmio);
 
-    qbus_set_hotplug_handler(BUS(sec_bus), DEVICE(d), NULL);
+    qbus_set_hotplug_handler(BUS(sec_bus), OBJECT(d), NULL);
 
     d->cap_present |= QEMU_PCI_CAP_SHPC;
     return 0;
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 0d680b71b5d6..be9e01d84eaf 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1737,7 +1737,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
                                 &sphb->memspace, &sphb->iospace,
                                 PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
     phb->bus = bus;
-    qbus_set_hotplug_handler(BUS(phb->bus), DEVICE(sphb), NULL);
+    qbus_set_hotplug_handler(BUS(phb->bus), OBJECT(sphb), NULL);
 
     /*
      * Initialize PHB address space.
diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
index 1bd6c8b45860..7573c40badbd 100644
--- a/hw/s390x/css-bridge.c
+++ b/hw/s390x/css-bridge.c
@@ -108,7 +108,7 @@ VirtualCssBus *virtual_css_bus_init(void)
     cbus = VIRTUAL_CSS_BUS(bus);
 
     /* Enable hotplugging */
-    qbus_set_hotplug_handler(bus, dev, &error_abort);
+    qbus_set_hotplug_handler(bus, OBJECT(dev), &error_abort);
 
     css_register_io_adapters(CSS_IO_ADAPTER_VIRTIO, true, false,
                              0, &error_abort);
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 15759b6514a5..c01eed2bd35b 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -708,7 +708,7 @@ static void s390_pcihost_realize(DeviceState *dev, Error **errp)
     pci_setup_iommu(b, s390_pci_dma_iommu, s);
 
     bus = BUS(b);
-    qbus_set_hotplug_handler(bus, dev, &local_err);
+    qbus_set_hotplug_handler(bus, OBJECT(dev), &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -716,7 +716,7 @@ static void s390_pcihost_realize(DeviceState *dev, Error **errp)
     phb->bus = b;
 
     s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, dev, NULL));
-    qbus_set_hotplug_handler(BUS(s->bus), dev, &local_err);
+    qbus_set_hotplug_handler(BUS(s->bus), OBJECT(dev), &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -844,7 +844,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
 
         bus = BUS(&pb->sec_bus);
-        qbus_set_hotplug_handler(bus, DEVICE(s), errp);
+        qbus_set_hotplug_handler(bus, OBJECT(s), errp);
 
         if (dev->hotplugged) {
             pci_default_write_config(pdev, PCI_PRIMARY_BUS, s->bus_no, 1);
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3aa99717e235..9cf6290c2986 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -893,7 +893,7 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
     scsi_bus_new(&s->bus, sizeof(s->bus), dev,
                  &virtio_scsi_scsi_info, vdev->bus_name);
     /* override default SCSI bus hotplug-handler, with virtio-scsi's one */
-    qbus_set_hotplug_handler(BUS(&s->bus), dev, &error_abort);
+    qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev), &error_abort);
 
     virtio_scsi_dataplane_setup(s, errp);
 }
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index a3a019e30a74..584b4be07e79 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1142,7 +1142,7 @@ pvscsi_realizefn(PCIDevice *pci_dev, Error **errp)
     scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(pci_dev),
                  &pvscsi_scsi_info, NULL);
     /* override default SCSI bus hotplug-handler, with pvscsi's one */
-    qbus_set_hotplug_handler(BUS(&s->bus), DEVICE(s), &error_abort);
+    qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(s), &error_abort);
     pvscsi_reset_state(s);
 }
 
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 8f716fc165a3..6b0137bb7699 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1322,7 +1322,7 @@ static void ccid_realize(USBDevice *dev, Error **errp)
     usb_desc_init(dev);
     qbus_create_inplace(&s->bus, sizeof(s->bus), TYPE_CCID_BUS, DEVICE(dev),
                         NULL);
-    qbus_set_hotplug_handler(BUS(&s->bus), DEVICE(dev), &error_abort);
+    qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev), &error_abort);
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP);
     s->bulk = usb_ep_get(dev, USB_TOKEN_IN, CCID_BULK_IN_EP);
     s->card = NULL;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index bc014c1c9fca..e11f6badcc2e 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -427,8 +427,7 @@ char *qdev_get_dev_path(DeviceState *dev);
 
 GSList *qdev_build_hotpluggable_device_list(Object *peripheral);
 
-void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
-                              Error **errp);
+void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp);
 
 void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp);
 

[Qemu-devel] [PATCH v2 08/13] spapr_pci: provide node start offset via spapr_populate_pci_dt()

[Greg Kurz]

From: Michael Roth <mdroth@linux.vnet.ibm.com>

PHB hotplug re-uses PHB device tree generation code and passes
it to a guest via RTAS. Doing this requires knowledge of where
exactly in the device tree the node describing the PHB begins.

Provide this via a new optional pointer that can be used to
store the PHB node's start offset.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c              |    2 +-
 hw/ppc/spapr_pci.c          |    5 ++++-
 include/hw/pci-host/spapr.h |    2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ef8984286587..ebb6fbbb2b69 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1297,7 +1297,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
 
     QLIST_FOREACH(phb, &spapr->phbs, list) {
         ret = spapr_populate_pci_dt(phb, PHANDLE_INTC, fdt,
-                                    spapr->irq->nr_msis);
+                                    spapr->irq->nr_msis, NULL);
         if (ret < 0) {
             error_report("couldn't setup PCI devices in fdt");
             exit(1);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index be9e01d84eaf..d8a285cc291d 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2130,7 +2130,7 @@ static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
 }
 
 int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
-                          uint32_t nr_msis)
+                          uint32_t nr_msis, int *node_offset)
 {
     int bus_off, i, j, ret;
     gchar *nodename;
@@ -2185,6 +2185,9 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
     nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
     _FDT(bus_off = fdt_add_subnode(fdt, 0, nodename));
     g_free(nodename);
+    if (node_offset) {
+        *node_offset = bus_off;
+    }
 
     /* Write PHB properties */
     _FDT(fdt_setprop_string(fdt, bus_off, "device_type", "pci"));
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 16f42d0c0254..caee5df4d52f 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -114,7 +114,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
 }
 
 int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
-                          uint32_t nr_msis);
+                          uint32_t nr_msis, int *node_offset);
 
 void spapr_pci_rtas_init(void);
 

[Qemu-devel] [PATCH v2 09/13] spapr_pci: add ibm, my-drc-index property for PHB hotplug

[Greg Kurz]

From: Michael Roth <mdroth@linux.vnet.ibm.com>

This is needed to denote a boot-time PHB as being hot-pluggable.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_pci.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index d8a285cc291d..f2b9066ecabe 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2180,6 +2180,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
     sPAPRTCETable *tcet;
     PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
     sPAPRFDT s_fdt;
+    sPAPRDRConnector *drc;
 
     /* Start populating the FDT */
     nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
@@ -2246,6 +2247,14 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
                  tcet->liobn, tcet->bus_offset,
                  tcet->nb_table << tcet->page_shift);
 
+    drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, phb->index);
+    if (drc) {
+        uint32_t drc_index = cpu_to_be32(spapr_drc_index(drc));
+
+        _FDT(fdt_setprop(fdt, bus_off, "ibm,my-drc-index", &drc_index,
+                         sizeof(drc_index)));
+    }
+
     /* Walk the bridges and program the bus numbers*/
     spapr_phb_pci_enumerate(phb);
     _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));

[Qemu-devel] [PATCH v2 10/13] spapr_irq: Expose the phandle of the interrupt controller

[Greg Kurz]

This will be used by PHB hotplug in order to create the "interrupt-map"
property of the PHB node.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/spapr_xive.c        |   34 ++++++++++++++++++++++++++++++++--
 hw/intc/xics_spapr.c        |   28 +++++++++++++++++++++++++++-
 hw/ppc/spapr_irq.c          |   13 ++++++++++++-
 include/hw/ppc/spapr_irq.h  |    1 +
 include/hw/ppc/spapr_xive.h |    2 ++
 include/hw/ppc/xics_spapr.h |    2 ++
 6 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index d391177ab81f..ffae680024d7 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -1414,6 +1414,12 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
     spapr_register_hypercall(H_INT_RESET, h_int_reset);
 }
 
+static gchar *xive_nodename(sPAPRXive *xive)
+{
+    return g_strdup_printf("interrupt-controller@%" PRIx64,
+                           xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
+}
+
 void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
                    uint32_t phandle)
 {
@@ -1450,8 +1456,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
                            XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
     timas[3] = cpu_to_be64(1ull << TM_SHIFT);
 
-    nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
-                           xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
+    nodename = xive_nodename(xive);
     _FDT(node = fdt_add_subnode(fdt, 0, nodename));
     g_free(nodename);
 
@@ -1479,3 +1484,28 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
     _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities",
                      plat_res_int_priorities, sizeof(plat_res_int_priorities)));
 }
+
+uint32_t spapr_get_phandle_xive(sPAPRMachineState *spapr, void *fdt,
+                                Error **errp)
+{
+    gchar *nodename = xive_nodename(spapr->xive);
+    int phandle = -1, offset;
+
+    offset = fdt_subnode_offset(fdt, 0, nodename);
+    if (offset < 0) {
+        error_setg(errp, "Can't find node \"%s\": %s", nodename,
+                   fdt_strerror(offset));
+        goto out;
+    }
+
+    phandle = fdt_get_phandle(spapr->fdt_blob, offset);
+    if (phandle < 0) {
+        error_setg(errp, "Can't get phandle of node \"%s\": %s",
+                   nodename, fdt_strerror(phandle));
+        goto out;
+    }
+
+out:
+    g_free(nodename);
+    return phandle;
+}
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index de6cc15b6474..7763b81d90dd 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -35,6 +35,7 @@
 #include "hw/ppc/xics_spapr.h"
 #include "hw/ppc/fdt.h"
 #include "qapi/visitor.h"
+#include "qapi/error.h"
 
 /*
  * Guest interfaces
@@ -245,6 +246,8 @@ void xics_spapr_init(sPAPRMachineState *spapr)
     spapr_register_hypercall(H_IPOLL, h_ipoll);
 }
 
+#define NODENAME "interrupt-controller"
+
 void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
                    uint32_t phandle)
 {
@@ -253,7 +256,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
     };
     int node;
 
-    _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
+    _FDT(node = fdt_add_subnode(fdt, 0, NODENAME));
 
     _FDT(fdt_setprop_string(fdt, node, "device_type",
                             "PowerPC-External-Interrupt-Presentation"));
@@ -266,3 +269,26 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
     _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle));
     _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle));
 }
+
+uint32_t spapr_get_phandle_xics(sPAPRMachineState *spapr, void *fdt,
+                                Error **errp)
+{
+    int phandle = -1, offset;
+
+    offset = fdt_subnode_offset(fdt, 0, NODENAME);
+    if (offset < 0) {
+        error_setg(errp, "Can't find node \"%s\": %s", NODENAME,
+                   fdt_strerror(offset));
+        goto out;
+    }
+
+    phandle = fdt_get_phandle(spapr->fdt_blob, offset);
+    if (phandle < 0) {
+        error_setg(errp, "Can't get phandle of node \"%s\": %s",
+                   NODENAME, fdt_strerror(phandle));
+        goto out;
+    }
+
+out:
+    return phandle;
+}
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 1da7a32348fc..ba0df9ae2e1b 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -255,6 +255,7 @@ sPAPRIrq spapr_irq_xics = {
     .post_load   = spapr_irq_post_load_xics,
     .reset       = spapr_irq_reset_xics,
     .set_irq     = spapr_irq_set_irq_xics,
+    .get_phandle = spapr_get_phandle_xics,
 };
 
 /*
@@ -411,6 +412,7 @@ sPAPRIrq spapr_irq_xive = {
     .post_load   = spapr_irq_post_load_xive,
     .reset       = spapr_irq_reset_xive,
     .set_irq     = spapr_irq_set_irq_xive,
+    .get_phandle = spapr_get_phandle_xive,
 };
 
 /*
@@ -569,6 +571,13 @@ static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val)
     spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
 }
 
+static uint32_t spapr_irq_get_phandle_dual(sPAPRMachineState *spapr, void *fdt,
+                                           Error **errp)
+{
+    return spapr_irq_current(spapr)->get_phandle(spapr, fdt, errp);
+}
+
+
 /*
  * Define values in sync with the XIVE and XICS backend
  */
@@ -589,7 +598,8 @@ sPAPRIrq spapr_irq_dual = {
     .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
     .post_load   = spapr_irq_post_load_dual,
     .reset       = spapr_irq_reset_dual,
-    .set_irq     = spapr_irq_set_irq_dual
+    .set_irq     = spapr_irq_set_irq_dual,
+    .get_phandle = spapr_irq_get_phandle_dual,
 };
 
 /*
@@ -706,4 +716,5 @@ sPAPRIrq spapr_irq_xics_legacy = {
     .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
     .post_load   = spapr_irq_post_load_xics,
     .set_irq     = spapr_irq_set_irq_xics,
+    .get_phandle = spapr_get_phandle_xics,
 };
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index 14b02c3aca33..990743a23582 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -47,6 +47,7 @@ typedef struct sPAPRIrq {
     int (*post_load)(sPAPRMachineState *spapr, int version_id);
     void (*reset)(sPAPRMachineState *spapr, Error **errp);
     void (*set_irq)(void *opaque, int srcno, int val);
+    uint32_t (*get_phandle)(sPAPRMachineState *spapr, void *fdt, Error **errp);
 } sPAPRIrq;
 
 extern sPAPRIrq spapr_irq_xics;
diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 7fdc25057420..53a34f14b258 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -49,4 +49,6 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
 void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
 void spapr_xive_mmio_set_enabled(sPAPRXive *xive, bool enable);
 
+uint32_t spapr_get_phandle_xive(sPAPRMachineState *spapr, void *fdt,
+                                Error **errp);
 #endif /* PPC_SPAPR_XIVE_H */
diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
index b1ab27d022cf..8899c5346375 100644
--- a/include/hw/ppc/xics_spapr.h
+++ b/include/hw/ppc/xics_spapr.h
@@ -31,6 +31,8 @@
 
 void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
                    uint32_t phandle);
+uint32_t spapr_get_phandle_xics(sPAPRMachineState *spapr, void *fdt,
+                                Error **errp);
 int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
 void xics_spapr_init(sPAPRMachineState *spapr);
 

[Qemu-devel] [PATCH v2 11/13] spapr_irq: Allow synchronization of a single irq state to KVM

[Greg Kurz]

When using the in-kernel interrupt controller, the state of all irqs is
synchronized to KVM at machine reset time. In the case of PHB hotplug, we
will need to synchronize LSIs manually.

Do this for the existing KVM XICS implementation and put a placeholder for
the upcoming KVM XIVE.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/xics_kvm.c         |   67 +++++++++++++++++++++++++-------------------
 hw/ppc/spapr_irq.c         |   31 ++++++++++++++++++++
 include/hw/ppc/spapr_irq.h |    2 +
 include/hw/ppc/xics.h      |    2 +
 4 files changed, 73 insertions(+), 29 deletions(-)

diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index dff13300504c..d3bbb2bcf19c 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -253,43 +253,52 @@ static void ics_synchronize_state(ICSState *ics)
     ics_get_kvm_state(ics);
 }
 
-static int ics_set_kvm_state(ICSState *ics, int version_id)
+int ics_set_kvm_state_one(ICSState *ics, unsigned srcno, Error **errp)
 {
+    ICSIRQState *irq;
     uint64_t state;
-    int i;
-    Error *local_err = NULL;
 
-    for (i = 0; i < ics->nr_irqs; i++) {
-        ICSIRQState *irq = &ics->irqs[i];
-        int ret;
+    assert(srcno < ics->nr_irqs);
 
-        state = irq->server;
-        state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
-            << KVM_XICS_PRIORITY_SHIFT;
-        if (irq->priority != irq->saved_priority) {
-            assert(irq->priority == 0xff);
-            state |= KVM_XICS_MASKED;
-        }
+    irq = &ics->irqs[srcno];
+    state = irq->server;
+    state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
+        << KVM_XICS_PRIORITY_SHIFT;
+    if (irq->priority != irq->saved_priority) {
+        assert(irq->priority == 0xff);
+        state |= KVM_XICS_MASKED;
+    }
 
-        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
-            state |= KVM_XICS_LEVEL_SENSITIVE;
-            if (irq->status & XICS_STATUS_ASSERTED) {
-                state |= KVM_XICS_PENDING;
-            }
-        } else {
-            if (irq->status & XICS_STATUS_MASKED_PENDING) {
-                state |= KVM_XICS_PENDING;
-            }
-        }
-        if (irq->status & XICS_STATUS_PRESENTED) {
-                state |= KVM_XICS_PRESENTED;
+    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
+        state |= KVM_XICS_LEVEL_SENSITIVE;
+        if (irq->status & XICS_STATUS_ASSERTED) {
+            state |= KVM_XICS_PENDING;
         }
-        if (irq->status & XICS_STATUS_QUEUED) {
-                state |= KVM_XICS_QUEUED;
+    } else {
+        if (irq->status & XICS_STATUS_MASKED_PENDING) {
+            state |= KVM_XICS_PENDING;
         }
+    }
+    if (irq->status & XICS_STATUS_PRESENTED) {
+        state |= KVM_XICS_PRESENTED;
+    }
+    if (irq->status & XICS_STATUS_QUEUED) {
+        state |= KVM_XICS_QUEUED;
+    }
+
+    return kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
+                             srcno + ics->offset, &state, true, errp);
+}
+
+static int ics_set_kvm_state(ICSState *ics, int version_id)
+{
+    int i;
+    Error *local_err = NULL;
+
+    for (i = 0; i < ics->nr_irqs; i++) {
+        int ret;
 
-        ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
-                                i + ics->offset, &state, true, &local_err);
+        ret = ics_set_kvm_state_one(ics, i, &local_err);
         if (local_err) {
             error_report_err(local_err);
             return ret;
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index ba0df9ae2e1b..2cf666c2ebc5 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -236,6 +236,17 @@ static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
     /* TODO: create the KVM XICS device */
 }
 
+static void spapr_irq_sync_to_kvm_xics(sPAPRMachineState *spapr, int irq,
+                                       Error **errp)
+{
+    MachineState *machine = MACHINE(spapr);
+    ICSState *ics = spapr->ics;
+
+    if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
+        ics_set_kvm_state_one(ics, irq - ics->offset, errp);
+    }
+}
+
 #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
 #define SPAPR_IRQ_XICS_NR_MSIS     \
     (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
@@ -256,6 +267,7 @@ sPAPRIrq spapr_irq_xics = {
     .reset       = spapr_irq_reset_xics,
     .set_irq     = spapr_irq_set_irq_xics,
     .get_phandle = spapr_get_phandle_xics,
+    .sync_to_kvm = spapr_irq_sync_to_kvm_xics,
 };
 
 /*
@@ -389,6 +401,12 @@ static void spapr_irq_set_irq_xive(void *opaque, int srcno, int val)
     xive_source_set_irq(&spapr->xive->source, srcno, val);
 }
 
+static void spapr_irq_sync_to_kvm_xive(sPAPRMachineState *spapr, int irq,
+                                       Error **errp)
+{
+    /* TODO: to be implemented when adding KVM XIVE support */
+}
+
 /*
  * XIVE uses the full IRQ number space. Set it to 8K to be compatible
  * with XICS.
@@ -413,6 +431,7 @@ sPAPRIrq spapr_irq_xive = {
     .reset       = spapr_irq_reset_xive,
     .set_irq     = spapr_irq_set_irq_xive,
     .get_phandle = spapr_get_phandle_xive,
+    .sync_to_kvm = spapr_irq_sync_to_kvm_xive,
 };
 
 /*
@@ -577,6 +596,11 @@ static uint32_t spapr_irq_get_phandle_dual(sPAPRMachineState *spapr, void *fdt,
     return spapr_irq_current(spapr)->get_phandle(spapr, fdt, errp);
 }
 
+static void spapr_irq_sync_to_kvm_dual(sPAPRMachineState *spapr, int irq,
+                                       Error **errp)
+{
+    spapr_irq_current(spapr)->sync_to_kvm(spapr, irq, errp);
+}
 
 /*
  * Define values in sync with the XIVE and XICS backend
@@ -600,6 +624,7 @@ sPAPRIrq spapr_irq_dual = {
     .reset       = spapr_irq_reset_dual,
     .set_irq     = spapr_irq_set_irq_dual,
     .get_phandle = spapr_irq_get_phandle_dual,
+    .sync_to_kvm = spapr_irq_sync_to_kvm_dual,
 };
 
 /*
@@ -645,6 +670,11 @@ void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp)
     }
 }
 
+void spapr_irq_sync_to_kvm(sPAPRMachineState *spapr, int irq, Error **errp)
+{
+    spapr->irq->sync_to_kvm(spapr, irq, errp);
+}
+
 /*
  * XICS legacy routines - to deprecate one day
  */
@@ -717,4 +747,5 @@ sPAPRIrq spapr_irq_xics_legacy = {
     .post_load   = spapr_irq_post_load_xics,
     .set_irq     = spapr_irq_set_irq_xics,
     .get_phandle = spapr_get_phandle_xics,
+    .sync_to_kvm = spapr_irq_sync_to_kvm_xics,
 };
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index 990743a23582..9c111f3211b3 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -48,6 +48,7 @@ typedef struct sPAPRIrq {
     void (*reset)(sPAPRMachineState *spapr, Error **errp);
     void (*set_irq)(void *opaque, int srcno, int val);
     uint32_t (*get_phandle)(sPAPRMachineState *spapr, void *fdt, Error **errp);
+    void (*sync_to_kvm)(sPAPRMachineState *spapr, int irq, Error **errp);
 } sPAPRIrq;
 
 extern sPAPRIrq spapr_irq_xics;
@@ -61,6 +62,7 @@ void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
 qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
 int spapr_irq_post_load(sPAPRMachineState *spapr, int version_id);
 void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp);
+void spapr_irq_sync_to_kvm(sPAPRMachineState *spapr, int irq, Error **errp);
 
 /*
  * XICS legacy routines
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index fad786e8b22d..52de166a2982 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -203,4 +203,6 @@ void icp_resend(ICPState *ss);
 Object *icp_create(Object *cpu, const char *type, XICSFabric *xi,
                    Error **errp);
 
+int ics_set_kvm_state_one(ICSState *ics, unsigned srcno, Error **errp);
+
 #endif /* XICS_H */

[Qemu-devel] [PATCH v2 12/13] spapr: add hotplug hooks for PHB hotplug

[Greg Kurz]

Hotplugging PHBs is a machine-level operation, but PHBs reside on the
main system bus, so we register spapr machine as the handler for the
main system bus.

We re-get the phandle of the interrupt controller systematically for
simplicity.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
v2: - reworked phandle handling
    - sync LSIs to KVM
---
 hw/ppc/spapr.c         |  125 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/spapr_drc.c     |    1 
 hw/ppc/spapr_pci.c     |   27 +++++-----
 include/hw/ppc/spapr.h |    1 
 4 files changed, 138 insertions(+), 16 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ebb6fbbb2b69..96979caa5b11 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2930,6 +2930,11 @@ static void spapr_machine_init(MachineState *machine)
     register_savevm_live(NULL, "spapr/htab", -1, 1,
                          &savevm_htab_handlers, spapr);
 
+    if (smc->dr_phb_enabled) {
+        qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine),
+                                 &error_fatal);
+    }
+
     qemu_register_boot_set(spapr_boot_set, spapr);
 
     if (kvm_enabled()) {
@@ -3727,6 +3732,112 @@ out:
     error_propagate(errp, local_err);
 }
 
+static void spapr_phb_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+                               Error **errp)
+{
+    sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
+    sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
+    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+    const unsigned windows_supported = spapr_phb_windows_supported(sphb);
+
+    if (sphb->index == (uint32_t)-1) {
+        error_setg(errp, "\"index\" for PAPR PHB is mandatory");
+        return;
+    }
+
+    /*
+     * This will check that sphb->index doesn't exceed the maximum number of
+     * PHBs for the current machine type.
+     */
+    smc->phb_placement(spapr, sphb->index,
+                       &sphb->buid, &sphb->io_win_addr,
+                       &sphb->mem_win_addr, &sphb->mem64_win_addr,
+                       windows_supported, sphb->dma_liobn, errp);
+}
+
+static void spapr_phb_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+                           Error **errp)
+{
+    sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
+    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
+    sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
+    void *fdt = NULL;
+    int fdt_start_offset;
+    int fdt_size;
+    Error *local_err = NULL;
+    sPAPRDRConnector *drc;
+    int ret;
+    bool hotplugged = spapr_drc_hotplugged(dev);
+    int phandle;
+
+    if (!smc->dr_phb_enabled) {
+        return;
+    }
+
+    drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->index);
+    /* hotplug hooks should check it's enabled before getting this far */
+    assert(drc);
+
+    if (hotplugged) {
+        phandle = spapr->irq->get_phandle(spapr, spapr->fdt_blob,
+                                          &local_err);
+        if (local_err) {
+            goto out;
+        }
+    } else {
+        phandle = PHANDLE_INTC;
+    }
+
+    fdt = create_device_tree(&fdt_size);
+    ret = spapr_populate_pci_dt(sphb, phandle, fdt, spapr->irq->nr_msis,
+                                &fdt_start_offset);
+    if (ret < 0) {
+        error_setg(&local_err, "unable to create FDT for %sPHB",
+                   dev->hotplugged ? "hotplugged " : "");
+        goto out;
+    }
+
+    if (hotplugged) {
+        /* generally SLOF creates these, for hotplug it's up to QEMU */
+        _FDT(fdt_setprop_string(fdt, fdt_start_offset, "name", "pci"));
+    }
+
+    spapr_drc_attach(drc, DEVICE(dev), fdt, fdt_start_offset, &local_err);
+
+out:
+    if (local_err) {
+        error_propagate(errp, local_err);
+        g_free(fdt);
+        return;
+    }
+
+    if (hotplugged) {
+        spapr_hotplug_req_add_by_index(drc);
+    } else if (drc) {
+        spapr_drc_reset(drc);
+    }
+}
+
+void spapr_phb_release(DeviceState *dev)
+{
+    object_unparent(OBJECT(dev));
+}
+
+static void spapr_phb_unplug_request(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
+{
+    sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(dev);
+    sPAPRDRConnector *drc;
+
+    drc = spapr_drc_by_id(TYPE_SPAPR_DRC_PHB, sphb->index);
+    assert(drc);
+
+    if (!spapr_drc_unplug_requested(drc)) {
+        spapr_drc_detach(drc);
+        spapr_hotplug_req_remove_by_index(drc);
+    }
+}
+
 static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
                                       DeviceState *dev, Error **errp)
 {
@@ -3734,6 +3845,8 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
         spapr_memory_plug(hotplug_dev, dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
         spapr_core_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
+        spapr_phb_plug(hotplug_dev, dev, errp);
     }
 }
 
@@ -3752,6 +3865,7 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
 {
     sPAPRMachineState *sms = SPAPR_MACHINE(OBJECT(hotplug_dev));
     MachineClass *mc = MACHINE_GET_CLASS(sms);
+    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
 
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
@@ -3771,6 +3885,12 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
             return;
         }
         spapr_core_unplug_request(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
+        if (!smc->dr_phb_enabled) {
+            error_setg(errp, "PHB hot unplug not supported on this machine");
+            return;
+        }
+        spapr_phb_unplug_request(hotplug_dev, dev, errp);
     }
 }
 
@@ -3781,6 +3901,8 @@ static void spapr_machine_device_pre_plug(HotplugHandler *hotplug_dev,
         spapr_memory_pre_plug(hotplug_dev, dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
         spapr_core_pre_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
+        spapr_phb_pre_plug(hotplug_dev, dev, errp);
     }
 }
 
@@ -3788,7 +3910,8 @@ static HotplugHandler *spapr_get_hotplug_handler(MachineState *machine,
                                                  DeviceState *dev)
 {
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
-        object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
+        object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
         return HOTPLUG_HANDLER(machine);
     }
     return NULL;
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 189ee681062a..7a2676716364 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -703,6 +703,7 @@ static void spapr_drc_phb_class_init(ObjectClass *k, void *data)
     drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB;
     drck->typename = "PHB";
     drck->drc_name_prefix = "PHB ";
+    drck->release = spapr_phb_release;
 }
 
 static const TypeInfo spapr_dr_connector_info = {
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index f2b9066ecabe..6ee285e84efb 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1641,21 +1641,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    if (sphb->index != (uint32_t)-1) {
-        Error *local_err = NULL;
-
-        smc->phb_placement(spapr, sphb->index,
-                           &sphb->buid, &sphb->io_win_addr,
-                           &sphb->mem_win_addr, &sphb->mem64_win_addr,
-                           windows_supported, sphb->dma_liobn, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-            return;
-        }
-    } else {
-        error_setg(errp, "\"index\" for PAPR PHB is mandatory");
-        return;
-    }
+    assert(sphb->index != (uint32_t)-1); /* checked in spapr_phb_pre_plug() */
 
     if (sphb->mem64_win_size != 0) {
         if (sphb->mem_win_size > SPAPR_PCI_MEM32_WIN_SIZE) {
@@ -1873,6 +1859,17 @@ static void spapr_phb_reset(DeviceState *qdev)
     if (spapr_phb_eeh_available(SPAPR_PCI_HOST_BRIDGE(qdev))) {
         spapr_phb_vfio_reset(qdev);
     }
+
+    if (spapr_drc_hotplugged(qdev)) {
+        sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+        int i;
+
+        for (i = 0; i < PCI_NUM_PINS; i++) {
+            uint32_t irq = sphb->lsi_table[i].irq;
+
+            spapr_irq_sync_to_kvm(spapr, irq, &error_abort);
+        }
+    }
 }
 
 static Property spapr_phb_properties[] = {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 4eb80c3d888c..92aae58916ff 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -766,6 +766,7 @@ int spapr_max_server_number(sPAPRMachineState *spapr);
 /* CPU and LMB DRC release callbacks. */
 void spapr_core_release(DeviceState *dev);
 void spapr_lmb_release(DeviceState *dev);
+void spapr_phb_release(DeviceState *dev);
 
 void spapr_rtc_read(sPAPRRTCState *rtc, struct tm *tm, uint32_t *ns);
 int spapr_rtc_import_offset(sPAPRRTCState *rtc, int64_t legacy_offset);

[Qemu-devel] [PATCH v2 13/13] spapr: enable PHB hotplug for default pseries machine type

[Greg Kurz]

From: Michael Roth <mdroth@linux.vnet.ibm.com>

The 'dr_phb_enabled' field of that class can be set as part of
machine-specific init code. It will be used to conditionally
enable creation of DRC objects and device-tree description to
facilitate hotplug of PHBs.

Since we can't migrate this state to older machine types,
default the option to true and disable it for older machine
types.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 96979caa5b11..59088203e0b2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4164,6 +4164,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF;
     spapr_caps_add_properties(smc, &error_abort);
     smc->irq = &spapr_irq_xics;
+    smc->dr_phb_enabled = true;
 }
 
 static const TypeInfo spapr_machine_info = {
@@ -4229,6 +4230,7 @@ static void spapr_machine_3_1_class_options(MachineClass *mc)
     compat_props_add(mc->compat_props, hw_compat_3_1, hw_compat_3_1_len);
     mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
     smc->update_dt_enabled = false;
+    smc->dr_phb_enabled = false;
 }
 
 DEFINE_SPAPR_MACHINE(3_1, "3.1", false);

Re: [Qemu-devel] [PATCH v2 02/13] spapr: Rename xics to intc in interrupt controller agnostic code

[Cédric Le Goater]

On 1/11/19 10:00 PM, Greg Kurz wrote:
> All this code is used with both the XICS and XIVE interrupt controllers.

I am not fond of the name : spapr_dt_intc_irq(), it seems a bit redundant.

Nevertheless,

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/ppc/spapr.c              |    6 +++---
>  hw/ppc/spapr_events.c       |    2 +-
>  hw/ppc/spapr_pci.c          |    6 +++---
>  hw/ppc/spapr_vio.c          |    2 +-
>  include/hw/pci-host/spapr.h |    2 +-
>  include/hw/ppc/spapr.h      |    2 +-
>  6 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 83081defde4e..26f8e55cc25e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -96,7 +96,7 @@
>  
>  #define MIN_RMA_SLOF            128UL
>  
> -#define PHANDLE_XICP            0x00001111
> +#define PHANDLE_INTC            0x00001111
>  
>  /* These two functions implement the VCPU id numbering: one to compute them
>   * all and one to identify thread 0 of a VCORE. Any change to the first one
> @@ -1276,7 +1276,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>  
>      /* /interrupt controller */
>      spapr->irq->dt_populate(spapr, spapr_max_server_number(spapr), fdt,
> -                          PHANDLE_XICP);
> +                          PHANDLE_INTC);
>  
>      ret = spapr_populate_memory(spapr, fdt);
>      if (ret < 0) {
> @@ -1296,7 +1296,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>      }
>  
>      QLIST_FOREACH(phb, &spapr->phbs, list) {
> -        ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt,
> +        ret = spapr_populate_pci_dt(phb, PHANDLE_INTC, fdt,
>                                      spapr->irq->nr_msis);
>          if (ret < 0) {
>              error_report("couldn't setup PCI devices in fdt");
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 32719a1b72d0..d4e75211954d 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -282,7 +282,7 @@ void spapr_dt_events(sPAPRMachineState *spapr, void *fdt)
>              continue;
>          }
>  
> -        spapr_dt_xics_irq(interrupts, source->irq, false);
> +        spapr_dt_intc_irq(interrupts, source->irq, false);
>  
>          _FDT(node_offset = fdt_add_subnode(fdt, event_sources, source_name));
>          _FDT(fdt_setprop(fdt, node_offset, "interrupts", interrupts,
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index b74f2632ecc6..24f95a400623 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -2066,7 +2066,7 @@ static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
>  
>  }
>  
> -int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
> +int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
>                            uint32_t nr_msis)
>  {
>      int bus_off, i, j, ret;
> @@ -2164,8 +2164,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
>              irqmap[1] = 0;
>              irqmap[2] = 0;
>              irqmap[3] = cpu_to_be32(j+1);
> -            irqmap[4] = cpu_to_be32(xics_phandle);
> -            spapr_dt_xics_irq(&irqmap[5], phb->lsi_table[lsi_num].irq, true);
> +            irqmap[4] = cpu_to_be32(intc_phandle);
> +            spapr_dt_intc_irq(&irqmap[5], phb->lsi_table[lsi_num].irq, true);
>          }
>      }
>      /* Write interrupt map */
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index 7e8a9ad09337..cfd9849d8dfb 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -158,7 +158,7 @@ static int vio_make_devnode(VIOsPAPRDevice *dev,
>      if (dev->irq) {
>          uint32_t ints_prop[2];
>  
> -        spapr_dt_xics_irq(ints_prop, dev->irq, false);
> +        spapr_dt_intc_irq(ints_prop, dev->irq, false);
>          ret = fdt_setprop(fdt, node_off, "interrupts", ints_prop,
>                            sizeof(ints_prop));
>          if (ret < 0) {
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 4eb3a2ce3eb8..e0e683c32469 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -113,7 +113,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
>      return spapr_qirq(spapr, phb->lsi_table[pin].irq);
>  }
>  
> -int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
> +int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
>                            uint32_t nr_msis);
>  
>  void spapr_pci_rtas_init(void);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 9e01a5a12e4a..7193ce094689 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -682,7 +682,7 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr);
>   * "interrupt-controller" node has its "#interrupt-cells" property set to 2 (ie,
>   * VIO devices, RTAS event sources and PHBs).
>   */
> -static inline void spapr_dt_xics_irq(uint32_t *intspec, int irq, bool is_lsi)
> +static inline void spapr_dt_intc_irq(uint32_t *intspec, int irq, bool is_lsi)
>  {
>      intspec[0] = cpu_to_be32(irq);
>      intspec[1] = is_lsi ? cpu_to_be32(1) : 0;
> 

Re: [Qemu-devel] [PATCH v2 10/13] spapr_irq: Expose the phandle of the interrupt controller

[Cédric Le Goater]

On 1/11/19 10:04 PM, Greg Kurz wrote:
> This will be used by PHB hotplug in order to create the "interrupt-map"
> property of the PHB node.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/intc/spapr_xive.c        |   34 ++++++++++++++++++++++++++++++++--
>  hw/intc/xics_spapr.c        |   28 +++++++++++++++++++++++++++-
>  hw/ppc/spapr_irq.c          |   13 ++++++++++++-
>  include/hw/ppc/spapr_irq.h  |    1 +
>  include/hw/ppc/spapr_xive.h |    2 ++
>  include/hw/ppc/xics_spapr.h |    2 ++
>  6 files changed, 76 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index d391177ab81f..ffae680024d7 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -1414,6 +1414,12 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
>      spapr_register_hypercall(H_INT_RESET, h_int_reset);
>  }
>  
> +static gchar *xive_nodename(sPAPRXive *xive)
> +{
> +    return g_strdup_printf("interrupt-controller@%" PRIx64,
> +                           xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> +}
> +
>  void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>                     uint32_t phandle)
>  {
> @@ -1450,8 +1456,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>                             XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
>      timas[3] = cpu_to_be64(1ull << TM_SHIFT);
>  
> -    nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
> -                           xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> +    nodename = xive_nodename(xive);
>      _FDT(node = fdt_add_subnode(fdt, 0, nodename));
>      g_free(nodename);
>  
> @@ -1479,3 +1484,28 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>      _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities",
>                       plat_res_int_priorities, sizeof(plat_res_int_priorities)));
>  }
> +
> +uint32_t spapr_get_phandle_xive(sPAPRMachineState *spapr, void *fdt,
> +                                Error **errp)
> +{
> +    gchar *nodename = xive_nodename(spapr->xive);
> +    int phandle = -1, offset;
> +
> +    offset = fdt_subnode_offset(fdt, 0, nodename);
> +    if (offset < 0) {
> +        error_setg(errp, "Can't find node \"%s\": %s", nodename,
> +                   fdt_strerror(offset));
> +        goto out;
> +    }
> +
> +    phandle = fdt_get_phandle(spapr->fdt_blob, offset);
> +    if (phandle < 0) {
> +        error_setg(errp, "Can't get phandle of node \"%s\": %s",
> +                   nodename, fdt_strerror(phandle));
> +        goto out;
> +    }
> +
> +out:
> +    g_free(nodename);
> +    return phandle;
> +}
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index de6cc15b6474..7763b81d90dd 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -35,6 +35,7 @@
>  #include "hw/ppc/xics_spapr.h"
>  #include "hw/ppc/fdt.h"
>  #include "qapi/visitor.h"
> +#include "qapi/error.h"
>  
>  /*
>   * Guest interfaces
> @@ -245,6 +246,8 @@ void xics_spapr_init(sPAPRMachineState *spapr)
>      spapr_register_hypercall(H_IPOLL, h_ipoll);
>  }
>  
> +#define NODENAME "interrupt-controller"
> +
>  void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>                     uint32_t phandle)
>  {
> @@ -253,7 +256,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>      };
>      int node;
>  
> -    _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
> +    _FDT(node = fdt_add_subnode(fdt, 0, NODENAME));
>  
>      _FDT(fdt_setprop_string(fdt, node, "device_type",
>                              "PowerPC-External-Interrupt-Presentation"));
> @@ -266,3 +269,26 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>      _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle));
>      _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle));
>  }
> +
> +uint32_t spapr_get_phandle_xics(sPAPRMachineState *spapr, void *fdt,
> +                                Error **errp)
> +{

This routine is redundant with the spapr_get_phandle_xive() I think. 

I would remove the (*get_phandle)() method and move the code under a high 
level spapr_irq_get_phandle() routine which would call a method of the 
sPAPR IRQ backend : 

	char* (*get_nodename)(sPAPRMachineState *spapr);

The get_nodename_xics and get_nodename_xive methods could be located 
under the spapr_irq.c file or introduce helpers that could be used
directly as sPAPR IRQ methods. 

Thanks,

C. 

> +    int phandle = -1, offset;
> +
> +    offset = fdt_subnode_offset(fdt, 0, NODENAME);
> +    if (offset < 0) {
> +        error_setg(errp, "Can't find node \"%s\": %s", NODENAME,
> +                   fdt_strerror(offset));
> +        goto out;
> +    }
> +
> +    phandle = fdt_get_phandle(spapr->fdt_blob, offset);
> +    if (phandle < 0) {
> +        error_setg(errp, "Can't get phandle of node \"%s\": %s",
> +                   NODENAME, fdt_strerror(phandle));
> +        goto out;
> +    }
> +
> +out:
> +    return phandle;
> +}
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 1da7a32348fc..ba0df9ae2e1b 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -255,6 +255,7 @@ sPAPRIrq spapr_irq_xics = {
>      .post_load   = spapr_irq_post_load_xics,
>      .reset       = spapr_irq_reset_xics,
>      .set_irq     = spapr_irq_set_irq_xics,
> +    .get_phandle = spapr_get_phandle_xics,
>  };
>  
>  /*
> @@ -411,6 +412,7 @@ sPAPRIrq spapr_irq_xive = {
>      .post_load   = spapr_irq_post_load_xive,
>      .reset       = spapr_irq_reset_xive,
>      .set_irq     = spapr_irq_set_irq_xive,
> +    .get_phandle = spapr_get_phandle_xive,
>  };
>  
>  /*
> @@ -569,6 +571,13 @@ static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val)
>      spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
>  }
>  
> +static uint32_t spapr_irq_get_phandle_dual(sPAPRMachineState *spapr, void *fdt,
> +                                           Error **errp)
> +{
> +    return spapr_irq_current(spapr)->get_phandle(spapr, fdt, errp);
> +}
> +
> +
>  /*
>   * Define values in sync with the XIVE and XICS backend
>   */
> @@ -589,7 +598,8 @@ sPAPRIrq spapr_irq_dual = {
>      .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
>      .post_load   = spapr_irq_post_load_dual,
>      .reset       = spapr_irq_reset_dual,
> -    .set_irq     = spapr_irq_set_irq_dual
> +    .set_irq     = spapr_irq_set_irq_dual,
> +    .get_phandle = spapr_irq_get_phandle_dual,
>  };
>  
>  /*
> @@ -706,4 +716,5 @@ sPAPRIrq spapr_irq_xics_legacy = {
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>      .post_load   = spapr_irq_post_load_xics,
>      .set_irq     = spapr_irq_set_irq_xics,
> +    .get_phandle = spapr_get_phandle_xics,
>  };
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 14b02c3aca33..990743a23582 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -47,6 +47,7 @@ typedef struct sPAPRIrq {
>      int (*post_load)(sPAPRMachineState *spapr, int version_id);
>      void (*reset)(sPAPRMachineState *spapr, Error **errp);
>      void (*set_irq)(void *opaque, int srcno, int val);
> +    uint32_t (*get_phandle)(sPAPRMachineState *spapr, void *fdt, Error **errp);
>  } sPAPRIrq;
>  
>  extern sPAPRIrq spapr_irq_xics;
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 7fdc25057420..53a34f14b258 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -49,4 +49,6 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>  void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
>  void spapr_xive_mmio_set_enabled(sPAPRXive *xive, bool enable);
>  
> +uint32_t spapr_get_phandle_xive(sPAPRMachineState *spapr, void *fdt,
> +                                Error **errp);
>  #endif /* PPC_SPAPR_XIVE_H */
> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
> index b1ab27d022cf..8899c5346375 100644
> --- a/include/hw/ppc/xics_spapr.h
> +++ b/include/hw/ppc/xics_spapr.h
> @@ -31,6 +31,8 @@
>  
>  void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>                     uint32_t phandle);
> +uint32_t spapr_get_phandle_xics(sPAPRMachineState *spapr, void *fdt,
> +                                Error **errp);
>  int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
>  void xics_spapr_init(sPAPRMachineState *spapr);
>  
> 

Re: [Qemu-devel] [PATCH v2 11/13] spapr_irq: Allow synchronization of a single irq state to KVM

[Cédric Le Goater]

On 1/11/19 10:04 PM, Greg Kurz wrote:
> When using the in-kernel interrupt controller, the state of all irqs is
> synchronized to KVM at machine reset time. In the case of PHB hotplug, we
> will need to synchronize LSIs manually.

Yes. This is because the interrupt sources in the KVM XICS device have 
already been initialized as MSIs. 

Can not we reset the source when it is claimed ? 

An alternative solution would be to initialize the SPAPR_IRQ_PCI_LSI range 
as LSIs at a KVM level.

Thanks,

C.

> Do this for the existing KVM XICS implementation and put a placeholder for
> the upcoming KVM XIVE.> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/intc/xics_kvm.c         |   67 +++++++++++++++++++++++++-------------------
>  hw/ppc/spapr_irq.c         |   31 ++++++++++++++++++++
>  include/hw/ppc/spapr_irq.h |    2 +
>  include/hw/ppc/xics.h      |    2 +
>  4 files changed, 73 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index dff13300504c..d3bbb2bcf19c 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -253,43 +253,52 @@ static void ics_synchronize_state(ICSState *ics)
>      ics_get_kvm_state(ics);
>  }
>  
> -static int ics_set_kvm_state(ICSState *ics, int version_id)
> +int ics_set_kvm_state_one(ICSState *ics, unsigned srcno, Error **errp)
>  {
> +    ICSIRQState *irq;
>      uint64_t state;
> -    int i;
> -    Error *local_err = NULL;
>  
> -    for (i = 0; i < ics->nr_irqs; i++) {
> -        ICSIRQState *irq = &ics->irqs[i];
> -        int ret;
> +    assert(srcno < ics->nr_irqs);
>  
> -        state = irq->server;
> -        state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
> -            << KVM_XICS_PRIORITY_SHIFT;
> -        if (irq->priority != irq->saved_priority) {
> -            assert(irq->priority == 0xff);
> -            state |= KVM_XICS_MASKED;
> -        }
> +    irq = &ics->irqs[srcno];
> +    state = irq->server;
> +    state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
> +        << KVM_XICS_PRIORITY_SHIFT;
> +    if (irq->priority != irq->saved_priority) {
> +        assert(irq->priority == 0xff);
> +        state |= KVM_XICS_MASKED;
> +    }
>  
> -        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
> -            state |= KVM_XICS_LEVEL_SENSITIVE;
> -            if (irq->status & XICS_STATUS_ASSERTED) {
> -                state |= KVM_XICS_PENDING;
> -            }
> -        } else {
> -            if (irq->status & XICS_STATUS_MASKED_PENDING) {
> -                state |= KVM_XICS_PENDING;
> -            }
> -        }
> -        if (irq->status & XICS_STATUS_PRESENTED) {
> -                state |= KVM_XICS_PRESENTED;
> +    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
> +        state |= KVM_XICS_LEVEL_SENSITIVE;
> +        if (irq->status & XICS_STATUS_ASSERTED) {
> +            state |= KVM_XICS_PENDING;
>          }
> -        if (irq->status & XICS_STATUS_QUEUED) {
> -                state |= KVM_XICS_QUEUED;
> +    } else {
> +        if (irq->status & XICS_STATUS_MASKED_PENDING) {
> +            state |= KVM_XICS_PENDING;
>          }
> +    }
> +    if (irq->status & XICS_STATUS_PRESENTED) {
> +        state |= KVM_XICS_PRESENTED;
> +    }
> +    if (irq->status & XICS_STATUS_QUEUED) {
> +        state |= KVM_XICS_QUEUED;
> +    }
> +
> +    return kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> +                             srcno + ics->offset, &state, true, errp);
> +}
> +
> +static int ics_set_kvm_state(ICSState *ics, int version_id)
> +{
> +    int i;
> +    Error *local_err = NULL;
> +
> +    for (i = 0; i < ics->nr_irqs; i++) {
> +        int ret;
>  
> -        ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> -                                i + ics->offset, &state, true, &local_err);
> +        ret = ics_set_kvm_state_one(ics, i, &local_err);
>          if (local_err) {
>              error_report_err(local_err);
>              return ret;
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index ba0df9ae2e1b..2cf666c2ebc5 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -236,6 +236,17 @@ static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
>      /* TODO: create the KVM XICS device */
>  }
>  
> +static void spapr_irq_sync_to_kvm_xics(sPAPRMachineState *spapr, int irq,
> +                                       Error **errp)
> +{
> +    MachineState *machine = MACHINE(spapr);
> +    ICSState *ics = spapr->ics;
> +
> +    if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
> +        ics_set_kvm_state_one(ics, irq - ics->offset, errp);
> +    }
> +}
> +
>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
>  #define SPAPR_IRQ_XICS_NR_MSIS     \
>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -256,6 +267,7 @@ sPAPRIrq spapr_irq_xics = {
>      .reset       = spapr_irq_reset_xics,
>      .set_irq     = spapr_irq_set_irq_xics,
>      .get_phandle = spapr_get_phandle_xics,
> +    .sync_to_kvm = spapr_irq_sync_to_kvm_xics,
>  };
>  
>  /*
> @@ -389,6 +401,12 @@ static void spapr_irq_set_irq_xive(void *opaque, int srcno, int val)
>      xive_source_set_irq(&spapr->xive->source, srcno, val);
>  }
>  
> +static void spapr_irq_sync_to_kvm_xive(sPAPRMachineState *spapr, int irq,
> +                                       Error **errp)
> +{
> +    /* TODO: to be implemented when adding KVM XIVE support */
> +}
> +
>  /*
>   * XIVE uses the full IRQ number space. Set it to 8K to be compatible
>   * with XICS.
> @@ -413,6 +431,7 @@ sPAPRIrq spapr_irq_xive = {
>      .reset       = spapr_irq_reset_xive,
>      .set_irq     = spapr_irq_set_irq_xive,
>      .get_phandle = spapr_get_phandle_xive,
> +    .sync_to_kvm = spapr_irq_sync_to_kvm_xive,
>  };
>  
>  /*
> @@ -577,6 +596,11 @@ static uint32_t spapr_irq_get_phandle_dual(sPAPRMachineState *spapr, void *fdt,
>      return spapr_irq_current(spapr)->get_phandle(spapr, fdt, errp);
>  }
>  
> +static void spapr_irq_sync_to_kvm_dual(sPAPRMachineState *spapr, int irq,
> +                                       Error **errp)
> +{
> +    spapr_irq_current(spapr)->sync_to_kvm(spapr, irq, errp);
> +}
>  
>  /*
>   * Define values in sync with the XIVE and XICS backend
> @@ -600,6 +624,7 @@ sPAPRIrq spapr_irq_dual = {
>      .reset       = spapr_irq_reset_dual,
>      .set_irq     = spapr_irq_set_irq_dual,
>      .get_phandle = spapr_irq_get_phandle_dual,
> +    .sync_to_kvm = spapr_irq_sync_to_kvm_dual,
>  };
>  
>  /*
> @@ -645,6 +670,11 @@ void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp)
>      }
>  }
>  
> +void spapr_irq_sync_to_kvm(sPAPRMachineState *spapr, int irq, Error **errp)
> +{
> +    spapr->irq->sync_to_kvm(spapr, irq, errp);
> +}
> +
>  /*
>   * XICS legacy routines - to deprecate one day
>   */
> @@ -717,4 +747,5 @@ sPAPRIrq spapr_irq_xics_legacy = {
>      .post_load   = spapr_irq_post_load_xics,
>      .set_irq     = spapr_irq_set_irq_xics,
>      .get_phandle = spapr_get_phandle_xics,
> +    .sync_to_kvm = spapr_irq_sync_to_kvm_xics,
>  };
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 990743a23582..9c111f3211b3 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -48,6 +48,7 @@ typedef struct sPAPRIrq {
>      void (*reset)(sPAPRMachineState *spapr, Error **errp);
>      void (*set_irq)(void *opaque, int srcno, int val);
>      uint32_t (*get_phandle)(sPAPRMachineState *spapr, void *fdt, Error **errp);
> +    void (*sync_to_kvm)(sPAPRMachineState *spapr, int irq, Error **errp);
>  } sPAPRIrq;
>  
>  extern sPAPRIrq spapr_irq_xics;
> @@ -61,6 +62,7 @@ void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
>  qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
>  int spapr_irq_post_load(sPAPRMachineState *spapr, int version_id);
>  void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp);
> +void spapr_irq_sync_to_kvm(sPAPRMachineState *spapr, int irq, Error **errp);
>  
>  /*
>   * XICS legacy routines
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index fad786e8b22d..52de166a2982 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -203,4 +203,6 @@ void icp_resend(ICPState *ss);
>  Object *icp_create(Object *cpu, const char *type, XICSFabric *xi,
>                     Error **errp);
>  
> +int ics_set_kvm_state_one(ICSState *ics, unsigned srcno, Error **errp);
> +
>  #endif /* XICS_H */
> 

Re: [Qemu-devel] [PATCH v2 10/13] spapr_irq: Expose the phandle of the interrupt controller

[Greg Kurz]

On Mon, 14 Jan 2019 09:11:47 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 1/11/19 10:04 PM, Greg Kurz wrote:
> > This will be used by PHB hotplug in order to create the "interrupt-map"
> > property of the PHB node.
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/intc/spapr_xive.c        |   34 ++++++++++++++++++++++++++++++++--
> >  hw/intc/xics_spapr.c        |   28 +++++++++++++++++++++++++++-
> >  hw/ppc/spapr_irq.c          |   13 ++++++++++++-
> >  include/hw/ppc/spapr_irq.h  |    1 +
> >  include/hw/ppc/spapr_xive.h |    2 ++
> >  include/hw/ppc/xics_spapr.h |    2 ++
> >  6 files changed, 76 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> > index d391177ab81f..ffae680024d7 100644
> > --- a/hw/intc/spapr_xive.c
> > +++ b/hw/intc/spapr_xive.c
> > @@ -1414,6 +1414,12 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
> >      spapr_register_hypercall(H_INT_RESET, h_int_reset);
> >  }
> >  
> > +static gchar *xive_nodename(sPAPRXive *xive)
> > +{
> > +    return g_strdup_printf("interrupt-controller@%" PRIx64,
> > +                           xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> > +}
> > +
> >  void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> >                     uint32_t phandle)
> >  {
> > @@ -1450,8 +1456,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> >                             XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
> >      timas[3] = cpu_to_be64(1ull << TM_SHIFT);
> >  
> > -    nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
> > -                           xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
> > +    nodename = xive_nodename(xive);
> >      _FDT(node = fdt_add_subnode(fdt, 0, nodename));
> >      g_free(nodename);
> >  
> > @@ -1479,3 +1484,28 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> >      _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities",
> >                       plat_res_int_priorities, sizeof(plat_res_int_priorities)));
> >  }
> > +
> > +uint32_t spapr_get_phandle_xive(sPAPRMachineState *spapr, void *fdt,
> > +                                Error **errp)
> > +{
> > +    gchar *nodename = xive_nodename(spapr->xive);
> > +    int phandle = -1, offset;
> > +
> > +    offset = fdt_subnode_offset(fdt, 0, nodename);
> > +    if (offset < 0) {
> > +        error_setg(errp, "Can't find node \"%s\": %s", nodename,
> > +                   fdt_strerror(offset));
> > +        goto out;
> > +    }
> > +
> > +    phandle = fdt_get_phandle(spapr->fdt_blob, offset);
> > +    if (phandle < 0) {
> > +        error_setg(errp, "Can't get phandle of node \"%s\": %s",
> > +                   nodename, fdt_strerror(phandle));
> > +        goto out;
> > +    }
> > +
> > +out:
> > +    g_free(nodename);
> > +    return phandle;
> > +}
> > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> > index de6cc15b6474..7763b81d90dd 100644
> > --- a/hw/intc/xics_spapr.c
> > +++ b/hw/intc/xics_spapr.c
> > @@ -35,6 +35,7 @@
> >  #include "hw/ppc/xics_spapr.h"
> >  #include "hw/ppc/fdt.h"
> >  #include "qapi/visitor.h"
> > +#include "qapi/error.h"
> >  
> >  /*
> >   * Guest interfaces
> > @@ -245,6 +246,8 @@ void xics_spapr_init(sPAPRMachineState *spapr)
> >      spapr_register_hypercall(H_IPOLL, h_ipoll);
> >  }
> >  
> > +#define NODENAME "interrupt-controller"
> > +
> >  void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> >                     uint32_t phandle)
> >  {
> > @@ -253,7 +256,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> >      };
> >      int node;
> >  
> > -    _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
> > +    _FDT(node = fdt_add_subnode(fdt, 0, NODENAME));
> >  
> >      _FDT(fdt_setprop_string(fdt, node, "device_type",
> >                              "PowerPC-External-Interrupt-Presentation"));
> > @@ -266,3 +269,26 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> >      _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle));
> >      _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle));
> >  }
> > +
> > +uint32_t spapr_get_phandle_xics(sPAPRMachineState *spapr, void *fdt,
> > +                                Error **errp)
> > +{  
> 
> This routine is redundant with the spapr_get_phandle_xive() I think. 
> 

Yes both routines do the same, except for the node name being a runtime
thing with XIVE and build time with XICS.

> I would remove the (*get_phandle)() method and move the code under a high 
> level spapr_irq_get_phandle() routine which would call a method of the 
> sPAPR IRQ backend : 
> 
> 	char* (*get_nodename)(sPAPRMachineState *spapr);
> 

Yeah I had this in v1 but I didn't like to do g_strdup() of a fixed
string in the XICS case. Anyway, I could possibly revert to doing that
but I guess the setting of the "phandle" property should be moved to
spapr_irq.c as well.

> The get_nodename_xics and get_nodename_xive methods could be located 
> under the spapr_irq.c file or introduce helpers that could be used
> directly as sPAPR IRQ methods. 
> 
> Thanks,
> 
> C. 
> 
> > +    int phandle = -1, offset;
> > +
> > +    offset = fdt_subnode_offset(fdt, 0, NODENAME);
> > +    if (offset < 0) {
> > +        error_setg(errp, "Can't find node \"%s\": %s", NODENAME,
> > +                   fdt_strerror(offset));
> > +        goto out;
> > +    }
> > +
> > +    phandle = fdt_get_phandle(spapr->fdt_blob, offset);
> > +    if (phandle < 0) {
> > +        error_setg(errp, "Can't get phandle of node \"%s\": %s",
> > +                   NODENAME, fdt_strerror(phandle));
> > +        goto out;
> > +    }
> > +
> > +out:
> > +    return phandle;
> > +}
> > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> > index 1da7a32348fc..ba0df9ae2e1b 100644
> > --- a/hw/ppc/spapr_irq.c
> > +++ b/hw/ppc/spapr_irq.c
> > @@ -255,6 +255,7 @@ sPAPRIrq spapr_irq_xics = {
> >      .post_load   = spapr_irq_post_load_xics,
> >      .reset       = spapr_irq_reset_xics,
> >      .set_irq     = spapr_irq_set_irq_xics,
> > +    .get_phandle = spapr_get_phandle_xics,
> >  };
> >  
> >  /*
> > @@ -411,6 +412,7 @@ sPAPRIrq spapr_irq_xive = {
> >      .post_load   = spapr_irq_post_load_xive,
> >      .reset       = spapr_irq_reset_xive,
> >      .set_irq     = spapr_irq_set_irq_xive,
> > +    .get_phandle = spapr_get_phandle_xive,
> >  };
> >  
> >  /*
> > @@ -569,6 +571,13 @@ static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val)
> >      spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
> >  }
> >  
> > +static uint32_t spapr_irq_get_phandle_dual(sPAPRMachineState *spapr, void *fdt,
> > +                                           Error **errp)
> > +{
> > +    return spapr_irq_current(spapr)->get_phandle(spapr, fdt, errp);
> > +}
> > +
> > +
> >  /*
> >   * Define values in sync with the XIVE and XICS backend
> >   */
> > @@ -589,7 +598,8 @@ sPAPRIrq spapr_irq_dual = {
> >      .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
> >      .post_load   = spapr_irq_post_load_dual,
> >      .reset       = spapr_irq_reset_dual,
> > -    .set_irq     = spapr_irq_set_irq_dual
> > +    .set_irq     = spapr_irq_set_irq_dual,
> > +    .get_phandle = spapr_irq_get_phandle_dual,
> >  };
> >  
> >  /*
> > @@ -706,4 +716,5 @@ sPAPRIrq spapr_irq_xics_legacy = {
> >      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> >      .post_load   = spapr_irq_post_load_xics,
> >      .set_irq     = spapr_irq_set_irq_xics,
> > +    .get_phandle = spapr_get_phandle_xics,
> >  };
> > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> > index 14b02c3aca33..990743a23582 100644
> > --- a/include/hw/ppc/spapr_irq.h
> > +++ b/include/hw/ppc/spapr_irq.h
> > @@ -47,6 +47,7 @@ typedef struct sPAPRIrq {
> >      int (*post_load)(sPAPRMachineState *spapr, int version_id);
> >      void (*reset)(sPAPRMachineState *spapr, Error **errp);
> >      void (*set_irq)(void *opaque, int srcno, int val);
> > +    uint32_t (*get_phandle)(sPAPRMachineState *spapr, void *fdt, Error **errp);
> >  } sPAPRIrq;
> >  
> >  extern sPAPRIrq spapr_irq_xics;
> > diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> > index 7fdc25057420..53a34f14b258 100644
> > --- a/include/hw/ppc/spapr_xive.h
> > +++ b/include/hw/ppc/spapr_xive.h
> > @@ -49,4 +49,6 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> >  void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
> >  void spapr_xive_mmio_set_enabled(sPAPRXive *xive, bool enable);
> >  
> > +uint32_t spapr_get_phandle_xive(sPAPRMachineState *spapr, void *fdt,
> > +                                Error **errp);
> >  #endif /* PPC_SPAPR_XIVE_H */
> > diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
> > index b1ab27d022cf..8899c5346375 100644
> > --- a/include/hw/ppc/xics_spapr.h
> > +++ b/include/hw/ppc/xics_spapr.h
> > @@ -31,6 +31,8 @@
> >  
> >  void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> >                     uint32_t phandle);
> > +uint32_t spapr_get_phandle_xics(sPAPRMachineState *spapr, void *fdt,
> > +                                Error **errp);
> >  int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
> >  void xics_spapr_init(sPAPRMachineState *spapr);
> >  
> >   
> 

Re: [Qemu-devel] [PATCH v2 10/13] spapr_irq: Expose the phandle of the interrupt controller

[Cédric Le Goater]

On 1/14/19 11:03 AM, Greg Kurz wrote:
> On Mon, 14 Jan 2019 09:11:47 +0100
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> On 1/11/19 10:04 PM, Greg Kurz wrote:
>>> This will be used by PHB hotplug in order to create the "interrupt-map"
>>> property of the PHB node.
>>>
>>> Signed-off-by: Greg Kurz <groug@kaod.org>
>>> ---
>>>  hw/intc/spapr_xive.c        |   34 ++++++++++++++++++++++++++++++++--
>>>  hw/intc/xics_spapr.c        |   28 +++++++++++++++++++++++++++-
>>>  hw/ppc/spapr_irq.c          |   13 ++++++++++++-
>>>  include/hw/ppc/spapr_irq.h  |    1 +
>>>  include/hw/ppc/spapr_xive.h |    2 ++
>>>  include/hw/ppc/xics_spapr.h |    2 ++
>>>  6 files changed, 76 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>>> index d391177ab81f..ffae680024d7 100644
>>> --- a/hw/intc/spapr_xive.c
>>> +++ b/hw/intc/spapr_xive.c
>>> @@ -1414,6 +1414,12 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr)
>>>      spapr_register_hypercall(H_INT_RESET, h_int_reset);
>>>  }
>>>  
>>> +static gchar *xive_nodename(sPAPRXive *xive)
>>> +{
>>> +    return g_strdup_printf("interrupt-controller@%" PRIx64,
>>> +                           xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
>>> +}
>>> +
>>>  void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>>>                     uint32_t phandle)
>>>  {
>>> @@ -1450,8 +1456,7 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>>>                             XIVE_TM_OS_PAGE * (1ull << TM_SHIFT));
>>>      timas[3] = cpu_to_be64(1ull << TM_SHIFT);
>>>  
>>> -    nodename = g_strdup_printf("interrupt-controller@%" PRIx64,
>>> -                           xive->tm_base + XIVE_TM_USER_PAGE * (1 << TM_SHIFT));
>>> +    nodename = xive_nodename(xive);
>>>      _FDT(node = fdt_add_subnode(fdt, 0, nodename));
>>>      g_free(nodename);
>>>  
>>> @@ -1479,3 +1484,28 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>>>      _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities",
>>>                       plat_res_int_priorities, sizeof(plat_res_int_priorities)));
>>>  }
>>> +
>>> +uint32_t spapr_get_phandle_xive(sPAPRMachineState *spapr, void *fdt,
>>> +                                Error **errp)
>>> +{
>>> +    gchar *nodename = xive_nodename(spapr->xive);
>>> +    int phandle = -1, offset;
>>> +
>>> +    offset = fdt_subnode_offset(fdt, 0, nodename);
>>> +    if (offset < 0) {
>>> +        error_setg(errp, "Can't find node \"%s\": %s", nodename,
>>> +                   fdt_strerror(offset));
>>> +        goto out;
>>> +    }
>>> +
>>> +    phandle = fdt_get_phandle(spapr->fdt_blob, offset);
>>> +    if (phandle < 0) {
>>> +        error_setg(errp, "Can't get phandle of node \"%s\": %s",
>>> +                   nodename, fdt_strerror(phandle));
>>> +        goto out;
>>> +    }
>>> +
>>> +out:
>>> +    g_free(nodename);
>>> +    return phandle;
>>> +}
>>> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
>>> index de6cc15b6474..7763b81d90dd 100644
>>> --- a/hw/intc/xics_spapr.c
>>> +++ b/hw/intc/xics_spapr.c
>>> @@ -35,6 +35,7 @@
>>>  #include "hw/ppc/xics_spapr.h"
>>>  #include "hw/ppc/fdt.h"
>>>  #include "qapi/visitor.h"
>>> +#include "qapi/error.h"
>>>  
>>>  /*
>>>   * Guest interfaces
>>> @@ -245,6 +246,8 @@ void xics_spapr_init(sPAPRMachineState *spapr)
>>>      spapr_register_hypercall(H_IPOLL, h_ipoll);
>>>  }
>>>  
>>> +#define NODENAME "interrupt-controller"
>>> +
>>>  void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>>>                     uint32_t phandle)
>>>  {
>>> @@ -253,7 +256,7 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>>>      };
>>>      int node;
>>>  
>>> -    _FDT(node = fdt_add_subnode(fdt, 0, "interrupt-controller"));
>>> +    _FDT(node = fdt_add_subnode(fdt, 0, NODENAME));
>>>  
>>>      _FDT(fdt_setprop_string(fdt, node, "device_type",
>>>                              "PowerPC-External-Interrupt-Presentation"));
>>> @@ -266,3 +269,26 @@ void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>>>      _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle));
>>>      _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle));
>>>  }
>>> +
>>> +uint32_t spapr_get_phandle_xics(sPAPRMachineState *spapr, void *fdt,
>>> +                                Error **errp)
>>> +{  
>>
>> This routine is redundant with the spapr_get_phandle_xive() I think. 
>>
> 
> Yes both routines do the same, except for the node name being a runtime
> thing with XIVE and build time with XICS.
> 
>> I would remove the (*get_phandle)() method and move the code under a high 
>> level spapr_irq_get_phandle() routine which would call a method of the 
>> sPAPR IRQ backend : 
>>
>> 	char* (*get_nodename)(sPAPRMachineState *spapr);
>>
> 
> Yeah I had this in v1 but I didn't like to do g_strdup() of a fixed
> string in the XICS case. 

you could introduce a static string and initialize it once. There is no
reason for it to change.

C. 

> Anyway, I could possibly revert to doing that
> but I guess the setting of the "phandle" property should be moved to
> spapr_irq.c as well.
> 
>> The get_nodename_xics and get_nodename_xive methods could be located 
>> under the spapr_irq.c file or introduce helpers that could be used
>> directly as sPAPR IRQ methods. 
>>
>> Thanks,
>>
>> C. 
>>
>>> +    int phandle = -1, offset;
>>> +
>>> +    offset = fdt_subnode_offset(fdt, 0, NODENAME);
>>> +    if (offset < 0) {
>>> +        error_setg(errp, "Can't find node \"%s\": %s", NODENAME,
>>> +                   fdt_strerror(offset));
>>> +        goto out;
>>> +    }
>>> +
>>> +    phandle = fdt_get_phandle(spapr->fdt_blob, offset);
>>> +    if (phandle < 0) {
>>> +        error_setg(errp, "Can't get phandle of node \"%s\": %s",
>>> +                   NODENAME, fdt_strerror(phandle));
>>> +        goto out;
>>> +    }
>>> +
>>> +out:
>>> +    return phandle;
>>> +}
>>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>>> index 1da7a32348fc..ba0df9ae2e1b 100644
>>> --- a/hw/ppc/spapr_irq.c
>>> +++ b/hw/ppc/spapr_irq.c
>>> @@ -255,6 +255,7 @@ sPAPRIrq spapr_irq_xics = {
>>>      .post_load   = spapr_irq_post_load_xics,
>>>      .reset       = spapr_irq_reset_xics,
>>>      .set_irq     = spapr_irq_set_irq_xics,
>>> +    .get_phandle = spapr_get_phandle_xics,
>>>  };
>>>  
>>>  /*
>>> @@ -411,6 +412,7 @@ sPAPRIrq spapr_irq_xive = {
>>>      .post_load   = spapr_irq_post_load_xive,
>>>      .reset       = spapr_irq_reset_xive,
>>>      .set_irq     = spapr_irq_set_irq_xive,
>>> +    .get_phandle = spapr_get_phandle_xive,
>>>  };
>>>  
>>>  /*
>>> @@ -569,6 +571,13 @@ static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val)
>>>      spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
>>>  }
>>>  
>>> +static uint32_t spapr_irq_get_phandle_dual(sPAPRMachineState *spapr, void *fdt,
>>> +                                           Error **errp)
>>> +{
>>> +    return spapr_irq_current(spapr)->get_phandle(spapr, fdt, errp);
>>> +}
>>> +
>>> +
>>>  /*
>>>   * Define values in sync with the XIVE and XICS backend
>>>   */
>>> @@ -589,7 +598,8 @@ sPAPRIrq spapr_irq_dual = {
>>>      .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
>>>      .post_load   = spapr_irq_post_load_dual,
>>>      .reset       = spapr_irq_reset_dual,
>>> -    .set_irq     = spapr_irq_set_irq_dual
>>> +    .set_irq     = spapr_irq_set_irq_dual,
>>> +    .get_phandle = spapr_irq_get_phandle_dual,
>>>  };
>>>  
>>>  /*
>>> @@ -706,4 +716,5 @@ sPAPRIrq spapr_irq_xics_legacy = {
>>>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>>>      .post_load   = spapr_irq_post_load_xics,
>>>      .set_irq     = spapr_irq_set_irq_xics,
>>> +    .get_phandle = spapr_get_phandle_xics,
>>>  };
>>> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
>>> index 14b02c3aca33..990743a23582 100644
>>> --- a/include/hw/ppc/spapr_irq.h
>>> +++ b/include/hw/ppc/spapr_irq.h
>>> @@ -47,6 +47,7 @@ typedef struct sPAPRIrq {
>>>      int (*post_load)(sPAPRMachineState *spapr, int version_id);
>>>      void (*reset)(sPAPRMachineState *spapr, Error **errp);
>>>      void (*set_irq)(void *opaque, int srcno, int val);
>>> +    uint32_t (*get_phandle)(sPAPRMachineState *spapr, void *fdt, Error **errp);
>>>  } sPAPRIrq;
>>>  
>>>  extern sPAPRIrq spapr_irq_xics;
>>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>>> index 7fdc25057420..53a34f14b258 100644
>>> --- a/include/hw/ppc/spapr_xive.h
>>> +++ b/include/hw/ppc/spapr_xive.h
>>> @@ -49,4 +49,6 @@ void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>>>  void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
>>>  void spapr_xive_mmio_set_enabled(sPAPRXive *xive, bool enable);
>>>  
>>> +uint32_t spapr_get_phandle_xive(sPAPRMachineState *spapr, void *fdt,
>>> +                                Error **errp);
>>>  #endif /* PPC_SPAPR_XIVE_H */
>>> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
>>> index b1ab27d022cf..8899c5346375 100644
>>> --- a/include/hw/ppc/xics_spapr.h
>>> +++ b/include/hw/ppc/xics_spapr.h
>>> @@ -31,6 +31,8 @@
>>>  
>>>  void spapr_dt_xics(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>>>                     uint32_t phandle);
>>> +uint32_t spapr_get_phandle_xics(sPAPRMachineState *spapr, void *fdt,
>>> +                                Error **errp);
>>>  int xics_kvm_init(sPAPRMachineState *spapr, Error **errp);
>>>  void xics_spapr_init(sPAPRMachineState *spapr);
>>>  
>>>   
>>
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 02/13] spapr: Rename xics to intc in interrupt controller agnostic code

[Greg Kurz]

On Mon, 14 Jan 2019 08:58:26 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 1/11/19 10:00 PM, Greg Kurz wrote:
> > All this code is used with both the XICS and XIVE interrupt controllers.  
> 
> I am not fond of the name : spapr_dt_intc_irq(), it seems a bit redundant.
> 

Yeah, I guess it could be spapr_dt_irq().

> Nevertheless,
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> Thanks,
> 
> C.
> 
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/ppc/spapr.c              |    6 +++---
> >  hw/ppc/spapr_events.c       |    2 +-
> >  hw/ppc/spapr_pci.c          |    6 +++---
> >  hw/ppc/spapr_vio.c          |    2 +-
> >  include/hw/pci-host/spapr.h |    2 +-
> >  include/hw/ppc/spapr.h      |    2 +-
> >  6 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 83081defde4e..26f8e55cc25e 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -96,7 +96,7 @@
> >  
> >  #define MIN_RMA_SLOF            128UL
> >  
> > -#define PHANDLE_XICP            0x00001111
> > +#define PHANDLE_INTC            0x00001111
> >  
> >  /* These two functions implement the VCPU id numbering: one to compute them
> >   * all and one to identify thread 0 of a VCORE. Any change to the first one
> > @@ -1276,7 +1276,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
> >  
> >      /* /interrupt controller */
> >      spapr->irq->dt_populate(spapr, spapr_max_server_number(spapr), fdt,
> > -                          PHANDLE_XICP);
> > +                          PHANDLE_INTC);
> >  
> >      ret = spapr_populate_memory(spapr, fdt);
> >      if (ret < 0) {
> > @@ -1296,7 +1296,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
> >      }
> >  
> >      QLIST_FOREACH(phb, &spapr->phbs, list) {
> > -        ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt,
> > +        ret = spapr_populate_pci_dt(phb, PHANDLE_INTC, fdt,
> >                                      spapr->irq->nr_msis);
> >          if (ret < 0) {
> >              error_report("couldn't setup PCI devices in fdt");
> > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> > index 32719a1b72d0..d4e75211954d 100644
> > --- a/hw/ppc/spapr_events.c
> > +++ b/hw/ppc/spapr_events.c
> > @@ -282,7 +282,7 @@ void spapr_dt_events(sPAPRMachineState *spapr, void *fdt)
> >              continue;
> >          }
> >  
> > -        spapr_dt_xics_irq(interrupts, source->irq, false);
> > +        spapr_dt_intc_irq(interrupts, source->irq, false);
> >  
> >          _FDT(node_offset = fdt_add_subnode(fdt, event_sources, source_name));
> >          _FDT(fdt_setprop(fdt, node_offset, "interrupts", interrupts,
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index b74f2632ecc6..24f95a400623 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -2066,7 +2066,7 @@ static void spapr_phb_pci_enumerate(sPAPRPHBState *phb)
> >  
> >  }
> >  
> > -int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
> > +int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
> >                            uint32_t nr_msis)
> >  {
> >      int bus_off, i, j, ret;
> > @@ -2164,8 +2164,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
> >              irqmap[1] = 0;
> >              irqmap[2] = 0;
> >              irqmap[3] = cpu_to_be32(j+1);
> > -            irqmap[4] = cpu_to_be32(xics_phandle);
> > -            spapr_dt_xics_irq(&irqmap[5], phb->lsi_table[lsi_num].irq, true);
> > +            irqmap[4] = cpu_to_be32(intc_phandle);
> > +            spapr_dt_intc_irq(&irqmap[5], phb->lsi_table[lsi_num].irq, true);
> >          }
> >      }
> >      /* Write interrupt map */
> > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> > index 7e8a9ad09337..cfd9849d8dfb 100644
> > --- a/hw/ppc/spapr_vio.c
> > +++ b/hw/ppc/spapr_vio.c
> > @@ -158,7 +158,7 @@ static int vio_make_devnode(VIOsPAPRDevice *dev,
> >      if (dev->irq) {
> >          uint32_t ints_prop[2];
> >  
> > -        spapr_dt_xics_irq(ints_prop, dev->irq, false);
> > +        spapr_dt_intc_irq(ints_prop, dev->irq, false);
> >          ret = fdt_setprop(fdt, node_off, "interrupts", ints_prop,
> >                            sizeof(ints_prop));
> >          if (ret < 0) {
> > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> > index 4eb3a2ce3eb8..e0e683c32469 100644
> > --- a/include/hw/pci-host/spapr.h
> > +++ b/include/hw/pci-host/spapr.h
> > @@ -113,7 +113,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
> >      return spapr_qirq(spapr, phb->lsi_table[pin].irq);
> >  }
> >  
> > -int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt,
> > +int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void *fdt,
> >                            uint32_t nr_msis);
> >  
> >  void spapr_pci_rtas_init(void);
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 9e01a5a12e4a..7193ce094689 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -682,7 +682,7 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr);
> >   * "interrupt-controller" node has its "#interrupt-cells" property set to 2 (ie,
> >   * VIO devices, RTAS event sources and PHBs).
> >   */
> > -static inline void spapr_dt_xics_irq(uint32_t *intspec, int irq, bool is_lsi)
> > +static inline void spapr_dt_intc_irq(uint32_t *intspec, int irq, bool is_lsi)
> >  {
> >      intspec[0] = cpu_to_be32(irq);
> >      intspec[1] = is_lsi ? cpu_to_be32(1) : 0;
> >   
> 
> 

Re: [Qemu-devel] [PATCH v2 07/13] qdev: pass an Object * to qbus_set_hotplug_handler()

[Cornelia Huck]

On Fri, 11 Jan 2019 22:02:50 +0100
Greg Kurz <groug@kaod.org> wrote:

> From: Michael Roth <mdroth@linux.vnet.ibm.com>
> 
> Certain devices types, like memory/CPU, are now being handled using a
> hotplug interface provided by a top-level MachineClass. Hotpluggable
> host bridges are another such device where it makes sense to use a
> machine-level hotplug handler. However, unlike those devices,
> host-bridges have a parent bus (the main system bus), and devices with
> a parent bus use a different mechanism for registering their hotplug
> handlers: qbus_set_hotplug_handler(). This interface currently expects
> a handler to be a subclass of DeviceClass, but this is not the case
> for MachineClass, which derives directly from ObjectClass.
> 
> Internally, the interface only requires an ObjectClass, so expose that
> in qbus_set_hotplug_handler().
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/acpi/pcihp.c               |    2 +-
>  hw/acpi/piix4.c               |    2 +-
>  hw/char/virtio-serial-bus.c   |    2 +-
>  hw/core/bus.c                 |   11 ++---------
>  hw/pci/pcie.c                 |    2 +-
>  hw/pci/shpc.c                 |    2 +-
>  hw/ppc/spapr_pci.c            |    2 +-
>  hw/s390x/css-bridge.c         |    2 +-
>  hw/s390x/s390-pci-bus.c       |    6 +++---
>  hw/scsi/virtio-scsi.c         |    2 +-
>  hw/scsi/vmw_pvscsi.c          |    2 +-
>  hw/usb/dev-smartcard-reader.c |    2 +-
>  include/hw/qdev-core.h        |    3 +--
>  13 files changed, 16 insertions(+), 24 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH v2 11/13] spapr_irq: Allow synchronization of a single irq state to KVM

[Greg Kurz]

On Mon, 14 Jan 2019 09:19:50 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 1/11/19 10:04 PM, Greg Kurz wrote:
> > When using the in-kernel interrupt controller, the state of all irqs is
> > synchronized to KVM at machine reset time. In the case of PHB hotplug, we
> > will need to synchronize LSIs manually.  
> 
> Yes. This is because the interrupt sources in the KVM XICS device have 
> already been initialized as MSIs. 
> 
> Can not we reset the source when it is claimed ? 
> 
> An alternative solution would be to initialize the SPAPR_IRQ_PCI_LSI range 
> as LSIs at a KVM level.
> 

I don't really want to add some dependency to sPAPR code in KVM XICS... what
about claiming LSIs for all possible PHBs at machine init time ?

> Thanks,
> 
> C.
> 
> > Do this for the existing KVM XICS implementation and put a placeholder for
> > the upcoming KVM XIVE.> 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/intc/xics_kvm.c         |   67 +++++++++++++++++++++++++-------------------
> >  hw/ppc/spapr_irq.c         |   31 ++++++++++++++++++++
> >  include/hw/ppc/spapr_irq.h |    2 +
> >  include/hw/ppc/xics.h      |    2 +
> >  4 files changed, 73 insertions(+), 29 deletions(-)
> > 
> > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> > index dff13300504c..d3bbb2bcf19c 100644
> > --- a/hw/intc/xics_kvm.c
> > +++ b/hw/intc/xics_kvm.c
> > @@ -253,43 +253,52 @@ static void ics_synchronize_state(ICSState *ics)
> >      ics_get_kvm_state(ics);
> >  }
> >  
> > -static int ics_set_kvm_state(ICSState *ics, int version_id)
> > +int ics_set_kvm_state_one(ICSState *ics, unsigned srcno, Error **errp)
> >  {
> > +    ICSIRQState *irq;
> >      uint64_t state;
> > -    int i;
> > -    Error *local_err = NULL;
> >  
> > -    for (i = 0; i < ics->nr_irqs; i++) {
> > -        ICSIRQState *irq = &ics->irqs[i];
> > -        int ret;
> > +    assert(srcno < ics->nr_irqs);
> >  
> > -        state = irq->server;
> > -        state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
> > -            << KVM_XICS_PRIORITY_SHIFT;
> > -        if (irq->priority != irq->saved_priority) {
> > -            assert(irq->priority == 0xff);
> > -            state |= KVM_XICS_MASKED;
> > -        }
> > +    irq = &ics->irqs[srcno];
> > +    state = irq->server;
> > +    state |= (uint64_t)(irq->saved_priority & KVM_XICS_PRIORITY_MASK)
> > +        << KVM_XICS_PRIORITY_SHIFT;
> > +    if (irq->priority != irq->saved_priority) {
> > +        assert(irq->priority == 0xff);
> > +        state |= KVM_XICS_MASKED;
> > +    }
> >  
> > -        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
> > -            state |= KVM_XICS_LEVEL_SENSITIVE;
> > -            if (irq->status & XICS_STATUS_ASSERTED) {
> > -                state |= KVM_XICS_PENDING;
> > -            }
> > -        } else {
> > -            if (irq->status & XICS_STATUS_MASKED_PENDING) {
> > -                state |= KVM_XICS_PENDING;
> > -            }
> > -        }
> > -        if (irq->status & XICS_STATUS_PRESENTED) {
> > -                state |= KVM_XICS_PRESENTED;
> > +    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
> > +        state |= KVM_XICS_LEVEL_SENSITIVE;
> > +        if (irq->status & XICS_STATUS_ASSERTED) {
> > +            state |= KVM_XICS_PENDING;
> >          }
> > -        if (irq->status & XICS_STATUS_QUEUED) {
> > -                state |= KVM_XICS_QUEUED;
> > +    } else {
> > +        if (irq->status & XICS_STATUS_MASKED_PENDING) {
> > +            state |= KVM_XICS_PENDING;
> >          }
> > +    }
> > +    if (irq->status & XICS_STATUS_PRESENTED) {
> > +        state |= KVM_XICS_PRESENTED;
> > +    }
> > +    if (irq->status & XICS_STATUS_QUEUED) {
> > +        state |= KVM_XICS_QUEUED;
> > +    }
> > +
> > +    return kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> > +                             srcno + ics->offset, &state, true, errp);
> > +}
> > +
> > +static int ics_set_kvm_state(ICSState *ics, int version_id)
> > +{
> > +    int i;
> > +    Error *local_err = NULL;
> > +
> > +    for (i = 0; i < ics->nr_irqs; i++) {
> > +        int ret;
> >  
> > -        ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> > -                                i + ics->offset, &state, true, &local_err);
> > +        ret = ics_set_kvm_state_one(ics, i, &local_err);
> >          if (local_err) {
> >              error_report_err(local_err);
> >              return ret;
> > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> > index ba0df9ae2e1b..2cf666c2ebc5 100644
> > --- a/hw/ppc/spapr_irq.c
> > +++ b/hw/ppc/spapr_irq.c
> > @@ -236,6 +236,17 @@ static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> >      /* TODO: create the KVM XICS device */
> >  }
> >  
> > +static void spapr_irq_sync_to_kvm_xics(sPAPRMachineState *spapr, int irq,
> > +                                       Error **errp)
> > +{
> > +    MachineState *machine = MACHINE(spapr);
> > +    ICSState *ics = spapr->ics;
> > +
> > +    if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
> > +        ics_set_kvm_state_one(ics, irq - ics->offset, errp);
> > +    }
> > +}
> > +
> >  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
> >  #define SPAPR_IRQ_XICS_NR_MSIS     \
> >      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> > @@ -256,6 +267,7 @@ sPAPRIrq spapr_irq_xics = {
> >      .reset       = spapr_irq_reset_xics,
> >      .set_irq     = spapr_irq_set_irq_xics,
> >      .get_phandle = spapr_get_phandle_xics,
> > +    .sync_to_kvm = spapr_irq_sync_to_kvm_xics,
> >  };
> >  
> >  /*
> > @@ -389,6 +401,12 @@ static void spapr_irq_set_irq_xive(void *opaque, int srcno, int val)
> >      xive_source_set_irq(&spapr->xive->source, srcno, val);
> >  }
> >  
> > +static void spapr_irq_sync_to_kvm_xive(sPAPRMachineState *spapr, int irq,
> > +                                       Error **errp)
> > +{
> > +    /* TODO: to be implemented when adding KVM XIVE support */
> > +}
> > +
> >  /*
> >   * XIVE uses the full IRQ number space. Set it to 8K to be compatible
> >   * with XICS.
> > @@ -413,6 +431,7 @@ sPAPRIrq spapr_irq_xive = {
> >      .reset       = spapr_irq_reset_xive,
> >      .set_irq     = spapr_irq_set_irq_xive,
> >      .get_phandle = spapr_get_phandle_xive,
> > +    .sync_to_kvm = spapr_irq_sync_to_kvm_xive,
> >  };
> >  
> >  /*
> > @@ -577,6 +596,11 @@ static uint32_t spapr_irq_get_phandle_dual(sPAPRMachineState *spapr, void *fdt,
> >      return spapr_irq_current(spapr)->get_phandle(spapr, fdt, errp);
> >  }
> >  
> > +static void spapr_irq_sync_to_kvm_dual(sPAPRMachineState *spapr, int irq,
> > +                                       Error **errp)
> > +{
> > +    spapr_irq_current(spapr)->sync_to_kvm(spapr, irq, errp);
> > +}
> >  
> >  /*
> >   * Define values in sync with the XIVE and XICS backend
> > @@ -600,6 +624,7 @@ sPAPRIrq spapr_irq_dual = {
> >      .reset       = spapr_irq_reset_dual,
> >      .set_irq     = spapr_irq_set_irq_dual,
> >      .get_phandle = spapr_irq_get_phandle_dual,
> > +    .sync_to_kvm = spapr_irq_sync_to_kvm_dual,
> >  };
> >  
> >  /*
> > @@ -645,6 +670,11 @@ void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp)
> >      }
> >  }
> >  
> > +void spapr_irq_sync_to_kvm(sPAPRMachineState *spapr, int irq, Error **errp)
> > +{
> > +    spapr->irq->sync_to_kvm(spapr, irq, errp);
> > +}
> > +
> >  /*
> >   * XICS legacy routines - to deprecate one day
> >   */
> > @@ -717,4 +747,5 @@ sPAPRIrq spapr_irq_xics_legacy = {
> >      .post_load   = spapr_irq_post_load_xics,
> >      .set_irq     = spapr_irq_set_irq_xics,
> >      .get_phandle = spapr_get_phandle_xics,
> > +    .sync_to_kvm = spapr_irq_sync_to_kvm_xics,
> >  };
> > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> > index 990743a23582..9c111f3211b3 100644
> > --- a/include/hw/ppc/spapr_irq.h
> > +++ b/include/hw/ppc/spapr_irq.h
> > @@ -48,6 +48,7 @@ typedef struct sPAPRIrq {
> >      void (*reset)(sPAPRMachineState *spapr, Error **errp);
> >      void (*set_irq)(void *opaque, int srcno, int val);
> >      uint32_t (*get_phandle)(sPAPRMachineState *spapr, void *fdt, Error **errp);
> > +    void (*sync_to_kvm)(sPAPRMachineState *spapr, int irq, Error **errp);
> >  } sPAPRIrq;
> >  
> >  extern sPAPRIrq spapr_irq_xics;
> > @@ -61,6 +62,7 @@ void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
> >  qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
> >  int spapr_irq_post_load(sPAPRMachineState *spapr, int version_id);
> >  void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp);
> > +void spapr_irq_sync_to_kvm(sPAPRMachineState *spapr, int irq, Error **errp);
> >  
> >  /*
> >   * XICS legacy routines
> > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> > index fad786e8b22d..52de166a2982 100644
> > --- a/include/hw/ppc/xics.h
> > +++ b/include/hw/ppc/xics.h
> > @@ -203,4 +203,6 @@ void icp_resend(ICPState *ss);
> >  Object *icp_create(Object *cpu, const char *type, XICSFabric *xi,
> >                     Error **errp);
> >  
> > +int ics_set_kvm_state_one(ICSState *ics, unsigned srcno, Error **errp);
> > +
> >  #endif /* XICS_H */
> >   
> 

Re: [Qemu-devel] [PATCH v2 11/13] spapr_irq: Allow synchronization of a single irq state to KVM

[Cédric Le Goater]

On 1/14/19 4:10 PM, Greg Kurz wrote:
> On Mon, 14 Jan 2019 09:19:50 +0100
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> On 1/11/19 10:04 PM, Greg Kurz wrote:
>>> When using the in-kernel interrupt controller, the state of all irqs is
>>> synchronized to KVM at machine reset time. In the case of PHB hotplug, we
>>> will need to synchronize LSIs manually.  
>>
>> Yes. This is because the interrupt sources in the KVM XICS device have 
>> already been initialized as MSIs. 
>>
>> Can not we reset the source when it is claimed ? 
>>
>> An alternative solution would be to initialize the SPAPR_IRQ_PCI_LSI range 
>> as LSIs at a KVM level.
>>
> 
> I don't really want to add some dependency to sPAPR code in KVM XICS... what
> about claiming LSIs for all possible PHBs at machine init time ?

you can not claim IRQ numbers twice. So if a new PHB is hot-plugged, 
it will fail the claim its LSIs.

C.  

Re: [Qemu-devel] [PATCH v2 11/13] spapr_irq: Allow synchronization of a single irq state to KVM

[Greg Kurz]

On Mon, 14 Jan 2019 17:29:48 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 1/14/19 4:10 PM, Greg Kurz wrote:
> > On Mon, 14 Jan 2019 09:19:50 +0100
> > Cédric Le Goater <clg@kaod.org> wrote:
> >   
> >> On 1/11/19 10:04 PM, Greg Kurz wrote:  
> >>> When using the in-kernel interrupt controller, the state of all irqs is
> >>> synchronized to KVM at machine reset time. In the case of PHB hotplug, we
> >>> will need to synchronize LSIs manually.    
> >>
> >> Yes. This is because the interrupt sources in the KVM XICS device have 
> >> already been initialized as MSIs. 
> >>
> >> Can not we reset the source when it is claimed ? 
> >>
> >> An alternative solution would be to initialize the SPAPR_IRQ_PCI_LSI range 
> >> as LSIs at a KVM level.
> >>  
> > 
> > I don't really want to add some dependency to sPAPR code in KVM XICS... what
> > about claiming LSIs for all possible PHBs at machine init time ?  
> 
> you can not claim IRQ numbers twice. So if a new PHB is hot-plugged, 
> it will fail the claim its LSIs.
> 

The intent is to claim all these LSIs for the lifetime of the machine, instead
of having each PHB to claim its own ones during realize.

> C.  

Re: [Qemu-devel] [PATCH v2 00/13] spapr: Add support for PHB hotplug

[Michael S. Tsirkin]

On Fri, Jan 11, 2019 at 09:59:55PM +0100, Greg Kurz wrote:
> This allows to hotplug/unplug PHBs. I could successfully test:
> - hotplug/unplug with e1000 device to validate LSIs
> - hotplug/unplug with virtio-net device to validate MSIs
> - some simple migration scenarios
> 
> Changes in v2:
> - rebased on current ppc-for-4.0
> - added some preliminary cleanup
> - call unrealize from realize error path
> - advertise PHB hotplug in last patch
> - reworked phandle related code
> - sync LSIs to KVM
> 
> Please comment.

PCI things all look fine:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Pls merge through the sPAPR tree.


> --
> Greg
> 
> ---
> 
> Greg Kurz (5):
>       spapr: Rename xics to intc in interrupt controller agnostic code
>       spapr_pci: add PHB unrealize
>       spapr_irq: Expose the phandle of the interrupt controller
>       spapr_irq: Allow synchronization of a single irq state to KVM
>       spapr: add hotplug hooks for PHB hotplug
> 
> Michael Roth (6):
>       spapr: create DR connectors for PHBs
>       spapr_events: add support for phb hotplug events
>       qdev: pass an Object * to qbus_set_hotplug_handler()
>       spapr_pci: provide node start offset via spapr_populate_pci_dt()
>       spapr_pci: add ibm, my-drc-index property for PHB hotplug
>       spapr: enable PHB hotplug for default pseries machine type
> 
> Nathan Fontenot (1):
>       spapr: populate PHB DRC entries for root DT node
> 
> Thomas Huth (1):
>       ppc: Move spapr-related prototypes from xics.h into a seperate header file
> 
> 
>  hw/acpi/pcihp.c               |    2 -
>  hw/acpi/piix4.c               |    2 -
>  hw/char/virtio-serial-bus.c   |    2 -
>  hw/core/bus.c                 |   11 +--
>  hw/intc/spapr_xive.c          |   34 ++++++++-
>  hw/intc/xics_kvm.c            |   68 ++++++++++--------
>  hw/intc/xics_spapr.c          |   29 +++++++-
>  hw/pci/pcie.c                 |    2 -
>  hw/pci/shpc.c                 |    2 -
>  hw/ppc/spapr.c                |  156 ++++++++++++++++++++++++++++++++++++++++-
>  hw/ppc/spapr_drc.c            |   18 +++++
>  hw/ppc/spapr_events.c         |    5 +
>  hw/ppc/spapr_irq.c            |   45 ++++++++++++
>  hw/ppc/spapr_pci.c            |  122 +++++++++++++++++++++++++-------
>  hw/ppc/spapr_vio.c            |    2 -
>  hw/s390x/css-bridge.c         |    2 -
>  hw/s390x/s390-pci-bus.c       |    6 +-
>  hw/scsi/virtio-scsi.c         |    2 -
>  hw/scsi/vmw_pvscsi.c          |    2 -
>  hw/usb/dev-smartcard-reader.c |    2 -
>  include/hw/pci-host/spapr.h   |    8 ++
>  include/hw/ppc/spapr.h        |    4 +
>  include/hw/ppc/spapr_drc.h    |    8 ++
>  include/hw/ppc/spapr_irq.h    |    3 +
>  include/hw/ppc/spapr_xive.h   |    2 +
>  include/hw/ppc/xics.h         |    9 +-
>  include/hw/ppc/xics_spapr.h   |   39 ++++++++++
>  include/hw/qdev-core.h        |    3 -
>  28 files changed, 492 insertions(+), 98 deletions(-)
>  create mode 100644 include/hw/ppc/xics_spapr.h

Re: [Qemu-devel] [PATCH v2 07/13] qdev: pass an Object * to qbus_set_hotplug_handler()

[Halil Pasic]

On Fri, 11 Jan 2019 22:02:50 +0100
Greg Kurz <groug@kaod.org> wrote:

> From: Michael Roth <mdroth@linux.vnet.ibm.com>
> 
> Certain devices types, like memory/CPU, are now being handled using a
> hotplug interface provided by a top-level MachineClass. Hotpluggable
> host bridges are another such device where it makes sense to use a
> machine-level hotplug handler. However, unlike those devices,
> host-bridges have a parent bus (the main system bus), and devices with
> a parent bus use a different mechanism for registering their hotplug
> handlers: qbus_set_hotplug_handler(). This interface currently expects
> a handler to be a subclass of DeviceClass, but this is not the case
> for MachineClass, which derives directly from ObjectClass.
> 
> Internally, the interface only requires an ObjectClass, so expose that
> in qbus_set_hotplug_handler().
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Acked-by: Halil Pasic <pasic@linux.ibm.com>

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 00/13] spapr: Add support for PHB hotplug

[Greg Kurz]

On Fri, 11 Jan 2019 21:59:55 +0100
Greg Kurz <groug@kaod.org> wrote:

> This allows to hotplug/unplug PHBs. I could successfully test:
> - hotplug/unplug with e1000 device to validate LSIs
> - hotplug/unplug with virtio-net device to validate MSIs
> - some simple migration scenarios
> 
> Changes in v2:
> - rebased on current ppc-for-4.0
> - added some preliminary cleanup
> - call unrealize from realize error path
> - advertise PHB hotplug in last patch
> - reworked phandle related code
> - sync LSIs to KVM
> 
> Please comment.
> 

David,

I had enough remarks from Cedric to go for a v3. I expect to send
it shortly. Don't spend time on reviewing the v2.

Cheers,

--
Greg

> --
> Greg
> 
> ---
> 
> Greg Kurz (5):
>       spapr: Rename xics to intc in interrupt controller agnostic code
>       spapr_pci: add PHB unrealize
>       spapr_irq: Expose the phandle of the interrupt controller
>       spapr_irq: Allow synchronization of a single irq state to KVM
>       spapr: add hotplug hooks for PHB hotplug
> 
> Michael Roth (6):
>       spapr: create DR connectors for PHBs
>       spapr_events: add support for phb hotplug events
>       qdev: pass an Object * to qbus_set_hotplug_handler()
>       spapr_pci: provide node start offset via spapr_populate_pci_dt()
>       spapr_pci: add ibm, my-drc-index property for PHB hotplug
>       spapr: enable PHB hotplug for default pseries machine type
> 
> Nathan Fontenot (1):
>       spapr: populate PHB DRC entries for root DT node
> 
> Thomas Huth (1):
>       ppc: Move spapr-related prototypes from xics.h into a seperate header file
> 
> 
>  hw/acpi/pcihp.c               |    2 -
>  hw/acpi/piix4.c               |    2 -
>  hw/char/virtio-serial-bus.c   |    2 -
>  hw/core/bus.c                 |   11 +--
>  hw/intc/spapr_xive.c          |   34 ++++++++-
>  hw/intc/xics_kvm.c            |   68 ++++++++++--------
>  hw/intc/xics_spapr.c          |   29 +++++++-
>  hw/pci/pcie.c                 |    2 -
>  hw/pci/shpc.c                 |    2 -
>  hw/ppc/spapr.c                |  156 ++++++++++++++++++++++++++++++++++++++++-
>  hw/ppc/spapr_drc.c            |   18 +++++
>  hw/ppc/spapr_events.c         |    5 +
>  hw/ppc/spapr_irq.c            |   45 ++++++++++++
>  hw/ppc/spapr_pci.c            |  122 +++++++++++++++++++++++++-------
>  hw/ppc/spapr_vio.c            |    2 -
>  hw/s390x/css-bridge.c         |    2 -
>  hw/s390x/s390-pci-bus.c       |    6 +-
>  hw/scsi/virtio-scsi.c         |    2 -
>  hw/scsi/vmw_pvscsi.c          |    2 -
>  hw/usb/dev-smartcard-reader.c |    2 -
>  include/hw/pci-host/spapr.h   |    8 ++
>  include/hw/ppc/spapr.h        |    4 +
>  include/hw/ppc/spapr_drc.h    |    8 ++
>  include/hw/ppc/spapr_irq.h    |    3 +
>  include/hw/ppc/spapr_xive.h   |    2 +
>  include/hw/ppc/xics.h         |    9 +-
>  include/hw/ppc/xics_spapr.h   |   39 ++++++++++
>  include/hw/qdev-core.h        |    3 -
>  28 files changed, 492 insertions(+), 98 deletions(-)
>  create mode 100644 include/hw/ppc/xics_spapr.h
> 
>