[RFC 0/1] hw/ipmi: add an aspeed IPMI iBT device model

[RFC 0/1] hw/ipmi: add an aspeed IPMI iBT device model

[Titus Rwantare]

This patch follows the Handing IPMI for emulating BMC patch set by Hao Wu.
Building on top of the work in [PATCH] hw/misc: Add an iBT device model posted
by Cédric Le Goater, this iBT model works as a backend to ipmi-host-extern.

Needed to run:
- [PATCH 4/8] hw/ipmi: Refactor IPMI interface, Hao Wu
- [PATCH 5/8] hw/ipmi: Take out common from ipmi_bmc_extern.c, Hao Wu
- [PATCH 6/8] hw/ipmi: Move handle_command to IPMICoreClass, Hao Wu
- [PATCH 7/8] hw/ipmi: Add an IPMI external host device, Hao Wu

Host commandline
    -chardev socket,id=ipmichr1,host=localhost,port=9999,reconnect=10 \
    -device ipmi-bmc-extern,chardev=ipmichr1,id=bmc0 \
    -device isa-ipmi-bt,bmc=bmc0,irq=10 -nodefaults

BMC commandline
    -chardev socket,id=ipmichr1,host=localhost,port=9999,server=on,wait=off \
    -device "ipmi-host-extern,chardev=ipmichr1,responder=/machine/soc/ibt"

Currently, IRQs are not functional, but it is able to connect.
The following printout is from the host:

[    6.869742] ipmi_si IPI0001:00: IPMI message handler: Found new BMC (man_id:
 0x002b79, prod_id: 0x0000, dev_id: 0x00)
[   12.393281] ipmi_si IPI0001:00: IPMI bt interface initialized

This patch is an RFC because of the missing IRQs and the need for other patches
to get merged.

Thank you all.

Titus Rwantare (1):
  hw/ipmi: add an aspeed ipmi iBT device model

 hw/ipmi/ipmi_extern.h        |   1 +
 include/hw/arm/aspeed_soc.h  |   2 +
 include/hw/ipmi/aspeed_ibt.h |  77 +++++++++
 hw/arm/aspeed_ast2600.c      |  12 ++
 hw/arm/aspeed_soc.c          |  12 ++
 hw/ipmi/aspeed_ibt.c         | 311 +++++++++++++++++++++++++++++++++++
 hw/ipmi/meson.build          |   1 +
 hw/ipmi/trace-events         |   7 +
 8 files changed, 423 insertions(+)
 create mode 100644 include/hw/ipmi/aspeed_ibt.h
 create mode 100644 hw/ipmi/aspeed_ibt.c

-- 
2.33.0.800.g4c38ced690-goog

[RFC 1/1] hw/ipmi: add an aspeed ipmi iBT device model

[Titus Rwantare]

Modifies [PATCH] hw/misc: Add an iBT device model
posted by Cédric Le Goater, to use IPMIInterface.

Signed-off-by: Titus Rwantare <titusr@google.com>
---
 hw/ipmi/ipmi_extern.h        |   1 +
 include/hw/arm/aspeed_soc.h  |   2 +
 include/hw/ipmi/aspeed_ibt.h |  77 +++++++++
 hw/arm/aspeed_ast2600.c      |  12 ++
 hw/arm/aspeed_soc.c          |  12 ++
 hw/ipmi/aspeed_ibt.c         | 311 +++++++++++++++++++++++++++++++++++
 hw/ipmi/meson.build          |   1 +
 hw/ipmi/trace-events         |   7 +
 8 files changed, 423 insertions(+)
 create mode 100644 include/hw/ipmi/aspeed_ibt.h
 create mode 100644 hw/ipmi/aspeed_ibt.c

diff --git a/hw/ipmi/ipmi_extern.h b/hw/ipmi/ipmi_extern.h
index e4aa80a0f6..40157ab0b3 100644
--- a/hw/ipmi/ipmi_extern.h
+++ b/hw/ipmi/ipmi_extern.h
@@ -27,6 +27,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/ipmi/ipmi.h"
+#include "chardev/char-fe.h"
 
 #define VM_MSG_CHAR        0xA0 /* Marks end of message */
 #define VM_CMD_CHAR        0xA1 /* Marks end of a command */
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 87d76c9259..f650ff83ae 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -30,6 +30,7 @@
 #include "hw/usb/hcd-ehci.h"
 #include "qom/object.h"
 #include "hw/misc/aspeed_lpc.h"
