All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Handing IPMI for emulating BMC
@ 2021-09-09 23:06 Hao Wu
  2021-09-09 23:06 ` [PATCH 1/8] docs: enable sphinx blockdiag extension Hao Wu
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Hao Wu @ 2021-09-09 23:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-arm, qemu-devel, wuhaotsh, venture, Avi.Fishman, kfting,
	hskinnemoen, titusr, peter.maydell

Baseboard Management Controllers (BMCs) are special
processors that monitors state of a computer, often
used in data center servers. They often communicate
via IPMI. As a result, it is important to emulate
the IPMI interface so that they can connect to host
machines.

This patch set aims to refactor the existing hw/ipmi
and make it handles both Core side and BMC side
emulations. We also added the implementation of the
KCS module for NPCM7XX BMC boards that work as a backend.
We have tested this patch on various NPCM7xx based
systems and they can communicate with a host that runs
`ipmi-bmc-extern`.

The structure is as follows:
Patch 1-3 contains some documentation written by
Havard Skinnomoen that how the emulation of existing
host-side IPMI and the new BMC-side IPMI works.
Patch 4-6 refactors the current IPMI code so that
they work for both host-side and BMC-side.
Patch 7 adds a new ipmi-host-extern which represents
BMC-side emulation that is similar to the current
ipmi-bmc-extern.
Patch 8 implements the KCS device in NPCM7XX boards. It
works as a backend to the ipmi-host-extern device. Since
the direction is different we can't directly use ipmi-kcs.c
for BMC emulation.

Hao Wu (5):
  hw/ipmi: Refactor IPMI interface
  hw/ipmi: Take out common from ipmi_bmc_extern.c
  hw/ipmi: Move handle_command to IPMICoreClass
  hw/ipmi: Add an IPMI external host device
  hw/ipmi: Add a KCS Module for NPCM7XX

Havard Skinnemoen (3):
  docs: enable sphinx blockdiag extension
  docs/specs: IPMI device emulation: main processor
  docs/specs: IPMI device emulation: BMC

 configs/devices/arm-softmmu/default.mak |   2 +
 docs/conf.py                            |   6 +-
 docs/specs/index.rst                    |   1 +
 docs/specs/ipmi.rst                     | 170 +++++++
 docs/system/arm/nuvoton.rst             |   1 -
 hw/arm/npcm7xx.c                        |  10 +-
 hw/ipmi/Kconfig                         |   5 +
 hw/ipmi/ipmi.c                          |  15 +-
 hw/ipmi/ipmi_bmc_extern.c               | 417 ++---------------
 hw/ipmi/ipmi_bmc_sim.c                  |  47 +-
 hw/ipmi/ipmi_bt.c                       |   6 +-
 hw/ipmi/ipmi_extern.c                   | 429 +++++++++++++++++
 hw/ipmi/ipmi_extern.h                   |  90 ++++
 hw/ipmi/ipmi_host_extern.c              | 170 +++++++
 hw/ipmi/ipmi_kcs.c                      |   8 +-
 hw/ipmi/isa_ipmi_bt.c                   |   4 +-
 hw/ipmi/isa_ipmi_kcs.c                  |   4 +-
 hw/ipmi/meson.build                     |   4 +-
 hw/ipmi/npcm7xx_kcs.c                   | 588 ++++++++++++++++++++++++
 hw/ipmi/pci_ipmi_bt.c                   |   4 +-
 hw/ipmi/pci_ipmi_kcs.c                  |   4 +-
 hw/ipmi/smbus_ipmi.c                    |  12 +-
 hw/ipmi/trace-events                    |   8 +
 hw/ipmi/trace.h                         |   1 +
 include/hw/arm/npcm7xx.h                |   2 +
 include/hw/ipmi/ipmi.h                  |  54 ++-
 include/hw/ipmi/npcm7xx_kcs.h           | 103 +++++
 meson.build                             |   1 +
 28 files changed, 1733 insertions(+), 433 deletions(-)
 create mode 100644 docs/specs/ipmi.rst
 create mode 100644 hw/ipmi/ipmi_extern.c
 create mode 100644 hw/ipmi/ipmi_extern.h
 create mode 100644 hw/ipmi/ipmi_host_extern.c
 create mode 100644 hw/ipmi/npcm7xx_kcs.c
 create mode 100644 hw/ipmi/trace-events
 create mode 100644 hw/ipmi/trace.h
 create mode 100644 include/hw/ipmi/npcm7xx_kcs.h

-- 
2.33.0.309.g3052b89438-goog



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

* [PATCH 1/8] docs: enable sphinx blockdiag extension
  2021-09-09 23:06 [PATCH 0/8] Handing IPMI for emulating BMC Hao Wu
@ 2021-09-09 23:06 ` Hao Wu
  2021-09-09 23:40   ` Corey Minyard
  2021-09-09 23:06 ` [PATCH 2/8] docs/specs: IPMI device emulation: main processor Hao Wu
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Hao Wu @ 2021-09-09 23:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-arm, qemu-devel, wuhaotsh, venture, Avi.Fishman, kfting,
	hskinnemoen, titusr, peter.maydell

From: Havard Skinnemoen <hskinnemoen@google.com>

Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
Signed-off-by: Hao Wu <hskinnemoen@google.com>
---
 docs/conf.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/docs/conf.py b/docs/conf.py
index ff6e92c6e2..ecd0be66a5 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -71,7 +71,11 @@
 # Add any Sphinx extension module names here, as strings. They can be
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
-extensions = ['kerneldoc', 'qmp_lexer', 'hxtool', 'depfile', 'qapidoc']
+extensions = ['kerneldoc', 'qmp_lexer', 'hxtool', 'depfile', 'qapidoc',
+        'sphinxcontrib.blockdiag']
+
+# Fontpath for blockdiag (truetype font)
+blockdiag_fontpath = '/usr/share/fonts/truetype/liberation/LiberationSans-Regular.ttf'
 
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
-- 
2.33.0.309.g3052b89438-goog



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

* [PATCH 2/8] docs/specs: IPMI device emulation: main processor
  2021-09-09 23:06 [PATCH 0/8] Handing IPMI for emulating BMC Hao Wu
  2021-09-09 23:06 ` [PATCH 1/8] docs: enable sphinx blockdiag extension Hao Wu
@ 2021-09-09 23:06 ` Hao Wu
  2021-09-09 23:48   ` Corey Minyard
  2021-09-09 23:06 ` [PATCH 3/8] docs/specs: IPMI device emulation: BMC Hao Wu
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Hao Wu @ 2021-09-09 23:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-arm, qemu-devel, wuhaotsh, venture, Avi.Fishman, kfting,
	hskinnemoen, titusr, peter.maydell

From: Havard Skinnemoen <hskinnemoen@google.com>

This document is an attempt to briefly document the existing IPMI
emulation support on the main processor. It provides the necessary
background for the BMC-side IPMI emulation proposed by the next patch.

Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
Signed-off-by: Hao Wu <wuhaotsh@google.com>
---
 docs/specs/index.rst |   1 +
 docs/specs/ipmi.rst  | 100 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 docs/specs/ipmi.rst

diff --git a/docs/specs/index.rst b/docs/specs/index.rst
index 65e9663916..1b5d177d53 100644
--- a/docs/specs/index.rst
+++ b/docs/specs/index.rst
@@ -11,6 +11,7 @@ guest hardware that is specific to QEMU.
    ppc-spapr-xive
    ppc-spapr-numa
    acpi_hw_reduced_hotplug
+   ipmi
    tpm
    acpi_hest_ghes
    acpi_cpu_hotplug
diff --git a/docs/specs/ipmi.rst b/docs/specs/ipmi.rst
new file mode 100644
index 0000000000..adb098b53d
--- /dev/null
+++ b/docs/specs/ipmi.rst
@@ -0,0 +1,100 @@
+=====================
+IPMI device emulation
+=====================
+
+QEMU supports emulating many types of machines. This includes machines that may
+serve as the main processor in an IPMI system, e.g. x86 or POWER server
+processors, as well as machines emulating ARM-based Baseband Management
+Controllers (BMCs), e.g. AST2xxx or NPCM7xxx systems-on-chip.
+
+Main processor emulation
+========================
+
+A server platform may include one of the following system interfaces for
+communicating with a BMC:
+
+* A Keyboard Controller Style (KCS) Interface, accessible via ISA
+  (``isa-ipmi-kcs``) or PCI (``pci-ipmi-kcs``).
+* A Block Transfer (BT) Interface, accessible via ISA (``isa-ipmi-bt``) or PCI
+  (``pci-ipmi-bt``).
+* An SMBus System Interface (SSIF; ``smbus-ipmi``).
+
+These interfaces can all be emulated by QEMU. To emulate the behavior of the
+BMC, the messaging interface emulators use one of the following backends:
+
+* A BMC simulator running within the QEMU process (``ipmi-bmc-sim``).
+* An external BMC simulator or emulator, connected over a chardev
+  (``ipmi-bmc-extern``). `ipmi_sim
+  <https://github.com/wrouesnel/openipmi/blob/master/lanserv/README.ipmi_sim>`_
+  from OpenIPMI is an example external BMC emulator.
+
+The following diagram shows how these entities relate to each other.
+
+.. blockdiag::
+
+    blockdiag main_processor_ipmi {
+        orientation = portrait
+        default_group_color = "none";
+        class msgif [color = lightblue];
+        class bmc [color = salmon];
+
+        ipmi_sim [color="aquamarine", label="External BMC"]
+        ipmi-bmc-extern <-> ipmi_sim [label="chardev"];
+
+        group {
+            orientation = portrait
+
+            ipmi-interface <-> ipmi-bmc;
+
+            group {
+                orientation = portrait
+
+                ipmi-interface [class = "msgif"];
+                isa-ipmi-kcs [class="msgif", stacked];
+
+                ipmi-interface <- isa-ipmi-kcs [hstyle = generalization];
+            }
+
+
+            group {
+                orientation = portrait
+
+                ipmi-bmc [class = "bmc"];
+                ipmi-bmc-sim [class="bmc"];
+                ipmi-bmc-extern [class="bmc"];
+
+                ipmi-bmc <- ipmi-bmc-sim [hstyle = generalization];
+                ipmi-bmc <- ipmi-bmc-extern [hstyle = generalization];
+            }
+
+        }
+    }
+
+IPMI System Interfaces
+----------------------
+
+The system software running on the main processor may use a *system interface*
+to communicate with the BMC. These are hardware devices attached to an ISA, PCI
+or i2c bus, and in QEMU, they all need to implement ``ipmi-interface``.
+This allows a BMC implementation to interact with the system interface in a
+standard way.
+
+IPMI BMC
+--------
+
+The system interface devices delegate emulation of BMC behavior to a BMC
+device, that is a subclass of ``ipmi-bmc``. This type of device is called
+a BMC because that's what it looks like to the main processor guest software.
+
+The BMC behavior may be simulated within the qemu process (``ipmi-bmc-sim``) or
+further delegated to an external emulator, or a real BMC. The
+``ipmi-bmc-extern`` device has a required ``chardev`` property which specifies
+the communications channel to the external BMC.
+
+Wire protocol
+=============
+
+The wire protocol used between ``ipmi-bmc-extern`` and the external BMC
+emulator is defined by `README.vm
+<https://github.com/wrouesnel/openipmi/blob/master/lanserv/README.vm>`_ from
+the OpenIPMI project.
-- 
2.33.0.309.g3052b89438-goog



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

* [PATCH 3/8] docs/specs: IPMI device emulation: BMC
  2021-09-09 23:06 [PATCH 0/8] Handing IPMI for emulating BMC Hao Wu
  2021-09-09 23:06 ` [PATCH 1/8] docs: enable sphinx blockdiag extension Hao Wu
  2021-09-09 23:06 ` [PATCH 2/8] docs/specs: IPMI device emulation: main processor Hao Wu
@ 2021-09-09 23:06 ` Hao Wu
  2021-09-09 23:06 ` [PATCH 4/8] hw/ipmi: Refactor IPMI interface Hao Wu
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Hao Wu @ 2021-09-09 23:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-arm, qemu-devel, wuhaotsh, venture, Avi.Fishman, kfting,
	hskinnemoen, titusr, peter.maydell

From: Havard Skinnemoen <hskinnemoen@google.com>

The IPMI document is expanded with a proposal to emulate BMC-side IPMI
devices. This allows a QEMU instance running server software to interact
with a different QEMU instance running BMC firmware, which should
closely model how a real server system works.

Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
Signed-off-by: Hao Wu <wuhaotsh@google.com>
---
 docs/specs/ipmi.rst | 70 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/docs/specs/ipmi.rst b/docs/specs/ipmi.rst
index adb098b53d..b5fe362043 100644
--- a/docs/specs/ipmi.rst
+++ b/docs/specs/ipmi.rst
@@ -91,6 +91,76 @@ further delegated to an external emulator, or a real BMC. The
 ``ipmi-bmc-extern`` device has a required ``chardev`` property which specifies
 the communications channel to the external BMC.
 
+Baseband Management Controller (BMC) emulation
+==============================================
+
+This section is about emulation of IPMI-related devices in a System-on-Chip
+(SoC) used as a Baseband Management Controller. This is not to be confused with
+emulating the BMC device as seen by the main processor.
+
+SoCs that are designed to be used as a BMC often have dedicated hardware that
+allows them to be connected to one or more of the IPMI System Interfaces. The
+BMC-side hardware interface is not standardized, so each type of SoC may need
+its own device implementation in QEMU, for example:
+
+* ``aspeed-ibt`` for emulating the Aspeed iBT peripheral.
+* ``npcm7xx-kcs`` for emulating the Nuvoton NPCM7xx Host-to-BMC Keyboard
+  Controller Style (KCS) channels.
+
+.. blockdiag::
+
+    blockdiag bmc_ipmi {
+        orientation = portrait
+        default_group_color = "none";
+        class interface [color = lightblue];
+        class host [color = salmon];
+
+        host [color="aquamarine", label="External Host"]
+
+        group {
+            orientation = portrait
+
+            group {
+                orientation = portrait
+
+                bmc-interface [class = "interface"]
+                npcm7xx-ipmi-kcs [class = "interface", stacked]
+
+                bmc-interface <- npcm7xx-ipmi-kcs [hstyle = generalization];
+            }
+
+            group {
+                orientation = portrait
+
+                bmc-host [class = "host"];
+                bmc-host-sim [class = "host"];
+                bmc-host-extern [class = "host"];
+
+                bmc-host <- bmc-host-sim [hstyle = generalization];
+                bmc-host <- bmc-host-extern [hstyle = generalization];
+            }
+
+            bmc-interface <-> bmc-host
+        }
+
+        bmc-host-extern <-> host [label="chardev"];
+    }
+
+IPMI Host
+---------
+
+Mirroring the main processor emulation, the interface devices delegate
+emulation of host behavior to a Host device that is a subclass of
+``ipmi-core``. This type of device is called a Host because that's what it
+looks like to the BMC guest software.
+
+The host behavior may be further delegated to an external emulator (e.g.
+another QEMU VM) through the ``ipmi-host-extern`` host implementation. This
+device has a required ``chardev`` property which specifies the communications
+channel to the external host and a required ``interface`` property which
+specifies the underlying IPMI interface. The wire format is the same as for
+``ipmi-bmc-extern``.
+
 Wire protocol
 =============
 
-- 
2.33.0.309.g3052b89438-goog



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

* [PATCH 4/8] hw/ipmi: Refactor IPMI interface
  2021-09-09 23:06 [PATCH 0/8] Handing IPMI for emulating BMC Hao Wu
                   ` (2 preceding siblings ...)
  2021-09-09 23:06 ` [PATCH 3/8] docs/specs: IPMI device emulation: BMC Hao Wu
