All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support
@ 2017-04-10 13:56 Cédric Le Goater
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 1/8] ppc/pnv: Add support for POWER8+ LPC Controller Cédric Le Goater
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-10 13:56 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater

Hello,

The first patches improve the LPC support for the POWER8NVL (nvlink)
systems and for multichip systems. Next, we add IPMI support to the
machine which is required to power off and reboot a PowerNV system. To
make use of it, a BT device and an BMC simulator need to be defined on
the command line:

    -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10

To improve the sensor and FRU support, one can use the following
options for the simulator:

    sdrfile=./palmetto-SDR.bin,fruareasize=256,frudatafile=./palmetto-FRU.bin


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

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

The full patchset is available here :

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

Thanks,

C.

Changes since v1:

 - moved the IRQ handler in pnv_lpc.c and introduced pnv_lpc_isa_irq_create()
 - only add the "primary" property on the LPC bus of chip 0
 - reworked the assignement of the ISA IO base in the 'reg' array
   property
 - changed the type of the 'bmc' attribute of the machine


Benjamin Herrenschmidt (1):
  ppc/pnv: Add support for POWER8+ LPC Controller

Cédric Le Goater (7):
  ppc/pnv: enable only one LPC bus
  ppc/pnv: scan ISA bus to populate device tree
  ppc/pnv: populate device tree for RTC devices
  ppc/pnv: populate device tree for serial devices
  ppc/pnv: populate device tree for IPMI BT devices
  ppc/pnv: add initial IPMI sensors for the BMC simulator
  ppc/pnv: generate an OEM SEL event on shutdown

 hw/ppc/Makefile.objs     |   2 +-
 hw/ppc/pnv.c             | 212 ++++++++++++++++++++++++++++++++++++++---------
 hw/ppc/pnv_bmc.c         | 122 +++++++++++++++++++++++++++
 hw/ppc/pnv_lpc.c         | 120 ++++++++++++++++++++++++---
 include/hw/ppc/pnv.h     |  11 +++
 include/hw/ppc/pnv_lpc.h |  10 +++
 6 files changed, 425 insertions(+), 52 deletions(-)
 create mode 100644 hw/ppc/pnv_bmc.c

-- 
2.7.4

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

* [Qemu-devel] [PATCH v2 1/8] ppc/pnv: Add support for POWER8+ LPC Controller
  2017-04-10 13:56 [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support Cédric Le Goater
@ 2017-04-10 13:56 ` Cédric Le Goater
  2017-04-11  2:23   ` David Gibson
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 2/8] ppc/pnv: enable only one LPC bus Cédric Le Goater
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-10 13:56 UTC (permalink / raw)
  To: David Gibson
  Cc: qemu-ppc, qemu-devel, Benjamin Herrenschmidt, Cédric Le Goater

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

It adds the Naples chip which supports proper LPC interrupts via the
LPC controller rather than via an external CPLD.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: - updated for qemu-2.9
      - ported on latest PowerNV patchset
      - moved the IRQ handler in pnv_lpc.c
      - introduced pnv_lpc_isa_irq_create() to create the ISA IRQs ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v1:

 - moved the IRQ handler in pnv_lpc.c
 - introduced pnv_lpc_isa_irq_create() to create the ISA IRQs 

 hw/ppc/pnv.c             | 45 +++-------------------
 hw/ppc/pnv_lpc.c         | 97 +++++++++++++++++++++++++++++++++++++++++++++++-
 include/hw/ppc/pnv_lpc.h |  8 ++++
 3 files changed, 108 insertions(+), 42 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 16f32c9bbd9d..27589b91d1cf 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -346,36 +346,6 @@ static void ppc_powernv_reset(void)
     cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt));
 }
 
-/* If we don't use the built-in LPC interrupt deserializer, we need
- * to provide a set of qirqs for the ISA bus or things will go bad.
- *
- * Most machines using pre-Naples chips (without said deserializer)
- * have a CPLD that will collect the SerIRQ and shoot them as a
- * single level interrupt to the P8 chip. So let's setup a hook
- * for doing just that.
- */
-static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level)
-{
-    PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine());
-    uint32_t old_state = pnv->cpld_irqstate;
-    PnvChip *chip = opaque;
-
-    if (level) {
-        pnv->cpld_irqstate |= 1u << n;
-    } else {
-        pnv->cpld_irqstate &= ~(1u << n);
-    }
-    if (pnv->cpld_irqstate != old_state) {
-        pnv_psi_irq_set(&chip->psi, PSIHB_IRQ_EXTERNAL,
-                        pnv->cpld_irqstate != 0);
-    }
-}
-
-static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level)
-{
-     /* XXX TODO */
-}
-
 static ISABus *pnv_isa_create(PnvChip *chip)
 {
     PnvLpcController *lpc = &chip->lpc;
@@ -390,16 +360,7 @@ static ISABus *pnv_isa_create(PnvChip *chip)
     isa_bus = isa_bus_new(NULL, &lpc->isa_mem, &lpc->isa_io,
                           &error_fatal);
 
-    /* Not all variants have a working serial irq decoder. If not,
-     * handling of LPC interrupts becomes a platform issue (some
-     * platforms have a CPLD to do it).
-     */
-    if (pcc->chip_type == PNV_CHIP_POWER8NVL) {
-        irqs = qemu_allocate_irqs(pnv_lpc_isa_irq_handler, chip, ISA_NUM_IRQS);
-    } else {
-        irqs = qemu_allocate_irqs(pnv_lpc_isa_irq_handler_cpld, chip,
-                                  ISA_NUM_IRQS);
-    }
+    irqs = pnv_lpc_isa_irq_create(lpc, pcc->chip_type, ISA_NUM_IRQS);
 
     isa_bus_irqs(isa_bus, irqs);
     return isa_bus;
@@ -699,6 +660,10 @@ static void pnv_chip_init(Object *obj)
     object_property_add_child(obj, "occ", OBJECT(&chip->occ), NULL);
     object_property_add_const_link(OBJECT(&chip->occ), "psi",
                                    OBJECT(&chip->psi), &error_abort);
+
+    /* The LPC controller needs PSI to generate interrupts */
+    object_property_add_const_link(OBJECT(&chip->lpc), "psi",
+                                   OBJECT(&chip->psi), &error_abort);
 }
 
 static void pnv_chip_icp_realize(PnvChip *chip, Error **errp)
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index 78db52415b11..baee366d386a 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -250,6 +250,34 @@ static const MemoryRegionOps pnv_lpc_xscom_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
+static void pnv_lpc_eval_irqs(PnvLpcController *lpc)
+{
+    bool lpc_to_opb_irq = false;
+
+    /* Update LPC controller to OPB line */
+    if (lpc->lpc_hc_irqser_ctrl & LPC_HC_IRQSER_EN) {
+        uint32_t irqs;
+
+        irqs = lpc->lpc_hc_irqstat & lpc->lpc_hc_irqmask;
+        lpc_to_opb_irq = (irqs != 0);
+    }
+
+    /* We don't honor the polarity register, it's pointless and unused
+     * anyway
+     */
+    if (lpc_to_opb_irq) {
+        lpc->opb_irq_input |= OPB_MASTER_IRQ_LPC;
+    } else {
+        lpc->opb_irq_input &= ~OPB_MASTER_IRQ_LPC;
+    }
+
+    /* Update OPB internal latch */
+    lpc->opb_irq_stat |= lpc->opb_irq_input & lpc->opb_irq_mask;
+
+    /* Reflect the interrupt */
+    pnv_psi_irq_set(lpc->psi, PSIHB_IRQ_LPC_I2C, lpc->opb_irq_stat != 0);
+}
+
 static uint64_t lpc_hc_read(void *opaque, hwaddr addr, unsigned size)
 {
     PnvLpcController *lpc = opaque;
@@ -300,12 +328,15 @@ static void lpc_hc_write(void *opaque, hwaddr addr, uint64_t val,
         break;
     case LPC_HC_IRQSER_CTRL:
         lpc->lpc_hc_irqser_ctrl = val;
+        pnv_lpc_eval_irqs(lpc);
         break;
     case LPC_HC_IRQMASK:
         lpc->lpc_hc_irqmask = val;
+        pnv_lpc_eval_irqs(lpc);
         break;
     case LPC_HC_IRQSTAT:
         lpc->lpc_hc_irqstat &= ~val;
+        pnv_lpc_eval_irqs(lpc);
         break;
     case LPC_HC_ERROR_ADDRESS:
         break;
@@ -363,14 +394,15 @@ static void opb_master_write(void *opaque, hwaddr addr,
     switch (addr) {
     case OPB_MASTER_LS_IRQ_STAT:
         lpc->opb_irq_stat &= ~val;
+        pnv_lpc_eval_irqs(lpc);
         break;
     case OPB_MASTER_LS_IRQ_MASK:
-        /* XXX Filter out reserved bits */
         lpc->opb_irq_mask = val;
+        pnv_lpc_eval_irqs(lpc);
         break;
     case OPB_MASTER_LS_IRQ_POL:
-        /* XXX Filter out reserved bits */
         lpc->opb_irq_pol = val;
+        pnv_lpc_eval_irqs(lpc);
         break;
     case OPB_MASTER_LS_IRQ_INPUT:
         /* Read only */
@@ -398,6 +430,8 @@ static const MemoryRegionOps opb_master_ops = {
 static void pnv_lpc_realize(DeviceState *dev, Error **errp)
 {
     PnvLpcController *lpc = PNV_LPC(dev);
+    Object *obj;
+    Error *error = NULL;
 
     /* Reg inits */
     lpc->lpc_hc_fw_rd_acc_size = LPC_HC_FW_RD_4B;
@@ -441,6 +475,15 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
     pnv_xscom_region_init(&lpc->xscom_regs, OBJECT(dev),
                           &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
                           PNV_XSCOM_LPC_SIZE);
+
+    /* get PSI object from chip */
+    obj = object_property_get_link(OBJECT(dev), "psi", &error);
+    if (!obj) {
+        error_setg(errp, "%s: required link 'psi' not found: %s",
+                   __func__, error_get_pretty(error));
+        return;
+    }
+    lpc->psi = PNV_PSI(obj);
 }
 
 static void pnv_lpc_class_init(ObjectClass *klass, void *data)
@@ -470,3 +513,53 @@ static void pnv_lpc_register_types(void)
 }
 
 type_init(pnv_lpc_register_types)
+
+/* If we don't use the built-in LPC interrupt deserializer, we need
+ * to provide a set of qirqs for the ISA bus or things will go bad.
+ *
+ * Most machines using pre-Naples chips (without said deserializer)
+ * have a CPLD that will collect the SerIRQ and shoot them as a
+ * single level interrupt to the P8 chip. So let's setup a hook
+ * for doing just that.
+ */
+static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level)
+{
+    PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine());
+    uint32_t old_state = pnv->cpld_irqstate;
+    PnvLpcController *lpc = opaque;
+
+    if (level) {
+        pnv->cpld_irqstate |= 1u << n;
+    } else {
+        pnv->cpld_irqstate &= ~(1u << n);
+    }
+
+    if (pnv->cpld_irqstate != old_state) {
+        pnv_psi_irq_set(lpc->psi, PSIHB_IRQ_EXTERNAL, pnv->cpld_irqstate != 0);
+    }
+}
+
+static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level)
+{
+    PnvLpcController *lpc = opaque;
+
+    /* The Naples HW latches the 1 levels, clearing is done by SW */
+    if (level) {
+        lpc->lpc_hc_irqstat |= LPC_HC_IRQ_SERIRQ0 >> n;
+        pnv_lpc_eval_irqs(lpc);
+    }
+}
+
+qemu_irq *pnv_lpc_isa_irq_create(PnvLpcController *lpc, int chip_type,
+                                 int nirqs)
+{
+    /* Not all variants have a working serial irq decoder. If not,
+     * handling of LPC interrupts becomes a platform issue (some
+     * platforms have a CPLD to do it).
+     */
+    if (chip_type == PNV_CHIP_POWER8NVL) {
+        return qemu_allocate_irqs(pnv_lpc_isa_irq_handler, lpc, nirqs);
+    } else {
+        return qemu_allocate_irqs(pnv_lpc_isa_irq_handler_cpld, lpc, nirqs);
+    }
+}
diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
index 38e5506975aa..ccf969af9448 100644
--- a/include/hw/ppc/pnv_lpc.h
+++ b/include/hw/ppc/pnv_lpc.h
@@ -23,6 +23,8 @@
 #define PNV_LPC(obj) \
      OBJECT_CHECK(PnvLpcController, (obj), TYPE_PNV_LPC)
 
+typedef struct PnvPsi PnvPsi;
+
 typedef struct PnvLpcController {
     DeviceState parent;
 
@@ -62,6 +64,12 @@ typedef struct PnvLpcController {
 
     /* XSCOM registers */
     MemoryRegion xscom_regs;
+
+    /* PSI to generate interrupts */
+    PnvPsi *psi;
 } PnvLpcController;
 
+qemu_irq *pnv_lpc_isa_irq_create(PnvLpcController *lpc, int chip_type,
+                                 int nirqs);
+
 #endif /* _PPC_PNV_LPC_H */
-- 
2.7.4

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

* [Qemu-devel] [PATCH v2 2/8] ppc/pnv: enable only one LPC bus
  2017-04-10 13:56 [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support Cédric Le Goater
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 1/8] ppc/pnv: Add support for POWER8+ LPC Controller Cédric Le Goater
@ 2017-04-10 13:56 ` Cédric Le Goater
  2017-04-11  2:40   ` David Gibson
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 3/8] ppc/pnv: scan ISA bus to populate device tree Cédric Le Goater
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-10 13:56 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater

The firmware (skiboot) chooses the default LPC bus of a multichip
systems using a "primary" property. The LPC bus of chip 0 should be
the only connected in the system. Let's advertise it in the device
tree.

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

 - the device tree is populated for all LPC busses of the system but
   only the one on chip 0 has the "primary" property.

 hw/ppc/pnv.c             |  2 ++
 hw/ppc/pnv_lpc.c         | 23 ++++++++++++++---------
 include/hw/ppc/pnv_lpc.h |  2 ++
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 27589b91d1cf..7d742b6e34e1 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -765,6 +765,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
     g_free(typename);
 
     /* Create LPC controller */
+    object_property_set_int(OBJECT(&chip->lpc), chip->chip_id, "chip-id",
+                            &error_fatal);
     object_property_set_bool(OBJECT(&chip->lpc), true, "realized",
                              &error_fatal);
     pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, &chip->lpc.xscom_regs);
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index baee366d386a..13d7a695678d 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -92,14 +92,6 @@ enum {
 #define LPC_HC_REGS_OPB_SIZE    0x00001000
 
 
-/*
- * TODO: the "primary" cell should only be added on chip 0. This is
- * how skiboot chooses the default LPC controller on multichip
- * systems.
- *
- * It would be easly done if we can change the populate() interface to
- * replace the PnvXScomInterface parameter by a PnvChip one
- */
 static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
 {
     const char compat[] = "ibm,power8-lpc\0ibm,lpc";
@@ -110,6 +102,7 @@ static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
         cpu_to_be32(lpc_pcba),
         cpu_to_be32(PNV_XSCOM_LPC_SIZE)
     };
+    PnvLpcController *lpc = PNV_LPC(dev);
 
     name = g_strdup_printf("isa@%x", lpc_pcba);
     offset = fdt_add_subnode(fdt, xscom_offset, name);
@@ -119,7 +112,13 @@ static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
     _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
     _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 2)));
     _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 1)));
