[PATCH for-5.0 0/8] ppc: Consolidate QOM links and pointers to the same object

[PATCH for-5.0 0/8] ppc: Consolidate QOM links and pointers to the same object

[Greg Kurz]

There's a recurring pattern in the code where a const link is added to a
newly instanciated object and the link is then used in the object's realize
function to keep a pointer to the QOM entity which the link points to.

void create_obj_B(Object *obj_a)
{
    Object *obj_b;

    obj_b = object_new(TYPE_B);
    object_property_add_const_link(obj_b, "link-to-a", obj_a, &error_abort);
}

void object_B_realize(DeviceState *dev, Error **errp)
{
    Object *obj_a;

    obj_a = object_property_get_link(OBJECT(dev), "link-to-a", errp);
    if (!obj_a) {
        return;
    }

    obj_b->obj_a = A(obj_a); // If obj_b->obj_a is changed, the link property
                             // still points to the original obj_a that was
                             // passed to object_property_add_const_link()
}

Confusing bugs could arise if the pointer and the link go out of sync for
some reason. This can be avoided if the property is defined to directly use
the pointer with object_property_add_link() and object_property_set_link().

This series just does that for all occurences of the fragile pattern in
the XIVE and PNV code.

--
Greg

---

Greg Kurz (8):
      xive: Link "cpu" property to XiveTCTX::cs pointer
      xive: Link "xive" property to XiveSource::xive pointer
      xive: Link "xive" property to XiveEndSource::xrtr pointer
      ppc/pnv: Link "psi" property to PnvLpc::psi pointer
      ppc/pnv: Link "psi" property to PnvOCC::psi pointer
      ppc/pnv: Link "chip" property to PnvHomer::chip pointer
      ppc/pnv: Link "chip" property to PnvCore::chip pointer
      ppc/pnv: Link "chip" property to PnvXive::chip pointer


 hw/intc/pnv_xive.c        |   29 +++++++++-----------
 hw/intc/spapr_xive.c      |    8 +++---
 hw/intc/xive.c            |   65 ++++++++++++++++++++++-----------------------
 hw/ppc/pnv.c              |   32 +++++++++++-----------
 hw/ppc/pnv_core.c         |   18 +++++++-----
 hw/ppc/pnv_homer.c        |   24 +++++++++--------
 hw/ppc/pnv_lpc.c          |   23 ++++++++--------
 hw/ppc/pnv_occ.c          |   23 ++++++++--------
 hw/ppc/pnv_psi.c          |    3 +-
 include/hw/ppc/pnv.h      |    2 +
 include/hw/ppc/pnv_core.h |    2 +
 11 files changed, 115 insertions(+), 114 deletions(-)

[PATCH for-5.0 1/8] xive: Link "cpu" property to XiveTCTX::cs pointer

[Greg Kurz]

The TCTX object has both a pointer and a "cpu" property pointing to the
vCPU object. Confusing bugs could arise if these ever go out of sync.

Change the property definition so that it explicitely sets the pointer.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/xive.c |   25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 75dce82fb205..5363fcaa416c 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -580,19 +580,11 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp)
     XiveTCTX *tctx = XIVE_TCTX(dev);
     PowerPCCPU *cpu;
     CPUPPCState *env;
-    Object *obj;
     Error *local_err = NULL;
 
-    obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
-    if (!obj) {
-        error_propagate(errp, local_err);
-        error_prepend(errp, "required link 'cpu' not found: ");
-        return;
-    }
-
-    cpu = POWERPC_CPU(obj);
-    tctx->cs = CPU(obj);
+    assert(tctx->cs);
 
+    cpu = POWERPC_CPU(tctx->cs);
     env = &cpu->env;
     switch (PPC_INPUT(env)) {
     case PPC_FLAGS_INPUT_POWER9:
@@ -676,10 +668,19 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data)
     dc->user_creatable = false;
 }
 
+static void xive_tctx_instance_init(Object *obj)
+{
+    object_property_add_link(obj, "cpu", TYPE_CPU,
+                             (Object **) &XIVE_TCTX(obj)->cs,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG, &error_abort);
+}
+
 static const TypeInfo xive_tctx_info = {
     .name          = TYPE_XIVE_TCTX,
     .parent        = TYPE_DEVICE,
     .instance_size = sizeof(XiveTCTX),
+    .instance_init = xive_tctx_instance_init,
     .class_init    = xive_tctx_class_init,
 };
 
@@ -691,8 +692,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
     obj = object_new(TYPE_XIVE_TCTX);
     object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
     object_unref(obj);