@ 2021-09-09 23:06 ` Hao Wu
  2021-09-10  0:26   ` Corey Minyard
  2021-09-09 23:06 ` [PATCH 5/8] hw/ipmi: Take out common from ipmi_bmc_extern.c Hao Wu
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Hao Wu @ 2021-09-09 23:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-arm, qemu-devel, wuhaotsh, venture, Avi.Fishman, kfting,
	hskinnemoen, titusr, peter.maydell

This patch refactors the IPMI interface so that it can be used by both
the BMC side and core-side simulation.

Detail changes:
(1) rename handle_rsp -> handle_msg so the name fits both BMC side and
    Core side.
(2) Add a new class IPMICore. This class represents a simulator/external
    connection for both BMC and Core side emulation. The old IPMIBmc is
    a sub-class of IPMICore.
(3) Change all ibe->parent.intf in hw/ipmi/ipmi_bmc_*.c to use type
    cast to IPMICore. Directly accessing parent QOM is against QEMU
    guide and by using type casting the code can fit both core side and
    BMC side emulation.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
---
 hw/ipmi/ipmi.c            | 15 +++++++++++---
 hw/ipmi/ipmi_bmc_extern.c | 37 +++++++++++++++++++++--------------
 hw/ipmi/ipmi_bmc_sim.c    | 41 ++++++++++++++++++++++++++-------------
 hw/ipmi/ipmi_bt.c         |  2 +-
 hw/ipmi/ipmi_kcs.c        |  2 +-
 hw/ipmi/isa_ipmi_bt.c     |  4 +++-
 hw/ipmi/isa_ipmi_kcs.c    |  4 +++-
 hw/ipmi/pci_ipmi_bt.c     |  4 +++-
 hw/ipmi/pci_ipmi_kcs.c    |  4 +++-
 hw/ipmi/smbus_ipmi.c      |  6 ++++--
 include/hw/ipmi/ipmi.h    | 40 ++++++++++++++++++++++++++++++--------
 11 files changed, 111 insertions(+), 48 deletions(-)

diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index 8d35c9fdd6..7da1b36fab 100644
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -92,13 +92,21 @@ static TypeInfo ipmi_interface_type_info = {
     .class_init = ipmi_interface_class_init,
 };
 
+static TypeInfo ipmi_core_type_info = {
+    .name = TYPE_IPMI_CORE,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(IPMICore),
+    .abstract = true,
+};
+
 static void isa_ipmi_bmc_check(const Object *obj, const char *name,
                                Object *val, Error **errp)
 {
-    IPMIBmc *bmc = IPMI_BMC(val);
+    IPMICore *ic = IPMI_CORE(val);
 
-    if (bmc->intf)
+    if (ic->intf) {
         error_setg(errp, "BMC object is already in use");
+    }
 }
 
 void ipmi_bmc_find_and_link(Object *obj, Object **bmc)
@@ -122,7 +130,7 @@ static void bmc_class_init(ObjectClass *oc, void *data)
 
 static TypeInfo ipmi_bmc_type_info = {
     .name = TYPE_IPMI_BMC,
-    .parent = TYPE_DEVICE,
+    .parent = TYPE_IPMI_CORE,
     .instance_size = sizeof(IPMIBmc),
     .abstract = true,
     .class_size = sizeof(IPMIBmcClass),
@@ -132,6 +140,7 @@ static TypeInfo ipmi_bmc_type_info = {
 static void ipmi_register_types(void)
 {
     type_register_static(&ipmi_interface_type_info);
+    type_register_static(&ipmi_core_type_info);
     type_register_static(&ipmi_bmc_type_info);
 }
 
diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
index acf2bab35f..a0c3a40e7c 100644
--- a/hw/ipmi/ipmi_bmc_extern.c
+++ b/hw/ipmi/ipmi_bmc_extern.c
@@ -147,8 +147,9 @@ static void continue_send(IPMIBmcExtern *ibe)
 
 static void extern_timeout(void *opaque)
 {
+    IPMICore *ic = opaque;
     IPMIBmcExtern *ibe = opaque;
-    IPMIInterface *s = ibe->parent.intf;
+    IPMIInterface *s = ic->intf;
 
     if (ibe->connected) {
         if (ibe->waiting_rsp && (ibe->outlen == 0)) {
@@ -158,7 +159,7 @@ static void extern_timeout(void *opaque)
             ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
             ibe->inbuf[2] = ibe->outbuf[2];
             ibe->inbuf[3] = IPMI_CC_TIMEOUT;
-            k->handle_rsp(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
+            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
         } else {
             continue_send(ibe);
         }
@@ -186,8 +187,9 @@ static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
                                        unsigned int max_cmd_len,
                                        uint8_t msg_id)
 {
+    IPMICore *ic = IPMI_CORE(b);
     IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(b);
-    IPMIInterface *s = ibe->parent.intf;
+    IPMIInterface *s = ic->intf;
     uint8_t err = 0, csum;
     unsigned int i;
 
@@ -213,7 +215,7 @@ static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
         rsp[1] = cmd[1];
         rsp[2] = err;
         ibe->waiting_rsp = false;
-        k->handle_rsp(s, msg_id, rsp, 3);
+        k->handle_msg(s, msg_id, rsp, 3);
         goto out;
     }
 
@@ -236,7 +238,8 @@ static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
 
 static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
 {
-    IPMIInterface *s = ibe->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibe);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
     switch (hw_op) {
@@ -284,7 +287,9 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
 
 static void handle_msg(IPMIBmcExtern *ibe)
 {
-    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(ibe->parent.intf);
+    IPMICore *ic = IPMI_CORE(ibe);
+    IPMIInterface *s = ic->intf;
+    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
     if (ibe->in_escape) {
         ipmi_debug("msg escape not ended\n");
@@ -306,7 +311,7 @@ static void handle_msg(IPMIBmcExtern *ibe)
 
     timer_del(ibe->extern_timer);
     ibe->waiting_rsp = false;
-    k->handle_rsp(ibe->parent.intf, ibe->inbuf[0], ibe->inbuf + 1, ibe->inpos - 1);
+    k->handle_msg(s, ibe->inbuf[0], ibe->inbuf + 1, ibe->inpos - 1);
 }
 
 static int can_receive(void *opaque)
@@ -382,8 +387,9 @@ static void receive(void *opaque, const uint8_t *buf, int size)
 
 static void chr_event(void *opaque, QEMUChrEvent event)
 {
+    IPMICore *ic = opaque;
     IPMIBmcExtern *ibe = opaque;
-    IPMIInterface *s = ibe->parent.intf;
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     unsigned char v;
 
@@ -398,17 +404,17 @@ static void chr_event(void *opaque, QEMUChrEvent event)
         ibe->outlen++;
         addchar(ibe, VM_CMD_CAPABILITIES);
         v = VM_CAPABILITIES_IRQ | VM_CAPABILITIES_ATTN;
-        if (k->do_hw_op(ibe->parent.intf, IPMI_POWEROFF_CHASSIS, 1) == 0) {
+        if (k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1) == 0) {
             v |= VM_CAPABILITIES_POWER;
         }
-        if (k->do_hw_op(ibe->parent.intf, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 1)
+        if (k->do_hw_op(s, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 1)
             == 0) {
             v |= VM_CAPABILITIES_GRACEFUL_SHUTDOWN;
         }
-        if (k->do_hw_op(ibe->parent.intf, IPMI_RESET_CHASSIS, 1) == 0) {
+        if (k->do_hw_op(s, IPMI_RESET_CHASSIS, 1) == 0) {
             v |= VM_CAPABILITIES_RESET;
         }
-        if (k->do_hw_op(ibe->parent.intf, IPMI_SEND_NMI, 1) == 0) {
+        if (k->do_hw_op(s, IPMI_SEND_NMI, 1) == 0) {
             v |= VM_CAPABILITIES_NMI;
         }
         addchar(ibe, v);
@@ -433,7 +439,7 @@ static void chr_event(void *opaque, QEMUChrEvent event)
             ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
             ibe->inbuf[2] = ibe->outbuf[2];
             ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
-            k->handle_rsp(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
+            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
         }
         break;
 
@@ -475,14 +481,15 @@ static int ipmi_bmc_extern_post_migrate(void *opaque, int version_id)
      * error on the interface if a response was being waited for.
      */
     if (ibe->waiting_rsp) {
-        IPMIInterface *ii = ibe->parent.intf;
+        IPMICore *ic = opaque;
+        IPMIInterface *ii = ic->intf;
         IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
 
         ibe->waiting_rsp = false;
         ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
         ibe->inbuf[2] = ibe->outbuf[2];
         ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
-        iic->handle_rsp(ii, ibe->outbuf[0], ibe->inbuf + 1, 3);
+        iic->handle_msg(ii, ibe->outbuf[0], ibe->inbuf + 1, 3);
     }
     return 0;
 }
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 905e091094..7cc4a22456 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -451,7 +451,8 @@ static int attn_irq_enabled(IPMIBmcSim *ibs)
 void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log)
 {
     IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(b);
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
     if (!IPMI_BMC_EVENT_MSG_BUF_ENABLED(ibs)) {
@@ -475,7 +476,8 @@ void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log)
 static void gen_event(IPMIBmcSim *ibs, unsigned int sens_num, uint8_t deassert,
                       uint8_t evd1, uint8_t evd2, uint8_t evd3)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     uint8_t evt[16];
     IPMISensor *sens = ibs->sensors + sens_num;
@@ -644,7 +646,8 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
                                     uint8_t msg_id)
 {
     IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(b);
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     const IPMICmdHandler *hdl;
     RspBuffer rsp = RSP_BUFFER_INITIALIZER;
@@ -690,14 +693,15 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
     hdl->cmd_handler(ibs, cmd, cmd_len, &rsp);
 
  out:
-    k->handle_rsp(s, msg_id, rsp.buffer, rsp.len);
+    k->handle_msg(s, msg_id, rsp.buffer, rsp.len);
 
     next_timeout(ibs);
 }
 
 static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
     if (!ibs->watchdog_running) {
@@ -788,7 +792,8 @@ static void chassis_control(IPMIBmcSim *ibs,
                             uint8_t *cmd, unsigned int cmd_len,
                             RspBuffer *rsp)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
     switch (cmd[2] & 0xf) {
@@ -845,7 +850,8 @@ static void get_device_id(IPMIBmcSim *ibs,
 
 static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     bool irqs_on;
 
@@ -861,7 +867,8 @@ static void cold_reset(IPMIBmcSim *ibs,
                        uint8_t *cmd, unsigned int cmd_len,
                        RspBuffer *rsp)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
     /* Disable all interrupts */
@@ -876,7 +883,8 @@ static void warm_reset(IPMIBmcSim *ibs,
                        uint8_t *cmd, unsigned int cmd_len,
                        RspBuffer *rsp)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
     if (k->reset) {
@@ -939,7 +947,8 @@ static void clr_msg_flags(IPMIBmcSim *ibs,
                           uint8_t *cmd, unsigned int cmd_len,
                           RspBuffer *rsp)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
     ibs->msg_flags &= ~cmd[2];
@@ -957,7 +966,8 @@ static void read_evt_msg_buf(IPMIBmcSim *ibs,
                              uint8_t *cmd, unsigned int cmd_len,
                              RspBuffer *rsp)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     unsigned int i;
 
@@ -989,7 +999,8 @@ static void get_msg(IPMIBmcSim *ibs,
     g_free(msg);
 
     if (QTAILQ_EMPTY(&ibs->rcvbufs)) {
-        IPMIInterface *s = ibs->parent.intf;
+        IPMICore *ic = IPMI_CORE(ibs);
+        IPMIInterface *s = ic->intf;
         IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
         ibs->msg_flags &= ~IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE;
@@ -1014,7 +1025,8 @@ static void send_msg(IPMIBmcSim *ibs,
                      uint8_t *cmd, unsigned int cmd_len,
                      RspBuffer *rsp)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     IPMIRcvBufEntry *msg;
     uint8_t *buf;
@@ -1130,7 +1142,8 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
                                uint8_t *cmd, unsigned int cmd_len,
                                RspBuffer *rsp)
 {
-    IPMIInterface *s = ibs->parent.intf;
+    IPMICore *ic = IPMI_CORE(ibs);
+    IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     unsigned int val;
 
diff --git a/hw/ipmi/ipmi_bt.c b/hw/ipmi/ipmi_bt.c
index 22f94fb98d..f76c369e4a 100644
--- a/hw/ipmi/ipmi_bt.c
+++ b/hw/ipmi/ipmi_bt.c
@@ -430,7 +430,7 @@ void ipmi_bt_class_init(IPMIInterfaceClass *iic)
 {
     iic->init = ipmi_bt_init;
     iic->set_atn = ipmi_bt_set_atn;
-    iic->handle_rsp = ipmi_bt_handle_rsp;
+    iic->handle_msg = ipmi_bt_handle_rsp;
     iic->handle_if_event = ipmi_bt_handle_event;
     iic->set_irq_enable = ipmi_bt_set_irq_enable;
     iic->reset = ipmi_bt_handle_reset;
diff --git a/hw/ipmi/ipmi_kcs.c b/hw/ipmi/ipmi_kcs.c
index a77612946a..e0f870e13a 100644
--- a/hw/ipmi/ipmi_kcs.c
+++ b/hw/ipmi/ipmi_kcs.c
@@ -417,7 +417,7 @@ void ipmi_kcs_class_init(IPMIInterfaceClass *iic)
 {
     iic->init = ipmi_kcs_init;
     iic->set_atn = ipmi_kcs_set_atn;
-    iic->handle_rsp = ipmi_kcs_handle_rsp;
+    iic->handle_msg = ipmi_kcs_handle_rsp;
     iic->handle_if_event = ipmi_kcs_handle_event;
     iic->set_irq_enable = ipmi_kcs_set_irq_enable;
 }
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index 02625eb94e..0f52fc4262 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -74,6 +74,7 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
     ISAIPMIBTDevice *iib = ISA_IPMI_BT(dev);
     IPMIInterface *ii = IPMI_INTERFACE(dev);
     IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMICore *ic;
 
     if (!iib->bt.bmc) {
         error_setg(errp, "IPMI device requires a bmc attribute to be set");
@@ -82,7 +83,8 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
 
     iib->uuid = ipmi_next_uuid();
 
-    iib->bt.bmc->intf = ii;
+    ic = IPMI_CORE(iib->bt.bmc);
+    ic->intf = ii;
     iib->bt.opaque = iib;
 
     iic->init(ii, 0, &err);
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index 3b23ad08b3..60cab90a21 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -73,6 +73,7 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
     ISAIPMIKCSDevice *iik = ISA_IPMI_KCS(dev);
     IPMIInterface *ii = IPMI_INTERFACE(dev);
     IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMICore *ic;
 
     if (!iik->kcs.bmc) {
         error_setg(errp, "IPMI device requires a bmc attribute to be set");
@@ -81,7 +82,8 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
 
     iik->uuid = ipmi_next_uuid();
 
-    iik->kcs.bmc->intf = ii;
+    ic = IPMI_CORE(iik->kcs.bmc);
+    ic->intf = ii;
     iik->kcs.opaque = iik;
 
     iic->init(ii, 0, &err);
diff --git a/hw/ipmi/pci_ipmi_bt.c b/hw/ipmi/pci_ipmi_bt.c
index b6e52730d3..751c15a31f 100644
--- a/hw/ipmi/pci_ipmi_bt.c
+++ b/hw/ipmi/pci_ipmi_bt.c
@@ -58,6 +58,7 @@ static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
     PCIIPMIBTDevice *pik = PCI_IPMI_BT(pd);
     IPMIInterface *ii = IPMI_INTERFACE(pd);
     IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMICore *ic;
 
     if (!pik->bt.bmc) {
         error_setg(errp, "IPMI device requires a bmc attribute to be set");
@@ -66,7 +67,8 @@ static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
 
     pik->uuid = ipmi_next_uuid();
 
-    pik->bt.bmc->intf = ii;
+    ic = IPMI_CORE(pik->bt.bmc);
+    ic->intf = ii;
     pik->bt.opaque = pik;
 
     pci_config_set_prog_interface(pd->config, 0x02); /* BT */
diff --git a/hw/ipmi/pci_ipmi_kcs.c b/hw/ipmi/pci_ipmi_kcs.c
index de13418862..44a645723c 100644
--- a/hw/ipmi/pci_ipmi_kcs.c
+++ b/hw/ipmi/pci_ipmi_kcs.c
@@ -58,6 +58,7 @@ static void pci_ipmi_kcs_realize(PCIDevice *pd, Error **errp)
     PCIIPMIKCSDevice *pik = PCI_IPMI_KCS(pd);
     IPMIInterface *ii = IPMI_INTERFACE(pd);
     IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    IPMICore *ic;
 
     if (!pik->kcs.bmc) {
         error_setg(errp, "IPMI device requires a bmc attribute to be set");
@@ -66,7 +67,8 @@ static void pci_ipmi_kcs_realize(PCIDevice *pd, Error **errp)
 
     pik->uuid = ipmi_next_uuid();
 
-    pik->kcs.bmc->intf = ii;
+    ic = IPMI_CORE(pik->kcs.bmc);
+    ic->intf = ii;
     pik->kcs.opaque = pik;
 
     pci_config_set_prog_interface(pd->config, 0x01); /* KCS */
diff --git a/hw/ipmi/smbus_ipmi.c b/hw/ipmi/smbus_ipmi.c
index 1fdf0a66b6..a2383f1212 100644
--- a/hw/ipmi/smbus_ipmi.c
+++ b/hw/ipmi/smbus_ipmi.c
@@ -315,6 +315,7 @@ static void smbus_ipmi_realize(DeviceState *dev, Error **errp)
 {
     SMBusIPMIDevice *sid = SMBUS_IPMI(dev);
     IPMIInterface *ii = IPMI_INTERFACE(dev);
+    IPMICore *ic;
 
     if (!sid->bmc) {
         error_setg(errp, "IPMI device requires a bmc attribute to be set");
@@ -323,7 +324,8 @@ static void smbus_ipmi_realize(DeviceState *dev, Error **errp)
 
     sid->uuid = ipmi_next_uuid();
 
-    sid->bmc->intf = ii;
+    ic = IPMI_CORE(sid->bmc);
+    ic->intf = ii;
 }
 
 static void smbus_ipmi_init(Object *obj)
@@ -359,7 +361,7 @@ static void smbus_ipmi_class_init(ObjectClass *oc, void *data)
     dc->vmsd = &vmstate_smbus_ipmi;
     dc->realize = smbus_ipmi_realize;
     iic->set_atn = smbus_ipmi_set_atn;
-    iic->handle_rsp = smbus_ipmi_handle_rsp;
+    iic->handle_msg = smbus_ipmi_handle_rsp;
     iic->handle_if_event = smbus_ipmi_handle_event;
     iic->set_irq_enable = smbus_ipmi_set_irq_enable;
     iic->get_fwinfo = smbus_ipmi_get_fwinfo;
diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
index 77a7213ed9..1fa8cd12e5 100644
--- a/include/hw/ipmi/ipmi.h
+++ b/include/hw/ipmi/ipmi.h
@@ -111,11 +111,11 @@ uint32_t ipmi_next_uuid(void);
 #define TYPE_IPMI_INTERFACE "ipmi-interface"
 #define IPMI_INTERFACE(obj) \
      INTERFACE_CHECK(IPMIInterface, (obj), TYPE_IPMI_INTERFACE)
+typedef struct IPMIInterface IPMIInterface;
 typedef struct IPMIInterfaceClass IPMIInterfaceClass;
 DECLARE_CLASS_CHECKERS(IPMIInterfaceClass, IPMI_INTERFACE,
                        TYPE_IPMI_INTERFACE)
-
-typedef struct IPMIInterface IPMIInterface;
+struct IPMICore;
 
 struct IPMIInterfaceClass {
     InterfaceClass parent;
@@ -156,9 +156,9 @@ struct IPMIInterfaceClass {
     void (*reset)(struct IPMIInterface *s, bool is_cold);
 
     /*
-     * Handle a response from the bmc.
+     * Handle an IPMI message.
      */
-    void (*handle_rsp)(struct IPMIInterface *s, uint8_t msg_id,
+    void (*handle_msg)(struct IPMIInterface *s, uint8_t msg_id,
                        unsigned char *rsp, unsigned int rsp_len);
 
     /*
@@ -166,12 +166,38 @@ struct IPMIInterfaceClass {
      */
     void *(*get_backend_data)(struct IPMIInterface *s);
 
+    /*
+     * Set the IPMI core.
+     */
+    void (*set_ipmi_handler)(struct IPMIInterface *s, struct IPMICore *ic);
+
     /*
      * Return the firmware info for a device.
      */
     void (*get_fwinfo)(struct IPMIInterface *s, IPMIFwInfo *info);
 };
 
+/*
+ * Define an IPMI core (Either BMC or Host simulator.)
+ */
+#define TYPE_IPMI_CORE "ipmi-core"
+OBJECT_DECLARE_TYPE(IPMICore, IPMICoreClass, IPMI_CORE)
+
+struct IPMICore {
+    DeviceState parent;
+
+    IPMIInterface *intf;
+};
+
+struct IPMICoreClass {
+    DeviceClass parent;
+
+    /*
+     * Handle a hardware command.
+     */
+    void (*handle_hw_op)(struct IPMICore *s, uint8_t hw_op, uint8_t operand);
+};
+
 /*
  * Define a BMC simulator (or perhaps a connection to a real BMC)
  */
@@ -180,15 +206,13 @@ OBJECT_DECLARE_TYPE(IPMIBmc, IPMIBmcClass,
                     IPMI_BMC)
 
 struct IPMIBmc {
-    DeviceState parent;
+    IPMICore parent;
 
     uint8_t slave_addr;
-
-    IPMIInterface *intf;
 };
 
 struct IPMIBmcClass {
-    DeviceClass parent;
+    IPMICoreClass parent;
 
     /* Called when the system resets to report to the bmc. */
     void (*handle_reset)(struct IPMIBmc *s);
-- 
2.33.0.309.g3052b89438-goog



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

* [PATCH 5/8] hw/ipmi: Take out common from ipmi_bmc_extern.c
  2021-09-09 23:06 [PATCH 0/8] Handing IPMI for emulating BMC Hao Wu
                   ` (3 preceding siblings ...)
  2021-09-09 23:06 ` [PATCH 4/8] hw/ipmi: Refactor IPMI interface Hao Wu
@ 2021-09-09 23:06 ` Hao Wu
  2021-09-10  0:27   ` Corey Minyard
  2021-09-09 23:06 ` [PATCH 6/8] hw/ipmi: Move handle_command to IPMICoreClass Hao Wu
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Hao Wu @ 2021-09-09 23:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-arm, qemu-devel, wuhaotsh, venture, Avi.Fishman, kfting,
	hskinnemoen, titusr, peter.maydell

This patch refactors ipmi_bmc_extern.c and takes out the parts that can
be used both ipmi_bmc_extern.c and bmc_host_extern.c to a common file
ipmi_extern.c.

Now we have a connection called IPMIExtern which handles the connection,
and IPMIBmcExtern that handles core-side emulation specific stuff.

Basically most of the message transaction are moved. The stuff remained
are basically hardware operations like handle_reset and handle_hw_op.
These stuff have different behaviors in core-side and BMC-side
emulation.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
---
 hw/ipmi/ipmi_bmc_extern.c | 420 ++++----------------------------------
 hw/ipmi/ipmi_extern.c     | 415 +++++++++++++++++++++++++++++++++++++
 hw/ipmi/ipmi_extern.h     |  90 ++++++++
 hw/ipmi/meson.build       |   2 +-
 4 files changed, 543 insertions(+), 384 deletions(-)
 create mode 100644 hw/ipmi/ipmi_extern.c
 create mode 100644 hw/ipmi/ipmi_extern.h

diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
index a0c3a40e7c..24979ecfd5 100644
--- a/hw/ipmi/ipmi_bmc_extern.c
+++ b/hw/ipmi/ipmi_bmc_extern.c
@@ -34,211 +34,43 @@
 #include "qemu/timer.h"
 #include "chardev/char-fe.h"
 #include "hw/ipmi/ipmi.h"
+#include "hw/ipmi/ipmi_extern.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
 #include "migration/vmstate.h"
 #include "qom/object.h"
 
-#define VM_MSG_CHAR        0xA0 /* Marks end of message */
-#define VM_CMD_CHAR        0xA1 /* Marks end of a command */
-#define VM_ESCAPE_CHAR     0xAA /* Set bit 4 from the next byte to 0 */
-
-#define VM_PROTOCOL_VERSION        1
-#define VM_CMD_VERSION             0xff /* A version number byte follows */
-#define VM_CMD_NOATTN              0x00
-#define VM_CMD_ATTN                0x01
-#define VM_CMD_ATTN_IRQ            0x02
-#define VM_CMD_POWEROFF            0x03
-#define VM_CMD_RESET               0x04
-#define VM_CMD_ENABLE_IRQ          0x05 /* Enable/disable the messaging irq */
-#define VM_CMD_DISABLE_IRQ         0x06
-#define VM_CMD_SEND_NMI            0x07
-#define VM_CMD_CAPABILITIES        0x08
-#define   VM_CAPABILITIES_POWER    0x01
-#define   VM_CAPABILITIES_RESET    0x02
-#define   VM_CAPABILITIES_IRQ      0x04
-#define   VM_CAPABILITIES_NMI      0x08
-#define   VM_CAPABILITIES_ATTN     0x10
-#define   VM_CAPABILITIES_GRACEFUL_SHUTDOWN 0x20
-#define VM_CMD_GRACEFUL_SHUTDOWN   0x09
-
 #define TYPE_IPMI_BMC_EXTERN "ipmi-bmc-extern"
 OBJECT_DECLARE_SIMPLE_TYPE(IPMIBmcExtern, IPMI_BMC_EXTERN)
+
 struct IPMIBmcExtern {
     IPMIBmc parent;
 
-    CharBackend chr;
-
-    bool connected;
-
-    unsigned char inbuf[MAX_IPMI_MSG_SIZE + 2];
-    unsigned int inpos;
-    bool in_escape;
-    bool in_too_many;
-    bool waiting_rsp;
-    bool sending_cmd;
-
-    unsigned char outbuf[(MAX_IPMI_MSG_SIZE + 2) * 2 + 1];
-    unsigned int outpos;
-    unsigned int outlen;
-
-    struct QEMUTimer *extern_timer;
+    IPMIExtern conn;
 
     /* A reset event is pending to be sent upstream. */
     bool send_reset;
 };
 
-static unsigned char
-ipmb_checksum(const unsigned char *data, int size, unsigned char start)
+static void continue_send_bmc(IPMIBmcExtern *ibe)
 {
-        unsigned char csum = start;
-
-        for (; size > 0; size--, data++) {
-                csum += *data;
-        }
-        return csum;
-}
-
-static void continue_send(IPMIBmcExtern *ibe)
-{
-    int ret;
-    if (ibe->outlen == 0) {
-        goto check_reset;
-    }
- send:
-    ret = qemu_chr_fe_write(&ibe->chr, ibe->outbuf + ibe->outpos,
-                            ibe->outlen - ibe->outpos);
-    if (ret > 0) {
-        ibe->outpos += ret;
-    }
-    if (ibe->outpos < ibe->outlen) {
-        /* Not fully transmitted, try again in a 10ms */
-        timer_mod_ns(ibe->extern_timer,
-                     qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 10000000);
-    } else {
-        /* Sent */
-        ibe->outlen = 0;
-        ibe->outpos = 0;
-        if (!ibe->sending_cmd) {
-            ibe->waiting_rsp = true;
-        } else {
-            ibe->sending_cmd = false;
-        }
-    check_reset:
-        if (ibe->connected && ibe->send_reset) {
+    if (continue_send(&ibe->conn)) {
+        if (ibe->conn.connected && ibe->send_reset) {
             /* Send the reset */
-            ibe->outbuf[0] = VM_CMD_RESET;
-            ibe->outbuf[1] = VM_CMD_CHAR;
-            ibe->outlen = 2;
-            ibe->outpos = 0;
+            ibe->conn.outbuf[0] = VM_CMD_RESET;
+            ibe->conn.outbuf[1] = VM_CMD_CHAR;
+            ibe->conn.outlen = 2;
+            ibe->conn.outpos = 0;
             ibe->send_reset = false;
-            ibe->sending_cmd = true;
-            goto send;
-        }
-
-        if (ibe->waiting_rsp) {
-            /* Make sure we get a response within 4 seconds. */
-            timer_mod_ns(ibe->extern_timer,
-                         qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 4000000000ULL);
+            ibe->conn.sending_cmd = true;
+            continue_send(&ibe->conn);
         }
     }
-    return;
 }
 
-static void extern_timeout(void *opaque)
+static void ipmi_bmc_handle_hw_op(IPMICore *ic, unsigned char hw_op,
+                                  uint8_t operand)
 {
-    IPMICore *ic = opaque;
-    IPMIBmcExtern *ibe = opaque;
-    IPMIInterface *s = ic->intf;
-
-    if (ibe->connected) {
-        if (ibe->waiting_rsp && (ibe->outlen == 0)) {
-            IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
-            /* The message response timed out, return an error. */
-            ibe->waiting_rsp = false;
-            ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
-            ibe->inbuf[2] = ibe->outbuf[2];
-            ibe->inbuf[3] = IPMI_CC_TIMEOUT;
-            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
-        } else {
-            continue_send(ibe);
-        }
-    }
-}
-
-static void addchar(IPMIBmcExtern *ibe, unsigned char ch)
-{
-    switch (ch) {
-    case VM_MSG_CHAR:
-    case VM_CMD_CHAR:
-    case VM_ESCAPE_CHAR:
-        ibe->outbuf[ibe->outlen] = VM_ESCAPE_CHAR;
-        ibe->outlen++;
-        ch |= 0x10;
-        /* fall through */
-    default:
-        ibe->outbuf[ibe->outlen] = ch;
-        ibe->outlen++;
-    }
-}
-
-static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
-                                       uint8_t *cmd, unsigned int cmd_len,
-                                       unsigned int max_cmd_len,
-                                       uint8_t msg_id)
-{
-    IPMICore *ic = IPMI_CORE(b);
-    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(b);
-    IPMIInterface *s = ic->intf;
-    uint8_t err = 0, csum;
-    unsigned int i;
-
-    if (ibe->outlen) {
-        /* We already have a command queued.  Shouldn't ever happen. */
-        error_report("IPMI KCS: Got command when not finished with the"
-                     " previous command");
-        abort();
-    }
-
-    /* If it's too short or it was truncated, return an error. */
-    if (cmd_len < 2) {
-        err = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
-    } else if ((cmd_len > max_cmd_len) || (cmd_len > MAX_IPMI_MSG_SIZE)) {
-        err = IPMI_CC_REQUEST_DATA_TRUNCATED;
-    } else if (!ibe->connected) {
-        err = IPMI_CC_BMC_INIT_IN_PROGRESS;
-    }
-    if (err) {
-        IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
-        unsigned char rsp[3];
-        rsp[0] = cmd[0] | 0x04;
-        rsp[1] = cmd[1];
-        rsp[2] = err;
-        ibe->waiting_rsp = false;
-        k->handle_msg(s, msg_id, rsp, 3);
-        goto out;
-    }
-
-    addchar(ibe, msg_id);
-    for (i = 0; i < cmd_len; i++) {
-        addchar(ibe, cmd[i]);
-    }
-    csum = ipmb_checksum(&msg_id, 1, 0);
-    addchar(ibe, -ipmb_checksum(cmd, cmd_len, csum));
-
-    ibe->outbuf[ibe->outlen] = VM_MSG_CHAR;
-    ibe->outlen++;
-
-    /* Start the transmit */
-    continue_send(ibe);
-
- out:
-    return;
-}
-
-static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
-{
-    IPMICore *ic = IPMI_CORE(ibe);
     IPMIInterface *s = ic->intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
@@ -285,169 +117,22 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
     }
 }
 
-static void handle_msg(IPMIBmcExtern *ibe)
-{
-    IPMICore *ic = IPMI_CORE(ibe);
-    IPMIInterface *s = ic->intf;
-    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
-
-    if (ibe->in_escape) {
-        ipmi_debug("msg escape not ended\n");
-        return;
-    }
-    if (ibe->inpos < 5) {
-        ipmi_debug("msg too short\n");
-        return;
-    }
-    if (ibe->in_too_many) {
-        ibe->inbuf[3] = IPMI_CC_REQUEST_DATA_TRUNCATED;
-        ibe->inpos = 4;
-    } else if (ipmb_checksum(ibe->inbuf, ibe->inpos, 0) != 0) {
-        ipmi_debug("msg checksum failure\n");
-        return;
-    } else {
-        ibe->inpos--; /* Remove checkum */
-    }
-
-    timer_del(ibe->extern_timer);
-    ibe->waiting_rsp = false;
-    k->handle_msg(s, ibe->inbuf[0], ibe->inbuf + 1, ibe->inpos - 1);
-}
-
-static int can_receive(void *opaque)
-{
-    return 1;
-}
-
-static void receive(void *opaque, const uint8_t *buf, int size)
+static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
+                                       uint8_t *cmd, unsigned int cmd_len,
+                                       unsigned int max_cmd_len,
+                                       uint8_t msg_id)
 {
-    IPMIBmcExtern *ibe = opaque;
-    int i;
-    unsigned char hw_op;
-
-    for (i = 0; i < size; i++) {
-        unsigned char ch = buf[i];
-
-        switch (ch) {
-        case VM_MSG_CHAR:
-            handle_msg(ibe);
-            ibe->in_too_many = false;
-            ibe->inpos = 0;
-            break;
-
-        case VM_CMD_CHAR:
-            if (ibe->in_too_many) {
-                ipmi_debug("cmd in too many\n");
-                ibe->in_too_many = false;
-                ibe->inpos = 0;
-                break;
-            }
-            if (ibe->in_escape) {
-                ipmi_debug("cmd in escape\n");
-                ibe->in_too_many = false;
-                ibe->inpos = 0;
-                ibe->in_escape = false;
-                break;
-            }
-            ibe->in_too_many = false;
-            if (ibe->inpos < 1) {
-                break;
-            }
-            hw_op = ibe->inbuf[0];
-            ibe->inpos = 0;
-            goto out_hw_op;
-            break;
-
-        case VM_ESCAPE_CHAR:
-            ibe->in_escape = true;
-            break;
-
-        default:
-            if (ibe->in_escape) {
-                ch &= ~0x10;
-                ibe->in_escape = false;
-            }
-            if (ibe->in_too_many) {
-                break;
-            }
-            if (ibe->inpos >= sizeof(ibe->inbuf)) {
-                ibe->in_too_many = true;
-                break;
-            }
-            ibe->inbuf[ibe->inpos] = ch;
-            ibe->inpos++;
-            break;
-        }
-    }
-    return;
+    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(b);
 
- out_hw_op:
-    handle_hw_op(ibe, hw_op);
+    ipmi_extern_handle_command(&ibe->conn, cmd, cmd_len, max_cmd_len, msg_id);
 }
 
-static void chr_event(void *opaque, QEMUChrEvent event)
+static void ipmi_bmc_extern_realize(DeviceState *dev, Error **errp)
 {
-    IPMICore *ic = opaque;
-    IPMIBmcExtern *ibe = opaque;
-    IPMIInterface *s = ic->intf;
-    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
-    unsigned char v;
-
-    switch (event) {
-    case CHR_EVENT_OPENED:
-        ibe->connected = true;
-        ibe->outpos = 0;
-        ibe->outlen = 0;
-        addchar(ibe, VM_CMD_VERSION);
-        addchar(ibe, VM_PROTOCOL_VERSION);
-        ibe->outbuf[ibe->outlen] = VM_CMD_CHAR;
-        ibe->outlen++;
-        addchar(ibe, VM_CMD_CAPABILITIES);
-        v = VM_CAPABILITIES_IRQ | VM_CAPABILITIES_ATTN;
-        if (k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1) == 0) {
-            v |= VM_CAPABILITIES_POWER;
-        }
-        if (k->do_hw_op(s, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 1)
-            == 0) {
-            v |= VM_CAPABILITIES_GRACEFUL_SHUTDOWN;
-        }
-        if (k->do_hw_op(s, IPMI_RESET_CHASSIS, 1) == 0) {
-            v |= VM_CAPABILITIES_RESET;
-        }
-        if (k->do_hw_op(s, IPMI_SEND_NMI, 1) == 0) {
-            v |= VM_CAPABILITIES_NMI;
-        }
-        addchar(ibe, v);
-        ibe->outbuf[ibe->outlen] = VM_CMD_CHAR;
-        ibe->outlen++;
-        ibe->sending_cmd = false;
-        continue_send(ibe);
-        break;
-
-    case CHR_EVENT_CLOSED:
-        if (!ibe->connected) {
-            return;
-        }
-        ibe->connected = false;
-        /*
-         * Don't hang the OS trying to handle the ATN bit, other end will
-         * resend on a reconnect.
-         */
-        k->set_atn(s, 0, 0);
-        if (ibe->waiting_rsp) {
-            ibe->waiting_rsp = false;
-            ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
-            ibe->inbuf[2] = ibe->outbuf[2];
-            ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
-            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
-        }
-        break;
+    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(dev);
 
-    case CHR_EVENT_BREAK:
-    case CHR_EVENT_MUX_IN:
-    case CHR_EVENT_MUX_OUT:
-        /* Ignore */
-        break;
+    if (!qdev_realize(DEVICE(&ibe->conn), NULL, errp)) {
+        return;
     }
 }
 