-    _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
+
+    /* The firmware (skiboot) chooses the default LPC bus of the
+     * system using a "primary" property.
+     */
+    if (lpc->chip_id == 0x0) {
+        _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
+    }
     _FDT((fdt_setprop(fdt, offset, "compatible", compat, sizeof(compat))));
     return 0;
 }
@@ -486,6 +485,11 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
     lpc->psi = PNV_PSI(obj);
 }
 
+static Property pnv_lpc_properties[] = {
+    DEFINE_PROP_UINT32("chip-id", PnvLpcController, chip_id, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void pnv_lpc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -494,6 +498,7 @@ static void pnv_lpc_class_init(ObjectClass *klass, void *data)
     xdc->populate = pnv_lpc_populate;
 
     dc->realize = pnv_lpc_realize;
+    dc->props = pnv_lpc_properties;
 }
 
 static const TypeInfo pnv_lpc_info = {
diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
index ccf969af9448..c78ee4a98c62 100644
--- a/include/hw/ppc/pnv_lpc.h
+++ b/include/hw/ppc/pnv_lpc.h
@@ -67,6 +67,8 @@ typedef struct PnvLpcController {
 
     /* PSI to generate interrupts */
     PnvPsi *psi;
+
+    uint32_t chip_id;
 } PnvLpcController;
 
 qemu_irq *pnv_lpc_isa_irq_create(PnvLpcController *lpc, int chip_type,
-- 
2.7.4

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

* [Qemu-devel] [PATCH v2 3/8] ppc/pnv: scan ISA bus to populate device tree
  2017-04-10 13:56 [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support Cédric Le Goater
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 1/8] ppc/pnv: Add support for POWER8+ LPC Controller Cédric Le Goater
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 2/8] ppc/pnv: enable only one LPC bus Cédric Le Goater
@ 2017-04-10 13:56 ` Cédric Le Goater
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 4/8] ppc/pnv: populate device tree for RTC devices Cédric Le Goater
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-10 13:56 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater

This is an empty shell that we will use to include nodes in the device
tree for ISA devices. We expect RTC, UART and IPMI BT devices.

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

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 7d742b6e34e1..2d32b20f2d6e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -281,6 +281,36 @@ static void powernv_populate_chip(PnvChip *chip, void *fdt)
     g_free(typename);
 }
 
+typedef struct ForeachPopulateArgs {
+    void *fdt;
+    int offset;
+} ForeachPopulateArgs;
+
+static int powernv_populate_isa_device(DeviceState *dev, void *opaque)
+{
+    return 0;
+}
+
+static void powernv_populate_isa(ISABus *bus, void *fdt)
+{
+    int lpc_offset;
+    ForeachPopulateArgs args;
+
+    lpc_offset = fdt_node_offset_by_compatible(fdt, -1, "ibm,lpc");
+    if (lpc_offset < 0) {
+        error_report("Could not find the lpc node !?");
+        return;
+    }
+
+    args.fdt = fdt;
+    args.offset = lpc_offset;
+
+    /* ISA devices are not necessarily parented to the ISA bus so we
+     * can not use object_child_foreach() */
+    qbus_walk_children(BUS(bus), powernv_populate_isa_device,
+                       NULL, NULL, NULL, &args);
+}
+
 static void *powernv_create_fdt(MachineState *machine)
 {
     const char plat_compat[] = "qemu,powernv\0ibm,powernv";
@@ -328,6 +358,8 @@ static void *powernv_create_fdt(MachineState *machine)
     for (i = 0; i < pnv->num_chips; i++) {
         powernv_populate_chip(pnv->chips[i], fdt);
     }
+
+    powernv_populate_isa(pnv->isa_bus, fdt);
     return fdt;
 }
 
-- 
2.7.4

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

* [Qemu-devel] [PATCH v2 4/8] ppc/pnv: populate device tree for RTC devices
  2017-04-10 13:56 [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support Cédric Le Goater
                   ` (2 preceding siblings ...)
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 3/8] ppc/pnv: scan ISA bus to populate device tree Cédric Le Goater
@ 2017-04-10 13:56 ` Cédric Le Goater
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 5/8] ppc/pnv: populate device tree for serial devices Cédric Le Goater
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-10 13:56 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater

The code could be common to any ISA device but we are missing the IO
length.

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

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 2d32b20f2d6e..94e9744d7e67 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -281,6 +281,26 @@ static void powernv_populate_chip(PnvChip *chip, void *fdt)
     g_free(typename);
 }
 
+static void powernv_populate_rtc(ISADevice *d, void *fdt, int lpc_off)
+{
+    uint32_t io_base = d->ioport_id;
+    uint32_t io_regs[] = {
+        cpu_to_be32(1),
+        cpu_to_be32(io_base),
+        cpu_to_be32(2)
+    };
+    char *name;
+    int node;
+
+    name = g_strdup_printf("%s@i%x", qdev_fw_name(DEVICE(d)), io_base);
+    node = fdt_add_subnode(fdt, lpc_off, name);
+    _FDT(node);
+    g_free(name);
+
+    _FDT((fdt_setprop(fdt, node, "reg", io_regs, sizeof(io_regs))));
+    _FDT((fdt_setprop_string(fdt, node, "compatible", "pnpPNP,b00")));
+}
+
 typedef struct ForeachPopulateArgs {
     void *fdt;
     int offset;
@@ -288,6 +308,16 @@ typedef struct ForeachPopulateArgs {
 
 static int powernv_populate_isa_device(DeviceState *dev, void *opaque)
 {
+    ForeachPopulateArgs *args = opaque;
+    ISADevice *d = ISA_DEVICE(dev);
+
+    if (object_dynamic_cast(OBJECT(dev), TYPE_MC146818_RTC)) {
+        powernv_populate_rtc(d, args->fdt, args->offset);
+    } else {
+        error_report("unknown isa device %s@i%x", qdev_fw_name(dev),
+                     d->ioport_id);
+    }
+
     return 0;
 }
 
-- 
2.7.4

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

* [Qemu-devel] [PATCH v2 5/8] ppc/pnv: populate device tree for serial devices
  2017-04-10 13:56 [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support Cédric Le Goater
                   ` (3 preceding siblings ...)
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 4/8] ppc/pnv: populate device tree for RTC devices Cédric Le Goater
@ 2017-04-10 13:56 ` Cédric Le Goater
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 6/8] ppc/pnv: populate device tree for IPMI BT devices Cédric Le Goater
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-10 13:56 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater

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

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 94e9744d7e67..7f2f9897f146 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -301,6 +301,37 @@ static void powernv_populate_rtc(ISADevice *d, void *fdt, int lpc_off)
     _FDT((fdt_setprop_string(fdt, node, "compatible", "pnpPNP,b00")));
 }
 
+static void powernv_populate_serial(ISADevice *d, void *fdt, int lpc_off)
+{
+    const char compatible[] = "ns16550\0pnpPNP,501";
+    uint32_t io_base = d->ioport_id;
+    uint32_t io_regs[] = {
+        cpu_to_be32(1),
+        cpu_to_be32(io_base),
+        cpu_to_be32(8)
+    };
+    char *name;
+    int node;
+
+    name = g_strdup_printf("%s@i%x", qdev_fw_name(DEVICE(d)), io_base);
+    node = fdt_add_subnode(fdt, lpc_off, name);
+    _FDT(node);
+    g_free(name);
+
+    _FDT((fdt_setprop(fdt, node, "reg", io_regs, sizeof(io_regs))));
+    _FDT((fdt_setprop(fdt, node, "compatible", compatible,
+                      sizeof(compatible))));
+
+    _FDT((fdt_setprop_cell(fdt, node, "clock-frequency", 1843200)));
+    _FDT((fdt_setprop_cell(fdt, node, "current-speed", 115200)));
+    _FDT((fdt_setprop_cell(fdt, node, "interrupts", d->isairq[0])));
+    _FDT((fdt_setprop_cell(fdt, node, "interrupt-parent",
+                           fdt_get_phandle(fdt, lpc_off))));
+
+    /* This is needed by Linux */
+    _FDT((fdt_setprop_string(fdt, node, "device_type", "serial")));
+}
+
 typedef struct ForeachPopulateArgs {
     void *fdt;
     int offset;
@@ -313,6 +344,8 @@ static int powernv_populate_isa_device(DeviceState *dev, void *opaque)
 
     if (object_dynamic_cast(OBJECT(dev), TYPE_MC146818_RTC)) {
         powernv_populate_rtc(d, args->fdt, args->offset);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_ISA_SERIAL)) {
+        powernv_populate_serial(d, args->fdt, args->offset);
     } else {
         error_report("unknown isa device %s@i%x", qdev_fw_name(dev),
                      d->ioport_id);
-- 
2.7.4

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

* [Qemu-devel] [PATCH v2 6/8] ppc/pnv: populate device tree for IPMI BT devices
  2017-04-10 13:56 [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support Cédric Le Goater
                   ` (4 preceding siblings ...)
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 5/8] ppc/pnv: populate device tree for serial devices Cédric Le Goater
@ 2017-04-10 13:56 ` Cédric Le Goater
  2017-04-11  2:43   ` David Gibson
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 7/8] ppc/pnv: add initial IPMI sensors for the BMC simulator Cédric Le Goater
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 8/8] ppc/pnv: generate an OEM SEL event on shutdown Cédric Le Goater
  7 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-10 13:56 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater

When an ipmi-bt device [1] is defined on the ISA bus, we need to
populate the device tree with the object properties. Such devices are
created with the command line options :

   -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10

[1] https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03168.html

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

 Changes sinve v1:

 - reworked the assignement of the ISA IO base in the 'reg' array
   property
 
 hw/ppc/pnv.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 7f2f9897f146..ec5d62a479b3 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -332,6 +332,39 @@ static void powernv_populate_serial(ISADevice *d, void *fdt, int lpc_off)
     _FDT((fdt_setprop_string(fdt, node, "device_type", "serial")));
 }
 
+static void powernv_populate_ipmi_bt(ISADevice *d, void *fdt, int lpc_off)
+{
+    const char compatible[] = "bt\0ipmi-bt";
+    uint32_t io_base = 0x0;
+    uint32_t io_regs[] = {
+        cpu_to_be32(1),
+        0, /* 'io_base' retrieved from the 'ioport' property of 'isa-ipmi-bt' */
+        cpu_to_be32(3)
+    };
+    uint32_t irq;
+    char *name;
+    int node;
+
+    io_base = object_property_get_int(OBJECT(d), "ioport", &error_fatal);
+    io_regs[1] = cpu_to_be32(io_base);
+
+    irq = object_property_get_int(OBJECT(d), "irq", &error_fatal);
+
+    name = g_strdup_printf("%s@i%x", qdev_fw_name(DEVICE(d)), io_base);
+    node = fdt_add_subnode(fdt, lpc_off, name);
+    _FDT(node);
+    g_free(name);
+
+    fdt_setprop(fdt, node, "reg", io_regs, sizeof(io_regs));
+    fdt_setprop(fdt, node, "compatible", compatible, sizeof(compatible));
+
+    /* Mark it as reserved to avoid Linux trying to claim it */
+    _FDT((fdt_setprop_string(fdt, node, "status", "reserved")));
+    _FDT((fdt_setprop_cell(fdt, node, "interrupts", irq)));
+    _FDT((fdt_setprop_cell(fdt, node, "interrupt-parent",
+                           fdt_get_phandle(fdt, lpc_off))));
+}
+
 typedef struct ForeachPopulateArgs {
     void *fdt;
     int offset;
@@ -346,6 +379,8 @@ static int powernv_populate_isa_device(DeviceState *dev, void *opaque)
         powernv_populate_rtc(d, args->fdt, args->offset);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_ISA_SERIAL)) {
         powernv_populate_serial(d, args->fdt, args->offset);
+    } else if (object_dynamic_cast(OBJECT(dev), "isa-ipmi-bt")) {
+        powernv_populate_ipmi_bt(d, args->fdt, args->offset);
     } else {
         error_report("unknown isa device %s@i%x", qdev_fw_name(dev),
                      d->ioport_id);
-- 
2.7.4

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

* [Qemu-devel] [PATCH v2 7/8] ppc/pnv: add initial IPMI sensors for the BMC simulator
  2017-04-10 13:56 [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support Cédric Le Goater
                   ` (5 preceding siblings ...)
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 6/8] ppc/pnv: populate device tree for IPMI BT devices Cédric Le Goater
@ 2017-04-10 13:56 ` Cédric Le Goater
  2017-04-11  2:44   ` David Gibson
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 8/8] ppc/pnv: generate an OEM SEL event on shutdown Cédric Le Goater
  7 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-10 13:56 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater

Skiboot, the firmware for the PowerNV platform, expects the BMC to
provide some specific IPMI sensors. These sensors are exposed in the
device tree and their values are updated by the firmware at boot time.

Sensors of interest are :

	"FW Boot Progress"
	"Boot Count"

As such a device is defined on the command line, we can only detect
its presence at reset time.

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

 - fixed test on sdr->header.rec_type which was incorrect
 - changed the type of the 'bmc' attribute of the machine and changed
   accordingly the prototype of the helper routines

 hw/ppc/Makefile.objs |  2 +-
 hw/ppc/pnv.c         | 21 ++++++++++++++
 hw/ppc/pnv_bmc.c     | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/pnv.h |  9 ++++++
 4 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/pnv_bmc.c

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index ef67ea820158..7efc68674819 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -6,7 +6,7 @@ obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
 obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
 obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o
 # IBM PowerNV
-obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o
+obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
 ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
 obj-y += spapr_pci_vfio.o
 endif
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index ec5d62a479b3..bb9bb746e04a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -35,6 +35,7 @@
 #include "qapi/visitor.h"
 #include "monitor/monitor.h"
 #include "hw/intc/intc.h"
+#include "hw/ipmi/ipmi.h"
 
 #include "hw/ppc/xics.h"
 #include "hw/ppc/pnv_xscom.h"
@@ -458,16 +459,36 @@ static void *powernv_create_fdt(MachineState *machine)
     }
 
     powernv_populate_isa(pnv->isa_bus, fdt);
+
+    if (pnv->bmc) {
+        pnv_bmc_populate_sensors(pnv->bmc, fdt);
+    }
+
     return fdt;
 }
 
 static void ppc_powernv_reset(void)
 {
     MachineState *machine = MACHINE(qdev_get_machine());
+    PnvMachineState *pnv = POWERNV_MACHINE(machine);
     void *fdt;
+    Object *obj;
 
     qemu_devices_reset();
 
+    /* OpenPOWER systems have a BMC, which can be defined on the
+     * command line with:
+     *
+     *   -device ipmi-bmc-sim,id=bmc0
+     *
+     * This is the internal simulator but it could also be an external
+     * BMC.
+     */
+    obj = object_resolve_path_type("", TYPE_IPMI_BMC, NULL);
+    if (obj) {
+        pnv->bmc = IPMI_BMC(obj);
+    }
+
     fdt = powernv_create_fdt(machine);
 
     /* Pack resulting tree */
diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
new file mode 100644
index 000000000000..a0820dca559b
--- /dev/null
+++ b/hw/ppc/pnv_bmc.c
@@ -0,0 +1,81 @@
+/*
+ * QEMU PowerNV, BMC related functions
+ *
+ * Copyright (c) 2016-2017, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/hw.h"
+#include "sysemu/sysemu.h"
+#include "target/ppc/cpu.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "hw/ipmi/ipmi.h"
+#include "hw/ppc/fdt.h"
+
+#include "hw/ppc/pnv.h"
+
+#include <libfdt.h>
+
+/* TODO: include definition in ipmi.h */
+#define IPMI_SDR_FULL_TYPE 1
+
+void pnv_bmc_populate_sensors(IPMIBmc *bmc, void *fdt)
+{
+    int offset;
+    int i;
+    const struct ipmi_sdr_compact *sdr;
+    uint16_t nextrec;
+
+    offset = fdt_add_subnode(fdt, 0, "/bmc");
+    _FDT(offset);
+
+    _FDT((fdt_setprop_string(fdt, offset, "name", "bmc")));
+    _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0x1)));
+    _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 0x0)));
+
+    offset = fdt_add_subnode(fdt, offset, "sensors");
+    _FDT(offset);
+
+    _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0x1)));
+    _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 0x0)));
+
+    for (i = 0; !ipmi_bmc_sdr_find(bmc, i, &sdr, &nextrec); i++) {
+        int off;
+        char *name;
+
+        if (sdr->header.rec_type != IPMI_SDR_COMPACT_TYPE &&
+            sdr->header.rec_type != IPMI_SDR_FULL_TYPE) {
+            continue;
+        }
+
+        name = g_strdup_printf("sensor@%x", sdr->sensor_owner_number);
+        off = fdt_add_subnode(fdt, offset, name);
+        _FDT(off);
+        g_free(name);
+
+        _FDT((fdt_setprop_cell(fdt, off, "reg", sdr->sensor_owner_number)));
+        _FDT((fdt_setprop_string(fdt, off, "name", "sensor")));
+        _FDT((fdt_setprop_string(fdt, off, "compatible", "ibm,ipmi-sensor")));
+        _FDT((fdt_setprop_cell(fdt, off, "ipmi-sensor-reading-type",
+                               sdr->reading_type)));
+        _FDT((fdt_setprop_cell(fdt, off, "ipmi-entity-id",
+                               sdr->entity_id)));
+        _FDT((fdt_setprop_cell(fdt, off, "ipmi-entity-instance",
+                               sdr->entity_instance)));
+        _FDT((fdt_setprop_cell(fdt, off, "ipmi-sensor-type",
+                               sdr->sensor_type)));
+    }
+}
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index b45a0d91c813..02f6cf565ca4 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -118,6 +118,8 @@ typedef struct PnvChipClass {
 #define POWERNV_MACHINE(obj) \
     OBJECT_CHECK(PnvMachineState, (obj), TYPE_POWERNV_MACHINE)
 
+typedef struct IPMIBmc IPMIBmc;
+
 typedef struct PnvMachineState {
     /*< private >*/
     MachineState parent_obj;
@@ -130,12 +132,19 @@ typedef struct PnvMachineState {
 
     ISABus       *isa_bus;
     uint32_t     cpld_irqstate;
+
+    IPMIBmc      *bmc;
 } PnvMachineState;
 
 #define PNV_FDT_ADDR          0x01000000
 #define PNV_TIMEBASE_FREQ     512000000ULL
 
 /*
+ * BMC helpers
+ */
+void pnv_bmc_populate_sensors(IPMIBmc *bmc, void *fdt);
+
+/*
  * POWER8 MMIO base addresses
  */
 #define PNV_XSCOM_SIZE        0x800000000ull
-- 
2.7.4

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

* [Qemu-devel] [PATCH v2 8/8] ppc/pnv: generate an OEM SEL event on shutdown
  2017-04-10 13:56 [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support Cédric Le Goater
                   ` (6 preceding siblings ...)
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 7/8] ppc/pnv: add initial IPMI sensors for the BMC simulator Cédric Le Goater
@ 2017-04-10 13:56 ` Cédric Le Goater
  7 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-10 13:56 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater

OpenPOWER systems expect to be notified with such an event before a
shutdown or a reboot. An OEM SEL message is sent with specific
identifiers and a user data containing the request : OFF or REBOOT.

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

 - changed the prototype of the helper routine

 hw/ppc/pnv.c         | 14 ++++++++++++++
 hw/ppc/pnv_bmc.c     | 41 +++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/pnv.h |  2 ++
 3 files changed, 57 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index bb9bb746e04a..f7ee592180dc 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -467,6 +467,15 @@ static void *powernv_create_fdt(MachineState *machine)
     return fdt;
 }
 
+static void pnv_powerdown_notify(Notifier *n, void *opaque)
+{
+    PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine());
+
+    if (pnv->bmc) {
+        pnv_bmc_powerdown(pnv->bmc);
+    }
+}
+
 static void ppc_powernv_reset(void)
 {
     MachineState *machine = MACHINE(qdev_get_machine());
@@ -620,6 +629,11 @@ static void ppc_powernv_init(MachineState *machine)
 
     /* Create an RTC ISA device too */
     rtc_init(pnv->isa_bus, 2000, NULL);
+
+    /* OpenPOWER systems use a IPMI SEL Event message to notify the
+     * host to powerdown */
+    pnv->powerdown_notifier.notify = pnv_powerdown_notify;
+    qemu_register_powerdown_notifier(&pnv->powerdown_notifier);
 }
 
 /*
diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
index a0820dca559b..7b60b4c36071 100644
--- a/hw/ppc/pnv_bmc.c
+++ b/hw/ppc/pnv_bmc.c
@@ -32,6 +32,47 @@
 /* TODO: include definition in ipmi.h */
 #define IPMI_SDR_FULL_TYPE 1
 
+/*
+ * OEM SEL Event data packet sent by BMC in response of a Read Event
+ * Message Buffer command
+ */
+typedef struct OemSel {
+    /* SEL header */
+    uint8_t id[2];
+    uint8_t type;
+    uint8_t timestamp[4];
+    uint8_t manuf_id[3];
+
+    /* OEM SEL data (6 bytes) follows */
+    uint8_t netfun;
+    uint8_t cmd;
+    uint8_t data[4];
+} OemSel;
+
+#define SOFT_OFF        0x00
+#define SOFT_REBOOT     0x01
+
+static void pnv_gen_oem_sel(IPMIBmc *bmc, uint8_t reboot)
+{
+    /* IPMI SEL Event are 16 bytes long */
+    OemSel sel = {
+        .id        = { 0x55 , 0x55 },
+        .type      = 0xC0, /* OEM */
+        .manuf_id  = { 0x0, 0x0, 0x0 },
+        .timestamp = { 0x0, 0x0, 0x0, 0x0 },
+        .netfun    = 0x3A, /* IBM */
+        .cmd       = 0x04, /* AMI OEM SEL Power Notification */
+        .data      = { reboot, 0xFF, 0xFF, 0xFF },
+    };
+
+    ipmi_bmc_gen_event(bmc, (uint8_t *) &sel, 0 /* do not log the event */);
+}
+
+void pnv_bmc_powerdown(IPMIBmc *bmc)
+{
+    pnv_gen_oem_sel(bmc, SOFT_OFF);
+}
+
 void pnv_bmc_populate_sensors(IPMIBmc *bmc, void *fdt)
 {
     int offset;
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 02f6cf565ca4..c1288f974d4a 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -134,6 +134,7 @@ typedef struct PnvMachineState {
     uint32_t     cpld_irqstate;
 
     IPMIBmc      *bmc;
+    Notifier     powerdown_notifier;
 } PnvMachineState;
 
 #define PNV_FDT_ADDR          0x01000000
@@ -143,6 +144,7 @@ typedef struct PnvMachineState {
  * BMC helpers
  */
 void pnv_bmc_populate_sensors(IPMIBmc *bmc, void *fdt);
+void pnv_bmc_powerdown(IPMIBmc *bmc);
 
 /*
  * POWER8 MMIO base addresses
-- 
2.7.4

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

* Re: [Qemu-devel] [PATCH v2 1/8] ppc/pnv: Add support for POWER8+ LPC Controller
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 1/8] ppc/pnv: Add support for POWER8+ LPC Controller Cédric Le Goater
@ 2017-04-11  2:23   ` David Gibson
  2017-04-11  6:31     ` Cédric Le Goater
  0 siblings, 1 reply; 16+ messages in thread
From: David Gibson @ 2017-04-11  2:23 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, Benjamin Herrenschmidt

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

On Mon, Apr 10, 2017 at 03:56:51PM +0200, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> It adds the Naples chip which supports proper LPC interrupts via the
> LPC controller rather than via an external CPLD.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> [clg: - updated for qemu-2.9
>       - ported on latest PowerNV patchset
>       - moved the IRQ handler in pnv_lpc.c
>       - introduced pnv_lpc_isa_irq_create() to create the ISA IRQs ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v1:
> 
>  - moved the IRQ handler in pnv_lpc.c
>  - introduced pnv_lpc_isa_irq_create() to create the ISA IRQs 
> 
>  hw/ppc/pnv.c             | 45 +++-------------------
>  hw/ppc/pnv_lpc.c         | 97 +++++++++++++++++++++++++++++++++++++++++++++++-
>  include/hw/ppc/pnv_lpc.h |  8 ++++
>  3 files changed, 108 insertions(+), 42 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 16f32c9bbd9d..27589b91d1cf 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -346,36 +346,6 @@ static void ppc_powernv_reset(void)
>      cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt));
>  }
>  
> -/* If we don't use the built-in LPC interrupt deserializer, we need
> - * to provide a set of qirqs for the ISA bus or things will go bad.
> - *
> - * Most machines using pre-Naples chips (without said deserializer)
> - * have a CPLD that will collect the SerIRQ and shoot them as a
> - * single level interrupt to the P8 chip. So let's setup a hook
> - * for doing just that.
> - */
> -static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level)
> -{
> -    PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine());
> -    uint32_t old_state = pnv->cpld_irqstate;
> -    PnvChip *chip = opaque;
> -
> -    if (level) {
> -        pnv->cpld_irqstate |= 1u << n;
> -    } else {
> -        pnv->cpld_irqstate &= ~(1u << n);
> -    }
> -    if (pnv->cpld_irqstate != old_state) {
> -        pnv_psi_irq_set(&chip->psi, PSIHB_IRQ_EXTERNAL,
> -                        pnv->cpld_irqstate != 0);
> -    }
> -}
> -
> -static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level)
> -{
> -     /* XXX TODO */
> -}
> -
>  static ISABus *pnv_isa_create(PnvChip *chip)
>  {
>      PnvLpcController *lpc = &chip->lpc;
> @@ -390,16 +360,7 @@ static ISABus *pnv_isa_create(PnvChip *chip)
>      isa_bus = isa_bus_new(NULL, &lpc->isa_mem, &lpc->isa_io,
>                            &error_fatal);
>  
> -    /* Not all variants have a working serial irq decoder. If not,
> -     * handling of LPC interrupts becomes a platform issue (some
> -     * platforms have a CPLD to do it).
> -     */
> -    if (pcc->chip_type == PNV_CHIP_POWER8NVL) {
> -        irqs = qemu_allocate_irqs(pnv_lpc_isa_irq_handler, chip, ISA_NUM_IRQS);
> -    } else {
> -        irqs = qemu_allocate_irqs(pnv_lpc_isa_irq_handler_cpld, chip,
> -                                  ISA_NUM_IRQS);
> -    }
> +    irqs = pnv_lpc_isa_irq_create(lpc, pcc->chip_type, ISA_NUM_IRQS);
>  
>      isa_bus_irqs(isa_bus, irqs);
>      return isa_bus;
> @@ -699,6 +660,10 @@ static void pnv_chip_init(Object *obj)
>      object_property_add_child(obj, "occ", OBJECT(&chip->occ), NULL);
>      object_property_add_const_link(OBJECT(&chip->occ), "psi",
>                                     OBJECT(&chip->psi), &error_abort);
> +
> +    /* The LPC controller needs PSI to generate interrupts */
> +    object_property_add_const_link(OBJECT(&chip->lpc), "psi",
> +                                   OBJECT(&chip->psi), &error_abort);
>  }
>  
>  static void pnv_chip_icp_realize(PnvChip *chip, Error **errp)
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index 78db52415b11..baee366d386a 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -250,6 +250,34 @@ static const MemoryRegionOps pnv_lpc_xscom_ops = {
>      .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> +static void pnv_lpc_eval_irqs(PnvLpcController *lpc)
> +{
> +    bool lpc_to_opb_irq = false;
> +
> +    /* Update LPC controller to OPB line */
> +    if (lpc->lpc_hc_irqser_ctrl & LPC_HC_IRQSER_EN) {
> +        uint32_t irqs;
> +
> +        irqs = lpc->lpc_hc_irqstat & lpc->lpc_hc_irqmask;
> +        lpc_to_opb_irq = (irqs != 0);
> +    }
> +
> +    /* We don't honor the polarity register, it's pointless and unused
> +     * anyway
> +     */
> +    if (lpc_to_opb_irq) {
> +        lpc->opb_irq_input |= OPB_MASTER_IRQ_LPC;
> +    } else {
> +        lpc->opb_irq_input &= ~OPB_MASTER_IRQ_LPC;
> +    }
> +
> +    /* Update OPB internal latch */
> +    lpc->opb_irq_stat |= lpc->opb_irq_input & lpc->opb_irq_mask;
> +
> +    /* Reflect the interrupt */
> +    pnv_psi_irq_set(lpc->psi, PSIHB_IRQ_LPC_I2C, lpc->opb_irq_stat != 0);
> +}
> +
>  static uint64_t lpc_hc_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      PnvLpcController *lpc = opaque;
> @@ -300,12 +328,15 @@ static void lpc_hc_write(void *opaque, hwaddr addr, uint64_t val,
>          break;
>      case LPC_HC_IRQSER_CTRL:
>          lpc->lpc_hc_irqser_ctrl = val;
> +        pnv_lpc_eval_irqs(lpc);
>          break;
>      case LPC_HC_IRQMASK:
>          lpc->lpc_hc_irqmask = val;
> +        pnv_lpc_eval_irqs(lpc);
>          break;
>      case LPC_HC_IRQSTAT:
>          lpc->lpc_hc_irqstat &= ~val;
> +        pnv_lpc_eval_irqs(lpc);
>          break;
>      case LPC_HC_ERROR_ADDRESS:
>          break;
> @@ -363,14 +394,15 @@ static void opb_master_write(void *opaque, hwaddr addr,
>      switch (addr) {
>      case OPB_MASTER_LS_IRQ_STAT:
>          lpc->opb_irq_stat &= ~val;
> +        pnv_lpc_eval_irqs(lpc);
>          break;
>      case OPB_MASTER_LS_IRQ_MASK:
> -        /* XXX Filter out reserved bits */
>          lpc->opb_irq_mask = val;
> +        pnv_lpc_eval_irqs(lpc);
>          break;
>      case OPB_MASTER_LS_IRQ_POL:
> -        /* XXX Filter out reserved bits */
>          lpc->opb_irq_pol = val;
> +        pnv_lpc_eval_irqs(lpc);
>          break;
>      case OPB_MASTER_LS_IRQ_INPUT:
>          /* Read only */
> @@ -398,6 +430,8 @@ static const MemoryRegionOps opb_master_ops = {
>  static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>  {
>      PnvLpcController *lpc = PNV_LPC(dev);
> +    Object *obj;
> +    Error *error = NULL;
>  
>      /* Reg inits */
>      lpc->lpc_hc_fw_rd_acc_size = LPC_HC_FW_RD_4B;
> @@ -441,6 +475,15 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_region_init(&lpc->xscom_regs, OBJECT(dev),
>                            &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
>                            PNV_XSCOM_LPC_SIZE);
> +
> +    /* get PSI object from chip */
> +    obj = object_property_get_link(OBJECT(dev), "psi", &error);
> +    if (!obj) {
> +        error_setg(errp, "%s: required link 'psi' not found: %s",
> +                   __func__, error_get_pretty(error));
> +        return;
> +    }
> +    lpc->psi = PNV_PSI(obj);
>  }
>  
>  static void pnv_lpc_class_init(ObjectClass *klass, void *data)
> @@ -470,3 +513,53 @@ static void pnv_lpc_register_types(void)
>  }
>  
>  type_init(pnv_lpc_register_types)
> +
> +/* If we don't use the built-in LPC interrupt deserializer, we need
> + * to provide a set of qirqs for the ISA bus or things will go bad.
> + *
> + * Most machines using pre-Naples chips (without said deserializer)
> + * have a CPLD that will collect the SerIRQ and shoot them as a
> + * single level interrupt to the P8 chip. So let's setup a hook
> + * for doing just that.
> + */
> +static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level)
> +{
> +    PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine());
> +    uint32_t old_state = pnv->cpld_irqstate;
> +    PnvLpcController *lpc = opaque;

You can use the existing PNV_LPC() and similar macros here, rather
than hard casting from a void *.  That's a bit safer.

> +
> +    if (level) {
> +        pnv->cpld_irqstate |= 1u << n;
> +    } else {
> +        pnv->cpld_irqstate &= ~(1u << n);
> +    }
> +
> +    if (pnv->cpld_irqstate != old_state) {
> +        pnv_psi_irq_set(lpc->psi, PSIHB_IRQ_EXTERNAL, pnv->cpld_irqstate != 0);
> +    }
> +}
> +
> +static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level)
> +{
> +    PnvLpcController *lpc = opaque;
> +
> +    /* The Naples HW latches the 1 levels, clearing is done by SW */
> +    if (level) {
> +        lpc->lpc_hc_irqstat |= LPC_HC_IRQ_SERIRQ0 >> n;
> +        pnv_lpc_eval_irqs(lpc);
> +    }
> +}
> +
> +qemu_irq *pnv_lpc_isa_irq_create(PnvLpcController *lpc, int chip_type,
> +                                 int nirqs)
> +{
> +    /* Not all variants have a working serial irq decoder. If not,
> +     * handling of LPC interrupts becomes a platform issue (some
> +     * platforms have a CPLD to do it).
> +     */
> +    if (chip_type == PNV_CHIP_POWER8NVL) {
> +        return qemu_allocate_irqs(pnv_lpc_isa_irq_handler, lpc, nirqs);
> +    } else {
> +        return qemu_allocate_irqs(pnv_lpc_isa_irq_handler_cpld, lpc, nirqs);
> +    }
> +}
> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
> index 38e5506975aa..ccf969af9448 100644
> --- a/include/hw/ppc/pnv_lpc.h
> +++ b/include/hw/ppc/pnv_lpc.h
> @@ -23,6 +23,8 @@
>  #define PNV_LPC(obj) \
>       OBJECT_CHECK(PnvLpcController, (obj), TYPE_PNV_LPC)
>  
> +typedef struct PnvPsi PnvPsi;
> +
>  typedef struct PnvLpcController {
>      DeviceState parent;
>  
> @@ -62,6 +64,12 @@ typedef struct PnvLpcController {
>  
>      /* XSCOM registers */
>      MemoryRegion xscom_regs;
> +
> +    /* PSI to generate interrupts */
> +    PnvPsi *psi;
>  } PnvLpcController;
>  
> +qemu_irq *pnv_lpc_isa_irq_create(PnvLpcController *lpc, int chip_type,
> +                                 int nirqs);
> +
>  #endif /* _PPC_PNV_LPC_H */

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

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

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

* Re: [Qemu-devel] [PATCH v2 2/8] ppc/pnv: enable only one LPC bus
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 2/8] ppc/pnv: enable only one LPC bus Cédric Le Goater
@ 2017-04-11  2:40   ` David Gibson
  2017-04-11  7:06     ` Cédric Le Goater
  0 siblings, 1 reply; 16+ messages in thread
From: David Gibson @ 2017-04-11  2:40 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel

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

On Mon, Apr 10, 2017 at 03:56:52PM +0200, Cédric Le Goater wrote:
> The firmware (skiboot) chooses the default LPC bus of a multichip
> systems using a "primary" property. The LPC bus of chip 0 should be
> the only connected in the system. Let's advertise it in the device
> tree.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  Changes since v1:
> 
>  - the device tree is populated for all LPC busses of the system but
>    only the one on chip 0 has the "primary" property.
> 
>  hw/ppc/pnv.c             |  2 ++
>  hw/ppc/pnv_lpc.c         | 23 ++++++++++++++---------
>  include/hw/ppc/pnv_lpc.h |  2 ++
>  3 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 27589b91d1cf..7d742b6e34e1 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -765,6 +765,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
>      g_free(typename);
>  
>      /* Create LPC controller */
> +    object_property_set_int(OBJECT(&chip->lpc), chip->chip_id, "chip-id",
> +                            &error_fatal);
>      object_property_set_bool(OBJECT(&chip->lpc), true, "realized",
>                               &error_fatal);
>      pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, &chip->lpc.xscom_regs);
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index baee366d386a..13d7a695678d 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -92,14 +92,6 @@ enum {
>  #define LPC_HC_REGS_OPB_SIZE    0x00001000
>  
>  
> -/*
> - * TODO: the "primary" cell should only be added on chip 0. This is
> - * how skiboot chooses the default LPC controller on multichip
> - * systems.
> - *
> - * It would be easly done if we can change the populate() interface to
> - * replace the PnvXScomInterface parameter by a PnvChip one
> - */
>  static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>  {
>      const char compat[] = "ibm,power8-lpc\0ibm,lpc";
> @@ -110,6 +102,7 @@ static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>          cpu_to_be32(lpc_pcba),
>          cpu_to_be32(PNV_XSCOM_LPC_SIZE)
>      };
> +    PnvLpcController *lpc = PNV_LPC(dev);
>  
>      name = g_strdup_printf("isa@%x", lpc_pcba);
>      offset = fdt_add_subnode(fdt, xscom_offset, name);
> @@ -119,7 +112,13 @@ static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>      _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
>      _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 2)));
>      _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 1)));
> -    _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
> +
> +    /* The firmware (skiboot) chooses the default LPC bus of the
> +     * system using a "primary" property.
> +     */
> +    if (lpc->chip_id == 0x0) {
> +        _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
> +    }

So, the choice of primary bus is really a machine level thing, rather
than chip level.

So I think it would make more sense for the machine to poke the
'primary' property into the device tree afterwards, rather than adding
it initially within the chip/LPC code.  That will then avoid having to
pass the chip-id property into the LPC.

>      _FDT((fdt_setprop(fdt, offset, "compatible", compat, sizeof(compat))));
>      return 0;
>  }
> @@ -486,6 +485,11 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>      lpc->psi = PNV_PSI(obj);
>  }
>  
> +static Property pnv_lpc_properties[] = {
> +    DEFINE_PROP_UINT32("chip-id", PnvLpcController, chip_id, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void pnv_lpc_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -494,6 +498,7 @@ static void pnv_lpc_class_init(ObjectClass *klass, void *data)
>      xdc->populate = pnv_lpc_populate;
>  
>      dc->realize = pnv_lpc_realize;
> +    dc->props = pnv_lpc_properties;
>  }
>  
>  static const TypeInfo pnv_lpc_info = {
> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
> index ccf969af9448..c78ee4a98c62 100644
> --- a/include/hw/ppc/pnv_lpc.h
> +++ b/include/hw/ppc/pnv_lpc.h
> @@ -67,6 +67,8 @@ typedef struct PnvLpcController {
>  
>      /* PSI to generate interrupts */
>      PnvPsi *psi;
> +
> +    uint32_t chip_id;
>  } PnvLpcController;
>  
>  qemu_irq *pnv_lpc_isa_irq_create(PnvLpcController *lpc, int chip_type,

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

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

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

* Re: [Qemu-devel] [PATCH v2 6/8] ppc/pnv: populate device tree for IPMI BT devices
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 6/8] ppc/pnv: populate device tree for IPMI BT devices Cédric Le Goater
@ 2017-04-11  2:43   ` David Gibson
  0 siblings, 0 replies; 16+ messages in thread
From: David Gibson @ 2017-04-11  2:43 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel

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

On Mon, Apr 10, 2017 at 03:56:56PM +0200, Cédric Le Goater wrote:
> When an ipmi-bt device [1] is defined on the ISA bus, we need to
> populate the device tree with the object properties. Such devices are
> created with the command line options :
> 
>    -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03168.html
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes sinve v1:
> 
>  - reworked the assignement of the ISA IO base in the 'reg' array
>    property
>  
>  hw/ppc/pnv.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 7f2f9897f146..ec5d62a479b3 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -332,6 +332,39 @@ static void powernv_populate_serial(ISADevice *d, void *fdt, int lpc_off)
>      _FDT((fdt_setprop_string(fdt, node, "device_type", "serial")));
>  }
>  
> +static void powernv_populate_ipmi_bt(ISADevice *d, void *fdt, int lpc_off)
> +{
> +    const char compatible[] = "bt\0ipmi-bt";
> +    uint32_t io_base = 0x0;

Remove the initializer here.  It's always set below, and having the
initializer could suppress a useful warning if the code below was
rearranged.

> +    uint32_t io_regs[] = {
> +        cpu_to_be32(1),
> +        0, /* 'io_base' retrieved from the 'ioport' property of 'isa-ipmi-bt' */
> +        cpu_to_be32(3)
> +    };
> +    uint32_t irq;
> +    char *name;
> +    int node;
> +
> +    io_base = object_property_get_int(OBJECT(d), "ioport", &error_fatal);
> +    io_regs[1] = cpu_to_be32(io_base);
> +
> +    irq = object_property_get_int(OBJECT(d), "irq", &error_fatal);
> +
> +    name = g_strdup_printf("%s@i%x", qdev_fw_name(DEVICE(d)), io_base);
> +    node = fdt_add_subnode(fdt, lpc_off, name);
> +    _FDT(node);
> +    g_free(name);
> +
> +    fdt_setprop(fdt, node, "reg", io_regs, sizeof(io_regs));
> +    fdt_setprop(fdt, node, "compatible", compatible, sizeof(compatible));
> +
> +    /* Mark it as reserved to avoid Linux trying to claim it */
> +    _FDT((fdt_setprop_string(fdt, node, "status", "reserved")));
> +    _FDT((fdt_setprop_cell(fdt, node, "interrupts", irq)));
> +    _FDT((fdt_setprop_cell(fdt, node, "interrupt-parent",
> +                           fdt_get_phandle(fdt, lpc_off))));
> +}
> +
>  typedef struct ForeachPopulateArgs {
>      void *fdt;
>      int offset;
> @@ -346,6 +379,8 @@ static int powernv_populate_isa_device(DeviceState *dev, void *opaque)
>          powernv_populate_rtc(d, args->fdt, args->offset);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_ISA_SERIAL)) {
>          powernv_populate_serial(d, args->fdt, args->offset);
> +    } else if (object_dynamic_cast(OBJECT(dev), "isa-ipmi-bt")) {
> +        powernv_populate_ipmi_bt(d, args->fdt, args->offset);
>      } else {
>          error_report("unknown isa device %s@i%x", qdev_fw_name(dev),
>                       d->ioport_id);

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

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

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

* Re: [Qemu-devel] [PATCH v2 7/8] ppc/pnv: add initial IPMI sensors for the BMC simulator
  2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 7/8] ppc/pnv: add initial IPMI sensors for the BMC simulator Cédric Le Goater