+#include "hw/ipmi/aspeed_ibt.h"
 
 #define ASPEED_SPIS_NUM  2
 #define ASPEED_EHCIS_NUM 2
@@ -65,6 +66,7 @@ struct AspeedSoCState {
     AspeedSDHCIState sdhci;
     AspeedSDHCIState emmc;
     AspeedLPCState lpc;
+    AspeedIBTState ibt;
     uint32_t uart_default;
 };
 
diff --git a/include/hw/ipmi/aspeed_ibt.h b/include/hw/ipmi/aspeed_ibt.h
new file mode 100644
index 0000000000..d90cc103d4
--- /dev/null
+++ b/include/hw/ipmi/aspeed_ibt.h
@@ -0,0 +1,77 @@
+/*
+ * ASPEED iBT Device
+ *
+ * Copyright (c) 2016-2021 Cédric Le Goater, IBM Corporation.
+ * Copyright 2021 Google LLC
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef ASPEED_IBT_H
+#define ASPEED_IBT_H
+
+#include "hw/ipmi/ipmi.h"
+#include "hw/sysbus.h"
+
+#define TYPE_ASPEED_IBT "aspeed.ibt"
+#define ASPEED_IBT(obj) OBJECT_CHECK(AspeedIBTState, (obj), TYPE_ASPEED_IBT)
+
+#define ASPEED_IBT_NR_REGS 7
+
+#define ASPEED_IBT_BUFFER_SIZE 64
+
+#define ASPEED_IBT_IO_REGION_SIZE 0x1C
+
+#define TO_REG(o) (o >> 2)
+
+/* from linux/char/ipmi/bt-bmc. */
+#define BT_CR0      0x0
+#define   BT_CR0_IO_BASE        16
+#define   BT_CR0_IRQ            12
+#define   BT_CR0_EN_CLR_SLV_RDP 0x8
+#define   BT_CR0_EN_CLR_SLV_WRP 0x4
+#define   BT_CR0_ENABLE_IBT     0x1
+#define BT_CR1      0x4
+#define   BT_CR1_IRQ_H2B        0x01
+#define   BT_CR1_IRQ_HBUSY      0x40
+#define BT_CR2      0x8
+#define   BT_CR2_IRQ_H2B        0x01
+#define   BT_CR2_IRQ_HBUSY      0x40
+#define BT_CR3      0xc
+#define BT_CTRL     0x10
+#define   BT_CTRL_B_BUSY        BIT(7) /* BMC is busy */
+#define   BT_CTRL_H_BUSY        BIT(6) /* Host is busy */
+#define   BT_CTRL_OEM0          BIT(5)
+#define   BT_CTRL_SMS_ATN       BIT(4) /* SMS/EVT Attention */
+#define   BT_CTRL_B2H_ATN       BIT(3) /* BMC to Host Attention */
+#define   BT_CTRL_H2B_ATN       BIT(2) /* Host to BMC Attention */
+#define   BT_CTRL_CLR_RD_PTR    BIT(1) /* Clear Read Ptr */
+#define   BT_CTRL_CLR_WR_PTR    BIT(0) /* Clear Write Ptr */
+#define BT_BMC2HOST 0x14
+#define BT_HOST2BMC 0x14
+#define BT_INTMASK  0x18
+#define   BT_INTMASK_BMC_HWRST      BIT(7)
+#define   BT_INTMASK_B2H_IRQ        BIT(1)
+#define   BT_INTMASK_B2H_IRQEN      BIT(0)
+
+typedef struct AspeedIBTState {
+    SysBusDevice parent;
+
+    IPMICore *handler;
+
+    uint8_t msg_id;
+    uint8_t recv_msg[ASPEED_IBT_BUFFER_SIZE];
+    uint8_t recv_msg_len;
+    int recv_msg_index;
+
+    uint8_t send_msg[ASPEED_IBT_BUFFER_SIZE];
+    uint8_t send_msg_len;
+
+    MemoryRegion iomem;
+    qemu_irq irq;
+
+    uint32_t regs[ASPEED_IBT_NR_REGS];
+} AspeedIBTState;
+
+
+#endif /* ASPEED_IBT_H */
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 9d70e8e060..f40fa69a53 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -216,6 +216,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
 
     snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
     object_initialize_child(obj, "hace", &s->hace, typename);