@@ -456,42 +141,14 @@ static void ipmi_bmc_extern_handle_reset(IPMIBmc *b)
     IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(b);
 
     ibe->send_reset = true;
-    continue_send(ibe);
-}
-
-static void ipmi_bmc_extern_realize(DeviceState *dev, Error **errp)
-{
-    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(dev);
-
-    if (!qemu_chr_fe_backend_connected(&ibe->chr)) {
-        error_setg(errp, "IPMI external bmc requires chardev attribute");
-        return;
-    }
-
-    qemu_chr_fe_set_handlers(&ibe->chr, can_receive, receive,
-                             chr_event, NULL, ibe, NULL, true);
+    continue_send_bmc(ibe);
 }
 
 static int ipmi_bmc_extern_post_migrate(void *opaque, int version_id)
 {
-    IPMIBmcExtern *ibe = opaque;
-
-    /*
-     * We don't directly restore waiting_rsp, Instead, we return an
-     * error on the interface if a response was being waited for.
-     */
-    if (ibe->waiting_rsp) {
-        IPMICore *ic = opaque;
-        IPMIInterface *ii = ic->intf;
-        IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
-
-        ibe->waiting_rsp = false;
-        ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
-        ibe->inbuf[2] = ibe->outbuf[2];
-        ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
-        iic->handle_msg(ii, ibe->outbuf[0], ibe->inbuf + 1, 3);
-    }
-    return 0;
+    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(opaque);
+
+    return ipmi_extern_post_migrate(&ibe->conn, version_id);
 }
 
 static const VMStateDescription vmstate_ipmi_bmc_extern = {
@@ -501,28 +158,24 @@ static const VMStateDescription vmstate_ipmi_bmc_extern = {
     .post_load = ipmi_bmc_extern_post_migrate,
     .fields      = (VMStateField[]) {
         VMSTATE_BOOL(send_reset, IPMIBmcExtern),
-        VMSTATE_BOOL(waiting_rsp, IPMIBmcExtern),
+        VMSTATE_BOOL(conn.waiting_rsp, IPMIBmcExtern),
         VMSTATE_END_OF_LIST()
     }
 };
 
 static void ipmi_bmc_extern_init(Object *obj)
 {
+    IPMICore *ic = IPMI_CORE(obj);
     IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(obj);
 
-    ibe->extern_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, extern_timeout, ibe);
+    object_initialize_child(obj, "extern", &ibe->conn,
+                            TYPE_IPMI_EXTERN);
+    ibe->conn.core = ic;
     vmstate_register(NULL, 0, &vmstate_ipmi_bmc_extern, ibe);
 }
 
-static void ipmi_bmc_extern_finalize(Object *obj)
-{
-    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(obj);
-
-    timer_free(ibe->extern_timer);
-}
-
 static Property ipmi_bmc_extern_properties[] = {
-    DEFINE_PROP_CHR("chardev", IPMIBmcExtern, chr),
+    DEFINE_PROP_CHR("chardev", IPMIBmcExtern, conn.chr),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -530,9 +183,11 @@ static void ipmi_bmc_extern_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
     IPMIBmcClass *bk = IPMI_BMC_CLASS(oc);
+    IPMICoreClass *ck = IPMI_CORE_CLASS(oc);
 
     bk->handle_command = ipmi_bmc_extern_handle_command;
     bk->handle_reset = ipmi_bmc_extern_handle_reset;
+    ck->handle_hw_op = ipmi_bmc_handle_hw_op;
     dc->hotpluggable = false;
     dc->realize = ipmi_bmc_extern_realize;
     device_class_set_props(dc, ipmi_bmc_extern_properties);
@@ -543,9 +198,8 @@ static const TypeInfo ipmi_bmc_extern_type = {
     .parent        = TYPE_IPMI_BMC,
     .instance_size = sizeof(IPMIBmcExtern),
     .instance_init = ipmi_bmc_extern_init,
-    .instance_finalize = ipmi_bmc_extern_finalize,
     .class_init    = ipmi_bmc_extern_class_init,
- };
+};
 
 static void ipmi_bmc_extern_register_types(void)
 {
diff --git a/hw/ipmi/ipmi_extern.c b/hw/ipmi/ipmi_extern.c
new file mode 100644
index 0000000000..f139eaef24
--- /dev/null
+++ b/hw/ipmi/ipmi_extern.c
@@ -0,0 +1,415 @@
+/*
+ * IPMI external connection
+ *
+ * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "qemu/timer.h"
+#include "chardev/char-fe.h"
+#include "hw/ipmi/ipmi.h"
+#include "hw/ipmi/ipmi_extern.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "qom/object.h"
+
+static unsigned char
+ipmb_checksum(const unsigned char *data, int size, unsigned char start)
+{
+        unsigned char csum = start;
+
+        for (; size > 0; size--, data++) {
+                csum += *data;
+        }
+        return csum;
+}
+
+/* Returns whether check_reset is required for IPMI_BMC_EXTERN. */
+bool continue_send(IPMIExtern *ibe)
+{
+    int ret;
+    if (ibe->outlen == 0) {
+        return true;
+    }
+    ret = qemu_chr_fe_write(&ibe->chr, ibe->outbuf + ibe->outpos,
+                            ibe->outlen - ibe->outpos);
+    if (ret > 0) {
+        ibe->outpos += ret;
+    }
+    if (ibe->outpos < ibe->outlen) {
+        /* Not fully transmitted, try again in a 10ms */
+        timer_mod_ns(ibe->extern_timer,
+                     qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 10000000);
+        return false;
+    } else {
+        /* Sent */
+        ibe->outlen = 0;
+        ibe->outpos = 0;
+        if (!ibe->bmc_side && !ibe->sending_cmd) {
+            ibe->waiting_rsp = true;
+        } else {
+            ibe->sending_cmd = false;
+        }
+
+        if (ibe->waiting_rsp) {
+            /* Make sure we get a response within 4 seconds. */
+            timer_mod_ns(ibe->extern_timer,
+                         qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 4000000000ULL);
+        }
+        return true;
+    }
+}
+
+static void extern_timeout(void *opaque)
+{
+    IPMIExtern *ibe = opaque;
+    IPMIInterface *s = ibe->core->intf;
+
+    if (ibe->connected) {
+        /*TODO: only core-side */
+        if (ibe->waiting_rsp && (ibe->outlen == 0)) {
+            IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
+            /* The message response timed out, return an error. */
+            ibe->waiting_rsp = false;
+            ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
+            ibe->inbuf[2] = ibe->outbuf[2];
+            ibe->inbuf[3] = IPMI_CC_TIMEOUT;
+            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
+        } else {
+            continue_send(ibe);
+        }
+    }
+}
+
+static void addchar(IPMIExtern *ibe, unsigned char ch)
+{
+    switch (ch) {
+    case VM_MSG_CHAR:
+    case VM_CMD_CHAR:
+    case VM_ESCAPE_CHAR:
+        ibe->outbuf[ibe->outlen] = VM_ESCAPE_CHAR;
+        ibe->outlen++;
+        ch |= 0x10;
+        /* fall through */
+    default:
+        ibe->outbuf[ibe->outlen] = ch;
+        ibe->outlen++;
+    }
+}
+
+void ipmi_extern_handle_command(IPMIExtern *ibe,
+                                       uint8_t *cmd, unsigned int cmd_len,
+                                       unsigned int max_cmd_len,
+                                       uint8_t msg_id)
+{
+    IPMIInterface *s = ibe->core->intf;
+    uint8_t err = 0, csum;
+    unsigned int i;
+
+    if (ibe->outlen) {
+        /* We already have a command queued.  Shouldn't ever happen. */
+        error_report("IPMI KCS: Got command when not finished with the"
+                     " previous command");
+        abort();
+    }
+
+    /* If it's too short or it was truncated, return an error. */
+    if (cmd_len < 2) {
+        err = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
+    } else if ((cmd_len > max_cmd_len) || (cmd_len > MAX_IPMI_MSG_SIZE)) {
+        err = IPMI_CC_REQUEST_DATA_TRUNCATED;
+    } else if (!ibe->connected) {
+        err = IPMI_CC_BMC_INIT_IN_PROGRESS;
+    }
+    if (err) {
+        IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
+        unsigned char rsp[3];
+        rsp[0] = cmd[0] | 0x04;
+        rsp[1] = cmd[1];
+        rsp[2] = err;
+        ibe->waiting_rsp = false;
+        k->handle_msg(s, msg_id, rsp, 3);
+        goto out;
+    }
+
+    addchar(ibe, msg_id);
+    for (i = 0; i < cmd_len; i++) {
+        addchar(ibe, cmd[i]);
+    }
+    csum = ipmb_checksum(&msg_id, 1, 0);
+    addchar(ibe, -ipmb_checksum(cmd, cmd_len, csum));
+
+    ibe->outbuf[ibe->outlen] = VM_MSG_CHAR;
+    ibe->outlen++;
+
+    /* Start the transmit */
+    continue_send(ibe);
+
+ out:
+    return;
+}
+
+static void handle_msg(IPMIExtern *ibe)
+{
+    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(ibe->core->intf);
+
+    if (ibe->in_escape) {
+        ipmi_debug("msg escape not ended\n");
+        return;
+    }
+    if (ibe->inpos < (ibe->bmc_side ? 4 : 5)) {
+        ipmi_debug("msg too short\n");
+        return;
+    }
+    if (ibe->in_too_many) {
+        ibe->inbuf[3] = IPMI_CC_REQUEST_DATA_TRUNCATED;
+        ibe->inpos = 4;
+    } else if (ipmb_checksum(ibe->inbuf, ibe->inpos, 0) != 0) {
+        ipmi_debug("msg checksum failure\n");
+        return;
+    } else {
+        ibe->inpos--; /* Remove checkum */
+    }
+
+    timer_del(ibe->extern_timer);
+    ibe->waiting_rsp = false;
+    k->handle_msg(ibe->core->intf, ibe->inbuf[0],
+                  ibe->inbuf + 1, ibe->inpos - 1);
+}
+
+static int can_receive(void *opaque)
+{
+    return 1;
+}
+
+static void receive(void *opaque, const uint8_t *buf, int size)
+{
+    IPMIExtern *ibe = opaque;
+    IPMICoreClass *ck = IPMI_CORE_GET_CLASS(ibe->core);
+    int i;
+    unsigned char hw_op;
+    unsigned char hw_operand = 0;
+
+    for (i = 0; i < size; i++) {
+        unsigned char ch = buf[i];
+
+        switch (ch) {
+        case VM_MSG_CHAR:
+            handle_msg(ibe);
+            ibe->in_too_many = false;
+            ibe->inpos = 0;
+            break;
+
+        case VM_CMD_CHAR:
+            if (ibe->in_too_many) {
+                ipmi_debug("cmd in too many\n");
+                ibe->in_too_many = false;
+                ibe->inpos = 0;
+                break;
+            }
+            if (ibe->in_escape) {
+                ipmi_debug("cmd in escape\n");
+                ibe->in_too_many = false;
+                ibe->inpos = 0;
+                ibe->in_escape = false;
+                break;
+            }
+            ibe->in_too_many = false;
+            if (ibe->inpos < 1) {
+                break;
+            }
+            hw_op = ibe->inbuf[0];
+            if (ibe->inpos > 1) {
+                hw_operand = ibe->inbuf[1];
+            }
+            ibe->inpos = 0;
+            goto out_hw_op;
+            break;
+
+        case VM_ESCAPE_CHAR:
+            ibe->in_escape = true;
+            break;
+
+        default:
+            if (ibe->in_escape) {
+                ch &= ~0x10;
+                ibe->in_escape = false;
+            }
+            if (ibe->in_too_many) {
+                break;
+            }
+            if (ibe->inpos >= sizeof(ibe->inbuf)) {
+                ibe->in_too_many = true;
+                break;
+            }
+            ibe->inbuf[ibe->inpos] = ch;
+            ibe->inpos++;
+            break;
+        }
+    }
+    return;
+
+ out_hw_op:
+    ck->handle_hw_op(ibe->core, hw_op, hw_operand);
+}
+
+static void chr_event(void *opaque, QEMUChrEvent event)
+{
+    IPMIExtern *ibe = opaque;
+    IPMIInterface *s = ibe->core->intf;
+    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
+    unsigned char v;
+
+    switch (event) {
+    case CHR_EVENT_OPENED:
+        ibe->connected = true;
+        ibe->outpos = 0;
+        ibe->outlen = 0;
+        addchar(ibe, VM_CMD_VERSION);
+        addchar(ibe, VM_PROTOCOL_VERSION);
+        ibe->outbuf[ibe->outlen] = VM_CMD_CHAR;
+        ibe->outlen++;
+        /* Only send capability for core side. */
+        if (!ibe->bmc_side) {
+            addchar(ibe, VM_CMD_CAPABILITIES);
+            v = VM_CAPABILITIES_IRQ | VM_CAPABILITIES_ATTN;
+            if (k->do_hw_op(ibe->core->intf, IPMI_POWEROFF_CHASSIS, 1) == 0) {
+                v |= VM_CAPABILITIES_POWER;
+            }
+            if (k->do_hw_op(ibe->core->intf, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 1)
+                == 0) {
+                v |= VM_CAPABILITIES_GRACEFUL_SHUTDOWN;
+            }
+            if (k->do_hw_op(ibe->core->intf, IPMI_RESET_CHASSIS, 1) == 0) {
+                v |= VM_CAPABILITIES_RESET;
+            }
+            if (k->do_hw_op(ibe->core->intf, IPMI_SEND_NMI, 1) == 0) {
+                v |= VM_CAPABILITIES_NMI;
+            }
+            addchar(ibe, v);
+            ibe->outbuf[ibe->outlen] = VM_CMD_CHAR;
+            ibe->outlen++;
+        }
+        ibe->sending_cmd = false;
+        continue_send(ibe);
+        break;
+
+    case CHR_EVENT_CLOSED:
+        if (!ibe->connected) {
+            return;
+        }
+        ibe->connected = false;
+        /*
+         * Don't hang the OS trying to handle the ATN bit, other end will
+         * resend on a reconnect.
+         */
+        k->set_atn(s, 0, 0);
+        if (ibe->waiting_rsp) {
+            ibe->waiting_rsp = false;
+            ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
+            ibe->inbuf[2] = ibe->outbuf[2];
+            ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
+            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
+        }
+        break;
+
+    case CHR_EVENT_BREAK:
+    case CHR_EVENT_MUX_IN:
+    case CHR_EVENT_MUX_OUT:
+        /* Ignore */
+        break;
+    }
+}
+
+static void ipmi_extern_realize(DeviceState *dev, Error **errp)
+{
+    IPMIExtern *ibe = IPMI_EXTERN(dev);
+
+    if (!qemu_chr_fe_backend_connected(&ibe->chr)) {
+        error_setg(errp, "IPMI external bmc requires chardev attribute");
+        return;
+    }
+
+    qemu_chr_fe_set_handlers(&ibe->chr, can_receive, receive,
+                             chr_event, NULL, ibe, NULL, true);
+}
+
+int ipmi_extern_post_migrate(IPMIExtern *ibe, int version_id)
+{
+    /*
+     * We don't directly restore waiting_rsp, Instead, we return an
+     * error on the interface if a response was being waited for.
+     */
+    if (ibe->waiting_rsp) {
+        IPMIInterface *ii = ibe->core->intf;
+        IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+
+        ibe->waiting_rsp = false;
+        ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
+        ibe->inbuf[2] = ibe->outbuf[2];
+        ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
+        iic->handle_msg(ii, ibe->outbuf[0], ibe->inbuf + 1, 3);
+    }
+    return 0;
+}
+
+static void ipmi_extern_init(Object *obj)
+{
+    IPMIExtern *ibe = IPMI_EXTERN(obj);
+
+    ibe->extern_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, extern_timeout, ibe);
+}
+
+static void ipmi_extern_finalize(Object *obj)
+{
+    IPMIExtern *ibe = IPMI_EXTERN(obj);
+
+    timer_del(ibe->extern_timer);
+    timer_free(ibe->extern_timer);
+}
+
+
+static void ipmi_extern_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->hotpluggable = false;
+    dc->realize = ipmi_extern_realize;
+}
+
+static const TypeInfo ipmi_extern_type = {
+    .name          = TYPE_IPMI_EXTERN,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(IPMIExtern),
+    .instance_init = ipmi_extern_init,
+    .instance_finalize = ipmi_extern_finalize,
+    .class_init    = ipmi_extern_class_init,
+ };
+
+static void ipmi_extern_register_types(void)
+{
+    type_register_static(&ipmi_extern_type);
+}
+
+type_init(ipmi_extern_register_types)
diff --git a/hw/ipmi/ipmi_extern.h b/hw/ipmi/ipmi_extern.h
new file mode 100644
index 0000000000..e4aa80a0f6
--- /dev/null
+++ b/hw/ipmi/ipmi_extern.h
@@ -0,0 +1,90 @@
+/*
+ * IPMI external connection
+ *
+ * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef HW_IPMI_EXTERN_H
+#define HW_IPMI_EXTERN_H
+
+#include "qemu/osdep.h"
+#include "hw/ipmi/ipmi.h"
+
+#define VM_MSG_CHAR        0xA0 /* Marks end of message */
+#define VM_CMD_CHAR        0xA1 /* Marks end of a command */
+#define VM_ESCAPE_CHAR     0xAA /* Set bit 4 from the next byte to 0 */
+
+#define VM_PROTOCOL_VERSION        1
+#define VM_CMD_VERSION             0xff /* A version number byte follows */
+#define VM_CMD_NOATTN              0x00
+#define VM_CMD_ATTN                0x01
+#define VM_CMD_ATTN_IRQ            0x02
+#define VM_CMD_POWEROFF            0x03
+#define VM_CMD_RESET               0x04
+#define VM_CMD_ENABLE_IRQ          0x05 /* Enable/disable the messaging irq */
+#define VM_CMD_DISABLE_IRQ         0x06
+#define VM_CMD_SEND_NMI            0x07
+#define VM_CMD_CAPABILITIES        0x08
+#define   VM_CAPABILITIES_POWER    0x01
+#define   VM_CAPABILITIES_RESET    0x02
+#define   VM_CAPABILITIES_IRQ      0x04
+#define   VM_CAPABILITIES_NMI      0x08
+#define   VM_CAPABILITIES_ATTN     0x10
+#define   VM_CAPABILITIES_GRACEFUL_SHUTDOWN 0x20
+#define VM_CMD_GRACEFUL_SHUTDOWN   0x09
+
+typedef struct IPMIExtern {
+    DeviceState parent;
+
+    CharBackend chr;
+
+    IPMICore *core;
+
+    bool bmc_side;
+    bool connected;
+
+    unsigned char inbuf[MAX_IPMI_MSG_SIZE + 2];
+    unsigned int inpos;
+    bool in_escape;
+    bool in_too_many;
+    bool waiting_rsp;
+    bool sending_cmd;
+
+    unsigned char outbuf[(MAX_IPMI_MSG_SIZE + 2) * 2 + 1];
+    unsigned int outpos;
+    unsigned int outlen;
+
+    struct QEMUTimer *extern_timer;
+} IPMIExtern;
+
+#define TYPE_IPMI_EXTERN "ipmi-extern"
+#define IPMI_EXTERN(obj) \
+    OBJECT_CHECK(IPMIExtern, (obj), TYPE_IPMI_EXTERN)
+
+void ipmi_extern_handle_command(IPMIExtern *ibe,
+                                uint8_t *cmd, unsigned int cmd_len,
+                                unsigned int max_cmd_len,
+                                uint8_t msg_id);
+
+bool continue_send(IPMIExtern *ibe);
+int ipmi_extern_post_migrate(IPMIExtern *ibe, int version_id);
+
+#endif /* HW_IPMI_EXTERN_H */
diff --git a/hw/ipmi/meson.build b/hw/ipmi/meson.build
index 9622ea2a2c..edd0bf9af9 100644
--- a/hw/ipmi/meson.build
+++ b/hw/ipmi/meson.build
@@ -1,5 +1,5 @@
 ipmi_ss = ss.source_set()
-ipmi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c', 'ipmi_kcs.c', 'ipmi_bt.c'))
+ipmi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c', 'ipmi_kcs.c', 'ipmi_bt.c', 'ipmi_extern.c'))
 ipmi_ss.add(when: 'CONFIG_IPMI_LOCAL', if_true: files('ipmi_bmc_sim.c'))
 ipmi_ss.add(when: 'CONFIG_IPMI_EXTERN', if_true: files('ipmi_bmc_extern.c'))
 ipmi_ss.add(when: 'CONFIG_ISA_IPMI_KCS', if_true: files('isa_ipmi_kcs.c'))
-- 
2.33.0.309.g3052b89438-goog



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

* [PATCH 6/8] hw/ipmi: Move handle_command to IPMICoreClass
  2021-09-09 23:06 [PATCH 0/8] Handing IPMI for emulating BMC Hao Wu
                   ` (4 preceding siblings ...)
  2021-09-09 23:06 ` [PATCH 5/8] hw/ipmi: Take out common from ipmi_bmc_extern.c Hao Wu