@ 2017-04-11  2:44   ` David Gibson
  0 siblings, 0 replies; 16+ messages in thread
From: David Gibson @ 2017-04-11  2:44 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel

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

On Mon, Apr 10, 2017 at 03:56:57PM +0200, Cédric Le Goater wrote:
> Skiboot, the firmware for the PowerNV platform, expects the BMC to
> provide some specific IPMI sensors. These sensors are exposed in the
> device tree and their values are updated by the firmware at boot time.
> 
> Sensors of interest are :
> 
> 	"FW Boot Progress"
> 	"Boot Count"
> 
> As such a device is defined on the command line, we can only detect
> its presence at reset time.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  Changes since v1:
> 
>  - fixed test on sdr->header.rec_type which was incorrect
>  - changed the type of the 'bmc' attribute of the machine and changed
>    accordingly the prototype of the helper routines
> 
>  hw/ppc/Makefile.objs |  2 +-
>  hw/ppc/pnv.c         | 21 ++++++++++++++
>  hw/ppc/pnv_bmc.c     | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/pnv.h |  9 ++++++
>  4 files changed, 112 insertions(+), 1 deletion(-)
>  create mode 100644 hw/ppc/pnv_bmc.c
> 
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index ef67ea820158..7efc68674819 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -6,7 +6,7 @@ obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
>  obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
>  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o
>  # IBM PowerNV
> -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o
> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>  obj-y += spapr_pci_vfio.o
>  endif
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index ec5d62a479b3..bb9bb746e04a 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -35,6 +35,7 @@
>  #include "qapi/visitor.h"
>  #include "monitor/monitor.h"
>  #include "hw/intc/intc.h"
> +#include "hw/ipmi/ipmi.h"
>  
>  #include "hw/ppc/xics.h"
>  #include "hw/ppc/pnv_xscom.h"
> @@ -458,16 +459,36 @@ static void *powernv_create_fdt(MachineState *machine)
>      }
>  
>      powernv_populate_isa(pnv->isa_bus, fdt);
> +
> +    if (pnv->bmc) {
> +        pnv_bmc_populate_sensors(pnv->bmc, fdt);
> +    }
> +
>      return fdt;
>  }
>  
>  static void ppc_powernv_reset(void)
>  {
>      MachineState *machine = MACHINE(qdev_get_machine());
> +    PnvMachineState *pnv = POWERNV_MACHINE(machine);
>      void *fdt;
> +    Object *obj;
>  
>      qemu_devices_reset();
>  
> +    /* OpenPOWER systems have a BMC, which can be defined on the
> +     * command line with:
> +     *
> +     *   -device ipmi-bmc-sim,id=bmc0
> +     *
> +     * This is the internal simulator but it could also be an external
> +     * BMC.
> +     */
> +    obj = object_resolve_path_type("", TYPE_IPMI_BMC, NULL);
> +    if (obj) {
> +        pnv->bmc = IPMI_BMC(obj);
> +    }
> +
>      fdt = powernv_create_fdt(machine);
>  
>      /* Pack resulting tree */
> diff --git a/hw/ppc/pnv_bmc.c b/hw/ppc/pnv_bmc.c
> new file mode 100644
> index 000000000000..a0820dca559b
> --- /dev/null
> +++ b/hw/ppc/pnv_bmc.c
> @@ -0,0 +1,81 @@
> +/*
> + * QEMU PowerNV, BMC related functions
> + *
> + * Copyright (c) 2016-2017, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/hw.h"
> +#include "sysemu/sysemu.h"
> +#include "target/ppc/cpu.h"
> +#include "qapi/error.h"
> +#include "qemu/log.h"
> +#include "hw/ipmi/ipmi.h"
> +#include "hw/ppc/fdt.h"
> +
> +#include "hw/ppc/pnv.h"
> +
> +#include <libfdt.h>
> +
> +/* TODO: include definition in ipmi.h */
> +#define IPMI_SDR_FULL_TYPE 1
> +
> +void pnv_bmc_populate_sensors(IPMIBmc *bmc, void *fdt)
> +{
> +    int offset;
> +    int i;
> +    const struct ipmi_sdr_compact *sdr;
> +    uint16_t nextrec;
> +
> +    offset = fdt_add_subnode(fdt, 0, "/bmc");
> +    _FDT(offset);
> +
> +    _FDT((fdt_setprop_string(fdt, offset, "name", "bmc")));
> +    _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0x1)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 0x0)));
> +
> +    offset = fdt_add_subnode(fdt, offset, "sensors");
> +    _FDT(offset);
> +
> +    _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0x1)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 0x0)));
> +
> +    for (i = 0; !ipmi_bmc_sdr_find(bmc, i, &sdr, &nextrec); i++) {
> +        int off;
> +        char *name;
> +
> +        if (sdr->header.rec_type != IPMI_SDR_COMPACT_TYPE &&
> +            sdr->header.rec_type != IPMI_SDR_FULL_TYPE) {
> +            continue;
> +        }
> +
> +        name = g_strdup_printf("sensor@%x", sdr->sensor_owner_number);
> +        off = fdt_add_subnode(fdt, offset, name);
> +        _FDT(off);
> +        g_free(name);
> +
> +        _FDT((fdt_setprop_cell(fdt, off, "reg", sdr->sensor_owner_number)));
> +        _FDT((fdt_setprop_string(fdt, off, "name", "sensor")));
> +        _FDT((fdt_setprop_string(fdt, off, "compatible", "ibm,ipmi-sensor")));
> +        _FDT((fdt_setprop_cell(fdt, off, "ipmi-sensor-reading-type",
> +                               sdr->reading_type)));
> +        _FDT((fdt_setprop_cell(fdt, off, "ipmi-entity-id",
> +                               sdr->entity_id)));
> +        _FDT((fdt_setprop_cell(fdt, off, "ipmi-entity-instance",
> +                               sdr->entity_instance)));
> +        _FDT((fdt_setprop_cell(fdt, off, "ipmi-sensor-type",
> +                               sdr->sensor_type)));
> +    }
> +}
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index b45a0d91c813..02f6cf565ca4 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -118,6 +118,8 @@ typedef struct PnvChipClass {
>  #define POWERNV_MACHINE(obj) \
>      OBJECT_CHECK(PnvMachineState, (obj), TYPE_POWERNV_MACHINE)
>  
> +typedef struct IPMIBmc IPMIBmc;
> +
>  typedef struct PnvMachineState {
>      /*< private >*/
>      MachineState parent_obj;
> @@ -130,12 +132,19 @@ typedef struct PnvMachineState {
>  
>      ISABus       *isa_bus;
>      uint32_t     cpld_irqstate;
> +
> +    IPMIBmc      *bmc;
>  } PnvMachineState;
>  
>  #define PNV_FDT_ADDR          0x01000000
>  #define PNV_TIMEBASE_FREQ     512000000ULL
>  
>  /*
> + * BMC helpers
> + */
> +void pnv_bmc_populate_sensors(IPMIBmc *bmc, void *fdt);
> +
> +/*
>   * POWER8 MMIO base addresses
>   */
>  #define PNV_XSCOM_SIZE        0x800000000ull

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

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

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

* Re: [Qemu-devel] [PATCH v2 1/8] ppc/pnv: Add support for POWER8+ LPC Controller
  2017-04-11  2:23   ` David Gibson