-    object_ref(cpu);
-    object_property_add_const_link(obj, "cpu", cpu, &error_abort);
+    object_property_set_link(obj, cpu, "cpu", &error_abort);
     object_property_set_bool(obj, true, "realized", &local_err);
     if (local_err) {
         goto error;
@@ -710,7 +710,6 @@ void xive_tctx_destroy(XiveTCTX *tctx)
 {
     Object *obj = OBJECT(tctx);
 
-    object_unref(object_property_get_link(obj, "cpu", &error_abort));
     object_unparent(obj);
 }
 

[PATCH for-5.0 2/8] xive: Link "xive" property to XiveSource::xive pointer

[Greg Kurz]

The source object has both a pointer and a "xive" property pointing to the
notifier object. Confusing bugs could arise if these ever go out of sync.

Change the property definition so that it explicitely sets the pointer.
The property isn't optional : not being able to set the link is a bug
and QEMU should rather abort than exit in this case.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/pnv_xive.c   |    4 ++--
 hw/intc/spapr_xive.c |    4 ++--
 hw/intc/xive.c       |   20 ++++++++++----------
 hw/ppc/pnv_psi.c     |    3 +--
 4 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 348f2fdd263d..9e23dc705dc3 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1695,8 +1695,8 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
      */
     object_property_set_int(OBJECT(xsrc), PNV_XIVE_NR_IRQS, "nr-irqs",
                             &error_fatal);
-    object_property_add_const_link(OBJECT(xsrc), "xive", OBJECT(xive),
-                                   &error_fatal);
+    object_property_set_link(OBJECT(xsrc), OBJECT(xive), "xive",
+                             &error_abort);
     object_property_set_bool(OBJECT(xsrc), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 9cb8d38a3bab..10890aeeeb5b 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -276,8 +276,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
      */
     object_property_set_int(OBJECT(xsrc), xive->nr_irqs, "nr-irqs",
                             &error_fatal);
-    object_property_add_const_link(OBJECT(xsrc), "xive", OBJECT(xive),
-                                   &error_fatal);
+    object_property_set_link(OBJECT(xsrc), OBJECT(xive), "xive",
+                             &error_abort);
     object_property_set_bool(OBJECT(xsrc), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 5363fcaa416c..b69bceb4d255 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1063,17 +1063,8 @@ static void xive_source_reset(void *dev)
 static void xive_source_realize(DeviceState *dev, Error **errp)
 {
     XiveSource *xsrc = XIVE_SOURCE(dev);
-    Object *obj;
-    Error *local_err = NULL;
-
-    obj = object_property_get_link(OBJECT(dev), "xive", &local_err);
-    if (!obj) {
-        error_propagate(errp, local_err);
-        error_prepend(errp, "required link 'xive' not found: ");
-        return;
-    }
 
-    xsrc->xive = XIVE_NOTIFIER(obj);
+    assert(xsrc->xive);
 
     if (!xsrc->nr_irqs) {
         error_setg(errp, "Number of interrupt needs to be greater than 0");
@@ -1122,6 +1113,14 @@ static Property xive_source_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void xive_source_instance_init(Object *obj)
+{
+    object_property_add_link(obj, "xive", TYPE_XIVE_NOTIFIER,
+                             (Object **) &XIVE_SOURCE(obj)->xive,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG, &error_abort);
+}
+
 static void xive_source_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1141,6 +1140,7 @@ static const TypeInfo xive_source_info = {
     .name          = TYPE_XIVE_SOURCE,
     .parent        = TYPE_DEVICE,
     .instance_size = sizeof(XiveSource),
+    .instance_init = xive_source_instance_init,
     .class_init    = xive_source_class_init,
 };
 
diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index 68d0dfacfe2b..a360515a86f8 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -851,8 +851,7 @@ static void pnv_psi_power9_realize(DeviceState *dev, Error **errp)
                             &error_fatal);
     object_property_set_int(OBJECT(xsrc), PSIHB9_NUM_IRQS, "nr-irqs",
                             &error_fatal);
-    object_property_add_const_link(OBJECT(xsrc), "xive", OBJECT(psi),
-                                   &error_fatal);
+    object_property_set_link(OBJECT(xsrc), OBJECT(psi), "xive", &error_abort);
     object_property_set_bool(OBJECT(xsrc), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);

[PATCH for-5.0 3/8] xive: Link "xive" property to XiveEndSource::xrtr pointer

[Greg Kurz]

The END source object has both a pointer and a "xive" property pointing to
the router object. Confusing bugs could arise if these ever go out of sync.

Change the property definition so that it explicitely sets the pointer.
The property isn't optional : not being able to set the link is a bug
and QEMU should rather abort than exit in this case.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/pnv_xive.c   |    4 ++--
 hw/intc/spapr_xive.c |    4 ++--
 hw/intc/xive.c       |   20 ++++++++++----------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 9e23dc705dc3..6aa7aeed6f83 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1705,8 +1705,8 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
 
     object_property_set_int(OBJECT(end_xsrc), PNV_XIVE_NR_ENDS, "nr-ends",
                             &error_fatal);
-    object_property_add_const_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
-                                   &error_fatal);
+    object_property_set_link(OBJECT(end_xsrc), OBJECT(xive), "xive",
+                             &error_abort);
     object_property_set_bool(OBJECT(end_xsrc), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 10890aeeeb5b..729246e906c9 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -290,8 +290,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
      */
     object_property_set_int(OBJECT(end_xsrc), xive->nr_irqs, "nr-ends",
                             &error_fatal);
-    object_property_add_const_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
-                                   &error_fatal);
+    object_property_set_link(OBJECT(end_xsrc), OBJECT(xive), "xive",
+                             &error_abort);
     object_property_set_bool(OBJECT(end_xsrc), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index b69bceb4d255..a32ce0d5bfc6 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1824,17 +1824,8 @@ static const MemoryRegionOps xive_end_source_ops = {
 static void xive_end_source_realize(DeviceState *dev, Error **errp)
 {
     XiveENDSource *xsrc = XIVE_END_SOURCE(dev);
-    Object *obj;
-    Error *local_err = NULL;
-
-    obj = object_property_get_link(OBJECT(dev), "xive", &local_err);
-    if (!obj) {
-        error_propagate(errp, local_err);
-        error_prepend(errp, "required link 'xive' not found: ");
-        return;
-    }
 
-    xsrc->xrtr = XIVE_ROUTER(obj);
+    assert(xsrc->xrtr);
 
     if (!xsrc->nr_ends) {
         error_setg(errp, "Number of interrupt needs to be greater than 0");
@@ -1863,6 +1854,14 @@ static Property xive_end_source_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void xive_end_source_instance_init(Object *obj)
+{
+    object_property_add_link(obj, "xive", TYPE_XIVE_ROUTER,
+                             (Object **) &XIVE_END_SOURCE(obj)->xrtr,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG, &error_abort);
+}
+
 static void xive_end_source_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1881,6 +1880,7 @@ static const TypeInfo xive_end_source_info = {
     .name          = TYPE_XIVE_END_SOURCE,
     .parent        = TYPE_DEVICE,
     .instance_size = sizeof(XiveENDSource),
+    .instance_init    = xive_end_source_instance_init,
     .class_init    = xive_end_source_class_init,
 };
 

[PATCH for-5.0 4/8] ppc/pnv: Link "psi" property to PnvLpc::psi pointer

[Greg Kurz]

The LPC object has both a pointer and a "psi" property pointing to the
PSI object. Confusing bugs could arise if these ever go out of sync.

Change the property definition so that it explicitely sets the pointer.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/pnv.c     |    8 ++++----
 hw/ppc/pnv_lpc.c |   23 ++++++++++++-----------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index d0c1d4227784..286901746f50 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -898,8 +898,6 @@ static void pnv_chip_power8_instance_init(Object *obj)
 
     object_initialize_child(obj, "lpc",  &chip8->lpc, sizeof(chip8->lpc),
                             TYPE_PNV8_LPC, &error_abort, NULL);
-    object_property_add_const_link(OBJECT(&chip8->lpc), "psi",
-                                   OBJECT(&chip8->psi), &error_abort);
 
     object_initialize_child(obj, "occ",  &chip8->occ, sizeof(chip8->occ),
                             TYPE_PNV8_OCC, &error_abort, NULL);
@@ -978,6 +976,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
                             &PNV_PSI(psi8)->xscom_regs);
 
     /* Create LPC controller */
+    object_property_set_link(OBJECT(&chip8->lpc), OBJECT(&chip8->psi), "psi",
+                             &error_abort);
     object_property_set_bool(OBJECT(&chip8->lpc), true, "realized",
                              &error_fatal);
     pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, &chip8->lpc.xscom_regs);
@@ -1098,8 +1098,6 @@ static void pnv_chip_power9_instance_init(Object *obj)
 
     object_initialize_child(obj, "lpc",  &chip9->lpc, sizeof(chip9->lpc),
                             TYPE_PNV9_LPC, &error_abort, NULL);
-    object_property_add_const_link(OBJECT(&chip9->lpc), "psi",
-                                   OBJECT(&chip9->psi), &error_abort);
 
     object_initialize_child(obj, "occ",  &chip9->occ, sizeof(chip9->occ),
                             TYPE_PNV9_OCC, &error_abort, NULL);
@@ -1198,6 +1196,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
                             &PNV_PSI(psi9)->xscom_regs);
 
     /* LPC */
+    object_property_set_link(OBJECT(&chip9->lpc), OBJECT(&chip9->psi), "psi",
+                             &error_abort);
     object_property_set_bool(OBJECT(&chip9->lpc), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index 9466d4a1be3b..f0040a26cfd0 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -24,7 +24,7 @@
 #include "qemu/module.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
-
+#include "hw/qdev-properties.h"
 #include "hw/ppc/pnv.h"
 #include "hw/ppc/pnv_lpc.h"
 #include "hw/ppc/pnv_xscom.h"
@@ -682,17 +682,8 @@ static const TypeInfo pnv_lpc_power9_info = {
 static void pnv_lpc_realize(DeviceState *dev, Error **errp)
 {
     PnvLpcController *lpc = PNV_LPC(dev);
-    Object *obj;
-    Error *local_err = NULL;
 
-    obj = object_property_get_link(OBJECT(dev), "psi", &local_err);
-    if (!obj) {
-        error_propagate(errp, local_err);
-        error_prepend(errp, "required link 'psi' not found: ");
-        return;
-    }
-    /* The LPC controller needs PSI to generate interrupts  */
-    lpc->psi = PNV_PSI(obj);
+    assert(lpc->psi);
 
     /* Reg inits */
     lpc->lpc_hc_fw_rd_acc_size = LPC_HC_FW_RD_4B;
@@ -734,6 +725,15 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
                                 &lpc->lpc_hc_regs);
 }
 
+static void pnv_lpc_instance_init(Object *obj)
+{
+    /* The LPC controller needs PSI to generate interrupts  */
+    object_property_add_link(obj, "psi", TYPE_PNV_PSI,
+                             (Object **) &PNV_LPC(obj)->psi,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG, &error_abort);
+}
+
 static void pnv_lpc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -745,6 +745,7 @@ static void pnv_lpc_class_init(ObjectClass *klass, void *data)
 static const TypeInfo pnv_lpc_info = {
     .name          = TYPE_PNV_LPC,
     .parent        = TYPE_DEVICE,
+    .instance_init = pnv_lpc_instance_init,
     .class_init    = pnv_lpc_class_init,
     .class_size    = sizeof(PnvLpcClass),
     .abstract      = true,

[PATCH for-5.0 5/8] ppc/pnv: Link "psi" property to PnvOCC::psi pointer

[Greg Kurz]

The OCC object has both a pointer and a "psi" property pointing to the
PSI object. Confusing bugs could arise if these ever go out of sync.

Change the property definition so that it explicitely sets the pointer.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/pnv.c     |    8 ++++----
 hw/ppc/pnv_occ.c |   23 ++++++++++++-----------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 286901746f50..96c5a23cd1eb 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -901,8 +901,6 @@ static void pnv_chip_power8_instance_init(Object *obj)
 
     object_initialize_child(obj, "occ",  &chip8->occ, sizeof(chip8->occ),
                             TYPE_PNV8_OCC, &error_abort, NULL);
-    object_property_add_const_link(OBJECT(&chip8->occ), "psi",
-                                   OBJECT(&chip8->psi), &error_abort);
 
     object_initialize_child(obj, "homer",  &chip8->homer, sizeof(chip8->homer),
                             TYPE_PNV8_HOMER, &error_abort, NULL);
@@ -997,6 +995,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
     }
 
     /* Create the simplified OCC model */
+    object_property_set_link(OBJECT(&chip8->occ), OBJECT(&chip8->psi), "psi",
+                             &error_abort);
     object_property_set_bool(OBJECT(&chip8->occ), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -1101,8 +1101,6 @@ static void pnv_chip_power9_instance_init(Object *obj)
 
     object_initialize_child(obj, "occ",  &chip9->occ, sizeof(chip9->occ),
                             TYPE_PNV9_OCC, &error_abort, NULL);
-    object_property_add_const_link(OBJECT(&chip9->occ), "psi",
-                                   OBJECT(&chip9->psi), &error_abort);
 
     object_initialize_child(obj, "homer",  &chip9->homer, sizeof(chip9->homer),
                             TYPE_PNV9_HOMER, &error_abort, NULL);
@@ -1210,6 +1208,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
                                             (uint64_t) PNV9_LPCM_BASE(chip));
 
     /* Create the simplified OCC model */
+    object_property_set_link(OBJECT(&chip9->occ), OBJECT(&chip9->psi), "psi",
+                             &error_abort);
     object_property_set_bool(OBJECT(&chip9->occ), true, "realized", &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/ppc/pnv_occ.c b/hw/ppc/pnv_occ.c
index 785653bb6710..084d580c1565 100644
--- a/hw/ppc/pnv_occ.c
+++ b/hw/ppc/pnv_occ.c
@@ -21,7 +21,7 @@
 #include "qapi/error.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
-
+#include "hw/qdev-properties.h"
 #include "hw/ppc/pnv.h"
 #include "hw/ppc/pnv_xscom.h"
 #include "hw/ppc/pnv_occ.h"
@@ -257,18 +257,10 @@ static void pnv_occ_realize(DeviceState *dev, Error **errp)
 {
     PnvOCC *occ = PNV_OCC(dev);
     PnvOCCClass *poc = PNV_OCC_GET_CLASS(occ);
-    Object *obj;
-    Error *local_err = NULL;
 
-    occ->occmisc = 0;
+    assert(occ->psi);
 
-    obj = object_property_get_link(OBJECT(dev), "psi", &local_err);
-    if (!obj) {
-        error_propagate(errp, local_err);
-        error_prepend(errp, "required link 'psi' not found: ");
-        return;
-    }
-    occ->psi = PNV_PSI(obj);
+    occ->occmisc = 0;
 
     /* XScom region for OCC registers */
     pnv_xscom_region_init(&occ->xscom_regs, OBJECT(dev), poc->xscom_ops,
@@ -279,6 +271,14 @@ static void pnv_occ_realize(DeviceState *dev, Error **errp)
                           occ, "occ-common-area", poc->sram_size);
 }
 
+static void pnv_occ_instance_init(Object *obj)
+{
+    object_property_add_link(obj, "psi", TYPE_PNV_PSI,
+                             (Object **) &PNV_OCC(obj)->psi,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG, &error_abort);
+}
+
 static void pnv_occ_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -291,6 +291,7 @@ static const TypeInfo pnv_occ_type_info = {
     .name          = TYPE_PNV_OCC,
     .parent        = TYPE_DEVICE,
     .instance_size = sizeof(PnvOCC),
+    .instance_init = pnv_occ_instance_init,
     .class_init    = pnv_occ_class_init,
     .class_size    = sizeof(PnvOCCClass),
     .abstract      = true,

[PATCH for-5.0 6/8] ppc/pnv: Link "chip" property to PnvHomer::chip pointer

[Greg Kurz]

The homer object has both a pointer and a "chip" property pointing to the
chip object. Confusing bugs could arise if these ever go out of sync.

Change the property definition so that it explicitely sets the pointer.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/pnv.c       |    8 ++++----
 hw/ppc/pnv_homer.c |   24 +++++++++++++-----------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 96c5a23cd1eb..232c817452c0 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -904,8 +904,6 @@ static void pnv_chip_power8_instance_init(Object *obj)
 
     object_initialize_child(obj, "homer",  &chip8->homer, sizeof(chip8->homer),
                             TYPE_PNV8_HOMER, &error_abort, NULL);
-    object_property_add_const_link(OBJECT(&chip8->homer), "chip", obj,
-                                   &error_abort);
 }
 
 static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
@@ -1009,6 +1007,8 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
                                 &chip8->occ.sram_regs);
 
     /* HOMER */
+    object_property_set_link(OBJECT(&chip8->homer), OBJECT(chip), "chip",
+                             &error_abort);
     object_property_set_bool(OBJECT(&chip8->homer), true, "realized",
                              &local_err);
     if (local_err) {
@@ -1104,8 +1104,6 @@ static void pnv_chip_power9_instance_init(Object *obj)
 
     object_initialize_child(obj, "homer",  &chip9->homer, sizeof(chip9->homer),
                             TYPE_PNV9_HOMER, &error_abort, NULL);
-    object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj,
-                                   &error_abort);
 }
 
 static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)
@@ -1222,6 +1220,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
                                 &chip9->occ.sram_regs);
 
     /* HOMER */
+    object_property_set_link(OBJECT(&chip9->homer), OBJECT(chip), "chip",
+                             &error_abort);
     object_property_set_bool(OBJECT(&chip9->homer), true, "realized",
                              &local_err);
     if (local_err) {
diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
index cc881a3b3289..cc08c1e34e2e 100644
--- a/hw/ppc/pnv_homer.c
+++ b/hw/ppc/pnv_homer.c
@@ -22,10 +22,10 @@
 #include "exec/memory.h"
 #include "sysemu/cpus.h"
 #include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
 #include "hw/ppc/pnv.h"
 #include "hw/ppc/pnv_homer.h"
 
-
 static bool core_max_array(PnvHomer *homer, hwaddr addr)
 {
     int i;
@@ -229,22 +229,23 @@ static void pnv_homer_realize(DeviceState *dev, Error **errp)
 {
     PnvHomer *homer = PNV_HOMER(dev);
     PnvHomerClass *hmrc = PNV_HOMER_GET_CLASS(homer);
-    Object *obj;
-    Error *local_err = NULL;
-
-    obj = object_property_get_link(OBJECT(dev), "chip", &local_err);
-    if (!obj) {
-        error_propagate(errp, local_err);
-        error_prepend(errp, "required link 'chip' not found: ");
-        return;
-    }
-    homer->chip = PNV_CHIP(obj);
+
+    assert(homer->chip);
+
     /* homer region */
     memory_region_init_io(&homer->regs, OBJECT(dev),
                           hmrc->homer_ops, homer, "homer-main-memory",
                           hmrc->homer_size);
 }
 
+static void pnv_homer_instance_init(Object *obj)
+{
+    object_property_add_link(obj, "chip", TYPE_PNV_CHIP,
+                             (Object **) &PNV_HOMER(obj)->chip,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG, &error_abort);
+}
+
 static void pnv_homer_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -258,6 +259,7 @@ static const TypeInfo pnv_homer_type_info = {
     .parent        = TYPE_DEVICE,
     .instance_size = sizeof(PnvHomer),
     .class_init    = pnv_homer_class_init,
+    .instance_init = pnv_homer_instance_init,
     .class_size    = sizeof(PnvHomerClass),
     .abstract      = true,
 };

[PATCH for-5.0 7/8] ppc/pnv: Link "chip" property to PnvCore::chip pointer

[Greg Kurz]

The core object has both a pointer and a "chip" property pointing to the
chip object. Confusing bugs could arise if these ever go out of sync.

Change the property definition so that it explicitely sets the pointer.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/pnv.c              |    4 ++--
 hw/ppc/pnv_core.c         |   18 ++++++++++--------
 include/hw/ppc/pnv_core.h |    2 +-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 232c817452c0..8851875bcfd7 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1326,8 +1326,8 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
         object_property_set_int(OBJECT(pnv_core),
                                 pcc->core_pir(chip, core_hwid),
                                 "pir", &error_fatal);
-        object_property_add_const_link(OBJECT(pnv_core), "chip",
-                                       OBJECT(chip), &error_fatal);
+        object_property_set_link(OBJECT(pnv_core), OBJECT(chip), "chip",
+                                 &error_abort);
         object_property_set_bool(OBJECT(pnv_core), true, "realized",
                                  &error_fatal);
 
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 61b3d3ce2250..56d185f6290d 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -217,15 +217,8 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
     void *obj;
     int i, j;
     char name[32];
-    Object *chip;
 
-    chip = object_property_get_link(OBJECT(dev), "chip", &local_err);
-    if (!chip) {
-        error_propagate_prepend(errp, local_err,
-                                "required link 'chip' not found: ");
-        return;
-    }
-    pc->chip = PNV_CHIP(chip);
+    assert(pc->chip);
 
     pc->threads = g_new(PowerPCCPU *, cc->nr_threads);
     for (i = 0; i < cc->nr_threads; i++) {
@@ -323,6 +316,14 @@ static void pnv_core_class_init(ObjectClass *oc, void *data)
     dc->props = pnv_core_properties;
 }
 
+static void pnv_core_instance_init(Object *obj)
+{
+    object_property_add_link(obj, "chip", TYPE_PNV_CHIP,
+                             (Object **) &PNV_CORE(obj)->chip,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG, &error_abort);
+}
+
 #define DEFINE_PNV_CORE_TYPE(family, cpu_model) \
     {                                           \
         .parent = TYPE_PNV_CORE,                \
@@ -335,6 +336,7 @@ static const TypeInfo pnv_core_infos[] = {
         .name           = TYPE_PNV_CORE,
         .parent         = TYPE_CPU_CORE,
         .instance_size  = sizeof(PnvCore),
+        .instance_init  = pnv_core_instance_init,
         .class_size     = sizeof(PnvCoreClass),
         .class_init = pnv_core_class_init,
         .abstract       = true,
diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index 55eee95104da..fce6d8d9b31b 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -36,11 +36,11 @@ typedef struct PnvChip PnvChip;
 typedef struct PnvCore {
     /*< private >*/
     CPUCore parent_obj;
+    PnvChip *chip;
 
     /*< public >*/
     PowerPCCPU **threads;
     uint32_t pir;
-    PnvChip *chip;
 
     MemoryRegion xscom_regs;
 } PnvCore;

Re: [PATCH for-5.0 7/8] ppc/pnv: Link "chip" property to PnvCore::chip pointer

[Cédric Le Goater]

> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> index 55eee95104da..fce6d8d9b31b 100644
> --- a/include/hw/ppc/pnv_core.h
> +++ b/include/hw/ppc/pnv_core.h
> @@ -36,11 +36,11 @@ typedef struct PnvChip PnvChip;
>  typedef struct PnvCore {
>      /*< private >*/
>      CPUCore parent_obj;
> +    PnvChip *chip;
>  
>      /*< public >*/
>      PowerPCCPU **threads;
>      uint32_t pir;
> -    PnvChip *chip;
>  
>      MemoryRegion xscom_regs;
>  } PnvCore;
> 


Why this change ? 

C.

Re: [PATCH for-5.0 7/8] ppc/pnv: Link "chip" property to PnvCore::chip pointer

[Greg Kurz]

On Fri, 15 Nov 2019 13:24:50 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> > diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> > index 55eee95104da..fce6d8d9b31b 100644
> > --- a/include/hw/ppc/pnv_core.h
> > +++ b/include/hw/ppc/pnv_core.h
> > @@ -36,11 +36,11 @@ typedef struct PnvChip PnvChip;
> >  typedef struct PnvCore {
> >      /*< private >*/
> >      CPUCore parent_obj;
> > +    PnvChip *chip;
> >  
> >      /*< public >*/
> >      PowerPCCPU **threads;
> >      uint32_t pir;
> > -    PnvChip *chip;
> >  
> >      MemoryRegion xscom_regs;
> >  } PnvCore;
> > 
> 
> 
> Why this change ? 
> 

Because PnvCore::chip is an PnvCore internal that shouldn't be
used outside pnv_core.c IMHO.

> C.

[PATCH for-5.0 8/8] ppc/pnv: Link "chip" property to PnvXive::chip pointer

[Greg Kurz]

The XIVE object has both a pointer and a "chip" property pointing to the
chip object. Confusing bugs could arise if these ever go out of sync.

Change the property definition so that it explicitely sets the pointer.

While here, rename pnv_xive_init() to pnv_xive_instance_init() for
clarity.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/pnv_xive.c   |   21 +++++++++------------
 hw/ppc/pnv.c         |    4 ++--
 include/hw/ppc/pnv.h |    2 +-
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 6aa7aeed6f83..158d16b328e3 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1651,7 +1651,7 @@ static void pnv_xive_reset(void *dev)
     }
 }
 
-static void pnv_xive_init(Object *obj)
+static void pnv_xive_instance_init(Object *obj)
 {
     PnvXive *xive = PNV_XIVE(obj);
 
@@ -1661,6 +1661,12 @@ static void pnv_xive_init(Object *obj)
     object_initialize_child(obj, "end_source", &xive->end_source,
                             sizeof(xive->end_source), TYPE_XIVE_END_SOURCE,
                             &error_abort, NULL);
+
+    /* The PnvChip id identifies the XIVE interrupt controller. */
+    object_property_add_link(obj, "chip", TYPE_PNV_CHIP,
+                             (Object **) &PNV_XIVE(obj)->chip,
+                             qdev_prop_allow_set_link_before_realize,
+                             OBJ_PROP_LINK_STRONG, &error_abort);
 }
 
 /*
@@ -1675,17 +1681,8 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
     XiveSource *xsrc = &xive->ipi_source;
     XiveENDSource *end_xsrc = &xive->end_source;
     Error *local_err = NULL;
-    Object *obj;
 
-    obj = object_property_get_link(OBJECT(dev), "chip", &local_err);
-    if (!obj) {
-        error_propagate(errp, local_err);
-        error_prepend(errp, "required link 'chip' not found: ");
-        return;
-    }
-
-    /* The PnvChip id identifies the XIVE interrupt controller. */
-    xive->chip = PNV_CHIP(obj);
+    assert(xive->chip);
 
     /*
      * The XiveSource and XiveENDSource objects are realized with the
@@ -1829,7 +1826,7 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
 static const TypeInfo pnv_xive_info = {
     .name          = TYPE_PNV_XIVE,
     .parent        = TYPE_XIVE_ROUTER,
-    .instance_init = pnv_xive_init,
+    .instance_init = pnv_xive_instance_init,
     .instance_size = sizeof(PnvXive),
     .class_init    = pnv_xive_class_init,
     .interfaces    = (InterfaceInfo[]) {
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 8851875bcfd7..d7130c3304f0 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1088,8 +1088,6 @@ static void pnv_chip_power9_instance_init(Object *obj)
 
     object_initialize_child(obj, "xive", &chip9->xive, sizeof(chip9->xive),
                             TYPE_PNV_XIVE, &error_abort, NULL);
-    object_property_add_const_link(OBJECT(&chip9->xive), "chip", obj,
-                                   &error_abort);
 
     object_initialize_child(obj, "psi",  &chip9->psi, sizeof(chip9->psi),
                             TYPE_PNV9_PSI, &error_abort, NULL);
@@ -1171,6 +1169,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
                             "pc-bar", &error_fatal);
     object_property_set_int(OBJECT(&chip9->xive), PNV9_XIVE_TM_BASE(chip),
                             "tm-bar", &error_fatal);
+    object_property_set_link(OBJECT(&chip9->xive), OBJECT(chip), "chip",
+                             &error_abort);
     object_property_set_bool(OBJECT(&chip9->xive), true, "realized",
                              &local_err);
     if (local_err) {
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 5ecd3ba6ed24..d82484ecf669 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -87,9 +87,9 @@ typedef struct Pnv8Chip {
 typedef struct Pnv9Chip {
     /*< private >*/
     PnvChip      parent_obj;
+    PnvXive      xive;
 
     /*< public >*/
-    PnvXive      xive;
     Pnv9Psi      psi;
     PnvLpcController lpc;
     PnvOCC       occ;

Re: [PATCH for-5.0 8/8] ppc/pnv: Link "chip" property to PnvXive::chip pointer

[Cédric Le Goater]

> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 5ecd3ba6ed24..d82484ecf669 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -87,9 +87,9 @@ typedef struct Pnv8Chip {
>  typedef struct Pnv9Chip {
>      /*< private >*/
>      PnvChip      parent_obj;
> +    PnvXive      xive;
>  
>      /*< public >*/
> -    PnvXive      xive;
>      Pnv9Psi      psi;
>      PnvLpcController lpc;
>      PnvOCC       occ;
> 

Why this change ? 

C.

Re: [PATCH for-5.0 8/8] ppc/pnv: Link "chip" property to PnvXive::chip pointer

[Greg Kurz]

On Fri, 15 Nov 2019 13:25:01 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > index 5ecd3ba6ed24..d82484ecf669 100644
> > --- a/include/hw/ppc/pnv.h
> > +++ b/include/hw/ppc/pnv.h
> > @@ -87,9 +87,9 @@ typedef struct Pnv8Chip {
> >  typedef struct Pnv9Chip {
> >      /*< private >*/
> >      PnvChip      parent_obj;
> > +    PnvXive      xive;
> >  
> >      /*< public >*/
> > -    PnvXive      xive;
> >      Pnv9Psi      psi;
> >      PnvLpcController lpc;
> >      PnvOCC       occ;
> > 
> 
> Why this change ? 
> 

Same reason as in patch 7/8.

> C.

Re: [PATCH for-5.0 0/8] ppc: Consolidate QOM links and pointers to the same object

[Cédric Le Goater]

On 15/11/2019 12:53, Greg Kurz wrote:
> There's a recurring pattern in the code where a const link is added to a
> newly instanciated object and the link is then used in the object's realize
> function to keep a pointer to the QOM entity which the link points to.
> 
> void create_obj_B(Object *obj_a)
> {
>     Object *obj_b;
> 
>     obj_b = object_new(TYPE_B);
>     object_property_add_const_link(obj_b, "link-to-a", obj_a, &error_abort);
> }
> 
> void object_B_realize(DeviceState *dev, Error **errp)
> {
>     Object *obj_a;
> 
>     obj_a = object_property_get_link(OBJECT(dev), "link-to-a", errp);
>     if (!obj_a) {
>         return;
>     }
> 
>     obj_b->obj_a = A(obj_a); // If obj_b->obj_a is changed, the link property
>                              // still points to the original obj_a that was
>                              // passed to object_property_add_const_link()
> }
> 
> Confusing bugs could arise if the pointer and the link go out of sync for
> some reason. This can be avoided if the property is defined to directly use
> the pointer with object_property_add_link() and object_property_set_link().
> 
> This series just does that for all occurences of the fragile pattern in
> the XIVE and PNV code.

Can we use DEFINE_PROP_LINK() instead ?

C. 


> 
> --
> Greg
> 
> ---
> 
> Greg Kurz (8):
>       xive: Link "cpu" property to XiveTCTX::cs pointer
>       xive: Link "xive" property to XiveSource::xive pointer
>       xive: Link "xive" property to XiveEndSource::xrtr pointer
>       ppc/pnv: Link "psi" property to PnvLpc::psi pointer
>       ppc/pnv: Link "psi" property to PnvOCC::psi pointer
>       ppc/pnv: Link "chip" property to PnvHomer::chip pointer
>       ppc/pnv: Link "chip" property to PnvCore::chip pointer
>       ppc/pnv: Link "chip" property to PnvXive::chip pointer
> 
> 
>  hw/intc/pnv_xive.c        |   29 +++++++++-----------
>  hw/intc/spapr_xive.c      |    8 +++---
>  hw/intc/xive.c            |   65 ++++++++++++++++++++++-----------------------
>  hw/ppc/pnv.c              |   32 +++++++++++-----------
>  hw/ppc/pnv_core.c         |   18 +++++++-----
>  hw/ppc/pnv_homer.c        |   24 +++++++++--------
>  hw/ppc/pnv_lpc.c          |   23 ++++++++--------
>  hw/ppc/pnv_occ.c          |   23 ++++++++--------
>  hw/ppc/pnv_psi.c          |    3 +-
>  include/hw/ppc/pnv.h      |    2 +
>  include/hw/ppc/pnv_core.h |    2 +
>  11 files changed, 115 insertions(+), 114 deletions(-)
> 

Re: [PATCH for-5.0 0/8] ppc: Consolidate QOM links and pointers to the same object

[Greg Kurz]

On Fri, 15 Nov 2019 13:16:22 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 15/11/2019 12:53, Greg Kurz wrote:
> > There's a recurring pattern in the code where a const link is added to a
> > newly instanciated object and the link is then used in the object's realize
> > function to keep a pointer to the QOM entity which the link points to.
> > 
> > void create_obj_B(Object *obj_a)
> > {
> >     Object *obj_b;
> > 
> >     obj_b = object_new(TYPE_B);
> >     object_property_add_const_link(obj_b, "link-to-a", obj_a, &error_abort);
> > }
> > 
> > void object_B_realize(DeviceState *dev, Error **errp)
> > {
> >     Object *obj_a;
> > 
> >     obj_a = object_property_get_link(OBJECT(dev), "link-to-a", errp);
> >     if (!obj_a) {
> >         return;
> >     }
> > 
> >     obj_b->obj_a = A(obj_a); // If obj_b->obj_a is changed, the link property
> >                              // still points to the original obj_a that was
> >                              // passed to object_property_add_const_link()
> > }
> > 
> > Confusing bugs could arise if the pointer and the link go out of sync for
> > some reason. This can be avoided if the property is defined to directly use
> > the pointer with object_property_add_link() and object_property_set_link().
> > 
> > This series just does that for all occurences of the fragile pattern in
> > the XIVE and PNV code.
> 
> Can we use DEFINE_PROP_LINK() instead ?
> 

Heh this seems to be even better indeed. I'll give a try.

> C. 
> 
> 
> > 
> > --
> > Greg
> > 
> > ---
> > 
> > Greg Kurz (8):
> >       xive: Link "cpu" property to XiveTCTX::cs pointer
> >       xive: Link "xive" property to XiveSource::xive pointer
> >       xive: Link "xive" property to XiveEndSource::xrtr pointer
> >       ppc/pnv: Link "psi" property to PnvLpc::psi pointer
> >       ppc/pnv: Link "psi" property to PnvOCC::psi pointer
> >       ppc/pnv: Link "chip" property to PnvHomer::chip pointer
> >       ppc/pnv: Link "chip" property to PnvCore::chip pointer
> >       ppc/pnv: Link "chip" property to PnvXive::chip pointer
> > 
> > 
> >  hw/intc/pnv_xive.c        |   29 +++++++++-----------
> >  hw/intc/spapr_xive.c      |    8 +++---
> >  hw/intc/xive.c            |   65 ++++++++++++++++++++++-----------------------
> >  hw/ppc/pnv.c              |   32 +++++++++++-----------
> >  hw/ppc/pnv_core.c         |   18 +++++++-----
> >  hw/ppc/pnv_homer.c        |   24 +++++++++--------
> >  hw/ppc/pnv_lpc.c          |   23 ++++++++--------
> >  hw/ppc/pnv_occ.c          |   23 ++++++++--------
> >  hw/ppc/pnv_psi.c          |    3 +-
> >  include/hw/ppc/pnv.h      |    2 +
> >  include/hw/ppc/pnv_core.h |    2 +
> >  11 files changed, 115 insertions(+), 114 deletions(-)
> > 
>