@ 2021-09-09 23:06 ` Hao Wu
  2021-09-09 23:06 ` [PATCH 7/8] hw/ipmi: Add an IPMI external host device Hao Wu
  2021-09-09 23:06 ` [PATCH 8/8] hw/ipmi: Add a KCS Module for NPCM7XX Hao Wu
  7 siblings, 0 replies; 14+ messages in thread
From: Hao Wu @ 2021-09-09 23:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-arm, qemu-devel, wuhaotsh, venture, Avi.Fishman, kfting,
	hskinnemoen, titusr, peter.maydell

Move the function handle_command to IPMICoreClass. This function is
shared between BMC-side emulation and Host-side emulation.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
---
 hw/ipmi/ipmi_bmc_extern.c |  4 ++--
 hw/ipmi/ipmi_bmc_sim.c    |  6 +++---
 hw/ipmi/ipmi_bt.c         |  4 ++--
 hw/ipmi/ipmi_extern.c     |  6 +++---
 hw/ipmi/ipmi_kcs.c        |  6 +++---
 hw/ipmi/smbus_ipmi.c      |  6 +++---
 include/hw/ipmi/ipmi.h    | 16 ++++++++--------
 7 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
index 24979ecfd5..f7b88763c1 100644
--- a/hw/ipmi/ipmi_bmc_extern.c
+++ b/hw/ipmi/ipmi_bmc_extern.c
@@ -117,7 +117,7 @@ static void ipmi_bmc_handle_hw_op(IPMICore *ic, unsigned char hw_op,
     }
 }
 
-static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
+static void ipmi_bmc_extern_handle_command(IPMICore *b,
                                        uint8_t *cmd, unsigned int cmd_len,
                                        unsigned int max_cmd_len,
                                        uint8_t msg_id)
@@ -185,8 +185,8 @@ static void ipmi_bmc_extern_class_init(ObjectClass *oc, void *data)
     IPMIBmcClass *bk = IPMI_BMC_CLASS(oc);
     IPMICoreClass *ck = IPMI_CORE_CLASS(oc);
 
-    bk->handle_command = ipmi_bmc_extern_handle_command;
     bk->handle_reset = ipmi_bmc_extern_handle_reset;
+    ck->handle_command = ipmi_bmc_extern_handle_command;
     ck->handle_hw_op = ipmi_bmc_handle_hw_op;
     dc->hotpluggable = false;
     dc->realize = ipmi_bmc_extern_realize;
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 7cc4a22456..ddbf150e78 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -640,7 +640,7 @@ static void next_timeout(IPMIBmcSim *ibs)
     timer_mod_ns(ibs->timer, next);
 }
 
-static void ipmi_sim_handle_command(IPMIBmc *b,
+static void ipmi_sim_handle_command(IPMICore *b,
                                     uint8_t *cmd, unsigned int cmd_len,
                                     unsigned int max_cmd_len,
                                     uint8_t msg_id)
@@ -2222,12 +2222,12 @@ static Property ipmi_sim_properties[] = {
 static void ipmi_sim_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
-    IPMIBmcClass *bk = IPMI_BMC_CLASS(oc);
+    IPMICoreClass *ck = IPMI_CORE_CLASS(oc);
 
     dc->hotpluggable = false;
     dc->realize = ipmi_sim_realize;
     device_class_set_props(dc, ipmi_sim_properties);
-    bk->handle_command = ipmi_sim_handle_command;
+    ck->handle_command = ipmi_sim_handle_command;
 }
 
 static const TypeInfo ipmi_sim_type = {
diff --git a/hw/ipmi/ipmi_bt.c b/hw/ipmi/ipmi_bt.c
index f76c369e4a..60a04f2a65 100644
--- a/hw/ipmi/ipmi_bt.c
+++ b/hw/ipmi/ipmi_bt.c
@@ -141,8 +141,8 @@ static void ipmi_bt_handle_event(IPMIInterface *ii)
     ib->waiting_seq = ib->inmsg[2];
     ib->inmsg[2] = ib->inmsg[1];
     {
-        IPMIBmcClass *bk = IPMI_BMC_GET_CLASS(ib->bmc);
-        bk->handle_command(ib->bmc, ib->inmsg + 2, ib->inlen - 2,
+        IPMICoreClass *ck = IPMI_CORE_GET_CLASS(ib->bmc);
+        ck->handle_command(IPMI_CORE(ib->bmc), ib->inmsg + 2, ib->inlen - 2,
                            sizeof(ib->inmsg), ib->waiting_rsp);
     }
  out:
diff --git a/hw/ipmi/ipmi_extern.c b/hw/ipmi/ipmi_extern.c
index f139eaef24..97dfed085f 100644
--- a/hw/ipmi/ipmi_extern.c
+++ b/hw/ipmi/ipmi_extern.c
@@ -119,9 +119,9 @@ static void addchar(IPMIExtern *ibe, unsigned char ch)
 }
 
 void ipmi_extern_handle_command(IPMIExtern *ibe,
-                                       uint8_t *cmd, unsigned int cmd_len,
-                                       unsigned int max_cmd_len,
-                                       uint8_t msg_id)
+                                uint8_t *cmd, unsigned int cmd_len,
+                                unsigned int max_cmd_len,
+                                uint8_t msg_id)
 {
     IPMIInterface *s = ibe->core->intf;
     uint8_t err = 0, csum;
diff --git a/hw/ipmi/ipmi_kcs.c b/hw/ipmi/ipmi_kcs.c
index e0f870e13a..4a77dbb7e7 100644
--- a/hw/ipmi/ipmi_kcs.c
+++ b/hw/ipmi/ipmi_kcs.c
@@ -162,12 +162,12 @@ static void ipmi_kcs_handle_event(IPMIInterface *ii)
             ik->inlen++;
         }
         if (ik->write_end) {
-            IPMIBmcClass *bk = IPMI_BMC_GET_CLASS(ik->bmc);
+            IPMICoreClass *ck = IPMI_CORE_GET_CLASS(ik->bmc);
             ik->outlen = 0;
             ik->write_end = 0;
             ik->outpos = 0;
-            bk->handle_command(ik->bmc, ik->inmsg, ik->inlen, sizeof(ik->inmsg),
-                               ik->waiting_rsp);
+            ck->handle_command(IPMI_CORE(ik->bmc), ik->inmsg, ik->inlen,
+                               sizeof(ik->inmsg), ik->waiting_rsp);
             goto out_noibf;
         } else if (ik->cmd_reg == IPMI_KCS_WRITE_END_CMD) {
             ik->cmd_reg = -1;
diff --git a/hw/ipmi/smbus_ipmi.c b/hw/ipmi/smbus_ipmi.c
index a2383f1212..e0248ac45f 100644
--- a/hw/ipmi/smbus_ipmi.c
+++ b/hw/ipmi/smbus_ipmi.c
@@ -107,7 +107,7 @@ static void smbus_ipmi_send_msg(SMBusIPMIDevice *sid)
 {
     uint8_t *msg = sid->inmsg;
     uint32_t len = sid->inlen;
-    IPMIBmcClass *bk = IPMI_BMC_GET_CLASS(sid->bmc);
+    IPMICoreClass *ck = IPMI_CORE_GET_CLASS(sid->bmc);
 
     sid->outlen = 0;
     sid->outpos = 0;
@@ -135,8 +135,8 @@ static void smbus_ipmi_send_msg(SMBusIPMIDevice *sid)
         return;
     }
 
-    bk->handle_command(sid->bmc, sid->inmsg, sid->inlen, sizeof(sid->inmsg),
-                       sid->waiting_rsp);
+    ck->handle_command(IPMI_CORE(sid->bmc), sid->inmsg, sid->inlen,
+                       sizeof(sid->inmsg), sid->waiting_rsp);
 }
 
 static uint8_t ipmi_receive_byte(SMBusDevice *dev)
diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
index 1fa8cd12e5..0083c73e4b 100644
--- a/include/hw/ipmi/ipmi.h
+++ b/include/hw/ipmi/ipmi.h
@@ -196,6 +196,14 @@ struct IPMICoreClass {
      * Handle a hardware command.
      */
     void (*handle_hw_op)(struct IPMICore *s, uint8_t hw_op, uint8_t operand);
+
+    /*
+     * Handle a command to the bmc.
+     */
+    void (*handle_command)(struct IPMICore *s,
+                           uint8_t *cmd, unsigned int cmd_len,
+                           unsigned int max_cmd_len,
+                           uint8_t msg_id);
 };
 
 /*
@@ -216,14 +224,6 @@ struct IPMIBmcClass {
 
     /* Called when the system resets to report to the bmc. */
     void (*handle_reset)(struct IPMIBmc *s);
-
-    /*
-     * Handle a command to the bmc.
-     */
-    void (*handle_command)(struct IPMIBmc *s,
-                           uint8_t *cmd, unsigned int cmd_len,
-                           unsigned int max_cmd_len,
-                           uint8_t msg_id);
 };
 
 /*
-- 
2.33.0.309.g3052b89438-goog



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

* [PATCH 7/8] hw/ipmi: Add an IPMI external host device
  2021-09-09 23:06 [PATCH 0/8] Handing IPMI for emulating BMC Hao Wu
                   ` (5 preceding siblings ...)
  2021-09-09 23:06 ` [PATCH 6/8] hw/ipmi: Move handle_command to IPMICoreClass Hao Wu
@ 2021-09-09 23:06 ` Hao Wu
  2021-09-10  0:53   ` Corey Minyard
  2021-09-09 23:06 ` [PATCH 8/8] hw/ipmi: Add a KCS Module for NPCM7XX Hao Wu
  7 siblings, 1 reply; 14+ messages in thread
From: Hao Wu @ 2021-09-09 23:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-arm, qemu-devel, wuhaotsh, venture, Avi.Fishman, kfting,
	hskinnemoen, titusr, peter.maydell

The IPMI external host device works for Baseband Management Controller
(BMC) emulations. It works as a representation of a host class that
connects to a given BMC.  It can connect to a real host hardware or a
emulated or simulated host device. In particular it can connect to a
host QEMU instance with device ipmi-bmc-extern.

For more details of IPMI host device in BMC emulation, see
docs/specs/ipmi.rst.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
---
 configs/devices/arm-softmmu/default.mak |   2 +
 hw/ipmi/Kconfig                         |   5 +
 hw/ipmi/ipmi_extern.c                   |  18 ++-
 hw/ipmi/ipmi_host_extern.c              | 170 ++++++++++++++++++++++++
 hw/ipmi/meson.build                     |   1 +
 5 files changed, 194 insertions(+), 2 deletions(-)
 create mode 100644 hw/ipmi/ipmi_host_extern.c

diff --git a/configs/devices/arm-softmmu/default.mak b/configs/devices/arm-softmmu/default.mak
index 6985a25377..82f0c6f8c3 100644
--- a/configs/devices/arm-softmmu/default.mak
+++ b/configs/devices/arm-softmmu/default.mak
@@ -25,6 +25,8 @@ CONFIG_GUMSTIX=y
 CONFIG_SPITZ=y
 CONFIG_TOSA=y
 CONFIG_Z2=y
+CONFIG_IPMI=y
+CONFIG_IPMI_HOST=y
 CONFIG_NPCM7XX=y
 CONFIG_COLLIE=y
 CONFIG_ASPEED_SOC=y
diff --git a/hw/ipmi/Kconfig b/hw/ipmi/Kconfig
index 9befd4f422..6722b1fbb0 100644
--- a/hw/ipmi/Kconfig
+++ b/hw/ipmi/Kconfig
@@ -11,6 +11,11 @@ config IPMI_EXTERN
     default y
     depends on IPMI
 
+config IPMI_HOST
+    bool
+    default y
+    depends on IPMI
+
 config ISA_IPMI_KCS
     bool
     depends on ISA_BUS