@ 2017-04-11  6:31     ` Cédric Le Goater
  0 siblings, 0 replies; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-11  6:31 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Benjamin Herrenschmidt

On 04/11/2017 04:23 AM, David Gibson wrote:
> On Mon, Apr 10, 2017 at 03:56:51PM +0200, Cédric Le Goater wrote:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>
>> It adds the Naples chip which supports proper LPC interrupts via the
>> LPC controller rather than via an external CPLD.
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> [clg: - updated for qemu-2.9
>>       - ported on latest PowerNV patchset
>>       - moved the IRQ handler in pnv_lpc.c
>>       - introduced pnv_lpc_isa_irq_create() to create the ISA IRQs ]
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>>  Changes since v1:
>>
>>  - moved the IRQ handler in pnv_lpc.c
>>  - introduced pnv_lpc_isa_irq_create() to create the ISA IRQs 
>>
>>  hw/ppc/pnv.c             | 45 +++-------------------
>>  hw/ppc/pnv_lpc.c         | 97 +++++++++++++++++++++++++++++++++++++++++++++++-
>>  include/hw/ppc/pnv_lpc.h |  8 ++++
>>  3 files changed, 108 insertions(+), 42 deletions(-)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 16f32c9bbd9d..27589b91d1cf 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -346,36 +346,6 @@ static void ppc_powernv_reset(void)
>>      cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt));
>>  }
>>  
>> -/* If we don't use the built-in LPC interrupt deserializer, we need
>> - * to provide a set of qirqs for the ISA bus or things will go bad.
>> - *
>> - * Most machines using pre-Naples chips (without said deserializer)
>> - * have a CPLD that will collect the SerIRQ and shoot them as a
>> - * single level interrupt to the P8 chip. So let's setup a hook
>> - * for doing just that.
>> - */
>> -static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level)
>> -{
>> -    PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine());
>> -    uint32_t old_state = pnv->cpld_irqstate;
>> -    PnvChip *chip = opaque;
>> -
>> -    if (level) {
>> -        pnv->cpld_irqstate |= 1u << n;
>> -    } else {
>> -        pnv->cpld_irqstate &= ~(1u << n);
>> -    }
>> -    if (pnv->cpld_irqstate != old_state) {
>> -        pnv_psi_irq_set(&chip->psi, PSIHB_IRQ_EXTERNAL,
>> -                        pnv->cpld_irqstate != 0);
>> -    }
>> -}
>> -
>> -static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level)
>> -{
>> -     /* XXX TODO */
>> -}
>> -
>>  static ISABus *pnv_isa_create(PnvChip *chip)
>>  {
>>      PnvLpcController *lpc = &chip->lpc;
>> @@ -390,16 +360,7 @@ static ISABus *pnv_isa_create(PnvChip *chip)
>>      isa_bus = isa_bus_new(NULL, &lpc->isa_mem, &lpc->isa_io,
>>                            &error_fatal);
>>  
>> -    /* Not all variants have a working serial irq decoder. If not,
>> -     * handling of LPC interrupts becomes a platform issue (some
>> -     * platforms have a CPLD to do it).
>> -     */
>> -    if (pcc->chip_type == PNV_CHIP_POWER8NVL) {
>> -        irqs = qemu_allocate_irqs(pnv_lpc_isa_irq_handler, chip, ISA_NUM_IRQS);
>> -    } else {
>> -        irqs = qemu_allocate_irqs(pnv_lpc_isa_irq_handler_cpld, chip,
>> -                                  ISA_NUM_IRQS);
>> -    }
>> +    irqs = pnv_lpc_isa_irq_create(lpc, pcc->chip_type, ISA_NUM_IRQS);
>>  
>>      isa_bus_irqs(isa_bus, irqs);
>>      return isa_bus;
>> @@ -699,6 +660,10 @@ static void pnv_chip_init(Object *obj)
>>      object_property_add_child(obj, "occ", OBJECT(&chip->occ), NULL);
>>      object_property_add_const_link(OBJECT(&chip->occ), "psi",
>>                                     OBJECT(&chip->psi), &error_abort);
>> +
>> +    /* The LPC controller needs PSI to generate interrupts */
>> +    object_property_add_const_link(OBJECT(&chip->lpc), "psi",
>> +                                   OBJECT(&chip->psi), &error_abort);
>>  }
>>  
>>  static void pnv_chip_icp_realize(PnvChip *chip, Error **errp)
>> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
>> index 78db52415b11..baee366d386a 100644
>> --- a/hw/ppc/pnv_lpc.c
>> +++ b/hw/ppc/pnv_lpc.c
>> @@ -250,6 +250,34 @@ static const MemoryRegionOps pnv_lpc_xscom_ops = {
>>      .endianness = DEVICE_BIG_ENDIAN,
>>  };
>>  
>> +static void pnv_lpc_eval_irqs(PnvLpcController *lpc)
>> +{
>> +    bool lpc_to_opb_irq = false;
>> +
>> +    /* Update LPC controller to OPB line */
>> +    if (lpc->lpc_hc_irqser_ctrl & LPC_HC_IRQSER_EN) {
>> +        uint32_t irqs;
>> +
>> +        irqs = lpc->lpc_hc_irqstat & lpc->lpc_hc_irqmask;
>> +        lpc_to_opb_irq = (irqs != 0);
>> +    }
>> +
>> +    /* We don't honor the polarity register, it's pointless and unused
>> +     * anyway
>> +     */
>> +    if (lpc_to_opb_irq) {
>> +        lpc->opb_irq_input |= OPB_MASTER_IRQ_LPC;
>> +    } else {
>> +        lpc->opb_irq_input &= ~OPB_MASTER_IRQ_LPC;
>> +    }
>> +
>> +    /* Update OPB internal latch */
>> +    lpc->opb_irq_stat |= lpc->opb_irq_input & lpc->opb_irq_mask;
>> +
>> +    /* Reflect the interrupt */
>> +    pnv_psi_irq_set(lpc->psi, PSIHB_IRQ_LPC_I2C, lpc->opb_irq_stat != 0);
>> +}
>> +
>>  static uint64_t lpc_hc_read(void *opaque, hwaddr addr, unsigned size)
>>  {
>>      PnvLpcController *lpc = opaque;
>> @@ -300,12 +328,15 @@ static void lpc_hc_write(void *opaque, hwaddr addr, uint64_t val,
>>          break;
>>      case LPC_HC_IRQSER_CTRL:
>>          lpc->lpc_hc_irqser_ctrl = val;
>> +        pnv_lpc_eval_irqs(lpc);
>>          break;
>>      case LPC_HC_IRQMASK:
>>          lpc->lpc_hc_irqmask = val;
>> +        pnv_lpc_eval_irqs(lpc);
>>          break;
>>      case LPC_HC_IRQSTAT:
>>          lpc->lpc_hc_irqstat &= ~val;
>> +        pnv_lpc_eval_irqs(lpc);
>>          break;
>>      case LPC_HC_ERROR_ADDRESS:
>>          break;
>> @@ -363,14 +394,15 @@ static void opb_master_write(void *opaque, hwaddr addr,
>>      switch (addr) {
>>      case OPB_MASTER_LS_IRQ_STAT:
>>          lpc->opb_irq_stat &= ~val;
>> +        pnv_lpc_eval_irqs(lpc);
>>          break;
>>      case OPB_MASTER_LS_IRQ_MASK:
>> -        /* XXX Filter out reserved bits */
>>          lpc->opb_irq_mask = val;
>> +        pnv_lpc_eval_irqs(lpc);
>>          break;
>>      case OPB_MASTER_LS_IRQ_POL:
>> -        /* XXX Filter out reserved bits */
>>          lpc->opb_irq_pol = val;
>> +        pnv_lpc_eval_irqs(lpc);
>>          break;
>>      case OPB_MASTER_LS_IRQ_INPUT:
>>          /* Read only */
>> @@ -398,6 +430,8 @@ static const MemoryRegionOps opb_master_ops = {
>>  static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>>  {
>>      PnvLpcController *lpc = PNV_LPC(dev);
>> +    Object *obj;
>> +    Error *error = NULL;
>>  
>>      /* Reg inits */
>>      lpc->lpc_hc_fw_rd_acc_size = LPC_HC_FW_RD_4B;
>> @@ -441,6 +475,15 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>>      pnv_xscom_region_init(&lpc->xscom_regs, OBJECT(dev),
>>                            &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
>>                            PNV_XSCOM_LPC_SIZE);
>> +
>> +    /* get PSI object from chip */
>> +    obj = object_property_get_link(OBJECT(dev), "psi", &error);
>> +    if (!obj) {
>> +        error_setg(errp, "%s: required link 'psi' not found: %s",
>> +                   __func__, error_get_pretty(error));
>> +        return;
>> +    }
>> +    lpc->psi = PNV_PSI(obj);
>>  }
>>  
>>  static void pnv_lpc_class_init(ObjectClass *klass, void *data)
>> @@ -470,3 +513,53 @@ static void pnv_lpc_register_types(void)
>>  }
>>  
>>  type_init(pnv_lpc_register_types)
>> +
>> +/* If we don't use the built-in LPC interrupt deserializer, we need
>> + * to provide a set of qirqs for the ISA bus or things will go bad.
>> + *
>> + * Most machines using pre-Naples chips (without said deserializer)
>> + * have a CPLD that will collect the SerIRQ and shoot them as a
>> + * single level interrupt to the P8 chip. So let's setup a hook
>> + * for doing just that.
>> + */
>> +static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level)
>> +{
>> +    PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine());
>> +    uint32_t old_state = pnv->cpld_irqstate;
>> +    PnvLpcController *lpc = opaque;
> 
> You can use the existing PNV_LPC() and similar macros here, rather
> than hard casting from a void *.  That's a bit safer.

ah yes. I missed that. 

Thanks,

C. 

>> +
>> +    if (level) {
>> +        pnv->cpld_irqstate |= 1u << n;
>> +    } else {
>> +        pnv->cpld_irqstate &= ~(1u << n);
>> +    }
>> +
>> +    if (pnv->cpld_irqstate != old_state) {
>> +        pnv_psi_irq_set(lpc->psi, PSIHB_IRQ_EXTERNAL, pnv->cpld_irqstate != 0);
>> +    }
>> +}
>> +
>> +static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level)
>> +{
>> +    PnvLpcController *lpc = opaque;
>> +
>> +    /* The Naples HW latches the 1 levels, clearing is done by SW */
>> +    if (level) {
>> +        lpc->lpc_hc_irqstat |= LPC_HC_IRQ_SERIRQ0 >> n;
>> +        pnv_lpc_eval_irqs(lpc);
>> +    }
>> +}
>> +
>> +qemu_irq *pnv_lpc_isa_irq_create(PnvLpcController *lpc, int chip_type,
>> +                                 int nirqs)
>> +{
>> +    /* Not all variants have a working serial irq decoder. If not,
>> +     * handling of LPC interrupts becomes a platform issue (some
>> +     * platforms have a CPLD to do it).
>> +     */
>> +    if (chip_type == PNV_CHIP_POWER8NVL) {
>> +        return qemu_allocate_irqs(pnv_lpc_isa_irq_handler, lpc, nirqs);
>> +    } else {
>> +        return qemu_allocate_irqs(pnv_lpc_isa_irq_handler_cpld, lpc, nirqs);
>> +    }
>> +}
>> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
>> index 38e5506975aa..ccf969af9448 100644
>> --- a/include/hw/ppc/pnv_lpc.h
>> +++ b/include/hw/ppc/pnv_lpc.h
>> @@ -23,6 +23,8 @@
>>  #define PNV_LPC(obj) \
>>       OBJECT_CHECK(PnvLpcController, (obj), TYPE_PNV_LPC)
>>  
>> +typedef struct PnvPsi PnvPsi;
>> +
>>  typedef struct PnvLpcController {
>>      DeviceState parent;
>>  
>> @@ -62,6 +64,12 @@ typedef struct PnvLpcController {
>>  
>>      /* XSCOM registers */
>>      MemoryRegion xscom_regs;
>> +
>> +    /* PSI to generate interrupts */
>> +    PnvPsi *psi;
>>  } PnvLpcController;
>>  
>> +qemu_irq *pnv_lpc_isa_irq_create(PnvLpcController *lpc, int chip_type,
>> +                                 int nirqs);
>> +
>>  #endif /* _PPC_PNV_LPC_H */
> 

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

* Re: [Qemu-devel] [PATCH v2 2/8] ppc/pnv: enable only one LPC bus
  2017-04-11  2:40   ` David Gibson