+
+    object_initialize_child(obj, "ibt", &s->ibt, TYPE_ASPEED_IBT);
 }
 
 /*
@@ -507,6 +509,16 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->hace), 0, sc->memmap[ASPEED_DEV_HACE]);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->hace), 0,
                        aspeed_soc_get_irq(s, ASPEED_DEV_HACE));
+
+    /* iBT */
+    if (!sysbus_realize(SYS_BUS_DEVICE(&s->ibt), errp)) {
+        return;
+    }
+    memory_region_add_subregion(&s->lpc.iomem,
+                   sc->memmap[ASPEED_DEV_IBT] - sc->memmap[ASPEED_DEV_LPC],
+                   &s->ibt.iomem);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->ibt), 0,
+                       aspeed_soc_get_irq(s, ASPEED_DEV_IBT));
 }
 
 static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index ed84502e23..13543d37c5 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -216,6 +216,8 @@ static void aspeed_soc_init(Object *obj)
 
     snprintf(typename, sizeof(typename), "aspeed.hace-%s", socname);
     object_initialize_child(obj, "hace", &s->hace, typename);
+
+    object_initialize_child(obj, "ibt", &s->ibt, TYPE_ASPEED_IBT);
 }
 
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
@@ -426,6 +428,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_kcs_4,
                        qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_kcs_4));
 
+    /* iBT */
+    if (!sysbus_realize(SYS_BUS_DEVICE(&s->ibt), errp)) {
+        return;
+    }
+    memory_region_add_subregion(&s->lpc.iomem,
+                   sc->memmap[ASPEED_DEV_IBT] - sc->memmap[ASPEED_DEV_LPC],
+                   &s->ibt.iomem);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->lpc), 1 + aspeed_lpc_ibt,
+                       qdev_get_gpio_in(DEVICE(&s->lpc), aspeed_lpc_ibt));
+
     /* HACE */
     object_property_set_link(OBJECT(&s->hace), "dram", OBJECT(s->dram_mr),
                              &error_abort);