diff --git a/hw/ipmi/ipmi_extern.c b/hw/ipmi/ipmi_extern.c
index 97dfed085f..0952dc5992 100644
--- a/hw/ipmi/ipmi_extern.c
+++ b/hw/ipmi/ipmi_extern.c
@@ -145,11 +145,25 @@ void ipmi_extern_handle_command(IPMIExtern *ibe,
     if (err) {
         IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
         unsigned char rsp[3];
+
         rsp[0] = cmd[0] | 0x04;
         rsp[1] = cmd[1];
         rsp[2] = err;
-        ibe->waiting_rsp = false;
-        k->handle_msg(s, msg_id, rsp, 3);
+
+        if (ibe->bmc_side) {
+            /* For BMC Side, send out an error message. */
+            addchar(ibe, msg_id);
+            for (i = 0; i < 3; ++i) {
+                addchar(ibe, rsp[i]);
+            }
+            csum = ipmb_checksum(&msg_id, 1, 0);
+            addchar(ibe, -ipmb_checksum(rsp, 3, csum));
+            continue_send(ibe);
+        } else {
+            /* For Core side, handle an error message. */
+            ibe->waiting_rsp = false;
+            k->handle_msg(s, msg_id, rsp, 3);
+        }
         goto out;
     }
 
diff --git a/hw/ipmi/ipmi_host_extern.c b/hw/ipmi/ipmi_host_extern.c
new file mode 100644
index 0000000000..4530631901
--- /dev/null
+++ b/hw/ipmi/ipmi_host_extern.c
@@ -0,0 +1,170 @@
+/*
+ * IPMI Host external connection
+ *
+ * Copyright 2021 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ */
+
+/*
+ * This is designed to connect to a host QEMU VM that runs ipmi_bmc_extern.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "chardev/char-fe.h"
+#include "hw/ipmi/ipmi.h"
+#include "hw/ipmi/ipmi_extern.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-properties-system.h"
+#include "migration/vmstate.h"
+#include "qom/object.h"
+
+#define TYPE_IPMI_HOST_EXTERN "ipmi-host-extern"
+OBJECT_DECLARE_SIMPLE_TYPE(IPMIHostExtern, IPMI_HOST_EXTERN)
+
+typedef struct IPMIHostExtern {
+    IPMICore parent;
+
+    IPMIExtern conn;
+
+    IPMIInterface *responder;
+
+    uint8_t capability;
+} IPMIHostExtern;
+
+/*
+ * Handle a command (typically IPMI response) from IPMI interface
+ * and send it out to the external host.
+ */
+static void ipmi_host_extern_handle_command(IPMICore *h, uint8_t *cmd,
+        unsigned cmd_len, unsigned max_cmd_len, uint8_t msg_id)
+{
+    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(h);
+
+    ipmi_extern_handle_command(&ihe->conn, cmd, cmd_len, max_cmd_len, msg_id);
+}
+
+/* This function handles a control command from the host. */
+static void ipmi_host_extern_handle_hw_op(IPMICore *ic, uint8_t cmd,
+                                          uint8_t operand)
+{
+    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(ic);
+
+    switch (cmd) {
+    case VM_CMD_VERSION:
+        /* The host informs us the protocol version. */
+        if (unlikely(operand != VM_PROTOCOL_VERSION)) {
+            ipmi_debug("Host protocol version %u is different from our version"
+                    " %u\n", operand, VM_PROTOCOL_VERSION);
+        }
+        break;
+
+    case VM_CMD_RESET:
+        /* The host tells us a reset has happened. */
+        break;
+
+    case VM_CMD_CAPABILITIES:
+        /* The host tells us its capability. */
+        ihe->capability = operand;
+        break;
+
+    default:
+        /* The host shouldn't send us this command. Just ignore if they do. */
+        ipmi_debug("Host cmd type %02x is invalid.\n", cmd);
+        break;
+    }
+}
+
+static void ipmi_host_extern_realize(DeviceState *dev, Error **errp)
+{
+    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(dev);
+    IPMIInterfaceClass *rk;
+
+    if (ihe->responder == NULL) {
+        error_setg(errp, "IPMI host extern requires responder attribute");
+        return;
+    }
+    rk = IPMI_INTERFACE_GET_CLASS(ihe->responder);
+    rk->set_ipmi_handler(ihe->responder, IPMI_CORE(ihe));
+    ihe->conn.core->intf = ihe->responder;
+
+    if (!qdev_realize(DEVICE(&ihe->conn), NULL, errp)) {
+        return;
+    }
+}
+
+static int ipmi_host_extern_post_migrate(void *opaque, int version_id)
+{
+    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(opaque);
+
+    return ipmi_extern_post_migrate(&ihe->conn, version_id);
+}
+
+static const VMStateDescription vmstate_ipmi_host_extern = {
+    .name = TYPE_IPMI_HOST_EXTERN,
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .post_load = ipmi_host_extern_post_migrate,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT8(capability, IPMIHostExtern),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void ipmi_host_extern_init(Object *obj)
+{
+    IPMICore *ic = IPMI_CORE(obj);
+    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(obj);
+
+    object_initialize_child(obj, "extern", &ihe->conn,
+                            TYPE_IPMI_EXTERN);
+    ihe->conn.core = ic;
+    ihe->conn.bmc_side = true;
+    vmstate_register(NULL, 0, &vmstate_ipmi_host_extern, ihe);
+}
+
+static Property ipmi_host_extern_properties[] = {
+    DEFINE_PROP_CHR("chardev", IPMIHostExtern, conn.chr),
+    DEFINE_PROP_LINK("responder", IPMIHostExtern, responder,
+                     TYPE_IPMI_INTERFACE, IPMIInterface *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ipmi_host_extern_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    IPMICoreClass *ck = IPMI_CORE_CLASS(oc);
+
+    device_class_set_props(dc, ipmi_host_extern_properties);
+
+    ck->handle_command = ipmi_host_extern_handle_command;
+    ck->handle_hw_op = ipmi_host_extern_handle_hw_op;
+    dc->hotpluggable = false;
+    dc->realize = ipmi_host_extern_realize;
+}
+
+static const TypeInfo ipmi_host_extern_type = {
+    .name          = TYPE_IPMI_HOST_EXTERN,
+    .parent        = TYPE_IPMI_CORE,
+    .instance_size = sizeof(IPMIHostExtern),
+    .instance_init = ipmi_host_extern_init,
+    .class_init    = ipmi_host_extern_class_init,
+};
+
+static void ipmi_host_extern_register_types(void)
+{
+    type_register_static(&ipmi_host_extern_type);
+}
+
+type_init(ipmi_host_extern_register_types)
diff --git a/hw/ipmi/meson.build b/hw/ipmi/meson.build
index edd0bf9af9..b1dd4710dc 100644
--- a/hw/ipmi/meson.build
+++ b/hw/ipmi/meson.build
@@ -7,5 +7,6 @@ ipmi_ss.add(when: 'CONFIG_PCI_IPMI_KCS', if_true: files('pci_ipmi_kcs.c'))
 ipmi_ss.add(when: 'CONFIG_ISA_IPMI_BT', if_true: files('isa_ipmi_bt.c'))
 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'))
 
 softmmu_ss.add_all(when: 'CONFIG_IPMI', if_true: ipmi_ss)
-- 
2.33.0.309.g3052b89438-goog



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

* [PATCH 8/8] hw/ipmi: Add a KCS Module for NPCM7XX
  2021-09-09 23:06 [PATCH 0/8] Handing IPMI for emulating BMC Hao Wu
                   ` (6 preceding siblings ...)
  2021-09-09 23:06 ` [PATCH 7/8] hw/ipmi: Add an IPMI external host device Hao Wu
@ 2021-09-09 23:06 ` Hao Wu
  7 siblings, 0 replies; 14+ messages in thread
From: Hao Wu @ 2021-09-09 23:06 UTC (permalink / raw)
  To: minyard
  Cc: qemu-arm, qemu-devel, wuhaotsh, venture, Avi.Fishman, kfting,
	hskinnemoen, titusr, peter.maydell

Add a KCS module for NPCM7xx SoC. This module implements the IPMI
responder interface and is responsible to communicate with an external
host via the KCS channels in an NPCM7xx SoC.

Note that we cannot directly use ipmi_kcs.c since the communication
direction is the opposite. For example, in READ_STATE, ipmi_kcs.c (core
side emulation) reads a message from BMC while npcm7xx_kcs.c
(BMC-side emulation) sends a message to the core.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
---
 docs/system/arm/nuvoton.rst   |   1 -
 hw/arm/npcm7xx.c              |  10 +-
 hw/ipmi/meson.build           |   1 +
 hw/ipmi/npcm7xx_kcs.c         | 588 ++++++++++++++++++++++++++++++++++
 hw/ipmi/trace-events          |   8 +
 hw/ipmi/trace.h               |   1 +
 include/hw/arm/npcm7xx.h      |   2 +
 include/hw/ipmi/npcm7xx_kcs.h | 103 ++++++
 meson.build                   |   1 +
 9 files changed, 713 insertions(+), 2 deletions(-)
 create mode 100644 hw/ipmi/npcm7xx_kcs.c
 create mode 100644 hw/ipmi/trace-events
 create mode 100644 hw/ipmi/trace.h
 create mode 100644 include/hw/ipmi/npcm7xx_kcs.h

diff --git a/docs/system/arm/nuvoton.rst b/docs/system/arm/nuvoton.rst
index 69f57c2886..0358323e76 100644
--- a/docs/system/arm/nuvoton.rst
+++ b/docs/system/arm/nuvoton.rst
@@ -54,7 +54,6 @@ Missing devices
  * LPC/eSPI host-to-BMC interface, including
 
    * Keyboard and mouse controller interface (KBCI)
-   * Keyboard Controller Style (KCS) channels
    * BIOS POST code FIFO
    * System Wake-up Control (SWC)
    * Shared memory (SHM)
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index 2ab0080e0b..319c5ba70e 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -45,6 +45,7 @@
 #define NPCM7XX_CLK_BA          (0xf0801000)
 #define NPCM7XX_MC_BA           (0xf0824000)
 #define NPCM7XX_RNG_BA          (0xf000b000)
+#define NPCM7XX_KCS_BA          (0xf0007000)
 
 /* USB Host modules */
 #define NPCM7XX_EHCI_BA         (0xf0806000)
@@ -81,6 +82,7 @@ enum NPCM7xxInterrupt {
     NPCM7XX_UART1_IRQ,
     NPCM7XX_UART2_IRQ,
     NPCM7XX_UART3_IRQ,
+    NPCM7XX_KCS_HIB_IRQ         = 9,
     NPCM7XX_EMC1RX_IRQ          = 15,
     NPCM7XX_EMC1TX_IRQ,
     NPCM7XX_TIMER0_IRQ          = 32,   /* Timer Module 0 */
@@ -423,6 +425,7 @@ static void npcm7xx_init(Object *obj)
                                 TYPE_NPCM7XX_SMBUS);
     }
 
+    object_initialize_child(obj, "kcs", &s->kcs, TYPE_NPCM7XX_KCS);
     object_initialize_child(obj, "ehci", &s->ehci, TYPE_NPCM7XX_EHCI);
     object_initialize_child(obj, "ohci", &s->ohci, TYPE_SYSBUS_OHCI);
 
@@ -598,6 +601,12 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
                            npcm7xx_irq(s, NPCM7XX_SMBUS0_IRQ + i));
     }
 
+    /* KCS modules*/
+    sysbus_realize(SYS_BUS_DEVICE(&s->kcs), &error_abort);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->kcs), 0, NPCM7XX_KCS_BA);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->kcs), 0,
+                       npcm7xx_irq(s, NPCM7XX_KCS_HIB_IRQ));
+
     /* USB Host */
     object_property_set_bool(OBJECT(&s->ehci), "companion-enable", true,
                              &error_abort);
@@ -710,7 +719,6 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
     create_unimplemented_device("npcm7xx.shm",          0xc0001000,   4 * KiB);
     create_unimplemented_device("npcm7xx.vdmx",         0xe0800000,   4 * KiB);
     create_unimplemented_device("npcm7xx.pcierc",       0xe1000000,  64 * KiB);
-    create_unimplemented_device("npcm7xx.kcs",          0xf0007000,   4 * KiB);
     create_unimplemented_device("npcm7xx.gfxi",         0xf000e000,   4 * KiB);
     create_unimplemented_device("npcm7xx.espi",         0xf009f000,   4 * KiB);
     create_unimplemented_device("npcm7xx.peci",         0xf0100000,   4 * KiB);
diff --git a/hw/ipmi/meson.build b/hw/ipmi/meson.build
index b1dd4710dc..3d8b030ba5 100644
--- a/hw/ipmi/meson.build
+++ b/hw/ipmi/meson.build
@@ -8,5 +8,6 @@ ipmi_ss.add(when: 'CONFIG_ISA_IPMI_BT', if_true: files('isa_ipmi_bt.c'))
 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'))
 
 softmmu_ss.add_all(when: 'CONFIG_IPMI', if_true: ipmi_ss)
diff --git a/hw/ipmi/npcm7xx_kcs.c b/hw/ipmi/npcm7xx_kcs.c
new file mode 100644
index 0000000000..197ddcd6e0
--- /dev/null
+++ b/hw/ipmi/npcm7xx_kcs.c
@@ -0,0 +1,588 @@
+/*
+ * Nuvoton NPCM7xx KCS Module
+ *
+ * Copyright 2021 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/ipmi/npcm7xx_kcs.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "qemu/units.h"
+#include "trace.h"
+
+/* Registers in each KCS channel. */
+typedef enum NPCM7xxKCSRegister {
+    NPCM7XX_KCSST,
+    NPCM7XX_KCSDO,
+    NPCM7XX_KCSDI,
+    NPCM7XX_KCSDOC,
+    NPCM7XX_KCSDOM,
+    NPCM7XX_KCSDIC,
+    NPCM7XX_KCSCTL,
+    NPCM7XX_KCSIC,
+    NPCM7XX_KCSIE,
+    NPCM7XX_KCS_REGS_END,
+} NPCM7xxKCSRegister;
+
+static const hwaddr npcm7xx_kcs_channel_base[] = {
+    0x0c, 0x1e, 0x30, 0x42
+};
+
+#define NPCM7XX_KCS_REG_DIFF    2
+
+/* Register field definitions. */
+#define NPCM7XX_CTL_OBEIE       BIT(1)
+#define NPCM7XX_CTL_IBFIE       BIT(0)
+
+/* IPMI 2.0 Table 9.1 status register bits */
+#define KCS_ST_STATE(rv)    extract8(rv, 6, 2)
+#define KCS_ST_CMD          BIT(3)
+#define KCS_ST_SMS_ATN      BIT(2)
+#define KCS_ST_IBF          BIT(1)
+#define KCS_ST_OBF          BIT(0)
+
+/* IPMI 2.0 Table 9.2 state bits */
+enum KCSState {
+    IDLE_STATE,
+    READ_STATE,
+    WRITE_STATE,
+    ERROR_STATE,
+};
+
+/* IPMI 2.0 Table 9.3 control codes */
+#define KCS_CMD_GET_STATUS_ABORT    0x60
+#define KCS_CMD_WRITE_START         0x61
+#define KCS_CMD_WRITE_END           0x62
+#define KCS_CMD_READ                0x68
+
+/* Host Side Operations. */
+
+static uint8_t npcm7xx_kcs_host_read_byte(NPCM7xxKCSChannel *c)
+{
+    uint8_t v;
+
+    v = c->dbbout;
+    c->status &= ~KCS_ST_OBF;
+    if (c->ctl & NPCM7XX_CTL_OBEIE) {
+        qemu_irq_raise(c->owner->irq);
+    }
+
+    trace_npcm7xx_kcs_host_read_byte(DEVICE(c)->canonical_path, v);
+    return v;
+}
+
+static void npcm7xx_kcs_host_write_byte(NPCM7xxKCSChannel *c, uint8_t value,
+        bool is_cmd)
+{
+    trace_npcm7xx_kcs_host_write_byte(DEVICE(c)->canonical_path, value);
+    c->dbbin = value;
+    c->status |= KCS_ST_IBF;
+    if (is_cmd) {
+        c->status |= KCS_ST_CMD;
+    } else {
+        c->status &= ~KCS_ST_CMD;
+    }
+    if (c->ctl & NPCM7XX_CTL_IBFIE) {
+        qemu_irq_raise(c->owner->irq);
+    }
+}
+
+static void npcm7xx_kcs_handle_event(NPCM7xxKCSChannel *c)
+{
+    uint8_t v;
+    IPMICoreClass *hk;
+
+    trace_npcm7xx_kcs_handle_event(DEVICE(c)->canonical_path,
+                                   KCS_ST_STATE(c->status));
+    switch (KCS_ST_STATE(c->status)) {
+    case IDLE_STATE:
+        if (c->status & KCS_ST_OBF) {
+            /* Read a dummy byte. */
+            npcm7xx_kcs_host_read_byte(c);
+            if (c->outlen > 0) {
+                /* Send to ipmi host when msg ends. */
+                if (c->host) {
+                    hk = IPMI_CORE_GET_CLASS(c->host);
+                    hk->handle_command(c->host, c->outmsg, c->outlen,
+                            MAX_IPMI_MSG_SIZE, c->last_msg_id);
+                }
+                /* The last byte has been read. return to empty state. */
+                c->outlen = 0;
+            }
+        }
+        if (c->inlen > 0) {
+            /* Send to bmc the next request */
+            npcm7xx_kcs_host_write_byte(c, KCS_CMD_WRITE_START, true);
+            c->last_byte_not_ready = true;
+        }
+        break;
+
+    case READ_STATE:
+        if (c->status & KCS_ST_OBF) {
+            /* Read in a byte from BMC */
+            v = npcm7xx_kcs_host_read_byte(c);
+            if (c->outlen < MAX_IPMI_MSG_SIZE) {
+                c->outmsg[c->outlen++] = v;
+            }
+            npcm7xx_kcs_host_write_byte(c, KCS_CMD_READ, false);
+        }
+        break;
+
+    case WRITE_STATE:
+        if (c->status & KCS_ST_IBF) {
+            /* The guest hasn't read the byte yet. We'll wait. */
+            break;
+        }
+        /* Clear OBF. */
+        c->status &= ~KCS_ST_OBF;
+        /* If it's the last byte written, we need to first send a command. */
+        if (c->last_byte_not_ready && c->inpos == c->inlen - 1) {
+            npcm7xx_kcs_host_write_byte(c, KCS_CMD_WRITE_END, true);
+            c->last_byte_not_ready = false;
+        } else {
+            npcm7xx_kcs_host_write_byte(c, c->inmsg[c->inpos++], false);
+            if (!c->last_byte_not_ready) {
+                /* The last byte has been sent. */
+                c->inlen = 0;
+                c->inpos = 0;
+            }
+        }
+        break;
+
+    case ERROR_STATE:
+        if (c->status & KCS_ST_OBF) {
+            /* Read in error byte from BMC */
+            npcm7xx_kcs_host_read_byte(c);
+        }
+        /* Force abort */
+        c->outlen = 0;
+        c->inlen = 0;
+        c->inpos = 0;
+        c->status = 0;
+        break;
+
+    default:
+        g_assert_not_reached();
+    }
+}
+
+/* Received a request from the host and send it to BMC core. */
+static void npcm7xx_kcs_handle_req(IPMIInterface *ii, uint8_t msg_id,
+                                unsigned char *req, unsigned req_len)
+{
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
+    NPCM7xxKCSChannel *c = iic->get_backend_data(ii);
+
+    /* Drop the request if the last request is still not handled. */
+    if (c->inlen > 0) {
+        return;
+    }
+    if (req_len == 0) {
+        return;
+    }
+    if (req_len > MAX_IPMI_MSG_SIZE) {
+        /* Truncate the extra bytes that don't fit. */
+        req_len = MAX_IPMI_MSG_SIZE;
+    }
+    memcpy(c->inmsg, req, req_len);
+    c->inpos = 0;
+    c->inlen = req_len;
+
+    c->last_msg_id = msg_id;
+
+    npcm7xx_kcs_handle_event(c);
+}
+
+/* Core Side Operations. */
+/* Return the channel index for addr. If the addr is out of range, return -1. */
+static int npcm7xx_kcs_channel_index(hwaddr addr)
+{
+    int index;
+
+    if (unlikely(addr < npcm7xx_kcs_channel_base[0])) {
+        return -1;
+    }
+    if (unlikely(addr >= npcm7xx_kcs_channel_base[NPCM7XX_KCS_NR_CHANNELS])) {
+        return -1;
+    }
+    if (unlikely(addr % NPCM7XX_KCS_REG_DIFF)) {
+        return -1;
+    }
+
+    for (index = 0; index < NPCM7XX_KCS_NR_CHANNELS; ++index) {
+        if (addr < npcm7xx_kcs_channel_base[index + 1]) {
+            return index;
+        }
+    }
+
+    g_assert_not_reached();
+}
+
+static NPCM7xxKCSRegister npcm7xx_kcs_reg_index(hwaddr addr, int channel)
+{
+    NPCM7xxKCSRegister reg;
+
+    reg = (addr - npcm7xx_kcs_channel_base[channel]) / NPCM7XX_KCS_REG_DIFF;
+    return reg;
+}
+
+static uint8_t npcm7xx_kcs_read_byte(NPCM7xxKCSChannel *c,
+        NPCM7xxKCSRegister reg)
+{
+    uint8_t v = 0;
+
+    v = c->dbbin;
+
+    c->status &= ~KCS_ST_IBF;
+    if (c->ctl & NPCM7XX_CTL_IBFIE) {
+        qemu_irq_lower(c->owner->irq);
+    }
+
+    if (reg == NPCM7XX_KCSDIC) {
+        qemu_log_mask(LOG_UNIMP,
+                "%s: Host nSCIPME interrupt is not implemented.\n",
+                __func__);
+    }
+
+    npcm7xx_kcs_handle_event(c);
+    return v;
+}
+
+static void npcm7xx_kcs_write_byte(NPCM7xxKCSChannel *c,
+        NPCM7xxKCSRegister reg, uint8_t value)
+{
+    c->dbbout = value;
+
+    c->status |= KCS_ST_OBF;
+    if (c->ctl & NPCM7XX_CTL_OBEIE) {
+        qemu_irq_lower(c->owner->irq);
+    }
+
+    if (reg == NPCM7XX_KCSDOC) {
+        qemu_log_mask(LOG_UNIMP,
+                "%s: Host nSCIPME interrupt is not implemented.\n",
+                __func__);
+    } else if (reg == NPCM7XX_KCSDOM) {
+        qemu_log_mask(LOG_UNIMP,
+                "%s: Host nSMI interrupt is not implemented.\n",
+                __func__);
+    }
+
+    npcm7xx_kcs_handle_event(c);
+}
+
+static uint8_t npcm7xx_kcs_read_status(NPCM7xxKCSChannel *c)
+{
+    uint8_t status = c->status;
+
+    return status;
+}
+
+static void npcm7xx_kcs_write_status(NPCM7xxKCSChannel *c, uint8_t value)
+{
+    static const uint8_t mask =
+        KCS_ST_CMD | KCS_ST_IBF | KCS_ST_OBF;
+
+    if (value & mask) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: read-only bits in 0x%02x ignored\n",
+                      __func__, value);
+        value &= ~mask;
+    }
+
+    c->status = (c->status & mask) | value;
+}
+
+static void npcm7xx_kcs_write_ctl(NPCM7xxKCSChannel *c, uint8_t value)
+{
+    if (value & ~(NPCM7XX_CTL_OBEIE | NPCM7XX_CTL_IBFIE)) {
+        qemu_log_mask(LOG_UNIMP, "%s: Host interrupts are not implemented.\n",
+                __func__);
+    }
+
+    c->ctl = value;
+}
+
+static uint64_t npcm7xx_kcs_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    NPCM7xxKCSState *s = opaque;
+    uint8_t value = 0;
+    int channel;
+    NPCM7xxKCSRegister reg;
+
+    channel = npcm7xx_kcs_channel_index(addr);
+    if (channel < 0 || channel >= NPCM7XX_KCS_NR_CHANNELS) {
+        qemu_log_mask(LOG_UNIMP,
+                      "%s: read from addr 0x%04" HWADDR_PRIx
+                      " is invalid or unimplemented.\n",
+                      __func__, addr);
+        return value;
+    }
+
+    reg = npcm7xx_kcs_reg_index(addr, channel);
+    switch (reg) {
+    case NPCM7XX_KCSDI:
+    case NPCM7XX_KCSDIC:
+        value = npcm7xx_kcs_read_byte(&s->channels[channel], reg);
+        break;
+    case NPCM7XX_KCSDO:
+    case NPCM7XX_KCSDOC:
+    case NPCM7XX_KCSDOM:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: register @ 0x%04" HWADDR_PRIx " is write-only\n",
+                      __func__, addr);
+        break;
+    case NPCM7XX_KCSST:
+        value = npcm7xx_kcs_read_status(&s->channels[channel]);
+        break;
+    case NPCM7XX_KCSCTL:
+        value = s->channels[channel].ctl;
+        break;
+    case NPCM7XX_KCSIC:
+        value = s->channels[channel].ic;
+        break;
+    case NPCM7XX_KCSIE:
+        value = s->channels[channel].ie;
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: read from invalid addr 0x%04" HWADDR_PRIx "\n",
+                      __func__, addr);
+    }
+
+    trace_npcm7xx_kcs_read(DEVICE(s)->canonical_path, channel, reg, value);
+    return value;
+}
+
+static void npcm7xx_kcs_write(void *opaque, hwaddr addr, uint64_t v,
+                                    unsigned int size)
+{
+    NPCM7xxKCSState *s = opaque;
+    int channel;
+    NPCM7xxKCSRegister reg;
+
+    channel = npcm7xx_kcs_channel_index(addr);
+    if (channel < 0 || channel >= NPCM7XX_KCS_NR_CHANNELS) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: write to addr 0x%04" HWADDR_PRIx
+                      " is invalid or unimplemented.\n",
+                      __func__, addr);
+        return;
+    }
+
+    reg = npcm7xx_kcs_reg_index(addr, channel);
+    trace_npcm7xx_kcs_write(DEVICE(s)->canonical_path, channel, reg, (uint8_t)v);
+    switch (reg) {
+    case NPCM7XX_KCSDI:
+    case NPCM7XX_KCSDIC:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: register @ 0x%04" HWADDR_PRIx " is read-only\n",
+                      __func__, addr);
+        break;
+    case NPCM7XX_KCSDO:
+    case NPCM7XX_KCSDOC:
+    case NPCM7XX_KCSDOM:
+        npcm7xx_kcs_write_byte(&s->channels[channel], reg, v);
+        break;
+    case NPCM7XX_KCSST:
+        npcm7xx_kcs_write_status(&s->channels[channel], v);
+        break;
+    case NPCM7XX_KCSCTL:
+        npcm7xx_kcs_write_ctl(&s->channels[channel], v);
+        break;
+    case NPCM7XX_KCSIC:
+        qemu_log_mask(LOG_UNIMP, "%s: Host interrupts are not implemented.\n",
+                __func__);
+        break;
+    case NPCM7XX_KCSIE:
+        qemu_log_mask(LOG_UNIMP, "%s: Host interrupts are not implemented.\n",
+                __func__);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: read from invalid addr 0x%04" HWADDR_PRIx "\n",
+                      __func__, addr);
+    }
+}
+
+static const MemoryRegionOps npcm7xx_kcs_ops = {
+    .read = npcm7xx_kcs_read,
+    .write = npcm7xx_kcs_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 1, /* KCS registers are 8-bits. */
+        .max_access_size = 1, /* KCS registers are 8-bits. */
+        .unaligned = false,
+    },
+};
+
+static const VMStateDescription vmstate_npcm7xx_kcs_channel = {
+    .name = "npcm7xx-kcs-channel",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(status, NPCM7xxKCSChannel),
+        VMSTATE_UINT8(dbbout, NPCM7xxKCSChannel),
+        VMSTATE_UINT8(dbbin, NPCM7xxKCSChannel),
+        VMSTATE_UINT8(ctl, NPCM7xxKCSChannel),
+        VMSTATE_UINT8(ic, NPCM7xxKCSChannel),
+        VMSTATE_UINT8(ie, NPCM7xxKCSChannel),
+        VMSTATE_END_OF_LIST(),
+    },
+};
+
+static void npcm7xx_kcs_channel_realize(DeviceState *dev, Error **errp)
+{
+    IPMIInterface *ii = IPMI_INTERFACE(dev);
+    NPCM7xxKCSChannel *c = NPCM7XX_KCS_CHANNEL(ii);
+
+    if (c->host) {
+        c->host->intf = ii;
+    }
+}
+
+static void *npcm7xx_kcs_get_backend_data(IPMIInterface *ii)
+{
+    NPCM7xxKCSChannel *c = NPCM7XX_KCS_CHANNEL(ii);
+
+    return c;
+}
+
+static void npcm7xx_kcs_set_ipmi_handler(IPMIInterface *ii, IPMICore *h)
+{
+    NPCM7xxKCSChannel *c = NPCM7XX_KCS_CHANNEL(ii);
+
+    c->host = h;
+}
+
+static void npcm7xx_kcs_set_atn(struct IPMIInterface *s, int val, int irq)
+{
+}
+
+static void npcm7xx_kcs_channel_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    IPMIInterfaceClass *iic = IPMI_INTERFACE_CLASS(klass);
+
+    QEMU_BUILD_BUG_ON(NPCM7XX_KCS_REGS_END != NPCM7XX_KCS_CHANNEL_NR_REGS);
+
+    dc->desc = "NPCM7xx KCS Channel";
+    dc->vmsd = &vmstate_npcm7xx_kcs_channel;
+    dc->realize = npcm7xx_kcs_channel_realize;
+
+    iic->get_backend_data = npcm7xx_kcs_get_backend_data;
+    iic->handle_msg = npcm7xx_kcs_handle_req;
+    iic->set_ipmi_handler = npcm7xx_kcs_set_ipmi_handler;
+    iic->set_atn = npcm7xx_kcs_set_atn;
+}
+
+static const TypeInfo npcm7xx_kcs_channel_info = {
+    .name               = TYPE_NPCM7XX_KCS_CHANNEL,
+    .parent             = TYPE_DEVICE,
+    .instance_size      = sizeof(NPCM7xxKCSChannel),
+    .class_init         = npcm7xx_kcs_channel_class_init,
+    .interfaces         = (InterfaceInfo[]) {
+        { TYPE_IPMI_INTERFACE },
+        { },
+    },
+};
+
+static void npcm7xx_kcs_enter_reset(Object *obj, ResetType type)
+{
+    NPCM7xxKCSState *s = NPCM7XX_KCS(obj);
+    int i;
+
+    for (i = 0; i < NPCM7XX_KCS_NR_CHANNELS; i++) {
+        NPCM7xxKCSChannel *c = &s->channels[i];
+
+        c->status = 0x00;
+        c->ctl = 0xc0;
+        c->ic = 0x41;
+        c->ie = 0x00;
+    }
+}
+
+static void npcm7xx_kcs_hold_reset(Object *obj)
+{
+    NPCM7xxKCSState *s = NPCM7XX_KCS(obj);
+
+    qemu_irq_lower(s->irq);
+}
+
+static const VMStateDescription vmstate_npcm7xx_kcs = {
+    .name = "npcm7xx-kcs",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST(),
+    },
+};
+
+static void npcm7xx_kcs_realize(DeviceState *dev, Error **errp)
+{
+    NPCM7xxKCSState *s = NPCM7XX_KCS(dev);
+    SysBusDevice *sbd = &s->parent;
+    int i;
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &npcm7xx_kcs_ops, s,
+                          TYPE_NPCM7XX_KCS, 4 * KiB);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq);
+
+    for (i = 0; i < NPCM7XX_KCS_NR_CHANNELS; i++) {
+        s->channels[i].owner = s;
+        if (!qdev_realize(DEVICE(&s->channels[i]), NULL, errp)) {
+            return;
+        }
+    }
+}
+
+static void npcm7xx_kcs_init(Object *obj)
+{
+    NPCM7xxKCSState *s = NPCM7XX_KCS(obj);
+    int i;
+
+    for (i = 0; i < NPCM7XX_KCS_NR_CHANNELS; i++) {
+        object_initialize_child(obj, g_strdup_printf("channels[%d]", i),
+                &s->channels[i], TYPE_NPCM7XX_KCS_CHANNEL);
+    }
+}
+
+static void npcm7xx_kcs_class_init(ObjectClass *klass, void *data)
+{
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "NPCM7xx Timer Controller";
+    dc->vmsd = &vmstate_npcm7xx_kcs;
+    dc->realize = npcm7xx_kcs_realize;
+    rc->phases.enter = npcm7xx_kcs_enter_reset;
+    rc->phases.hold = npcm7xx_kcs_hold_reset;
+}
+
+static const TypeInfo npcm7xx_kcs_info = {
+    .name               = TYPE_NPCM7XX_KCS,
+    .parent             = TYPE_SYS_BUS_DEVICE,
+    .instance_size      = sizeof(NPCM7xxKCSState),
+    .class_init         = npcm7xx_kcs_class_init,
+    .instance_init      = npcm7xx_kcs_init,
+};
+
+static void npcm7xx_kcs_register_type(void)
+{
+    type_register_static(&npcm7xx_kcs_channel_info);
+    type_register_static(&npcm7xx_kcs_info);
+}
+type_init(npcm7xx_kcs_register_type);
diff --git a/hw/ipmi/trace-events b/hw/ipmi/trace-events
new file mode 100644
index 0000000000..66d40bccbc
--- /dev/null
+++ b/hw/ipmi/trace-events
@@ -0,0 +1,8 @@
+# See docs/devel/tracing.txt for syntax documentation.
+
+# npcm7xx_kcs.c
+npcm7xx_kcs_read(const char *id, int channel, int reg, uint8_t value) " %s channel: %" PRIi32 ", reg: %" PRIi32 ", value 0x%02" PRIx8
+npcm7xx_kcs_write(const char *id, int channel, int reg, uint8_t value) " %s channel: %" PRIi32 ", reg: %" PRIi32 ", value 0x%02" PRIx8
+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
diff --git a/hw/ipmi/trace.h b/hw/ipmi/trace.h
new file mode 100644
index 0000000000..60a72fb4be
--- /dev/null
+++ b/hw/ipmi/trace.h
@@ -0,0 +1 @@
+#include "trace/trace-hw_ipmi.h"
diff --git a/include/hw/arm/npcm7xx.h b/include/hw/arm/npcm7xx.h
index 61ecc57ab9..ea22fba06a 100644
--- a/include/hw/arm/npcm7xx.h
+++ b/include/hw/arm/npcm7xx.h
@@ -22,6 +22,7 @@
 #include "hw/cpu/a9mpcore.h"
 #include "hw/gpio/npcm7xx_gpio.h"
 #include "hw/i2c/npcm7xx_smbus.h"
+#include "hw/ipmi/npcm7xx_kcs.h"
 #include "hw/mem/npcm7xx_mc.h"
 #include "hw/misc/npcm7xx_clk.h"
 #include "hw/misc/npcm7xx_gcr.h"
@@ -99,6 +100,7 @@ typedef struct NPCM7xxState {
     NPCM7xxRNGState     rng;
     NPCM7xxGPIOState    gpio[8];
     NPCM7xxSMBusState   smbus[16];
+    NPCM7xxKCSState     kcs;
     EHCISysBusState     ehci;
     OHCISysBusState     ohci;
     NPCM7xxFIUState     fiu[2];
diff --git a/include/hw/ipmi/npcm7xx_kcs.h b/include/hw/ipmi/npcm7xx_kcs.h
new file mode 100644
index 0000000000..5954772ef1
--- /dev/null
+++ b/include/hw/ipmi/npcm7xx_kcs.h
@@ -0,0 +1,103 @@
+/*
+ * Nuvoton NPCM7xx KCS Module
+ *
+ * Copyright 2020 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ */
+#ifndef NPCM7XX_KCS_H
+#define NPCM7XX_KCS_H
+
+#include "exec/memory.h"
+#include "hw/ipmi/ipmi.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+
+#define NPCM7XX_KCS_NR_CHANNELS             3
+/*
+ * Number of registers in each KCS channel. Don't change this without
+ * incrementing the version_id in the vmstate.
+ */
+#define NPCM7XX_KCS_CHANNEL_NR_REGS         9
+
+typedef struct NPCM7xxKCSState NPCM7xxKCSState;
+
+/**
+ * struct NPCM7xxKCSChannel - KCS Channel that can be read or written by the
+ * host.
+ * @parent: Parent device.
+ * @owner: The KCS module that manages this KCS channel.
+ * @status: The status of this KCS module.
+ * @dbbout: The output buffer to the host.
+ * @dbbin: The input buffer from the host.
+ * @ctl: The control register.
+ * @ic: The host interrupt control register. Not implemented.
+ * @ie: The host interrupt enable register. Not implemented.
+ * @inmsg: The input message from the host. To be put in dbbin.
+ * @inpos: The current position of input message.
+ * @inlen: The length of input message.
+ * @outmsg: The input message from the host. To be put in dbbout.
+ * @outpos: The current position of output message.
+ * @outlen: The length of output message.
+ * @last_byte_not_ready: The last byte in inmsg is not ready to be sent.
+ * @last_msg_id: The message id of last incoming request from host.
+ */
+typedef struct NPCM7xxKCSChannel {
+    DeviceState             parent;
+
+    NPCM7xxKCSState         *owner;
+    IPMICore                *host;
+    /* Core side registers. */
+    uint8_t                 status;
+    uint8_t                 dbbout;
+    uint8_t                 dbbin;
+    uint8_t                 ctl;
+    uint8_t                 ic;
+    uint8_t                 ie;
+
+    /* Host side buffers. */
+    uint8_t                 inmsg[MAX_IPMI_MSG_SIZE];
+    uint32_t                inpos;
+    uint32_t                inlen;
+    uint8_t                 outmsg[MAX_IPMI_MSG_SIZE];
+    uint32_t                outpos;
+    uint32_t                outlen;
+
+    /* Flags. */
+    bool                    last_byte_not_ready;
+    uint8_t                 last_msg_id;
+} NPCM7xxKCSChannel;
+
+/**
+ * struct NPCM7xxKCSState - Keyboard Control Style (KCS) Module device state.
+ * @parent: System bus device.
+ * @iomem: Memory region through which registers are accessed.
+ * @irq: GIC interrupt line to fire on reading or writing the buffer.
+ * @channels: The KCS channels this module manages.
+ */
+struct NPCM7xxKCSState {
+    SysBusDevice            parent;
+
+    MemoryRegion            iomem;
+
+    qemu_irq                irq;
+    NPCM7xxKCSChannel       channels[NPCM7XX_KCS_NR_CHANNELS];
+};
+
+#define TYPE_NPCM7XX_KCS_CHANNEL "npcm7xx-kcs-channel"
+#define NPCM7XX_KCS_CHANNEL(obj)                                      \
+    OBJECT_CHECK(NPCM7xxKCSChannel, (obj), TYPE_NPCM7XX_KCS_CHANNEL)
+
+#define TYPE_NPCM7XX_KCS "npcm7xx-kcs"
+#define NPCM7XX_KCS(obj)                                              \
+    OBJECT_CHECK(NPCM7xxKCSState, (obj), TYPE_NPCM7XX_KCS)
+
+#endif /* NPCM7XX_KCS_H */
diff --git a/meson.build b/meson.build
index 7e58e6279b..6856f7aa5d 100644
--- a/meson.build
+++ b/meson.build
@@ -2128,6 +2128,7 @@ if have_system
     'hw/ide',
     'hw/input',
     'hw/intc',
+    'hw/ipmi',
     'hw/isa',
     'hw/mem',
     'hw/mips',
-- 
2.33.0.309.g3052b89438-goog



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

* Re: [PATCH 1/8] docs: enable sphinx blockdiag extension
  2021-09-09 23:06 ` [PATCH 1/8] docs: enable sphinx blockdiag extension Hao Wu
@ 2021-09-09 23:40   ` Corey Minyard
  0 siblings, 0 replies; 14+ messages in thread
From: Corey Minyard @ 2021-09-09 23:40 UTC (permalink / raw)
  To: Hao Wu
  Cc: peter.maydell, titusr, venture, hskinnemoen, qemu-devel, kfting,
	qemu-arm, Avi.Fishman

On Thu, Sep 09, 2021 at 04:06:13PM -0700, Hao Wu wrote:
> From: Havard Skinnemoen <hskinnemoen@google.com>
> 

Can you add some reason for this?  I also can't approve this, you'll
need permission from whoever is responsible for this file.

-corey

> Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
> Signed-off-by: Hao Wu <hskinnemoen@google.com>
> ---
>  docs/conf.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/conf.py b/docs/conf.py
> index ff6e92c6e2..ecd0be66a5 100644
> --- a/docs/conf.py
> +++ b/docs/conf.py
> @@ -71,7 +71,11 @@
>  # Add any Sphinx extension module names here, as strings. They can be
>  # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
>  # ones.
> -extensions = ['kerneldoc', 'qmp_lexer', 'hxtool', 'depfile', 'qapidoc']
> +extensions = ['kerneldoc', 'qmp_lexer', 'hxtool', 'depfile', 'qapidoc',
> +        'sphinxcontrib.blockdiag']
> +
> +# Fontpath for blockdiag (truetype font)
> +blockdiag_fontpath = '/usr/share/fonts/truetype/liberation/LiberationSans-Regular.ttf'
>  
>  # Add any paths that contain templates here, relative to this directory.
>  templates_path = ['_templates']
> -- 
> 2.33.0.309.g3052b89438-goog
> 


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

* Re: [PATCH 2/8] docs/specs: IPMI device emulation: main processor
  2021-09-09 23:06 ` [PATCH 2/8] docs/specs: IPMI device emulation: main processor Hao Wu
@ 2021-09-09 23:48   ` Corey Minyard
  0 siblings, 0 replies; 14+ messages in thread
From: Corey Minyard @ 2021-09-09 23:48 UTC (permalink / raw)
  To: Hao Wu
  Cc: peter.maydell, titusr, venture, hskinnemoen, qemu-devel, kfting,
	qemu-arm, Avi.Fishman

On Thu, Sep 09, 2021 at 04:06:14PM -0700, Hao Wu wrote:
> From: Havard Skinnemoen <hskinnemoen@google.com>
> 
> This document is an attempt to briefly document the existing IPMI
> emulation support on the main processor. It provides the necessary
> background for the BMC-side IPMI emulation proposed by the next patch.

I'm good with this for the most part.  One nit inline below.

This block diagrams depend on the previous patch.

> 
> Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
>  docs/specs/index.rst |   1 +
>  docs/specs/ipmi.rst  | 100 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 101 insertions(+)
>  create mode 100644 docs/specs/ipmi.rst
> 
> diff --git a/docs/specs/index.rst b/docs/specs/index.rst
> index 65e9663916..1b5d177d53 100644
> --- a/docs/specs/index.rst
> +++ b/docs/specs/index.rst
> @@ -11,6 +11,7 @@ guest hardware that is specific to QEMU.
>     ppc-spapr-xive
>     ppc-spapr-numa
>     acpi_hw_reduced_hotplug
> +   ipmi
>     tpm
>     acpi_hest_ghes
>     acpi_cpu_hotplug
> diff --git a/docs/specs/ipmi.rst b/docs/specs/ipmi.rst
> new file mode 100644
> index 0000000000..adb098b53d
> --- /dev/null
> +++ b/docs/specs/ipmi.rst
> @@ -0,0 +1,100 @@
> +=====================
> +IPMI device emulation
> +=====================
> +
> +QEMU supports emulating many types of machines. This includes machines that may
> +serve as the main processor in an IPMI system, e.g. x86 or POWER server
> +processors, as well as machines emulating ARM-based Baseband Management
> +Controllers (BMCs), e.g. AST2xxx or NPCM7xxx systems-on-chip.
> +
> +Main processor emulation
> +========================
> +
> +A server platform may include one of the following system interfaces for
> +communicating with a BMC:
> +
> +* A Keyboard Controller Style (KCS) Interface, accessible via ISA
> +  (``isa-ipmi-kcs``) or PCI (``pci-ipmi-kcs``).
> +* A Block Transfer (BT) Interface, accessible via ISA (``isa-ipmi-bt``) or PCI
> +  (``pci-ipmi-bt``).
> +* An SMBus System Interface (SSIF; ``smbus-ipmi``).
> +
> +These interfaces can all be emulated by QEMU. To emulate the behavior of the
> +BMC, the messaging interface emulators use one of the following backends:
> +
> +* A BMC simulator running within the QEMU process (``ipmi-bmc-sim``).
> +* An external BMC simulator or emulator, connected over a chardev
> +  (``ipmi-bmc-extern``). `ipmi_sim
> +  <https://github.com/wrouesnel/openipmi/blob/master/lanserv/README.ipmi_sim>`_

Here, and in the other reference below, can you reference the official
repository instead of your own?

> +  from OpenIPMI is an example external BMC emulator.
> +
> +The following diagram shows how these entities relate to each other.
> +
> +.. blockdiag::
> +
> +    blockdiag main_processor_ipmi {
> +        orientation = portrait
> +        default_group_color = "none";
> +        class msgif [color = lightblue];
> +        class bmc [color = salmon];
> +
> +        ipmi_sim [color="aquamarine", label="External BMC"]
> +        ipmi-bmc-extern <-> ipmi_sim [label="chardev"];
> +
> +        group {
> +            orientation = portrait
> +
> +            ipmi-interface <-> ipmi-bmc;
> +
> +            group {
> +                orientation = portrait
> +
> +                ipmi-interface [class = "msgif"];
> +                isa-ipmi-kcs [class="msgif", stacked];
> +
> +                ipmi-interface <- isa-ipmi-kcs [hstyle = generalization];
> +            }
> +
> +
> +            group {
> +                orientation = portrait
> +
> +                ipmi-bmc [class = "bmc"];
> +                ipmi-bmc-sim [class="bmc"];
> +                ipmi-bmc-extern [class="bmc"];
> +
> +                ipmi-bmc <- ipmi-bmc-sim [hstyle = generalization];
> +                ipmi-bmc <- ipmi-bmc-extern [hstyle = generalization];
> +            }
> +
> +        }
> +    }
> +
> +IPMI System Interfaces
> +----------------------
> +
> +The system software running on the main processor may use a *system interface*
> +to communicate with the BMC. These are hardware devices attached to an ISA, PCI
> +or i2c bus, and in QEMU, they all need to implement ``ipmi-interface``.
> +This allows a BMC implementation to interact with the system interface in a
> +standard way.
> +
> +IPMI BMC
> +--------
> +
> +The system interface devices delegate emulation of BMC behavior to a BMC
> +device, that is a subclass of ``ipmi-bmc``. This type of device is called
> +a BMC because that's what it looks like to the main processor guest software.
> +
> +The BMC behavior may be simulated within the qemu process (``ipmi-bmc-sim``) or
> +further delegated to an external emulator, or a real BMC. The
> +``ipmi-bmc-extern`` device has a required ``chardev`` property which specifies
> +the communications channel to the external BMC.
> +
> +Wire protocol
> +=============
> +
> +The wire protocol used between ``ipmi-bmc-extern`` and the external BMC
> +emulator is defined by `README.vm
> +<https://github.com/wrouesnel/openipmi/blob/master/lanserv/README.vm>`_ from
> +the OpenIPMI project.
> -- 
> 2.33.0.309.g3052b89438-goog
> 


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

* Re: [PATCH 4/8] hw/ipmi: Refactor IPMI interface
  2021-09-09 23:06 ` [PATCH 4/8] hw/ipmi: Refactor IPMI interface Hao Wu
@ 2021-09-10  0:26   ` Corey Minyard
  0 siblings, 0 replies; 14+ messages in thread
From: Corey Minyard @ 2021-09-10  0:26 UTC (permalink / raw)
  To: Hao Wu
  Cc: peter.maydell, titusr, venture, hskinnemoen, qemu-devel, kfting,
	qemu-arm, Avi.Fishman

On Thu, Sep 09, 2021 at 04:06:16PM -0700, Hao Wu wrote:
> This patch refactors the IPMI interface so that it can be used by both
> the BMC side and core-side simulation.
> 
> Detail changes:
> (1) rename handle_rsp -> handle_msg so the name fits both BMC side and
>     Core side.
> (2) Add a new class IPMICore. This class represents a simulator/external
>     connection for both BMC and Core side emulation. The old IPMIBmc is
>     a sub-class of IPMICore.
> (3) Change all ibe->parent.intf in hw/ipmi/ipmi_bmc_*.c to use type
>     cast to IPMICore. Directly accessing parent QOM is against QEMU
>     guide and by using type casting the code can fit both core side and
>     BMC side emulation.

I think I'm ok with this.  It fixes some fundamental issues (maybe that
should be separate patches, but I think it's ok here).

I was kind of against this at first, I was thinking a completely
separate code for the BMC-side hardware emulation.  But I can see that
so much code would be duplicated that it would be silly.

It would be better if there was an ipmi_bmc.[ch] that had the
BMC-specific things, split out from the ipmi.[ch] code.  I'm not 100%
sure how important that is, but it would be less confusing.  Do you
agree?

A couple of comments inline...

-corey

> 
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
>  hw/ipmi/ipmi.c            | 15 +++++++++++---
>  hw/ipmi/ipmi_bmc_extern.c | 37 +++++++++++++++++++++--------------
>  hw/ipmi/ipmi_bmc_sim.c    | 41 ++++++++++++++++++++++++++-------------
>  hw/ipmi/ipmi_bt.c         |  2 +-
>  hw/ipmi/ipmi_kcs.c        |  2 +-
>  hw/ipmi/isa_ipmi_bt.c     |  4 +++-
>  hw/ipmi/isa_ipmi_kcs.c    |  4 +++-
>  hw/ipmi/pci_ipmi_bt.c     |  4 +++-
>  hw/ipmi/pci_ipmi_kcs.c    |  4 +++-
>  hw/ipmi/smbus_ipmi.c      |  6 ++++--
>  include/hw/ipmi/ipmi.h    | 40 ++++++++++++++++++++++++++++++--------
>  11 files changed, 111 insertions(+), 48 deletions(-)
> 
> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
> index 8d35c9fdd6..7da1b36fab 100644
> --- a/hw/ipmi/ipmi.c
> +++ b/hw/ipmi/ipmi.c
> @@ -92,13 +92,21 @@ static TypeInfo ipmi_interface_type_info = {
>      .class_init = ipmi_interface_class_init,
>  };
>  
> +static TypeInfo ipmi_core_type_info = {
> +    .name = TYPE_IPMI_CORE,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(IPMICore),
> +    .abstract = true,
> +};
> +
>  static void isa_ipmi_bmc_check(const Object *obj, const char *name,
>                                 Object *val, Error **errp)
>  {
> -    IPMIBmc *bmc = IPMI_BMC(val);
> +    IPMICore *ic = IPMI_CORE(val);
>  
> -    if (bmc->intf)
> +    if (ic->intf) {
>          error_setg(errp, "BMC object is already in use");
> +    }
>  }
>  
>  void ipmi_bmc_find_and_link(Object *obj, Object **bmc)
> @@ -122,7 +130,7 @@ static void bmc_class_init(ObjectClass *oc, void *data)
>  
>  static TypeInfo ipmi_bmc_type_info = {
>      .name = TYPE_IPMI_BMC,
> -    .parent = TYPE_DEVICE,
> +    .parent = TYPE_IPMI_CORE,
>      .instance_size = sizeof(IPMIBmc),
>      .abstract = true,
>      .class_size = sizeof(IPMIBmcClass),
> @@ -132,6 +140,7 @@ static TypeInfo ipmi_bmc_type_info = {
>  static void ipmi_register_types(void)
>  {
>      type_register_static(&ipmi_interface_type_info);
> +    type_register_static(&ipmi_core_type_info);
>      type_register_static(&ipmi_bmc_type_info);
>  }
>  
> diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
> index acf2bab35f..a0c3a40e7c 100644
> --- a/hw/ipmi/ipmi_bmc_extern.c
> +++ b/hw/ipmi/ipmi_bmc_extern.c
> @@ -147,8 +147,9 @@ static void continue_send(IPMIBmcExtern *ibe)
>  
>  static void extern_timeout(void *opaque)
>  {
> +    IPMICore *ic = opaque;
>      IPMIBmcExtern *ibe = opaque;

Shouldn't the above be a cast from ic?

> -    IPMIInterface *s = ibe->parent.intf;
> +    IPMIInterface *s = ic->intf;
>  
>      if (ibe->connected) {
>          if (ibe->waiting_rsp && (ibe->outlen == 0)) {
> @@ -158,7 +159,7 @@ static void extern_timeout(void *opaque)
>              ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
>              ibe->inbuf[2] = ibe->outbuf[2];
>              ibe->inbuf[3] = IPMI_CC_TIMEOUT;
> -            k->handle_rsp(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
> +            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
>          } else {
>              continue_send(ibe);
>          }
> @@ -186,8 +187,9 @@ static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
>                                         unsigned int max_cmd_len,
>                                         uint8_t msg_id)
>  {
> +    IPMICore *ic = IPMI_CORE(b);
>      IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(b);
> -    IPMIInterface *s = ibe->parent.intf;
> +    IPMIInterface *s = ic->intf;
>      uint8_t err = 0, csum;
>      unsigned int i;
>  
> @@ -213,7 +215,7 @@ static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
>          rsp[1] = cmd[1];
>          rsp[2] = err;
>          ibe->waiting_rsp = false;
> -        k->handle_rsp(s, msg_id, rsp, 3);
> +        k->handle_msg(s, msg_id, rsp, 3);
>          goto out;
>      }
>  
> @@ -236,7 +238,8 @@ static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
>  
>  static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
>  {
> -    IPMIInterface *s = ibe->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibe);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>      switch (hw_op) {
> @@ -284,7 +287,9 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
>  
>  static void handle_msg(IPMIBmcExtern *ibe)
>  {
> -    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(ibe->parent.intf);
> +    IPMICore *ic = IPMI_CORE(ibe);
> +    IPMIInterface *s = ic->intf;
> +    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>      if (ibe->in_escape) {
>          ipmi_debug("msg escape not ended\n");
> @@ -306,7 +311,7 @@ static void handle_msg(IPMIBmcExtern *ibe)
>  
>      timer_del(ibe->extern_timer);
>      ibe->waiting_rsp = false;
> -    k->handle_rsp(ibe->parent.intf, ibe->inbuf[0], ibe->inbuf + 1, ibe->inpos - 1);
> +    k->handle_msg(s, ibe->inbuf[0], ibe->inbuf + 1, ibe->inpos - 1);
>  }
>  
>  static int can_receive(void *opaque)
> @@ -382,8 +387,9 @@ static void receive(void *opaque, const uint8_t *buf, int size)
>  
>  static void chr_event(void *opaque, QEMUChrEvent event)
>  {
> +    IPMICore *ic = opaque;
>      IPMIBmcExtern *ibe = opaque;

Shouldn't the above be a cast from ic?

> -    IPMIInterface *s = ibe->parent.intf;
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>      unsigned char v;
>  
> @@ -398,17 +404,17 @@ static void chr_event(void *opaque, QEMUChrEvent event)
>          ibe->outlen++;
>          addchar(ibe, VM_CMD_CAPABILITIES);
>          v = VM_CAPABILITIES_IRQ | VM_CAPABILITIES_ATTN;
> -        if (k->do_hw_op(ibe->parent.intf, IPMI_POWEROFF_CHASSIS, 1) == 0) {
> +        if (k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1) == 0) {
>              v |= VM_CAPABILITIES_POWER;
>          }
> -        if (k->do_hw_op(ibe->parent.intf, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 1)
> +        if (k->do_hw_op(s, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 1)
>              == 0) {
>              v |= VM_CAPABILITIES_GRACEFUL_SHUTDOWN;
>          }
> -        if (k->do_hw_op(ibe->parent.intf, IPMI_RESET_CHASSIS, 1) == 0) {
> +        if (k->do_hw_op(s, IPMI_RESET_CHASSIS, 1) == 0) {
>              v |= VM_CAPABILITIES_RESET;
>          }
> -        if (k->do_hw_op(ibe->parent.intf, IPMI_SEND_NMI, 1) == 0) {
> +        if (k->do_hw_op(s, IPMI_SEND_NMI, 1) == 0) {
>              v |= VM_CAPABILITIES_NMI;
>          }
>          addchar(ibe, v);
> @@ -433,7 +439,7 @@ static void chr_event(void *opaque, QEMUChrEvent event)
>              ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
>              ibe->inbuf[2] = ibe->outbuf[2];
>              ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
> -            k->handle_rsp(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
> +            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
>          }
>          break;
>  
> @@ -475,14 +481,15 @@ static int ipmi_bmc_extern_post_migrate(void *opaque, int version_id)
>       * error on the interface if a response was being waited for.
>       */
>      if (ibe->waiting_rsp) {
> -        IPMIInterface *ii = ibe->parent.intf;
> +        IPMICore *ic = opaque;
> +        IPMIInterface *ii = ic->intf;
>          IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
>  
>          ibe->waiting_rsp = false;
>          ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
>          ibe->inbuf[2] = ibe->outbuf[2];
>          ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
> -        iic->handle_rsp(ii, ibe->outbuf[0], ibe->inbuf + 1, 3);
> +        iic->handle_msg(ii, ibe->outbuf[0], ibe->inbuf + 1, 3);
>      }
>      return 0;
>  }
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 905e091094..7cc4a22456 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -451,7 +451,8 @@ static int attn_irq_enabled(IPMIBmcSim *ibs)
>  void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log)
>  {
>      IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(b);
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>      if (!IPMI_BMC_EVENT_MSG_BUF_ENABLED(ibs)) {
> @@ -475,7 +476,8 @@ void ipmi_bmc_gen_event(IPMIBmc *b, uint8_t *evt, bool log)
>  static void gen_event(IPMIBmcSim *ibs, unsigned int sens_num, uint8_t deassert,
>                        uint8_t evd1, uint8_t evd2, uint8_t evd3)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>      uint8_t evt[16];
>      IPMISensor *sens = ibs->sensors + sens_num;
> @@ -644,7 +646,8 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>                                      uint8_t msg_id)
>  {
>      IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(b);
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>      const IPMICmdHandler *hdl;
>      RspBuffer rsp = RSP_BUFFER_INITIALIZER;
> @@ -690,14 +693,15 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>      hdl->cmd_handler(ibs, cmd, cmd_len, &rsp);
>  
>   out:
> -    k->handle_rsp(s, msg_id, rsp.buffer, rsp.len);
> +    k->handle_msg(s, msg_id, rsp.buffer, rsp.len);
>  
>      next_timeout(ibs);
>  }
>  
>  static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>      if (!ibs->watchdog_running) {
> @@ -788,7 +792,8 @@ static void chassis_control(IPMIBmcSim *ibs,
>                              uint8_t *cmd, unsigned int cmd_len,
>                              RspBuffer *rsp)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>      switch (cmd[2] & 0xf) {
> @@ -845,7 +850,8 @@ static void get_device_id(IPMIBmcSim *ibs,
>  
>  static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>      bool irqs_on;
>  
> @@ -861,7 +867,8 @@ static void cold_reset(IPMIBmcSim *ibs,
>                         uint8_t *cmd, unsigned int cmd_len,
>                         RspBuffer *rsp)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>      /* Disable all interrupts */
> @@ -876,7 +883,8 @@ static void warm_reset(IPMIBmcSim *ibs,
>                         uint8_t *cmd, unsigned int cmd_len,
>                         RspBuffer *rsp)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>      if (k->reset) {
> @@ -939,7 +947,8 @@ static void clr_msg_flags(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
>                            RspBuffer *rsp)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>      ibs->msg_flags &= ~cmd[2];
> @@ -957,7 +966,8 @@ static void read_evt_msg_buf(IPMIBmcSim *ibs,
>                               uint8_t *cmd, unsigned int cmd_len,
>                               RspBuffer *rsp)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>      unsigned int i;
>  
> @@ -989,7 +999,8 @@ static void get_msg(IPMIBmcSim *ibs,
>      g_free(msg);
>  
>      if (QTAILQ_EMPTY(&ibs->rcvbufs)) {
> -        IPMIInterface *s = ibs->parent.intf;
> +        IPMICore *ic = IPMI_CORE(ibs);
> +        IPMIInterface *s = ic->intf;
>          IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>          ibs->msg_flags &= ~IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE;
> @@ -1014,7 +1025,8 @@ static void send_msg(IPMIBmcSim *ibs,
>                       uint8_t *cmd, unsigned int cmd_len,
>                       RspBuffer *rsp)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>      IPMIRcvBufEntry *msg;
>      uint8_t *buf;
> @@ -1130,7 +1142,8 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>                                 uint8_t *cmd, unsigned int cmd_len,
>                                 RspBuffer *rsp)
>  {
> -    IPMIInterface *s = ibs->parent.intf;
> +    IPMICore *ic = IPMI_CORE(ibs);
> +    IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>      unsigned int val;
>  
> diff --git a/hw/ipmi/ipmi_bt.c b/hw/ipmi/ipmi_bt.c
> index 22f94fb98d..f76c369e4a 100644
> --- a/hw/ipmi/ipmi_bt.c
> +++ b/hw/ipmi/ipmi_bt.c
> @@ -430,7 +430,7 @@ void ipmi_bt_class_init(IPMIInterfaceClass *iic)
>  {
>      iic->init = ipmi_bt_init;
>      iic->set_atn = ipmi_bt_set_atn;
> -    iic->handle_rsp = ipmi_bt_handle_rsp;
> +    iic->handle_msg = ipmi_bt_handle_rsp;
>      iic->handle_if_event = ipmi_bt_handle_event;
>      iic->set_irq_enable = ipmi_bt_set_irq_enable;
>      iic->reset = ipmi_bt_handle_reset;
> diff --git a/hw/ipmi/ipmi_kcs.c b/hw/ipmi/ipmi_kcs.c
> index a77612946a..e0f870e13a 100644
> --- a/hw/ipmi/ipmi_kcs.c
> +++ b/hw/ipmi/ipmi_kcs.c
> @@ -417,7 +417,7 @@ void ipmi_kcs_class_init(IPMIInterfaceClass *iic)
>  {
>      iic->init = ipmi_kcs_init;
>      iic->set_atn = ipmi_kcs_set_atn;
> -    iic->handle_rsp = ipmi_kcs_handle_rsp;
> +    iic->handle_msg = ipmi_kcs_handle_rsp;
>      iic->handle_if_event = ipmi_kcs_handle_event;
>      iic->set_irq_enable = ipmi_kcs_set_irq_enable;
>  }
> diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
> index 02625eb94e..0f52fc4262 100644
> --- a/hw/ipmi/isa_ipmi_bt.c
> +++ b/hw/ipmi/isa_ipmi_bt.c
> @@ -74,6 +74,7 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
>      ISAIPMIBTDevice *iib = ISA_IPMI_BT(dev);
>      IPMIInterface *ii = IPMI_INTERFACE(dev);
>      IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
> +    IPMICore *ic;
>  
>      if (!iib->bt.bmc) {
>          error_setg(errp, "IPMI device requires a bmc attribute to be set");
> @@ -82,7 +83,8 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
>  
>      iib->uuid = ipmi_next_uuid();
>  
> -    iib->bt.bmc->intf = ii;
> +    ic = IPMI_CORE(iib->bt.bmc);
> +    ic->intf = ii;
>      iib->bt.opaque = iib;
>  
>      iic->init(ii, 0, &err);
> diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
> index 3b23ad08b3..60cab90a21 100644
> --- a/hw/ipmi/isa_ipmi_kcs.c
> +++ b/hw/ipmi/isa_ipmi_kcs.c
> @@ -73,6 +73,7 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
>      ISAIPMIKCSDevice *iik = ISA_IPMI_KCS(dev);
>      IPMIInterface *ii = IPMI_INTERFACE(dev);
>      IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
> +    IPMICore *ic;
>  
>      if (!iik->kcs.bmc) {
>          error_setg(errp, "IPMI device requires a bmc attribute to be set");
> @@ -81,7 +82,8 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
>  
>      iik->uuid = ipmi_next_uuid();
>  
> -    iik->kcs.bmc->intf = ii;
> +    ic = IPMI_CORE(iik->kcs.bmc);
> +    ic->intf = ii;
>      iik->kcs.opaque = iik;
>  
>      iic->init(ii, 0, &err);
> diff --git a/hw/ipmi/pci_ipmi_bt.c b/hw/ipmi/pci_ipmi_bt.c
> index b6e52730d3..751c15a31f 100644
> --- a/hw/ipmi/pci_ipmi_bt.c
> +++ b/hw/ipmi/pci_ipmi_bt.c
> @@ -58,6 +58,7 @@ static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
>      PCIIPMIBTDevice *pik = PCI_IPMI_BT(pd);
>      IPMIInterface *ii = IPMI_INTERFACE(pd);
>      IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
> +    IPMICore *ic;
>  
>      if (!pik->bt.bmc) {
>          error_setg(errp, "IPMI device requires a bmc attribute to be set");
> @@ -66,7 +67,8 @@ static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
>  
>      pik->uuid = ipmi_next_uuid();
>  
> -    pik->bt.bmc->intf = ii;
> +    ic = IPMI_CORE(pik->bt.bmc);
> +    ic->intf = ii;
>      pik->bt.opaque = pik;
>  
>      pci_config_set_prog_interface(pd->config, 0x02); /* BT */
> diff --git a/hw/ipmi/pci_ipmi_kcs.c b/hw/ipmi/pci_ipmi_kcs.c
> index de13418862..44a645723c 100644
> --- a/hw/ipmi/pci_ipmi_kcs.c
> +++ b/hw/ipmi/pci_ipmi_kcs.c
> @@ -58,6 +58,7 @@ static void pci_ipmi_kcs_realize(PCIDevice *pd, Error **errp)
>      PCIIPMIKCSDevice *pik = PCI_IPMI_KCS(pd);
>      IPMIInterface *ii = IPMI_INTERFACE(pd);
>      IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
> +    IPMICore *ic;
>  
>      if (!pik->kcs.bmc) {
>          error_setg(errp, "IPMI device requires a bmc attribute to be set");
> @@ -66,7 +67,8 @@ static void pci_ipmi_kcs_realize(PCIDevice *pd, Error **errp)
>  
>      pik->uuid = ipmi_next_uuid();
>  
> -    pik->kcs.bmc->intf = ii;
> +    ic = IPMI_CORE(pik->kcs.bmc);
> +    ic->intf = ii;
>      pik->kcs.opaque = pik;
>  
>      pci_config_set_prog_interface(pd->config, 0x01); /* KCS */
> diff --git a/hw/ipmi/smbus_ipmi.c b/hw/ipmi/smbus_ipmi.c
> index 1fdf0a66b6..a2383f1212 100644
> --- a/hw/ipmi/smbus_ipmi.c
> +++ b/hw/ipmi/smbus_ipmi.c
> @@ -315,6 +315,7 @@ static void smbus_ipmi_realize(DeviceState *dev, Error **errp)
>  {
>      SMBusIPMIDevice *sid = SMBUS_IPMI(dev);
>      IPMIInterface *ii = IPMI_INTERFACE(dev);
> +    IPMICore *ic;
>  
>      if (!sid->bmc) {
>          error_setg(errp, "IPMI device requires a bmc attribute to be set");
> @@ -323,7 +324,8 @@ static void smbus_ipmi_realize(DeviceState *dev, Error **errp)
>  
>      sid->uuid = ipmi_next_uuid();
>  
> -    sid->bmc->intf = ii;
> +    ic = IPMI_CORE(sid->bmc);
> +    ic->intf = ii;
>  }
>  
>  static void smbus_ipmi_init(Object *obj)
> @@ -359,7 +361,7 @@ static void smbus_ipmi_class_init(ObjectClass *oc, void *data)
>      dc->vmsd = &vmstate_smbus_ipmi;
>      dc->realize = smbus_ipmi_realize;
>      iic->set_atn = smbus_ipmi_set_atn;
> -    iic->handle_rsp = smbus_ipmi_handle_rsp;
> +    iic->handle_msg = smbus_ipmi_handle_rsp;
>      iic->handle_if_event = smbus_ipmi_handle_event;
>      iic->set_irq_enable = smbus_ipmi_set_irq_enable;
>      iic->get_fwinfo = smbus_ipmi_get_fwinfo;
> diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
> index 77a7213ed9..1fa8cd12e5 100644
> --- a/include/hw/ipmi/ipmi.h
> +++ b/include/hw/ipmi/ipmi.h
> @@ -111,11 +111,11 @@ uint32_t ipmi_next_uuid(void);
>  #define TYPE_IPMI_INTERFACE "ipmi-interface"
>  #define IPMI_INTERFACE(obj) \
>       INTERFACE_CHECK(IPMIInterface, (obj), TYPE_IPMI_INTERFACE)
> +typedef struct IPMIInterface IPMIInterface;
>  typedef struct IPMIInterfaceClass IPMIInterfaceClass;
>  DECLARE_CLASS_CHECKERS(IPMIInterfaceClass, IPMI_INTERFACE,
>                         TYPE_IPMI_INTERFACE)
> -
> -typedef struct IPMIInterface IPMIInterface;
> +struct IPMICore;
>  
>  struct IPMIInterfaceClass {
>      InterfaceClass parent;
> @@ -156,9 +156,9 @@ struct IPMIInterfaceClass {
>      void (*reset)(struct IPMIInterface *s, bool is_cold);
>  
>      /*
> -     * Handle a response from the bmc.
> +     * Handle an IPMI message.

Could you add a little more explaination here.  Like:

Handle an IPMI message from the remote side.  This is either a message
from the BMC being handled by an IPMI interface on the host side, or a
message from the host side being handled by a BMC emulator running in
this VM.

>       */
> -    void (*handle_rsp)(struct IPMIInterface *s, uint8_t msg_id,
> +    void (*handle_msg)(struct IPMIInterface *s, uint8_t msg_id,
>                         unsigned char *rsp, unsigned int rsp_len);
>  
>      /*
> @@ -166,12 +166,38 @@ struct IPMIInterfaceClass {
>       */
>      void *(*get_backend_data)(struct IPMIInterface *s);
>  
> +    /*
> +     * Set the IPMI core.
> +     */
> +    void (*set_ipmi_handler)(struct IPMIInterface *s, struct IPMICore *ic);
> +
>      /*
>       * Return the firmware info for a device.
>       */
>      void (*get_fwinfo)(struct IPMIInterface *s, IPMIFwInfo *info);
>  };
>  
> +/*
> + * Define an IPMI core (Either BMC or Host simulator.)
> + */
> +#define TYPE_IPMI_CORE "ipmi-core"
> +OBJECT_DECLARE_TYPE(IPMICore, IPMICoreClass, IPMI_CORE)
> +
> +struct IPMICore {
> +    DeviceState parent;
> +
> +    IPMIInterface *intf;
> +};
> +
> +struct IPMICoreClass {
> +    DeviceClass parent;
> +
> +    /*
> +     * Handle a hardware command.
> +     */
> +    void (*handle_hw_op)(struct IPMICore *s, uint8_t hw_op, uint8_t operand);
> +};
> +
>  /*
>   * Define a BMC simulator (or perhaps a connection to a real BMC)
>   */
> @@ -180,15 +206,13 @@ OBJECT_DECLARE_TYPE(IPMIBmc, IPMIBmcClass,
>                      IPMI_BMC)
>  
>  struct IPMIBmc {
> -    DeviceState parent;
> +    IPMICore parent;
>  
>      uint8_t slave_addr;
> -
> -    IPMIInterface *intf;
>  };
>  
>  struct IPMIBmcClass {
> -    DeviceClass parent;
> +    IPMICoreClass parent;
>  
>      /* Called when the system resets to report to the bmc. */
>      void (*handle_reset)(struct IPMIBmc *s);
> -- 
> 2.33.0.309.g3052b89438-goog
> 


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

* Re: [PATCH 5/8] hw/ipmi: Take out common from ipmi_bmc_extern.c
  2021-09-09 23:06 ` [PATCH 5/8] hw/ipmi: Take out common from ipmi_bmc_extern.c Hao Wu
@ 2021-09-10  0:27   ` Corey Minyard
  0 siblings, 0 replies; 14+ messages in thread
From: Corey Minyard @ 2021-09-10  0:27 UTC (permalink / raw)
  To: Hao Wu
  Cc: peter.maydell, titusr, venture, hskinnemoen, qemu-devel, kfting,
	qemu-arm, Avi.Fishman

On Thu, Sep 09, 2021 at 04:06:17PM -0700, Hao Wu wrote:
> This patch refactors ipmi_bmc_extern.c and takes out the parts that can
> be used both ipmi_bmc_extern.c and bmc_host_extern.c to a common file
> ipmi_extern.c.
> 
> Now we have a connection called IPMIExtern which handles the connection,
> and IPMIBmcExtern that handles core-side emulation specific stuff.
> 
> Basically most of the message transaction are moved. The stuff remained
> are basically hardware operations like handle_reset and handle_hw_op.
> These stuff have different behaviors in core-side and BMC-side
> emulation.

Yeah, you are going to need this.

-corey

> 
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
>  hw/ipmi/ipmi_bmc_extern.c | 420 ++++----------------------------------
>  hw/ipmi/ipmi_extern.c     | 415 +++++++++++++++++++++++++++++++++++++
>  hw/ipmi/ipmi_extern.h     |  90 ++++++++
>  hw/ipmi/meson.build       |   2 +-
>  4 files changed, 543 insertions(+), 384 deletions(-)
>  create mode 100644 hw/ipmi/ipmi_extern.c
>  create mode 100644 hw/ipmi/ipmi_extern.h
> 
> diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
> index a0c3a40e7c..24979ecfd5 100644
> --- a/hw/ipmi/ipmi_bmc_extern.c
> +++ b/hw/ipmi/ipmi_bmc_extern.c
> @@ -34,211 +34,43 @@
>  #include "qemu/timer.h"
>  #include "chardev/char-fe.h"
>  #include "hw/ipmi/ipmi.h"
> +#include "hw/ipmi/ipmi_extern.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/qdev-properties-system.h"
>  #include "migration/vmstate.h"
>  #include "qom/object.h"
>  
> -#define VM_MSG_CHAR        0xA0 /* Marks end of message */
> -#define VM_CMD_CHAR        0xA1 /* Marks end of a command */
> -#define VM_ESCAPE_CHAR     0xAA /* Set bit 4 from the next byte to 0 */
> -
> -#define VM_PROTOCOL_VERSION        1
> -#define VM_CMD_VERSION             0xff /* A version number byte follows */
> -#define VM_CMD_NOATTN              0x00
> -#define VM_CMD_ATTN                0x01
> -#define VM_CMD_ATTN_IRQ            0x02
> -#define VM_CMD_POWEROFF            0x03
> -#define VM_CMD_RESET               0x04
> -#define VM_CMD_ENABLE_IRQ          0x05 /* Enable/disable the messaging irq */
> -#define VM_CMD_DISABLE_IRQ         0x06
> -#define VM_CMD_SEND_NMI            0x07
> -#define VM_CMD_CAPABILITIES        0x08
> -#define   VM_CAPABILITIES_POWER    0x01
> -#define   VM_CAPABILITIES_RESET    0x02
> -#define   VM_CAPABILITIES_IRQ      0x04
> -#define   VM_CAPABILITIES_NMI      0x08
> -#define   VM_CAPABILITIES_ATTN     0x10
> -#define   VM_CAPABILITIES_GRACEFUL_SHUTDOWN 0x20
> -#define VM_CMD_GRACEFUL_SHUTDOWN   0x09
> -
>  #define TYPE_IPMI_BMC_EXTERN "ipmi-bmc-extern"
>  OBJECT_DECLARE_SIMPLE_TYPE(IPMIBmcExtern, IPMI_BMC_EXTERN)
> +
>  struct IPMIBmcExtern {
>      IPMIBmc parent;
>  
> -    CharBackend chr;
> -
> -    bool connected;
> -
> -    unsigned char inbuf[MAX_IPMI_MSG_SIZE + 2];
> -    unsigned int inpos;
> -    bool in_escape;
> -    bool in_too_many;
> -    bool waiting_rsp;
> -    bool sending_cmd;
> -
> -    unsigned char outbuf[(MAX_IPMI_MSG_SIZE + 2) * 2 + 1];
> -    unsigned int outpos;
> -    unsigned int outlen;
> -
> -    struct QEMUTimer *extern_timer;
> +    IPMIExtern conn;
>  
>      /* A reset event is pending to be sent upstream. */
>      bool send_reset;
>  };
>  
> -static unsigned char
> -ipmb_checksum(const unsigned char *data, int size, unsigned char start)
> +static void continue_send_bmc(IPMIBmcExtern *ibe)
>  {
> -        unsigned char csum = start;
> -
> -        for (; size > 0; size--, data++) {
> -                csum += *data;
> -        }
> -        return csum;
> -}
> -
> -static void continue_send(IPMIBmcExtern *ibe)
> -{
> -    int ret;
> -    if (ibe->outlen == 0) {
> -        goto check_reset;
> -    }
> - send:
> -    ret = qemu_chr_fe_write(&ibe->chr, ibe->outbuf + ibe->outpos,
> -                            ibe->outlen - ibe->outpos);
> -    if (ret > 0) {
> -        ibe->outpos += ret;
> -    }
> -    if (ibe->outpos < ibe->outlen) {
> -        /* Not fully transmitted, try again in a 10ms */
> -        timer_mod_ns(ibe->extern_timer,
> -                     qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 10000000);
> -    } else {
> -        /* Sent */
> -        ibe->outlen = 0;
> -        ibe->outpos = 0;
> -        if (!ibe->sending_cmd) {
> -            ibe->waiting_rsp = true;
> -        } else {
> -            ibe->sending_cmd = false;
> -        }
> -    check_reset:
> -        if (ibe->connected && ibe->send_reset) {
> +    if (continue_send(&ibe->conn)) {
> +        if (ibe->conn.connected && ibe->send_reset) {
>              /* Send the reset */
> -            ibe->outbuf[0] = VM_CMD_RESET;
> -            ibe->outbuf[1] = VM_CMD_CHAR;
> -            ibe->outlen = 2;
> -            ibe->outpos = 0;
> +            ibe->conn.outbuf[0] = VM_CMD_RESET;
> +            ibe->conn.outbuf[1] = VM_CMD_CHAR;
> +            ibe->conn.outlen = 2;
> +            ibe->conn.outpos = 0;
>              ibe->send_reset = false;
> -            ibe->sending_cmd = true;
> -            goto send;
> -        }
> -
> -        if (ibe->waiting_rsp) {
> -            /* Make sure we get a response within 4 seconds. */
> -            timer_mod_ns(ibe->extern_timer,
> -                         qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 4000000000ULL);
> +            ibe->conn.sending_cmd = true;
> +            continue_send(&ibe->conn);
>          }
>      }
> -    return;
>  }
>  
> -static void extern_timeout(void *opaque)
> +static void ipmi_bmc_handle_hw_op(IPMICore *ic, unsigned char hw_op,
> +                                  uint8_t operand)
>  {
> -    IPMICore *ic = opaque;
> -    IPMIBmcExtern *ibe = opaque;
> -    IPMIInterface *s = ic->intf;
> -
> -    if (ibe->connected) {
> -        if (ibe->waiting_rsp && (ibe->outlen == 0)) {
> -            IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> -            /* The message response timed out, return an error. */
> -            ibe->waiting_rsp = false;
> -            ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
> -            ibe->inbuf[2] = ibe->outbuf[2];
> -            ibe->inbuf[3] = IPMI_CC_TIMEOUT;
> -            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
> -        } else {
> -            continue_send(ibe);
> -        }
> -    }
> -}
> -
> -static void addchar(IPMIBmcExtern *ibe, unsigned char ch)
> -{
> -    switch (ch) {
> -    case VM_MSG_CHAR:
> -    case VM_CMD_CHAR:
> -    case VM_ESCAPE_CHAR:
> -        ibe->outbuf[ibe->outlen] = VM_ESCAPE_CHAR;
> -        ibe->outlen++;
> -        ch |= 0x10;
> -        /* fall through */
> -    default:
> -        ibe->outbuf[ibe->outlen] = ch;
> -        ibe->outlen++;
> -    }
> -}
> -
> -static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
> -                                       uint8_t *cmd, unsigned int cmd_len,
> -                                       unsigned int max_cmd_len,
> -                                       uint8_t msg_id)
> -{
> -    IPMICore *ic = IPMI_CORE(b);
> -    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(b);
> -    IPMIInterface *s = ic->intf;
> -    uint8_t err = 0, csum;
> -    unsigned int i;
> -
> -    if (ibe->outlen) {
> -        /* We already have a command queued.  Shouldn't ever happen. */
> -        error_report("IPMI KCS: Got command when not finished with the"
> -                     " previous command");
> -        abort();
> -    }
> -
> -    /* If it's too short or it was truncated, return an error. */
> -    if (cmd_len < 2) {
> -        err = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> -    } else if ((cmd_len > max_cmd_len) || (cmd_len > MAX_IPMI_MSG_SIZE)) {
> -        err = IPMI_CC_REQUEST_DATA_TRUNCATED;
> -    } else if (!ibe->connected) {
> -        err = IPMI_CC_BMC_INIT_IN_PROGRESS;
> -    }
> -    if (err) {
> -        IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> -        unsigned char rsp[3];
> -        rsp[0] = cmd[0] | 0x04;
> -        rsp[1] = cmd[1];
> -        rsp[2] = err;
> -        ibe->waiting_rsp = false;
> -        k->handle_msg(s, msg_id, rsp, 3);
> -        goto out;
> -    }
> -
> -    addchar(ibe, msg_id);
> -    for (i = 0; i < cmd_len; i++) {
> -        addchar(ibe, cmd[i]);
> -    }
> -    csum = ipmb_checksum(&msg_id, 1, 0);
> -    addchar(ibe, -ipmb_checksum(cmd, cmd_len, csum));
> -
> -    ibe->outbuf[ibe->outlen] = VM_MSG_CHAR;
> -    ibe->outlen++;
> -
> -    /* Start the transmit */
> -    continue_send(ibe);
> -
> - out:
> -    return;
> -}
> -
> -static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
> -{
> -    IPMICore *ic = IPMI_CORE(ibe);
>      IPMIInterface *s = ic->intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
> @@ -285,169 +117,22 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op)
>      }
>  }
>  
> -static void handle_msg(IPMIBmcExtern *ibe)
> -{
> -    IPMICore *ic = IPMI_CORE(ibe);
> -    IPMIInterface *s = ic->intf;
> -    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> -
> -    if (ibe->in_escape) {
> -        ipmi_debug("msg escape not ended\n");
> -        return;
> -    }
> -    if (ibe->inpos < 5) {
> -        ipmi_debug("msg too short\n");
> -        return;
> -    }
> -    if (ibe->in_too_many) {
> -        ibe->inbuf[3] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> -        ibe->inpos = 4;
> -    } else if (ipmb_checksum(ibe->inbuf, ibe->inpos, 0) != 0) {
> -        ipmi_debug("msg checksum failure\n");
> -        return;
> -    } else {
> -        ibe->inpos--; /* Remove checkum */
> -    }
> -
> -    timer_del(ibe->extern_timer);
> -    ibe->waiting_rsp = false;
> -    k->handle_msg(s, ibe->inbuf[0], ibe->inbuf + 1, ibe->inpos - 1);
> -}
> -
> -static int can_receive(void *opaque)
> -{
> -    return 1;
> -}
> -
> -static void receive(void *opaque, const uint8_t *buf, int size)
> +static void ipmi_bmc_extern_handle_command(IPMIBmc *b,
> +                                       uint8_t *cmd, unsigned int cmd_len,
> +                                       unsigned int max_cmd_len,
> +                                       uint8_t msg_id)
>  {
> -    IPMIBmcExtern *ibe = opaque;
> -    int i;
> -    unsigned char hw_op;
> -
> -    for (i = 0; i < size; i++) {
> -        unsigned char ch = buf[i];
> -
> -        switch (ch) {
> -        case VM_MSG_CHAR:
> -            handle_msg(ibe);
> -            ibe->in_too_many = false;
> -            ibe->inpos = 0;
> -            break;
> -
> -        case VM_CMD_CHAR:
> -            if (ibe->in_too_many) {
> -                ipmi_debug("cmd in too many\n");
> -                ibe->in_too_many = false;
> -                ibe->inpos = 0;
> -                break;
> -            }
> -            if (ibe->in_escape) {
> -                ipmi_debug("cmd in escape\n");
> -                ibe->in_too_many = false;
> -                ibe->inpos = 0;
> -                ibe->in_escape = false;
> -                break;
> -            }
> -            ibe->in_too_many = false;
> -            if (ibe->inpos < 1) {
> -                break;
> -            }
> -            hw_op = ibe->inbuf[0];
> -            ibe->inpos = 0;
> -            goto out_hw_op;
> -            break;
> -
> -        case VM_ESCAPE_CHAR:
> -            ibe->in_escape = true;
> -            break;
> -
> -        default:
> -            if (ibe->in_escape) {
> -                ch &= ~0x10;
> -                ibe->in_escape = false;
> -            }
> -            if (ibe->in_too_many) {
> -                break;
> -            }
> -            if (ibe->inpos >= sizeof(ibe->inbuf)) {
> -                ibe->in_too_many = true;
> -                break;
> -            }
> -            ibe->inbuf[ibe->inpos] = ch;
> -            ibe->inpos++;
> -            break;
> -        }
> -    }
> -    return;
> +    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(b);
>  
> - out_hw_op:
> -    handle_hw_op(ibe, hw_op);
> +    ipmi_extern_handle_command(&ibe->conn, cmd, cmd_len, max_cmd_len, msg_id);
>  }
>  
> -static void chr_event(void *opaque, QEMUChrEvent event)
> +static void ipmi_bmc_extern_realize(DeviceState *dev, Error **errp)
>  {
> -    IPMICore *ic = opaque;
> -    IPMIBmcExtern *ibe = opaque;
> -    IPMIInterface *s = ic->intf;
> -    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> -    unsigned char v;
> -
> -    switch (event) {
> -    case CHR_EVENT_OPENED:
> -        ibe->connected = true;
> -        ibe->outpos = 0;
> -        ibe->outlen = 0;
> -        addchar(ibe, VM_CMD_VERSION);
> -        addchar(ibe, VM_PROTOCOL_VERSION);
> -        ibe->outbuf[ibe->outlen] = VM_CMD_CHAR;
> -        ibe->outlen++;
> -        addchar(ibe, VM_CMD_CAPABILITIES);
> -        v = VM_CAPABILITIES_IRQ | VM_CAPABILITIES_ATTN;
> -        if (k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1) == 0) {
> -            v |= VM_CAPABILITIES_POWER;
> -        }
> -        if (k->do_hw_op(s, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 1)
> -            == 0) {
> -            v |= VM_CAPABILITIES_GRACEFUL_SHUTDOWN;
> -        }
> -        if (k->do_hw_op(s, IPMI_RESET_CHASSIS, 1) == 0) {
> -            v |= VM_CAPABILITIES_RESET;
> -        }
> -        if (k->do_hw_op(s, IPMI_SEND_NMI, 1) == 0) {
> -            v |= VM_CAPABILITIES_NMI;
> -        }
> -        addchar(ibe, v);
> -        ibe->outbuf[ibe->outlen] = VM_CMD_CHAR;
> -        ibe->outlen++;
> -        ibe->sending_cmd = false;
> -        continue_send(ibe);
> -        break;
> -
> -    case CHR_EVENT_CLOSED:
> -        if (!ibe->connected) {
> -            return;
> -        }
> -        ibe->connected = false;
> -        /*
> -         * Don't hang the OS trying to handle the ATN bit, other end will
> -         * resend on a reconnect.
> -         */
> -        k->set_atn(s, 0, 0);
> -        if (ibe->waiting_rsp) {
> -            ibe->waiting_rsp = false;
> -            ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
> -            ibe->inbuf[2] = ibe->outbuf[2];
> -            ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
> -            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
> -        }
> -        break;
> +    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(dev);
>  
> -    case CHR_EVENT_BREAK:
> -    case CHR_EVENT_MUX_IN:
> -    case CHR_EVENT_MUX_OUT:
> -        /* Ignore */
> -        break;
> +    if (!qdev_realize(DEVICE(&ibe->conn), NULL, errp)) {
> +        return;
>      }
>  }
>  
> @@ -456,42 +141,14 @@ static void ipmi_bmc_extern_handle_reset(IPMIBmc *b)
>      IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(b);
>  
>      ibe->send_reset = true;
> -    continue_send(ibe);
> -}
> -
> -static void ipmi_bmc_extern_realize(DeviceState *dev, Error **errp)
> -{
> -    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(dev);
> -
> -    if (!qemu_chr_fe_backend_connected(&ibe->chr)) {
> -        error_setg(errp, "IPMI external bmc requires chardev attribute");
> -        return;
> -    }
> -
> -    qemu_chr_fe_set_handlers(&ibe->chr, can_receive, receive,
> -                             chr_event, NULL, ibe, NULL, true);
> +    continue_send_bmc(ibe);
>  }
>  
>  static int ipmi_bmc_extern_post_migrate(void *opaque, int version_id)
>  {
> -    IPMIBmcExtern *ibe = opaque;
> -
> -    /*
> -     * We don't directly restore waiting_rsp, Instead, we return an
> -     * error on the interface if a response was being waited for.
> -     */
> -    if (ibe->waiting_rsp) {
> -        IPMICore *ic = opaque;
> -        IPMIInterface *ii = ic->intf;
> -        IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
> -
> -        ibe->waiting_rsp = false;
> -        ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
> -        ibe->inbuf[2] = ibe->outbuf[2];
> -        ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
> -        iic->handle_msg(ii, ibe->outbuf[0], ibe->inbuf + 1, 3);
> -    }
> -    return 0;
> +    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(opaque);
> +
> +    return ipmi_extern_post_migrate(&ibe->conn, version_id);
>  }
>  
>  static const VMStateDescription vmstate_ipmi_bmc_extern = {
> @@ -501,28 +158,24 @@ static const VMStateDescription vmstate_ipmi_bmc_extern = {
>      .post_load = ipmi_bmc_extern_post_migrate,
>      .fields      = (VMStateField[]) {
>          VMSTATE_BOOL(send_reset, IPMIBmcExtern),
> -        VMSTATE_BOOL(waiting_rsp, IPMIBmcExtern),
> +        VMSTATE_BOOL(conn.waiting_rsp, IPMIBmcExtern),
>          VMSTATE_END_OF_LIST()
>      }
>  };
>  
>  static void ipmi_bmc_extern_init(Object *obj)
>  {
> +    IPMICore *ic = IPMI_CORE(obj);
>      IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(obj);
>  
> -    ibe->extern_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, extern_timeout, ibe);
> +    object_initialize_child(obj, "extern", &ibe->conn,
> +                            TYPE_IPMI_EXTERN);
> +    ibe->conn.core = ic;
>      vmstate_register(NULL, 0, &vmstate_ipmi_bmc_extern, ibe);
>  }
>  
> -static void ipmi_bmc_extern_finalize(Object *obj)
> -{
> -    IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(obj);
> -
> -    timer_free(ibe->extern_timer);
> -}
> -
>  static Property ipmi_bmc_extern_properties[] = {
> -    DEFINE_PROP_CHR("chardev", IPMIBmcExtern, chr),
> +    DEFINE_PROP_CHR("chardev", IPMIBmcExtern, conn.chr),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -530,9 +183,11 @@ static void ipmi_bmc_extern_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
>      IPMIBmcClass *bk = IPMI_BMC_CLASS(oc);
> +    IPMICoreClass *ck = IPMI_CORE_CLASS(oc);
>  
>      bk->handle_command = ipmi_bmc_extern_handle_command;
>      bk->handle_reset = ipmi_bmc_extern_handle_reset;
> +    ck->handle_hw_op = ipmi_bmc_handle_hw_op;
>      dc->hotpluggable = false;
>      dc->realize = ipmi_bmc_extern_realize;
>      device_class_set_props(dc, ipmi_bmc_extern_properties);
> @@ -543,9 +198,8 @@ static const TypeInfo ipmi_bmc_extern_type = {
>      .parent        = TYPE_IPMI_BMC,
>      .instance_size = sizeof(IPMIBmcExtern),
>      .instance_init = ipmi_bmc_extern_init,
> -    .instance_finalize = ipmi_bmc_extern_finalize,
>      .class_init    = ipmi_bmc_extern_class_init,
> - };
> +};
>  
>  static void ipmi_bmc_extern_register_types(void)
>  {
> diff --git a/hw/ipmi/ipmi_extern.c b/hw/ipmi/ipmi_extern.c
> new file mode 100644
> index 0000000000..f139eaef24
> --- /dev/null
> +++ b/hw/ipmi/ipmi_extern.c
> @@ -0,0 +1,415 @@
> +/*
> + * IPMI external connection
> + *
> + * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qemu/module.h"
> +#include "qapi/error.h"
> +#include "qemu/timer.h"
> +#include "chardev/char-fe.h"
> +#include "hw/ipmi/ipmi.h"
> +#include "hw/ipmi/ipmi_extern.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +#include "qom/object.h"
> +
> +static unsigned char
> +ipmb_checksum(const unsigned char *data, int size, unsigned char start)
> +{
> +        unsigned char csum = start;
> +
> +        for (; size > 0; size--, data++) {
> +                csum += *data;
> +        }
> +        return csum;
> +}
> +
> +/* Returns whether check_reset is required for IPMI_BMC_EXTERN. */
> +bool continue_send(IPMIExtern *ibe)
> +{
> +    int ret;
> +    if (ibe->outlen == 0) {
> +        return true;
> +    }
> +    ret = qemu_chr_fe_write(&ibe->chr, ibe->outbuf + ibe->outpos,
> +                            ibe->outlen - ibe->outpos);
> +    if (ret > 0) {
> +        ibe->outpos += ret;
> +    }
> +    if (ibe->outpos < ibe->outlen) {
> +        /* Not fully transmitted, try again in a 10ms */
> +        timer_mod_ns(ibe->extern_timer,
> +                     qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 10000000);
> +        return false;
> +    } else {
> +        /* Sent */
> +        ibe->outlen = 0;
> +        ibe->outpos = 0;
> +        if (!ibe->bmc_side && !ibe->sending_cmd) {
> +            ibe->waiting_rsp = true;
> +        } else {
> +            ibe->sending_cmd = false;
> +        }
> +
> +        if (ibe->waiting_rsp) {
> +            /* Make sure we get a response within 4 seconds. */
> +            timer_mod_ns(ibe->extern_timer,
> +                         qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 4000000000ULL);
> +        }
> +        return true;
> +    }
> +}
> +
> +static void extern_timeout(void *opaque)
> +{
> +    IPMIExtern *ibe = opaque;
> +    IPMIInterface *s = ibe->core->intf;
> +
> +    if (ibe->connected) {
> +        /*TODO: only core-side */
> +        if (ibe->waiting_rsp && (ibe->outlen == 0)) {
> +            IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> +            /* The message response timed out, return an error. */
> +            ibe->waiting_rsp = false;
> +            ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
> +            ibe->inbuf[2] = ibe->outbuf[2];
> +            ibe->inbuf[3] = IPMI_CC_TIMEOUT;
> +            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
> +        } else {
> +            continue_send(ibe);
> +        }
> +    }
> +}
> +
> +static void addchar(IPMIExtern *ibe, unsigned char ch)
> +{
> +    switch (ch) {
> +    case VM_MSG_CHAR:
> +    case VM_CMD_CHAR:
> +    case VM_ESCAPE_CHAR:
> +        ibe->outbuf[ibe->outlen] = VM_ESCAPE_CHAR;
> +        ibe->outlen++;
> +        ch |= 0x10;
> +        /* fall through */
> +    default:
> +        ibe->outbuf[ibe->outlen] = ch;
> +        ibe->outlen++;
> +    }
> +}
> +
> +void ipmi_extern_handle_command(IPMIExtern *ibe,
> +                                       uint8_t *cmd, unsigned int cmd_len,
> +                                       unsigned int max_cmd_len,
> +                                       uint8_t msg_id)
> +{
> +    IPMIInterface *s = ibe->core->intf;
> +    uint8_t err = 0, csum;
> +    unsigned int i;
> +
> +    if (ibe->outlen) {
> +        /* We already have a command queued.  Shouldn't ever happen. */
> +        error_report("IPMI KCS: Got command when not finished with the"
> +                     " previous command");
> +        abort();
> +    }
> +
> +    /* If it's too short or it was truncated, return an error. */
> +    if (cmd_len < 2) {
> +        err = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +    } else if ((cmd_len > max_cmd_len) || (cmd_len > MAX_IPMI_MSG_SIZE)) {
> +        err = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +    } else if (!ibe->connected) {
> +        err = IPMI_CC_BMC_INIT_IN_PROGRESS;
> +    }
> +    if (err) {
> +        IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> +        unsigned char rsp[3];
> +        rsp[0] = cmd[0] | 0x04;
> +        rsp[1] = cmd[1];
> +        rsp[2] = err;
> +        ibe->waiting_rsp = false;
> +        k->handle_msg(s, msg_id, rsp, 3);
> +        goto out;
> +    }
> +
> +    addchar(ibe, msg_id);
> +    for (i = 0; i < cmd_len; i++) {
> +        addchar(ibe, cmd[i]);
> +    }
> +    csum = ipmb_checksum(&msg_id, 1, 0);
> +    addchar(ibe, -ipmb_checksum(cmd, cmd_len, csum));
> +
> +    ibe->outbuf[ibe->outlen] = VM_MSG_CHAR;
> +    ibe->outlen++;
> +
> +    /* Start the transmit */
> +    continue_send(ibe);
> +
> + out:
> +    return;
> +}
> +
> +static void handle_msg(IPMIExtern *ibe)
> +{
> +    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(ibe->core->intf);
> +
> +    if (ibe->in_escape) {
> +        ipmi_debug("msg escape not ended\n");
> +        return;
> +    }
> +    if (ibe->inpos < (ibe->bmc_side ? 4 : 5)) {
> +        ipmi_debug("msg too short\n");
> +        return;
> +    }
> +    if (ibe->in_too_many) {
> +        ibe->inbuf[3] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +        ibe->inpos = 4;
> +    } else if (ipmb_checksum(ibe->inbuf, ibe->inpos, 0) != 0) {
> +        ipmi_debug("msg checksum failure\n");
> +        return;
> +    } else {
> +        ibe->inpos--; /* Remove checkum */
> +    }
> +
> +    timer_del(ibe->extern_timer);
> +    ibe->waiting_rsp = false;
> +    k->handle_msg(ibe->core->intf, ibe->inbuf[0],
> +                  ibe->inbuf + 1, ibe->inpos - 1);
> +}
> +
> +static int can_receive(void *opaque)
> +{
> +    return 1;
> +}
> +
> +static void receive(void *opaque, const uint8_t *buf, int size)
> +{
> +    IPMIExtern *ibe = opaque;
> +    IPMICoreClass *ck = IPMI_CORE_GET_CLASS(ibe->core);
> +    int i;
> +    unsigned char hw_op;
> +    unsigned char hw_operand = 0;
> +
> +    for (i = 0; i < size; i++) {
> +        unsigned char ch = buf[i];
> +
> +        switch (ch) {
> +        case VM_MSG_CHAR:
> +            handle_msg(ibe);
> +            ibe->in_too_many = false;
> +            ibe->inpos = 0;
> +            break;
> +
> +        case VM_CMD_CHAR:
> +            if (ibe->in_too_many) {
> +                ipmi_debug("cmd in too many\n");
> +                ibe->in_too_many = false;
> +                ibe->inpos = 0;
> +                break;
> +            }
> +            if (ibe->in_escape) {
> +                ipmi_debug("cmd in escape\n");
> +                ibe->in_too_many = false;
> +                ibe->inpos = 0;
> +                ibe->in_escape = false;
> +                break;
> +            }
> +            ibe->in_too_many = false;
> +            if (ibe->inpos < 1) {
> +                break;
> +            }
> +            hw_op = ibe->inbuf[0];
> +            if (ibe->inpos > 1) {
> +                hw_operand = ibe->inbuf[1];
> +            }
> +            ibe->inpos = 0;
> +            goto out_hw_op;
> +            break;
> +
> +        case VM_ESCAPE_CHAR:
> +            ibe->in_escape = true;
> +            break;
> +
> +        default:
> +            if (ibe->in_escape) {
> +                ch &= ~0x10;
> +                ibe->in_escape = false;
> +            }
> +            if (ibe->in_too_many) {
> +                break;
> +            }
> +            if (ibe->inpos >= sizeof(ibe->inbuf)) {
> +                ibe->in_too_many = true;
> +                break;
> +            }
> +            ibe->inbuf[ibe->inpos] = ch;
> +            ibe->inpos++;
> +            break;
> +        }
> +    }
> +    return;
> +
> + out_hw_op:
> +    ck->handle_hw_op(ibe->core, hw_op, hw_operand);
> +}
> +
> +static void chr_event(void *opaque, QEMUChrEvent event)
> +{
> +    IPMIExtern *ibe = opaque;
> +    IPMIInterface *s = ibe->core->intf;
> +    IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> +    unsigned char v;
> +
> +    switch (event) {
> +    case CHR_EVENT_OPENED:
> +        ibe->connected = true;
> +        ibe->outpos = 0;
> +        ibe->outlen = 0;
> +        addchar(ibe, VM_CMD_VERSION);
> +        addchar(ibe, VM_PROTOCOL_VERSION);
> +        ibe->outbuf[ibe->outlen] = VM_CMD_CHAR;
> +        ibe->outlen++;
> +        /* Only send capability for core side. */
> +        if (!ibe->bmc_side) {
> +            addchar(ibe, VM_CMD_CAPABILITIES);
> +            v = VM_CAPABILITIES_IRQ | VM_CAPABILITIES_ATTN;
> +            if (k->do_hw_op(ibe->core->intf, IPMI_POWEROFF_CHASSIS, 1) == 0) {
> +                v |= VM_CAPABILITIES_POWER;
> +            }
> +            if (k->do_hw_op(ibe->core->intf, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 1)
> +                == 0) {
> +                v |= VM_CAPABILITIES_GRACEFUL_SHUTDOWN;
> +            }
> +            if (k->do_hw_op(ibe->core->intf, IPMI_RESET_CHASSIS, 1) == 0) {
> +                v |= VM_CAPABILITIES_RESET;
> +            }
> +            if (k->do_hw_op(ibe->core->intf, IPMI_SEND_NMI, 1) == 0) {
> +                v |= VM_CAPABILITIES_NMI;
> +            }
> +            addchar(ibe, v);
> +            ibe->outbuf[ibe->outlen] = VM_CMD_CHAR;
> +            ibe->outlen++;
> +        }
> +        ibe->sending_cmd = false;
> +        continue_send(ibe);
> +        break;
> +
> +    case CHR_EVENT_CLOSED:
> +        if (!ibe->connected) {
> +            return;
> +        }
> +        ibe->connected = false;
> +        /*
> +         * Don't hang the OS trying to handle the ATN bit, other end will
> +         * resend on a reconnect.
> +         */
> +        k->set_atn(s, 0, 0);
> +        if (ibe->waiting_rsp) {
> +            ibe->waiting_rsp = false;
> +            ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
> +            ibe->inbuf[2] = ibe->outbuf[2];
> +            ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
> +            k->handle_msg(s, ibe->outbuf[0], ibe->inbuf + 1, 3);
> +        }
> +        break;
> +
> +    case CHR_EVENT_BREAK:
> +    case CHR_EVENT_MUX_IN:
> +    case CHR_EVENT_MUX_OUT:
> +        /* Ignore */
> +        break;
> +    }
> +}
> +
> +static void ipmi_extern_realize(DeviceState *dev, Error **errp)
> +{
> +    IPMIExtern *ibe = IPMI_EXTERN(dev);
> +
> +    if (!qemu_chr_fe_backend_connected(&ibe->chr)) {
> +        error_setg(errp, "IPMI external bmc requires chardev attribute");
> +        return;
> +    }
> +
> +    qemu_chr_fe_set_handlers(&ibe->chr, can_receive, receive,
> +                             chr_event, NULL, ibe, NULL, true);
> +}
> +
> +int ipmi_extern_post_migrate(IPMIExtern *ibe, int version_id)
> +{
> +    /*
> +     * We don't directly restore waiting_rsp, Instead, we return an
> +     * error on the interface if a response was being waited for.
> +     */
> +    if (ibe->waiting_rsp) {
> +        IPMIInterface *ii = ibe->core->intf;
> +        IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
> +
> +        ibe->waiting_rsp = false;
> +        ibe->inbuf[1] = ibe->outbuf[1] | 0x04;
> +        ibe->inbuf[2] = ibe->outbuf[2];
> +        ibe->inbuf[3] = IPMI_CC_BMC_INIT_IN_PROGRESS;
> +        iic->handle_msg(ii, ibe->outbuf[0], ibe->inbuf + 1, 3);
> +    }
> +    return 0;
> +}
> +
> +static void ipmi_extern_init(Object *obj)
> +{
> +    IPMIExtern *ibe = IPMI_EXTERN(obj);
> +
> +    ibe->extern_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, extern_timeout, ibe);
> +}
> +
> +static void ipmi_extern_finalize(Object *obj)
> +{
> +    IPMIExtern *ibe = IPMI_EXTERN(obj);
> +
> +    timer_del(ibe->extern_timer);
> +    timer_free(ibe->extern_timer);
> +}
> +
> +
> +static void ipmi_extern_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->hotpluggable = false;
> +    dc->realize = ipmi_extern_realize;
> +}
> +
> +static const TypeInfo ipmi_extern_type = {
> +    .name          = TYPE_IPMI_EXTERN,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(IPMIExtern),
> +    .instance_init = ipmi_extern_init,
> +    .instance_finalize = ipmi_extern_finalize,
> +    .class_init    = ipmi_extern_class_init,
> + };
> +
> +static void ipmi_extern_register_types(void)
> +{
> +    type_register_static(&ipmi_extern_type);
> +}
> +
> +type_init(ipmi_extern_register_types)
> diff --git a/hw/ipmi/ipmi_extern.h b/hw/ipmi/ipmi_extern.h
> new file mode 100644
> index 0000000000..e4aa80a0f6
> --- /dev/null
> +++ b/hw/ipmi/ipmi_extern.h
> @@ -0,0 +1,90 @@
> +/*
> + * IPMI external connection
> + *
> + * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef HW_IPMI_EXTERN_H
> +#define HW_IPMI_EXTERN_H
> +
> +#include "qemu/osdep.h"
> +#include "hw/ipmi/ipmi.h"
> +
> +#define VM_MSG_CHAR        0xA0 /* Marks end of message */
> +#define VM_CMD_CHAR        0xA1 /* Marks end of a command */
> +#define VM_ESCAPE_CHAR     0xAA /* Set bit 4 from the next byte to 0 */
> +
> +#define VM_PROTOCOL_VERSION        1
> +#define VM_CMD_VERSION             0xff /* A version number byte follows */
> +#define VM_CMD_NOATTN              0x00
> +#define VM_CMD_ATTN                0x01
> +#define VM_CMD_ATTN_IRQ            0x02
> +#define VM_CMD_POWEROFF            0x03
> +#define VM_CMD_RESET               0x04
> +#define VM_CMD_ENABLE_IRQ          0x05 /* Enable/disable the messaging irq */
> +#define VM_CMD_DISABLE_IRQ         0x06
> +#define VM_CMD_SEND_NMI            0x07
> +#define VM_CMD_CAPABILITIES        0x08
> +#define   VM_CAPABILITIES_POWER    0x01
> +#define   VM_CAPABILITIES_RESET    0x02
> +#define   VM_CAPABILITIES_IRQ      0x04
> +#define   VM_CAPABILITIES_NMI      0x08
> +#define   VM_CAPABILITIES_ATTN     0x10
> +#define   VM_CAPABILITIES_GRACEFUL_SHUTDOWN 0x20
> +#define VM_CMD_GRACEFUL_SHUTDOWN   0x09
> +
> +typedef struct IPMIExtern {
> +    DeviceState parent;
> +
> +    CharBackend chr;
> +
> +    IPMICore *core;
> +
> +    bool bmc_side;
> +    bool connected;
> +
> +    unsigned char inbuf[MAX_IPMI_MSG_SIZE + 2];
> +    unsigned int inpos;
> +    bool in_escape;
> +    bool in_too_many;
> +    bool waiting_rsp;
> +    bool sending_cmd;
> +
> +    unsigned char outbuf[(MAX_IPMI_MSG_SIZE + 2) * 2 + 1];
> +    unsigned int outpos;
> +    unsigned int outlen;
> +
> +    struct QEMUTimer *extern_timer;
> +} IPMIExtern;
> +
> +#define TYPE_IPMI_EXTERN "ipmi-extern"
> +#define IPMI_EXTERN(obj) \
> +    OBJECT_CHECK(IPMIExtern, (obj), TYPE_IPMI_EXTERN)
> +
> +void ipmi_extern_handle_command(IPMIExtern *ibe,
> +                                uint8_t *cmd, unsigned int cmd_len,
> +                                unsigned int max_cmd_len,
> +                                uint8_t msg_id);
> +
> +bool continue_send(IPMIExtern *ibe);
> +int ipmi_extern_post_migrate(IPMIExtern *ibe, int version_id);
> +
> +#endif /* HW_IPMI_EXTERN_H */
> diff --git a/hw/ipmi/meson.build b/hw/ipmi/meson.build
> index 9622ea2a2c..edd0bf9af9 100644
> --- a/hw/ipmi/meson.build
> +++ b/hw/ipmi/meson.build
> @@ -1,5 +1,5 @@
>  ipmi_ss = ss.source_set()
> -ipmi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c', 'ipmi_kcs.c', 'ipmi_bt.c'))
> +ipmi_ss.add(when: 'CONFIG_IPMI', if_true: files('ipmi.c', 'ipmi_kcs.c', 'ipmi_bt.c', 'ipmi_extern.c'))
>  ipmi_ss.add(when: 'CONFIG_IPMI_LOCAL', if_true: files('ipmi_bmc_sim.c'))
>  ipmi_ss.add(when: 'CONFIG_IPMI_EXTERN', if_true: files('ipmi_bmc_extern.c'))
>  ipmi_ss.add(when: 'CONFIG_ISA_IPMI_KCS', if_true: files('isa_ipmi_kcs.c'))
> -- 
> 2.33.0.309.g3052b89438-goog
> 


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

* Re: [PATCH 7/8] hw/ipmi: Add an IPMI external host device
  2021-09-09 23:06 ` [PATCH 7/8] hw/ipmi: Add an IPMI external host device Hao Wu
@ 2021-09-10  0:53   ` Corey Minyard
  0 siblings, 0 replies; 14+ messages in thread
From: Corey Minyard @ 2021-09-10  0:53 UTC (permalink / raw)
  To: Hao Wu
  Cc: peter.maydell, titusr, venture, hskinnemoen, qemu-devel, kfting,
	qemu-arm, Avi.Fishman

On Thu, Sep 09, 2021 at 04:06:19PM -0700, Hao Wu wrote:
> The IPMI external host device works for Baseband Management Controller
> (BMC) emulations. It works as a representation of a host class that
> connects to a given BMC.  It can connect to a real host hardware or a
> emulated or simulated host device. In particular it can connect to a
> host QEMU instance with device ipmi-bmc-extern.

This is reasonable, I think.

The terminology here is confusing, though.  I'm not sure exactly what to
do about it.  So we have right now:

  host interfaces - KCS, BT, SSIF
  bmc - either an emulated or extern BMC the host talks to

And what you are proposing is:

  core - anything that supplies IPMI message for processing by
    the code in the VM.  So this is the internal/external BMC
    and the bmc-side of things.
  bmc - either an emulated or extern BMC the host talks to
  bmc-side - Receives messages from a host running ipmi_bmc_extern
  interfaces - KCS, BT, SSIF, and the interface to the bmc-side
    VM.

What I would propose is something like:

  core - anything that supplies IPMI message for processing by
    the code in the VM.  So this is the internal/external BMC
    and the bmc-side of things.
  bmc-host - either an emulated or extern BMC the host talks to
  bmc-client - Receives messages from a host running ipmi_bmc_extern
  interfaces - All IPMI interfaces
  interfaces-host - KCS, BT, SSIF
  interfaces-client - the interface to the bmc-side VM.

I'm not too excited about the name "client", though.  But I think a
class hierarchy like above would be more clear about what things are,
and it's not that different than what you are proposing.

I'm really just thinking out loud, though.

-corey

> 
> For more details of IPMI host device in BMC emulation, see
> docs/specs/ipmi.rst.
> 
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
>  configs/devices/arm-softmmu/default.mak |   2 +
>  hw/ipmi/Kconfig                         |   5 +
>  hw/ipmi/ipmi_extern.c                   |  18 ++-
>  hw/ipmi/ipmi_host_extern.c              | 170 ++++++++++++++++++++++++
>  hw/ipmi/meson.build                     |   1 +
>  5 files changed, 194 insertions(+), 2 deletions(-)
>  create mode 100644 hw/ipmi/ipmi_host_extern.c
> 
> diff --git a/configs/devices/arm-softmmu/default.mak b/configs/devices/arm-softmmu/default.mak
> index 6985a25377..82f0c6f8c3 100644
> --- a/configs/devices/arm-softmmu/default.mak
> +++ b/configs/devices/arm-softmmu/default.mak
> @@ -25,6 +25,8 @@ CONFIG_GUMSTIX=y
>  CONFIG_SPITZ=y
>  CONFIG_TOSA=y
>  CONFIG_Z2=y
> +CONFIG_IPMI=y
> +CONFIG_IPMI_HOST=y
>  CONFIG_NPCM7XX=y
>  CONFIG_COLLIE=y
>  CONFIG_ASPEED_SOC=y
> diff --git a/hw/ipmi/Kconfig b/hw/ipmi/Kconfig
> index 9befd4f422..6722b1fbb0 100644
> --- a/hw/ipmi/Kconfig
> +++ b/hw/ipmi/Kconfig
> @@ -11,6 +11,11 @@ config IPMI_EXTERN
>      default y
>      depends on IPMI
>  
> +config IPMI_HOST
> +    bool
> +    default y
> +    depends on IPMI
> +
>  config ISA_IPMI_KCS
>      bool
>      depends on ISA_BUS
> diff --git a/hw/ipmi/ipmi_extern.c b/hw/ipmi/ipmi_extern.c
> index 97dfed085f..0952dc5992 100644
> --- a/hw/ipmi/ipmi_extern.c
> +++ b/hw/ipmi/ipmi_extern.c
> @@ -145,11 +145,25 @@ void ipmi_extern_handle_command(IPMIExtern *ibe,
>      if (err) {
>          IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>          unsigned char rsp[3];
> +
>          rsp[0] = cmd[0] | 0x04;
>          rsp[1] = cmd[1];
>          rsp[2] = err;
> -        ibe->waiting_rsp = false;
> -        k->handle_msg(s, msg_id, rsp, 3);
> +
> +        if (ibe->bmc_side) {
> +            /* For BMC Side, send out an error message. */
> +            addchar(ibe, msg_id);
> +            for (i = 0; i < 3; ++i) {
> +                addchar(ibe, rsp[i]);
> +            }
> +            csum = ipmb_checksum(&msg_id, 1, 0);
> +            addchar(ibe, -ipmb_checksum(rsp, 3, csum));
> +            continue_send(ibe);
> +        } else {
> +            /* For Core side, handle an error message. */
> +            ibe->waiting_rsp = false;
> +            k->handle_msg(s, msg_id, rsp, 3);
> +        }
>          goto out;
>      }
>  
> diff --git a/hw/ipmi/ipmi_host_extern.c b/hw/ipmi/ipmi_host_extern.c
> new file mode 100644
> index 0000000000..4530631901
> --- /dev/null
> +++ b/hw/ipmi/ipmi_host_extern.c
> @@ -0,0 +1,170 @@
> +/*
> + * IPMI Host external connection
> + *
> + * Copyright 2021 Google LLC
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + */
> +
> +/*
> + * This is designed to connect to a host QEMU VM that runs ipmi_bmc_extern.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qemu/module.h"
> +#include "qapi/error.h"
> +#include "chardev/char-fe.h"
> +#include "hw/ipmi/ipmi.h"
> +#include "hw/ipmi/ipmi_extern.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/qdev-properties-system.h"
> +#include "migration/vmstate.h"
> +#include "qom/object.h"
> +
> +#define TYPE_IPMI_HOST_EXTERN "ipmi-host-extern"
> +OBJECT_DECLARE_SIMPLE_TYPE(IPMIHostExtern, IPMI_HOST_EXTERN)
> +
> +typedef struct IPMIHostExtern {
> +    IPMICore parent;
> +
> +    IPMIExtern conn;
> +
> +    IPMIInterface *responder;
> +
> +    uint8_t capability;
> +} IPMIHostExtern;
> +
> +/*
> + * Handle a command (typically IPMI response) from IPMI interface
> + * and send it out to the external host.
> + */
> +static void ipmi_host_extern_handle_command(IPMICore *h, uint8_t *cmd,
> +        unsigned cmd_len, unsigned max_cmd_len, uint8_t msg_id)
> +{
> +    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(h);
> +
> +    ipmi_extern_handle_command(&ihe->conn, cmd, cmd_len, max_cmd_len, msg_id);
> +}
> +
> +/* This function handles a control command from the host. */
> +static void ipmi_host_extern_handle_hw_op(IPMICore *ic, uint8_t cmd,
> +                                          uint8_t operand)
> +{
> +    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(ic);
> +
> +    switch (cmd) {
> +    case VM_CMD_VERSION:
> +        /* The host informs us the protocol version. */
> +        if (unlikely(operand != VM_PROTOCOL_VERSION)) {
> +            ipmi_debug("Host protocol version %u is different from our version"
> +                    " %u\n", operand, VM_PROTOCOL_VERSION);
> +        }
> +        break;
> +
> +    case VM_CMD_RESET:
> +        /* The host tells us a reset has happened. */
> +        break;
> +
> +    case VM_CMD_CAPABILITIES:
> +        /* The host tells us its capability. */
> +        ihe->capability = operand;
> +        break;
> +
> +    default:
> +        /* The host shouldn't send us this command. Just ignore if they do. */
> +        ipmi_debug("Host cmd type %02x is invalid.\n", cmd);
> +        break;
> +    }
> +}
> +
> +static void ipmi_host_extern_realize(DeviceState *dev, Error **errp)
> +{
> +    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(dev);
> +    IPMIInterfaceClass *rk;
> +
> +    if (ihe->responder == NULL) {
> +        error_setg(errp, "IPMI host extern requires responder attribute");
> +        return;
> +    }
> +    rk = IPMI_INTERFACE_GET_CLASS(ihe->responder);
> +    rk->set_ipmi_handler(ihe->responder, IPMI_CORE(ihe));
> +    ihe->conn.core->intf = ihe->responder;
> +
> +    if (!qdev_realize(DEVICE(&ihe->conn), NULL, errp)) {
> +        return;
> +    }
> +}
> +
> +static int ipmi_host_extern_post_migrate(void *opaque, int version_id)
> +{
> +    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(opaque);
> +
> +    return ipmi_extern_post_migrate(&ihe->conn, version_id);
> +}
> +
> +static const VMStateDescription vmstate_ipmi_host_extern = {
> +    .name = TYPE_IPMI_HOST_EXTERN,
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .post_load = ipmi_host_extern_post_migrate,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT8(capability, IPMIHostExtern),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void ipmi_host_extern_init(Object *obj)
> +{
> +    IPMICore *ic = IPMI_CORE(obj);
> +    IPMIHostExtern *ihe = IPMI_HOST_EXTERN(obj);
> +
> +    object_initialize_child(obj, "extern", &ihe->conn,
> +                            TYPE_IPMI_EXTERN);
> +    ihe->conn.core = ic;
> +    ihe->conn.bmc_side = true;
> +    vmstate_register(NULL, 0, &vmstate_ipmi_host_extern, ihe);
> +}
> +
> +static Property ipmi_host_extern_properties[] = {
> +    DEFINE_PROP_CHR("chardev", IPMIHostExtern, conn.chr),
> +    DEFINE_PROP_LINK("responder", IPMIHostExtern, responder,
> +                     TYPE_IPMI_INTERFACE, IPMIInterface *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void ipmi_host_extern_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    IPMICoreClass *ck = IPMI_CORE_CLASS(oc);
> +
> +    device_class_set_props(dc, ipmi_host_extern_properties);
> +
> +    ck->handle_command = ipmi_host_extern_handle_command;
> +    ck->handle_hw_op = ipmi_host_extern_handle_hw_op;
> +    dc->hotpluggable = false;
> +    dc->realize = ipmi_host_extern_realize;
> +}
> +
> +static const TypeInfo ipmi_host_extern_type = {
> +    .name          = TYPE_IPMI_HOST_EXTERN,
> +    .parent        = TYPE_IPMI_CORE,
> +    .instance_size = sizeof(IPMIHostExtern),
> +    .instance_init = ipmi_host_extern_init,
> +    .class_init    = ipmi_host_extern_class_init,
> +};
> +
> +static void ipmi_host_extern_register_types(void)
> +{
> +    type_register_static(&ipmi_host_extern_type);
> +}
> +
> +type_init(ipmi_host_extern_register_types)
> diff --git a/hw/ipmi/meson.build b/hw/ipmi/meson.build
> index edd0bf9af9..b1dd4710dc 100644
> --- a/hw/ipmi/meson.build
> +++ b/hw/ipmi/meson.build
> @@ -7,5 +7,6 @@ ipmi_ss.add(when: 'CONFIG_PCI_IPMI_KCS', if_true: files('pci_ipmi_kcs.c'))
>  ipmi_ss.add(when: 'CONFIG_ISA_IPMI_BT', if_true: files('isa_ipmi_bt.c'))
>  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'))
>  
>  softmmu_ss.add_all(when: 'CONFIG_IPMI', if_true: ipmi_ss)
> -- 
> 2.33.0.309.g3052b89438-goog
> 


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

end of thread, other threads:[~2021-09-10  0:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-09 23:06 [PATCH 0/8] Handing IPMI for emulating BMC Hao Wu
2021-09-09 23:06 ` [PATCH 1/8] docs: enable sphinx blockdiag extension Hao Wu
2021-09-09 23:40   ` Corey Minyard
2021-09-09 23:06 ` [PATCH 2/8] docs/specs: IPMI device emulation: main processor Hao Wu
2021-09-09 23:48   ` Corey Minyard
2021-09-09 23:06 ` [PATCH 3/8] docs/specs: IPMI device emulation: BMC Hao Wu
2021-09-09 23:06 ` [PATCH 4/8] hw/ipmi: Refactor IPMI interface Hao Wu
2021-09-10  0:26   ` Corey Minyard
2021-09-09 23:06 ` [PATCH 5/8] hw/ipmi: Take out common from ipmi_bmc_extern.c Hao Wu
2021-09-10  0:27   ` Corey Minyard
2021-09-09 23:06 ` [PATCH 6/8] hw/ipmi: Move handle_command to IPMICoreClass Hao Wu
2021-09-09 23:06 ` [PATCH 7/8] hw/ipmi: Add an IPMI external host device Hao Wu
2021-09-10  0:53   ` Corey Minyard
2021-09-09 23:06 ` [PATCH 8/8] hw/ipmi: Add a KCS Module for NPCM7XX Hao Wu

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.