@ 2017-04-11  7:06     ` Cédric Le Goater
  2017-04-11 10:19       ` David Gibson
  0 siblings, 1 reply; 16+ messages in thread
From: Cédric Le Goater @ 2017-04-11  7:06 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel

On 04/11/2017 04:40 AM, David Gibson wrote:
> On Mon, Apr 10, 2017 at 03:56:52PM +0200, Cédric Le Goater wrote:
>> The firmware (skiboot) chooses the default LPC bus of a multichip
>> systems using a "primary" property. The LPC bus of chip 0 should be
>> the only connected in the system. Let's advertise it in the device
>> tree.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  Changes since v1:
>>
>>  - the device tree is populated for all LPC busses of the system but
>>    only the one on chip 0 has the "primary" property.
>>
>>  hw/ppc/pnv.c             |  2 ++
>>  hw/ppc/pnv_lpc.c         | 23 ++++++++++++++---------
>>  include/hw/ppc/pnv_lpc.h |  2 ++
>>  3 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 27589b91d1cf..7d742b6e34e1 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -765,6 +765,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
>>      g_free(typename);
>>  
>>      /* Create LPC controller */
>> +    object_property_set_int(OBJECT(&chip->lpc), chip->chip_id, "chip-id",
>> +                            &error_fatal);
>>      object_property_set_bool(OBJECT(&chip->lpc), true, "realized",
>>                               &error_fatal);
>>      pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, &chip->lpc.xscom_regs);
>> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
>> index baee366d386a..13d7a695678d 100644
>> --- a/hw/ppc/pnv_lpc.c
>> +++ b/hw/ppc/pnv_lpc.c
>> @@ -92,14 +92,6 @@ enum {
>>  #define LPC_HC_REGS_OPB_SIZE    0x00001000
>>  
>>  
>> -/*
>> - * TODO: the "primary" cell should only be added on chip 0. This is
>> - * how skiboot chooses the default LPC controller on multichip
>> - * systems.
>> - *
>> - * It would be easly done if we can change the populate() interface to
>> - * replace the PnvXScomInterface parameter by a PnvChip one
>> - */
>>  static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>>  {
>>      const char compat[] = "ibm,power8-lpc\0ibm,lpc";
>> @@ -110,6 +102,7 @@ static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>>          cpu_to_be32(lpc_pcba),
>>          cpu_to_be32(PNV_XSCOM_LPC_SIZE)
>>      };
>> +    PnvLpcController *lpc = PNV_LPC(dev);
>>  
>>      name = g_strdup_printf("isa@%x", lpc_pcba);
>>      offset = fdt_add_subnode(fdt, xscom_offset, name);
>> @@ -119,7 +112,13 @@ static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>>      _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
>>      _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 2)));
>>      _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 1)));
>> -    _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
>> +
>> +    /* The firmware (skiboot) chooses the default LPC bus of the
>> +     * system using a "primary" property.
>> +     */
>> +    if (lpc->chip_id == 0x0) {
>> +        _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
>> +    }
> 
> So, the choice of primary bus is really a machine level thing, rather
> than chip level.
> 
> So I think it would make more sense for the machine to poke the
> 'primary' property into the device tree afterwards, rather than adding
> it initially within the chip/LPC code.  That will then avoid having to
> pass the chip-id property into the LPC.