diff --git a/hw/ipmi/aspeed_ibt.c b/hw/ipmi/aspeed_ibt.c
new file mode 100644
index 0000000000..94503da1b0
--- /dev/null
+++ b/hw/ipmi/aspeed_ibt.c
@@ -0,0 +1,311 @@
+/*
+ * ASPEED iBT Device
+ *
+ * Copyright (c) 2016-2021 Cédric Le Goater, IBM Corporation.
+ * Copyright 2021 Google LLC
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-properties-system.h"
+#include "hw/ipmi/aspeed_ibt.h"
+#include "hw/ipmi/ipmi_extern.h"
+#include "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/bitops.h"
+#include "qemu/error-report.h"
+#include "qemu/log.h"
+#include "sysemu/qtest.h"
+#include "sysemu/sysemu.h"
+#include "trace.h"
+
+static void aspeed_ibt_dump_msg(const char *func, unsigned char *msg,
+                                unsigned int len)
+{
+    if (trace_event_get_state_backends(TRACE_ASPEED_IBT_CHR_DUMP_MSG)) {
+        int size = len * 3 + 1;
+        char tmp[size];
+        int i, n = 0;
+
+        for (i = 0; i < len; i++) {
+            n += snprintf(tmp + n, size - n, "%02x:", msg[i]);
+        }
+        tmp[size - 1] = 0;
+
+        trace_aspeed_ibt_chr_dump_msg(func, tmp, len);
+    }
+}
+
+/* convert and send IPMI message from BMC to remote host */
+static void aspeed_ibt_b2h_write(AspeedIBTState *ibt)
+{
+    IPMICoreClass *hk = IPMI_CORE_GET_CLASS(ibt->handler);
+
+    aspeed_ibt_dump_msg(__func__, ibt->send_msg, ibt->send_msg_len);
+    /* separate seq from raw contents */
+    ibt->msg_id = ibt->send_msg[2];
+    ibt->send_msg[2] = ibt->send_msg[1];
+    /* length and seq not included in vm message */
+    hk->handle_command(ibt->handler, ibt->send_msg + 2,
+                        ibt->send_msg_len - 2, MAX_IPMI_MSG_SIZE,
+                        ibt->msg_id);
+}
+
+
+/* BMC firmware writing to memory region */
+static void aspeed_ibt_write(void *opaque, hwaddr offset, uint64_t data,
+                             uint32_t size)
+{
+    AspeedIBTState *ibt = ASPEED_IBT(opaque);
+
+    trace_aspeed_ibt_write(offset, data);
+    switch (offset) {
+    case BT_CTRL:
+        /* CLR_WR_PTR: cleared before a message is written */
+        if (data & BT_CTRL_CLR_WR_PTR) {
+            memset(ibt->send_msg, 0, sizeof(ibt->send_msg));
+            ibt->send_msg_len = 0;
+            trace_aspeed_ibt_event("CLR_WR_PTR");
+        }
+
+        /* CLR_RD_PTR: cleared before a message is read */
+        else if (data & BT_CTRL_CLR_RD_PTR) {
+            ibt->recv_msg_index = -1;
+            trace_aspeed_ibt_event("CLR_RD_PTR");
+        }
+
+        /*
+         * H2B_ATN: raised by host to end message, cleared by BMC
+         * before reading message
+         */
+        else if (data & BT_CTRL_H2B_ATN) {
+            ibt->regs[TO_REG(BT_CTRL)] &= ~BT_CTRL_H2B_ATN;
+            trace_aspeed_ibt_event("H2B_ATN");
+        }
+
+        /* B_BUSY: raised and cleared by BMC when message is read */
+        else if (data & BT_CTRL_B_BUSY) {
+            ibt->regs[TO_REG(BT_CTRL)] ^= BT_CTRL_B_BUSY;
+            trace_aspeed_ibt_event("B_BUSY");
+        }
+
+        /*
+         * B2H_ATN: raised by BMC and cleared by host
+         *
+         * Also simulate the host busy bit which is set while the host
+         * is reading the message from the BMC
+         */
+        else if (data & BT_CTRL_B2H_ATN) {
+            trace_aspeed_ibt_event("B2H_ATN");
+            ibt->regs[TO_REG(BT_CTRL)] |= (BT_CTRL_B2H_ATN | BT_CTRL_H_BUSY);
+
+            aspeed_ibt_b2h_write(ibt);
+
+            ibt->regs[TO_REG(BT_CTRL)] &= ~(BT_CTRL_B2H_ATN | BT_CTRL_H_BUSY);
+        }
+
+        /* Anything else is unexpected */
+        else {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected CTRL setting\n",
+                          __func__);
+        }
+
+        /* Message was read by BMC. we can reset the receive state */
+        if (!(ibt->regs[TO_REG(BT_CTRL)] & BT_CTRL_B_BUSY)) {
+            trace_aspeed_ibt_event("B_BUSY cleared");
+            ibt->recv_msg_len = 0;
+        }
+        break;
+
+    case BT_BMC2HOST:
+        if (ibt->send_msg_len < sizeof(ibt->send_msg)) {
+            trace_aspeed_ibt_event("BMC2HOST");
+            ibt->send_msg[ibt->send_msg_len++] = data & 0xff;
+        }
+        break;
+
+    case BT_CR0: /* TODO: iBT config */
+    case BT_CR1: /* interrupt enable */
+    case BT_CR3: /* unused */
+    case BT_INTMASK:
+        ibt->regs[TO_REG(offset)] = (uint32_t) data;
+        break;
+
+    case BT_CR2: /* interrupt status. writing 1 clears. */
+        ibt->regs[TO_REG(offset)] ^= (uint32_t) data;
+        qemu_irq_lower(ibt->irq);
+        break;
+
+    default:
+        qemu_log_mask(LOG_UNIMP, "%s: not implemented 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        break;
+    }
+}
+
+/* BMC firmware reading from memory region */
+static uint64_t aspeed_ibt_read(void *opaque, hwaddr offset, unsigned size)
+{
+    AspeedIBTState *ibt = ASPEED_IBT(opaque);
+    uint64_t val = 0;
+
+    switch (offset) {
+    case BT_HOST2BMC: /* shares offset with B2H */
+        trace_aspeed_ibt_event("HOST2BMC");
+        /*
+         * The IPMI BT interface requires the first byte to be the
+         * length of the message
+         */
+        if (ibt->recv_msg_index == -1) {
+            val = ibt->recv_msg_len;
+            ibt->recv_msg_index++;
+        } else if (ibt->recv_msg_index < ibt->recv_msg_len) {
+            val = ibt->recv_msg[ibt->recv_msg_index++];
+        }
+        break;
+
+    case BT_CR0:
+    case BT_CR1:
+    case BT_CR2:
+    case BT_CR3:
+    case BT_CTRL:
+    case BT_INTMASK:
+        val = ibt->regs[TO_REG(offset)];
+        break;
+
+    default:
+        qemu_log_mask(LOG_UNIMP, "%s: not implemented 0x%" HWADDR_PRIx "\n",
+                      __func__, offset);
+        return 0;
+    }
+
+    trace_aspeed_ibt_read(offset, val);
+    return val;
+}
+
+/* send a request from a host to the BMC core */
+static void aspeed_ibt_handle_msg(IPMIInterface *ii, uint8_t msg_id,
+                                 unsigned char *req, unsigned int req_len)
+{
+    AspeedIBTState *ibt = ASPEED_IBT(ii);
+
+    if (req_len == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: zero length request", __func__);
+        return;
+    }
+
+    if (req_len > ASPEED_IBT_BUFFER_SIZE - 1) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: request of %d bytes is too long",
+                      __func__, req_len);
+        req_len = ASPEED_IBT_BUFFER_SIZE - 1;
+    }
+
+    /* include length and reuse msg_id as seq in message */
+    ibt->recv_msg[0] = req[0];
+    ibt->recv_msg[1] = msg_id;
+    memcpy(ibt->recv_msg + 2, req + 1, req_len - 1);
+    ibt->recv_msg_len = req_len + 1;
+
+
+    ibt->regs[TO_REG(BT_CTRL)] |= BT_CTRL_H2B_ATN;
+    aspeed_ibt_dump_msg(__func__, ibt->recv_msg, ibt->recv_msg_len);
+
+}
+
+static void *aspeed_ibt_backend_data(IPMIInterface *ii)
+{
+    return ii;
+}
+
+static void aspeed_ibt_set_ipmi_handler(IPMIInterface *ii, IPMICore *h)
+{
+    AspeedIBTState *ibt = ASPEED_IBT(ii);
+    ibt->handler = h;
+}
+
+static const MemoryRegionOps aspeed_ibt_ops = {
+    .read = aspeed_ibt_read,
+    .write = aspeed_ibt_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+};
+
+static void aspeed_ibt_enter_reset(Object *obj, ResetType type)
+{
+    AspeedIBTState *ibt = ASPEED_IBT(obj);
+
+    memset(ibt->regs, 0, sizeof(ibt->regs));
+
+    memset(ibt->recv_msg, 0, sizeof(ibt->recv_msg));
+    ibt->recv_msg_len = 0;
+    ibt->recv_msg_index = -1;
+
+    memset(ibt->send_msg, 0, sizeof(ibt->send_msg));
+    ibt->send_msg_len = 0;
+}
+
+
+static void aspeed_ibt_realize(DeviceState *dev, Error **errp)
+{
+    AspeedIBTState *ibt = ASPEED_IBT(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    IPMIInterface *ii = IPMI_INTERFACE(dev);
+
+    sysbus_init_irq(sbd, &ibt->irq);
+    memory_region_init_io(&ibt->iomem, OBJECT(ibt), &aspeed_ibt_ops, ibt,
+                          TYPE_ASPEED_IBT, ASPEED_IBT_IO_REGION_SIZE);
+    sysbus_init_mmio(sbd, &ibt->iomem);
+
+
+    if (ibt->handler) {
+        ibt->handler->intf = ii;
+    }
+}
+
+static const VMStateDescription vmstate_aspeed_ibt = {
+    .name = TYPE_ASPEED_IBT,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, AspeedIBTState, ASPEED_IBT_NR_REGS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void aspeed_ibt_class_init(ObjectClass *klass, void *data)
+{
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_CLASS(klass);
+
+    dc->desc = "ASPEED IPMI BT Host Controller";
+    dc->vmsd = &vmstate_aspeed_ibt;
+    dc->realize = aspeed_ibt_realize;
+    rc->phases.enter = aspeed_ibt_enter_reset;
+
+    iic->handle_msg = aspeed_ibt_handle_msg;
+    iic->get_backend_data = aspeed_ibt_backend_data;
+    iic->set_ipmi_handler = aspeed_ibt_set_ipmi_handler;
+}
+
+static const TypeInfo aspeed_ibt_info[] = {
+    {
+        .name = TYPE_ASPEED_IBT,
+        .parent = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(AspeedIBTState),
+        .class_init = aspeed_ibt_class_init,
+        .interfaces = (InterfaceInfo[]) {
+            { TYPE_IPMI_INTERFACE },
+            { },
+        },
+    },
+};
+
+DEFINE_TYPES(aspeed_ibt_info);
diff --git a/hw/ipmi/meson.build b/hw/ipmi/meson.build
index 3d8b030ba5..7e7bced38e 100644
--- a/hw/ipmi/meson.build
+++ b/hw/ipmi/meson.build
@@ -9,5 +9,6 @@ ipmi_ss.add(when: 'CONFIG_PCI_IPMI_BT', if_true: files('pci_ipmi_bt.c'))
 ipmi_ss.add(when: 'CONFIG_IPMI_SSIF', if_true: files('smbus_ipmi.c'))
 ipmi_ss.add(when: 'CONFIG_IPMI_HOST', if_true: files('ipmi_host_extern.c'))
 ipmi_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_kcs.c'))
+ipmi_ss.add(when: 'CONFIG_IPMI_HOST', if_true: files('aspeed_ibt.c'))
 
 softmmu_ss.add_all(when: 'CONFIG_IPMI', if_true: ipmi_ss)
diff --git a/hw/ipmi/trace-events b/hw/ipmi/trace-events
index 66d40bccbc..efbedfdd62 100644
--- a/hw/ipmi/trace-events
+++ b/hw/ipmi/trace-events
@@ -6,3 +6,10 @@ npcm7xx_kcs_write(const char *id, int channel, int reg, uint8_t value) " %s chan
 npcm7xx_kcs_handle_event(const char *id, uint8_t status) " %s: %" PRIu8
 npcm7xx_kcs_host_read_byte(const char *id, uint8_t value) " %s: value 0x%02" PRIx8
 npcm7xx_kcs_host_write_byte(const char *id, uint8_t value) " %s: value 0x%02" PRIx8
+
+# aspeed_ibt.c
+aspeed_ibt_chr_dump_msg(const char *func, const char *buf, uint32_t len) "%s: %s #%d bytes"
+aspeed_ibt_chr_event(bool connected) "connected:%d"
+aspeed_ibt_read(uint64_t offset, uint64_t value) "offset:0x%" PRIx64 " value:0x%" PRIx64
+aspeed_ibt_write(uint64_t offset, uint64_t value) "offset:0x%" PRIx64 " value:0x%" PRIx64
+aspeed_ibt_event(const char* event) "%s"
-- 
2.33.0.800.g4c38ced690-goog

Re: [RFC 0/1] hw/ipmi: add an aspeed IPMI iBT device model

[Cédric Le Goater]

Hello Titus,

On 9/29/21 00:39, Titus Rwantare wrote:
> This patch follows the Handing IPMI for emulating BMC patch set by Hao Wu.
> Building on top of the work in [PATCH] hw/misc: Add an iBT device model posted
> by Cédric Le Goater, this iBT model works as a backend to ipmi-host-extern.

Could you please resend with my patch which is still available here :

  https://github.com/legoater/qemu/commit/c6b679690f32534e8992d96752d90d2c4aa48130

or here :

  http://patchwork.ozlabs.org/project/qemu-devel/patch/20210407171637.777743-20-clg@kaod.org/

and in another patch, the modifications you made on top of mine. That
will help understand where the problem could be.

You can modify patches but you need to keep the original author name,
commit log, signed-off-by to certify its origin. Then list the changes
you have made and add your signed-off-by.

The patch you sent seems to be mostly mine and I don't see any of the
above.

Here is some more info in the kernel documentation :

   https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

I don't think we have the same kind of documentation in QEMU but it's
the same process.

> Needed to run:
> - [PATCH 4/8] hw/ipmi: Refactor IPMI interface, Hao Wu
> - [PATCH 5/8] hw/ipmi: Take out common from ipmi_bmc_extern.c, Hao Wu
> - [PATCH 6/8] hw/ipmi: Move handle_command to IPMICoreClass, Hao Wu
> - [PATCH 7/8] hw/ipmi: Add an IPMI external host device, Hao Wu

I have no idea what these patches do and where they are. They seem to
modify the core IPMI framework and it is important to understand the
overall changes. Please include them in the patchset first since they
are initial requirements.

> Host commandline
>      -chardev socket,id=ipmichr1,host=localhost,port=9999,reconnect=10 \
>      -device ipmi-bmc-extern,chardev=ipmichr1,id=bmc0 \
>      -device isa-ipmi-bt,bmc=bmc0,irq=10 -nodefaults
> 
> BMC commandline
>      -chardev socket,id=ipmichr1,host=localhost,port=9999,server=on,wait=off \
>      -device "ipmi-host-extern,chardev=ipmichr1,responder=/machine/soc/ibt"

Could we have the full command lines also ?


> Currently, IRQs are not functional, but it is able to connect.
> The following printout is from the host:
> 
> [    6.869742] ipmi_si IPI0001:00: IPMI message handler: Found new BMC (man_id:
>   0x002b79, prod_id: 0x0000, dev_id: 0x00)
> [   12.393281] ipmi_si IPI0001:00: IPMI bt interface initialized
> 
> This patch is an RFC because of the missing IRQs and the need for other patches
> to get merged.

We will need the whole project to start reviewing.

Thanks,

C.

> 
> Thank you all.
> 
> Titus Rwantare (1):
>    hw/ipmi: add an aspeed ipmi iBT device model
> 
>   hw/ipmi/ipmi_extern.h        |   1 +
>   include/hw/arm/aspeed_soc.h  |   2 +
>   include/hw/ipmi/aspeed_ibt.h |  77 +++++++++
>   hw/arm/aspeed_ast2600.c      |  12 ++
>   hw/arm/aspeed_soc.c          |  12 ++
>   hw/ipmi/aspeed_ibt.c         | 311 +++++++++++++++++++++++++++++++++++
>   hw/ipmi/meson.build          |   1 +
>   hw/ipmi/trace-events         |   7 +
>   8 files changed, 423 insertions(+)
>   create mode 100644 include/hw/ipmi/aspeed_ibt.h
>   create mode 100644 hw/ipmi/aspeed_ibt.c
> 

Re: [RFC 0/1] hw/ipmi: add an aspeed IPMI iBT device model

[Titus Rwantare]

On Tue, 28 Sept 2021 at 23:24, Cédric Le Goater <clg@kaod.org> wrote:
>
> Hello Titus,
>
> On 9/29/21 00:39, Titus Rwantare wrote:
> > This patch follows the Handing IPMI for emulating BMC patch set by Hao Wu.
> > Building on top of the work in [PATCH] hw/misc: Add an iBT device model posted
> > by Cédric Le Goater, this iBT model works as a backend to ipmi-host-extern.
>
> Could you please resend with my patch which is still available here :
>
>   https://github.com/legoater/qemu/commit/c6b679690f32534e8992d96752d90d2c4aa48130
>
> or here :
>
>   http://patchwork.ozlabs.org/project/qemu-devel/patch/20210407171637.777743-20-clg@kaod.org/
>
> and in another patch, the modifications you made on top of mine. That
> will help understand where the problem could be.
>
> You can modify patches but you need to keep the original author name,
> commit log, signed-off-by to certify its origin. Then list the changes
> you have made and add your signed-off-by.
>
> The patch you sent seems to be mostly mine and I don't see any of the
> above.
>
> Here is some more info in the kernel documentation :
>
>    https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
>
> I don't think we have the same kind of documentation in QEMU but it's
> the same process.

Thanks, I'm still learning.

>
> > Needed to run:
> > - [PATCH 4/8] hw/ipmi: Refactor IPMI interface, Hao Wu
> > - [PATCH 5/8] hw/ipmi: Take out common from ipmi_bmc_extern.c, Hao Wu
> > - [PATCH 6/8] hw/ipmi: Move handle_command to IPMICoreClass, Hao Wu
> > - [PATCH 7/8] hw/ipmi: Add an IPMI external host device, Hao Wu
>
> I have no idea what these patches do and where they are. They seem to
> modify the core IPMI framework and it is important to understand the
> overall changes. Please include them in the patchset first since they
> are initial requirements.
>

I've pushed everything here: https://github.com/Rwantare/qemu/tree/aspeed-ibt
The other patches are out for review and will likely change before merging

> > Host commandline
> >      -chardev socket,id=ipmichr1,host=localhost,port=9999,reconnect=10 \
> >      -device ipmi-bmc-extern,chardev=ipmichr1,id=bmc0 \
> >      -device isa-ipmi-bt,bmc=bmc0,irq=10 -nodefaults
> >
> > BMC commandline
> >      -chardev socket,id=ipmichr1,host=localhost,port=9999,server=on,wait=off \
> >      -device "ipmi-host-extern,chardev=ipmichr1,responder=/machine/soc/ibt"
>
> Could we have the full command lines also ?
>

Sure, there's not much else there, one of the bmc firmwares I'm testing with
is a quanta-q71l-bmc which should build from OpenBMC.

Host:
qemu-system-x86_64 \
-nographic -m 4G \
-kernel arch/x86/boot/bzImage -append "console=ttyS0 earlyprintk=ttyS0" \
-serial /dev/tty -monitor /dev/pts/9 -initrd u-root.cpio \
-chardev socket,id=ipmichr0,host=localhost,port=9999,reconnect=10 \
-device ipmi-bmc-extern,chardev=ipmichr0,id=bmc0 \
-device isa-ipmi-bt,bmc=bmc0,irq=10 -nodefaults

BMC:
qemu-system-arm \
-machine quanta-q71l-bmc \
-nographic \
-drive file=path/to/openbmc/image,if=mtd,bus=0,unit=0,snapshot=on \
-chardev socket,id=ipmichr1,host=localhost,port=9999,server=on,wait=off \
-device "ipmi-host-extern,chardev=ipmichr1,responder=/machine/soc/ibt"

>
> > Currently, IRQs are not functional, but it is able to connect.
> > The following printout is from the host:
> >
> > [    6.869742] ipmi_si IPI0001:00: IPMI message handler: Found new BMC (man_id:
> >   0x002b79, prod_id: 0x0000, dev_id: 0x00)
> > [   12.393281] ipmi_si IPI0001:00: IPMI bt interface initialized
> >
> > This patch is an RFC because of the missing IRQs and the need for other patches
> > to get merged.
>
> We will need the whole project to start reviewing.
>
> Thanks,
>
> C.

Noted, I think I should be able to send the BT commits from
https://github.com/Rwantare/qemu/tree/aspeed-ibt
after the others are reviewed and merged.
For now, the irq setup is my point of confusion.

Thanks,
-Titus