ok. I will see how I can do that. I think that I will change 
pnv_xscom_populate() to return the xscom offset in the device
tree. From this node, I can look for the ISA bus and add the 
"primary" property if this is chip 0.

Thanks,

C. 

> 
>>      _FDT((fdt_setprop(fdt, offset, "compatible", compat, sizeof(compat))));
>>      return 0;
>>  }
>> @@ -486,6 +485,11 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>>      lpc->psi = PNV_PSI(obj);
>>  }
>>  
>> +static Property pnv_lpc_properties[] = {
>> +    DEFINE_PROP_UINT32("chip-id", PnvLpcController, chip_id, 0),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>>  static void pnv_lpc_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -494,6 +498,7 @@ static void pnv_lpc_class_init(ObjectClass *klass, void *data)
>>      xdc->populate = pnv_lpc_populate;
>>  
>>      dc->realize = pnv_lpc_realize;
>> +    dc->props = pnv_lpc_properties;
>>  }
>>  
>>  static const TypeInfo pnv_lpc_info = {
>> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
>> index ccf969af9448..c78ee4a98c62 100644
>> --- a/include/hw/ppc/pnv_lpc.h
>> +++ b/include/hw/ppc/pnv_lpc.h
>> @@ -67,6 +67,8 @@ typedef struct PnvLpcController {
>>  
>>      /* PSI to generate interrupts */
>>      PnvPsi *psi;
>> +
>> +    uint32_t chip_id;
>>  } PnvLpcController;
>>  
>>  qemu_irq *pnv_lpc_isa_irq_create(PnvLpcController *lpc, int chip_type,
> 

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

* Re: [Qemu-devel] [PATCH v2 2/8] ppc/pnv: enable only one LPC bus
  2017-04-11  7:06     ` Cédric Le Goater
@ 2017-04-11 10:19       ` David Gibson
  0 siblings, 0 replies; 16+ messages in thread
From: David Gibson @ 2017-04-11 10:19 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel

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

On Tue, Apr 11, 2017 at 09:06:16AM +0200, Cédric Le Goater wrote:
> On 04/11/2017 04:40 AM, David Gibson wrote:
> > On Mon, Apr 10, 2017 at 03:56:52PM +0200, Cédric Le Goater wrote:
> >> The firmware (skiboot) chooses the default LPC bus of a multichip
> >> systems using a "primary" property. The LPC bus of chip 0 should be
> >> the only connected in the system. Let's advertise it in the device
> >> tree.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> ---
> >>  Changes since v1:
> >>
> >>  - the device tree is populated for all LPC busses of the system but
> >>    only the one on chip 0 has the "primary" property.
> >>
> >>  hw/ppc/pnv.c             |  2 ++
> >>  hw/ppc/pnv_lpc.c         | 23 ++++++++++++++---------
> >>  include/hw/ppc/pnv_lpc.h |  2 ++
> >>  3 files changed, 18 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> >> index 27589b91d1cf..7d742b6e34e1 100644
> >> --- a/hw/ppc/pnv.c
> >> +++ b/hw/ppc/pnv.c
> >> @@ -765,6 +765,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
> >>      g_free(typename);
> >>  
> >>      /* Create LPC controller */
> >> +    object_property_set_int(OBJECT(&chip->lpc), chip->chip_id, "chip-id",
> >> +                            &error_fatal);
> >>      object_property_set_bool(OBJECT(&chip->lpc), true, "realized",
> >>                               &error_fatal);
> >>      pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, &chip->lpc.xscom_regs);
> >> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> >> index baee366d386a..13d7a695678d 100644
> >> --- a/hw/ppc/pnv_lpc.c
> >> +++ b/hw/ppc/pnv_lpc.c
> >> @@ -92,14 +92,6 @@ enum {
> >>  #define LPC_HC_REGS_OPB_SIZE    0x00001000
> >>  
> >>  
> >> -/*
> >> - * TODO: the "primary" cell should only be added on chip 0. This is
> >> - * how skiboot chooses the default LPC controller on multichip
> >> - * systems.
> >> - *
> >> - * It would be easly done if we can change the populate() interface to
> >> - * replace the PnvXScomInterface parameter by a PnvChip one
> >> - */
> >>  static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
> >>  {
> >>      const char compat[] = "ibm,power8-lpc\0ibm,lpc";
> >> @@ -110,6 +102,7 @@ static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
> >>          cpu_to_be32(lpc_pcba),
> >>          cpu_to_be32(PNV_XSCOM_LPC_SIZE)
> >>      };
> >> +    PnvLpcController *lpc = PNV_LPC(dev);
> >>  
> >>      name = g_strdup_printf("isa@%x", lpc_pcba);
> >>      offset = fdt_add_subnode(fdt, xscom_offset, name);
> >> @@ -119,7 +112,13 @@ static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
> >>      _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
> >>      _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 2)));
> >>      _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 1)));
> >> -    _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
> >> +
> >> +    /* The firmware (skiboot) chooses the default LPC bus of the
> >> +     * system using a "primary" property.
> >> +     */
> >> +    if (lpc->chip_id == 0x0) {
> >> +        _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
> >> +    }
> > 
> > So, the choice of primary bus is really a machine level thing, rather
> > than chip level.
> > 
> > So I think it would make more sense for the machine to poke the
> > 'primary' property into the device tree afterwards, rather than adding
> > it initially within the chip/LPC code.  That will then avoid having to
> > pass the chip-id property into the LPC.
> 
> ok. I will see how I can do that. I think that I will change 
> pnv_xscom_populate() to return the xscom offset in the device
> tree. From this node, I can look for the ISA bus and add the 
> "primary" property if this is chip 0.

I wouldn't even bother storing the offset, which may not be safe
anyway.  You can just look it up again by path from the machine.

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

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

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

end of thread, other threads:[~2017-04-11 10:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-10 13:56 [Qemu-devel] [PATCH v2 0/8] pnv: improvement of LPC support and IPMI support Cédric Le Goater
2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 1/8] ppc/pnv: Add support for POWER8+ LPC Controller Cédric Le Goater
2017-04-11  2:23   ` David Gibson
2017-04-11  6:31     ` Cédric Le Goater
2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 2/8] ppc/pnv: enable only one LPC bus Cédric Le Goater
2017-04-11  2:40   ` David Gibson
2017-04-11  7:06     ` Cédric Le Goater
2017-04-11 10:19       ` David Gibson
2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 3/8] ppc/pnv: scan ISA bus to populate device tree Cédric Le Goater
2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 4/8] ppc/pnv: populate device tree for RTC devices Cédric Le Goater
2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 5/8] ppc/pnv: populate device tree for serial devices Cédric Le Goater
2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 6/8] ppc/pnv: populate device tree for IPMI BT devices Cédric Le Goater
2017-04-11  2:43   ` David Gibson
2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 7/8] ppc/pnv: add initial IPMI sensors for the BMC simulator Cédric Le Goater
2017-04-11  2:44   ` David Gibson
2017-04-10 13:56 ` [Qemu-devel] [PATCH v2 8/8] ppc/pnv: generate an OEM SEL event on shutdown Cédric Le Goater

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