Re: [Qemu-devel] [PATCH v4 03/17] ipmi: Add a local BMC simulation

[Cédric Le Goater]

A few comments below,

On 11/12/2015 08:02 PM, address@hidden wrote:
> From: Corey Minyard <address@hidden>
> 
> This provides a minimal local BMC, basically enough to comply with the
> spec and provide a complete watchdog timer (including a sensor, SDR,
> and event).
> 
> Signed-off-by: Corey Minyard <address@hidden>
> ---
>  default-configs/i386-softmmu.mak   |    1 +
>  default-configs/x86_64-softmmu.mak |    1 +
>  hw/ipmi/Makefile.objs              |    1 +
>  hw/ipmi/ipmi_bmc_sim.c             | 1731 
> ++++++++++++++++++++++++++++++++++++
>  4 files changed, 1734 insertions(+)
>  create mode 100644 hw/ipmi/ipmi_bmc_sim.c
> 
> diff --git a/default-configs/i386-softmmu.mak 
> b/default-configs/i386-softmmu.mak
> index 8fa751a..00a0346 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -10,6 +10,7 @@ CONFIG_VMWARE_VGA=y
>  CONFIG_VIRTIO_VGA=y
>  CONFIG_VMMOUSE=y
>  CONFIG_IPMI=y
> +CONFIG_IPMI_LOCAL=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> diff --git a/default-configs/x86_64-softmmu.mak 
> b/default-configs/x86_64-softmmu.mak
> index 6767f4f..39a619f 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -10,6 +10,7 @@ CONFIG_VMWARE_VGA=y
>  CONFIG_VIRTIO_VGA=y
>  CONFIG_VMMOUSE=y
>  CONFIG_IPMI=y
> +CONFIG_IPMI_LOCAL=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
> index 65bde11..875271c 100644
> --- a/hw/ipmi/Makefile.objs
> +++ b/hw/ipmi/Makefile.objs
> @@ -1 +1,2 @@
>  common-obj-$(CONFIG_IPMI) += ipmi.o
> +common-obj-$(CONFIG_IPMI_LOCAL) += ipmi_bmc_sim.o
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> new file mode 100644
> index 0000000..d246029
> --- /dev/null
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -0,0 +1,1731 @@
> +/*
> + * IPMI BMC emulation
> + *
> + * 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 <stdio.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include "qemu/timer.h"
> +#include "hw/ipmi/ipmi.h"
> +#include "qemu/error-report.h"
> +
> +#define IPMI_NETFN_CHASSIS            0x00
> +#define IPMI_NETFN_CHASSIS_MAXCMD         0x03
> +
> +#define IPMI_CMD_GET_CHASSIS_CAPABILITIES 0x00
> +#define IPMI_CMD_GET_CHASSIS_STATUS       0x01
> +#define IPMI_CMD_CHASSIS_CONTROL          0x02
> +
> +#define IPMI_NETFN_SENSOR_EVENT       0x04
> +#define IPMI_NETFN_SENSOR_EVENT_MAXCMD    0x2e
> +
> +#define IPMI_CMD_SET_SENSOR_EVT_ENABLE    0x28
> +#define IPMI_CMD_GET_SENSOR_EVT_ENABLE    0x29
> +#define IPMI_CMD_REARM_SENSOR_EVTS        0x2a
> +#define IPMI_CMD_GET_SENSOR_EVT_STATUS    0x2b
> +#define IPMI_CMD_GET_SENSOR_READING       0x2d

I have started adding a few commands and got scared by 
IPMI_CMD_SET_SENSOR_READING spec. By any chance, would 
you  have one in stock ? 

> +
> +/* #define IPMI_NETFN_APP             0x06 In ipmi.h */
> +#define IPMI_NETFN_APP_MAXCMD             0x36
> +
> +#define IPMI_CMD_GET_DEVICE_ID            0x01
> +#define IPMI_CMD_COLD_RESET               0x02
> +#define IPMI_CMD_WARM_RESET               0x03
> +#define IPMI_CMD_RESET_WATCHDOG_TIMER     0x22
> +#define IPMI_CMD_SET_WATCHDOG_TIMER       0x24
> +#define IPMI_CMD_GET_WATCHDOG_TIMER       0x25
> +#define IPMI_CMD_SET_BMC_GLOBAL_ENABLES   0x2e
> +#define IPMI_CMD_GET_BMC_GLOBAL_ENABLES   0x2f
> +#define IPMI_CMD_CLR_MSG_FLAGS            0x30
> +#define IPMI_CMD_GET_MSG_FLAGS            0x31
> +#define IPMI_CMD_GET_MSG                  0x33
> +#define IPMI_CMD_SEND_MSG                 0x34
> +#define IPMI_CMD_READ_EVT_MSG_BUF         0x35
> +
> +#define IPMI_NETFN_STORAGE            0x0a
> +#define IPMI_NETFN_STORAGE_MAXCMD         0x4a
> +
> +#define IPMI_CMD_GET_SDR_REP_INFO         0x20
> +#define IPMI_CMD_GET_SDR_REP_ALLOC_INFO   0x21
> +#define IPMI_CMD_RESERVE_SDR_REP          0x22
> +#define IPMI_CMD_GET_SDR                  0x23
> +#define IPMI_CMD_ADD_SDR                  0x24
> +#define IPMI_CMD_PARTIAL_ADD_SDR          0x25
> +#define IPMI_CMD_DELETE_SDR               0x26
> +#define IPMI_CMD_CLEAR_SDR_REP            0x27
> +#define IPMI_CMD_GET_SDR_REP_TIME         0x28
> +#define IPMI_CMD_SET_SDR_REP_TIME         0x29
> +#define IPMI_CMD_ENTER_SDR_REP_UPD_MODE   0x2A
> +#define IPMI_CMD_EXIT_SDR_REP_UPD_MODE    0x2B
> +#define IPMI_CMD_RUN_INIT_AGENT           0x2C
> +#define IPMI_CMD_GET_SEL_INFO             0x40
> +#define IPMI_CMD_GET_SEL_ALLOC_INFO       0x41
> +#define IPMI_CMD_RESERVE_SEL              0x42
> +#define IPMI_CMD_GET_SEL_ENTRY            0x43
> +#define IPMI_CMD_ADD_SEL_ENTRY            0x44
> +#define IPMI_CMD_PARTIAL_ADD_SEL_ENTRY    0x45
> +#define IPMI_CMD_DELETE_SEL_ENTRY         0x46
> +#define IPMI_CMD_CLEAR_SEL                0x47
> +#define IPMI_CMD_GET_SEL_TIME             0x48
> +#define IPMI_CMD_SET_SEL_TIME             0x49
> +
> +
> +/* Same as a timespec struct. */
> +struct ipmi_time {
> +    long tv_sec;
> +    long tv_nsec;
> +};
> +
> +#define MAX_SEL_SIZE 128
> +
> +typedef struct IPMISel {
> +    uint8_t sel[MAX_SEL_SIZE][16];
> +    unsigned int next_free;
> +    long time_offset;
> +    uint16_t reservation;
> +    uint8_t last_addition[4];
> +    uint8_t last_clear[4];
> +    uint8_t overflow;
> +} IPMISel;
> +
> +#define MAX_SDR_SIZE 16384
> +
> +typedef struct IPMISdr {
> +    uint8_t sdr[MAX_SDR_SIZE];
> +    unsigned int next_free;
> +    uint16_t next_rec_id;
> +    uint16_t reservation;
> +    uint8_t last_addition[4];
> +    uint8_t last_clear[4];
> +    uint8_t overflow;
> +} IPMISdr;
> +
> +typedef struct IPMISensor {
> +    uint8_t status;
> +    uint8_t reading;
> +    uint16_t states_suppt;
> +    uint16_t assert_suppt;
> +    uint16_t deassert_suppt;
> +    uint16_t states;
> +    uint16_t assert_states;
> +    uint16_t deassert_states;
> +    uint16_t assert_enable;
> +    uint16_t deassert_enable;
> +    uint8_t  sensor_type;
> +    uint8_t  evt_reading_type_code;
> +} IPMISensor;
> +#define IPMI_SENSOR_GET_PRESENT(s)       ((s)->status & 0x01)
> +#define IPMI_SENSOR_SET_PRESENT(s, v)    ((s)->status = (s->status & ~0x01) 
> | \
> +                                             !!(v))
> +#define IPMI_SENSOR_GET_SCAN_ON(s)       ((s)->status & 0x40)
> +#define IPMI_SENSOR_SET_SCAN_ON(s, v)    ((s)->status = (s->status & ~0x40) 
> | \
> +                                             ((!!(v)) << 6))
> +#define IPMI_SENSOR_GET_EVENTS_ON(s)     ((s)->status & 0x80)
> +#define IPMI_SENSOR_SET_EVENTS_ON(s, v)  ((s)->status = (s->status & ~0x80) 
> | \
> +                                             ((!!(v)) << 7))
> +#define IPMI_SENSOR_GET_RET_STATUS(s)    ((s)->status & 0xc0)
> +#define IPMI_SENSOR_SET_RET_STATUS(s, v) ((s)->status = (s->status & ~0xc0) 
> | \
> +                                             (v & 0xc0))
> +#define IPMI_SENSOR_IS_DISCRETE(s) ((s)->evt_reading_type_code != 1)
> +
> +#define MAX_SENSORS 20

Could we extend that a bit ? like 80 ? There are real systems with more 
than 180 sensors. 

> +#define IPMI_WATCHDOG_SENSOR 0
> +
> +typedef struct IPMIBmcSim IPMIBmcSim;
> +
> +#define MAX_NETFNS 64
> +typedef void (*IPMICmdHandler)(IPMIBmcSim *s,
> +                               uint8_t *cmd, unsigned int cmd_len,
> +                               uint8_t *rsp, unsigned int *rsp_len,
> +                               unsigned int max_rsp_len);
> +typedef struct IPMINetfn {
> +    unsigned int cmd_nums;
> +    const IPMICmdHandler *cmd_handlers;
> +} IPMINetfn;
> +
> +typedef struct IPMIRcvBufEntry {
> +    QTAILQ_ENTRY(IPMIRcvBufEntry) entry;
> +    uint8_t len;
> +    uint8_t buf[MAX_IPMI_MSG_SIZE];
> +} IPMIRcvBufEntry;
> +
> +#define TYPE_IPMI_BMC_SIMULATOR "ipmi-bmc-sim"
> +#define IPMI_BMC_SIMULATOR(obj) OBJECT_CHECK(IPMIBmcSim, (obj), \
> +                                        TYPE_IPMI_BMC_SIMULATOR)
> +struct IPMIBmcSim {
> +    IPMIBmc parent;
> +
> +    QEMUTimer *timer;
> +
> +    uint8_t bmc_global_enables;
> +    uint8_t msg_flags;
> +
> +    bool     watchdog_initialized;
> +    uint8_t  watchdog_use;
> +    uint8_t  watchdog_action;
> +    uint8_t  watchdog_pretimeout; /* In seconds */
> +    bool     watchdog_expired;
> +    uint16_t watchdog_timeout; /* in 100's of milliseconds */
> +
> +    bool     watchdog_running;
> +    bool     watchdog_preaction_ran;
> +    int64_t  watchdog_expiry;
> +
> +    uint8_t device_id;
> +    uint8_t ipmi_version;
> +    uint8_t device_rev;
> +    uint8_t fwrev1;
> +    uint8_t fwrev2;
> +    uint8_t mfg_id[3];
> +    uint8_t product_id[2];
> +
> +    IPMISel sel;
> +    IPMISdr sdr;
> +    IPMISensor sensors[MAX_SENSORS];
> +
> +    /* Odd netfns are for responses, so we only need the even ones. */
> +    const IPMINetfn *netfns[MAX_NETFNS / 2];
> +
> +    QemuMutex lock;
> +    /* We allow one event in the buffer */
> +    uint8_t evtbuf[16];
> +
> +    QTAILQ_HEAD(, IPMIRcvBufEntry) rcvbufs;
> +};
> +
> +#define IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK        (1 << 3)
> +#define IPMI_BMC_MSG_FLAG_EVT_BUF_FULL                 (1 << 1)
> +#define IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE                (1 << 0)
> +#define IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK_SET(s) \
> +    (IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK & (s)->msg_flags)
> +#define IPMI_BMC_MSG_FLAG_EVT_BUF_FULL_SET(s) \
> +    (IPMI_BMC_MSG_FLAG_EVT_BUF_FULL & (s)->msg_flags)
> +#define IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE_SET(s) \
> +    (IPMI_BMC_MSG_FLAG_RCV_MSG_QUEUE & (s)->msg_flags)
> +
> +#define IPMI_BMC_RCV_MSG_QUEUE_INT_BIT    0
> +#define IPMI_BMC_EVBUF_FULL_INT_BIT       1
> +#define IPMI_BMC_EVENT_MSG_BUF_BIT        2
> +#define IPMI_BMC_EVENT_LOG_BIT            3
> +#define IPMI_BMC_MSG_INTS_ON(s) ((s)->bmc_global_enables & \
> +                                 (1 << IPMI_BMC_RCV_MSG_QUEUE_INT_BIT))
> +#define IPMI_BMC_EVBUF_FULL_INT_ENABLED(s) ((s)->bmc_global_enables & \
> +                                        (1 << IPMI_BMC_EVBUF_FULL_INT_BIT))
> +#define IPMI_BMC_EVENT_LOG_ENABLED(s) ((s)->bmc_global_enables & \
> +                                       (1 << IPMI_BMC_EVENT_LOG_BIT))
> +#define IPMI_BMC_EVENT_MSG_BUF_ENABLED(s) ((s)->bmc_global_enables & \
> +                                           (1 << IPMI_BMC_EVENT_MSG_BUF_BIT))
> +
> +#define IPMI_BMC_WATCHDOG_USE_MASK 0xc7
> +#define IPMI_BMC_WATCHDOG_ACTION_MASK 0x77
> +#define IPMI_BMC_WATCHDOG_GET_USE(s) ((s)->watchdog_use & 0x7)
> +#define IPMI_BMC_WATCHDOG_GET_DONT_LOG(s) (((s)->watchdog_use >> 7) & 0x1)
> +#define IPMI_BMC_WATCHDOG_GET_DONT_STOP(s) (((s)->watchdog_use >> 6) & 0x1)
> +#define IPMI_BMC_WATCHDOG_GET_PRE_ACTION(s) (((s)->watchdog_action >> 4) & 
> 0x7)
> +#define IPMI_BMC_WATCHDOG_PRE_NONE               0
> +#define IPMI_BMC_WATCHDOG_PRE_SMI                1
> +#define IPMI_BMC_WATCHDOG_PRE_NMI                2
> +#define IPMI_BMC_WATCHDOG_PRE_MSG_INT            3
> +#define IPMI_BMC_WATCHDOG_GET_ACTION(s) ((s)->watchdog_action & 0x7)
> +#define IPMI_BMC_WATCHDOG_ACTION_NONE            0
> +#define IPMI_BMC_WATCHDOG_ACTION_RESET           1
> +#define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN      2
> +#define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE     3
> +
> +
> +/* Add a byte to the response. */
> +#define IPMI_ADD_RSP_DATA(b) \
> +    do {                                                   \
> +        if (*rsp_len >= max_rsp_len) {                     \
> +            rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;       \
> +            goto out;                                      \
> +        }                                                  \
> +        rsp[(*rsp_len)++] = (b);                           \
> +    } while (0)
> +
> +/* Verify that the received command is a certain length. */
> +#define IPMI_CHECK_CMD_LEN(l) \
> +    if (cmd_len < l) {                                     \
> +        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;      \
> +        goto out; \
> +    }
> +
> +/* Check that the reservation in the command is valid. */
> +#define IPMI_CHECK_RESERVATION(off, r) \
> +    do {                                                   \
> +        if ((cmd[off] | (cmd[off + 1] << 8)) != r) {       \
> +            rsp[2] = IPMI_CC_INVALID_RESERVATION;          \
> +            goto out;                                      \
> +        }                                                  \
> +    } while (0)
> +
> +
> +static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs);
> +
> +static void ipmi_gettime(struct ipmi_time *time)
> +{
> +    int64_t stime;
> +
> +    stime = qemu_clock_get_ns(QEMU_CLOCK_HOST);
> +    time->tv_sec = stime / 1000000000LL;
> +    time->tv_nsec = stime % 1000000000LL;
> +}
> +
> +static int64_t ipmi_getmonotime(void)
> +{
> +    return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +}
> +
> +static void ipmi_timeout(void *opaque)
> +{
> +    IPMIBmcSim *ibs = opaque;
> +
> +    ipmi_sim_handle_timeout(ibs);
> +}
> +
> +static void set_timestamp(IPMIBmcSim *ibs, uint8_t *ts)
> +{
> +    unsigned int val;
> +    struct ipmi_time now;
> +
> +    ipmi_gettime(&now);
> +    val = now.tv_sec + ibs->sel.time_offset;
> +    ts[0] = val & 0xff;
> +    ts[1] = (val >> 8) & 0xff;
> +    ts[2] = (val >> 16) & 0xff;
> +    ts[3] = (val >> 24) & 0xff;
> +}
> +
> +static void sdr_inc_reservation(IPMISdr *sdr)
> +{
> +    sdr->reservation++;
> +    if (sdr->reservation == 0) {
> +        sdr->reservation = 1;
> +    }
> +}

Could we externalize the following routine ? for platforms needing 
to add a few initial sdrs.
 
> +static int sdr_add_entry(IPMIBmcSim *ibs, const uint8_t *entry,
> +                         unsigned int len, uint16_t *recid)
> +{
> +    if ((len < 5) || (len > 255)) {
> +        return 1;
> +    }
> +
> +    if (entry[ibs->sdr.next_free + 4] != len) {

I think you meant :

        if (entry[4] != len) {
no ? 


> +        return 1;
> +    }
> +
> +    if (ibs->sdr.next_free + len > MAX_SDR_SIZE) {
> +        ibs->sdr.overflow = 1;
> +        return 1;
> +    }
> +
> +    memcpy(ibs->sdr.sdr + ibs->sdr.next_free, entry, len);
> +    ibs->sdr.sdr[ibs->sdr.next_free] = ibs->sdr.next_rec_id & 0xff;
> +    ibs->sdr.sdr[ibs->sdr.next_free+1] = (ibs->sdr.next_rec_id >> 8) & 0xff;
> +    ibs->sdr.sdr[ibs->sdr.next_free+2] = 0x51; /* Conform to IPMI 1.5 spec */
> +
> +    if (recid) {
> +        *recid = ibs->sdr.next_rec_id;
> +    }
> +    ibs->sdr.next_rec_id++;
> +    set_timestamp(ibs, ibs->sdr.last_addition);
> +    ibs->sdr.next_free += len;
> +    sdr_inc_reservation(&ibs->sdr);
> +    return 0;
> +}
> +

[ ... ]

> +static void register_cmds(IPMIBmcSim *s)
> +{
> +    ipmi_register_netfn(s, IPMI_NETFN_CHASSIS, &chassis_netfn);
> +    ipmi_register_netfn(s, IPMI_NETFN_SENSOR_EVENT, &sensor_event_netfn);
> +    ipmi_register_netfn(s, IPMI_NETFN_APP, &app_netfn);
> +    ipmi_register_netfn(s, IPMI_NETFN_STORAGE, &storage_netfn);
> +}
> +
> +static const uint8_t init_sdrs[] = {
> +    /* Watchdog device */
> +    0x00, 0x00, 0x51, 0x02,   40, 0x20, 0x00, 0x00,

Regarding the 'length' byte, the specs says : 

        Record Length: Number of bytes of data following the Record Length 
field.

So it should be 35 and not 40 ?  This change will problably 
have some consequences in the routine above.

> +    0x23, 0x01, 0x63, 0x00, 0x23, 0x6f, 0x0f, 0x01,
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xc8,
> +    'W',  'a',  't',  'c',  'h',  'd',  'o',  'g',
> +    /* End */
> +    0xff, 0xff, 0x00, 0x00, 0x00
> +};
> +
> +static void ipmi_sim_init(Object *obj)
> +{
> +    IPMIBmc *b = IPMI_BMC(obj);
> +    unsigned int i;
> +    unsigned int next_recid, recid;
> +    IPMIBmcSim *ibs = IPMI_BMC_SIMULATOR(b);
> +
> +    qemu_mutex_init(&ibs->lock);
> +    QTAILQ_INIT(&ibs->rcvbufs);
> +
> +    ibs->bmc_global_enables = (1 << IPMI_BMC_EVENT_LOG_BIT);
> +    ibs->device_id = 0x20;
> +    ibs->ipmi_version = 0x02; /* IPMI 2.0 */
> +    for (i = 0; i < 4; i++) {
> +        ibs->sel.last_addition[i] = 0xff;
> +        ibs->sel.last_clear[i] = 0xff;
> +        ibs->sdr.last_addition[i] = 0xff;
> +        ibs->sdr.last_clear[i] = 0xff;
> +    }
> +
> +    next_recid = 0;
> +    for (i = 0;;) {
> +        int len;
> +        if ((i + 5) > sizeof(init_sdrs)) {
> +            error_report("Problem with recid 0x%4.4x\n", i);
> +            return;
> +        }
> +        len = init_sdrs[i + 4];
> +        recid = init_sdrs[i] | (init_sdrs[i + 1] << 8);
> +        if (recid == 0xffff) {
> +            break;
> +        }
> +        if ((i + len) > sizeof(init_sdrs)) {
> +            error_report("Problem with recid 0x%4.4x\n", i);
> +            return;
> +        }
> +        if (recid != next_recid) {
> +            error_report("Problem with recid 0x%4.4x\n", i);
> +            return;
> +        }

May be remove the check on recid  which is useless here ? 

> +        sdr_add_entry(ibs, init_sdrs + i, len, NULL);
> +        i += len;
> +    }
> +
> +    ipmi_init_sensors_from_sdrs(ibs);
> +    register_cmds(ibs);
> +
> +    ibs->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ipmi_timeout, ibs);
> +}
> +
> +static void ipmi_sim_class_init(ObjectClass *oc, void *data)
> +{
> +    IPMIBmcClass *bk = IPMI_BMC_CLASS(oc);
> +
> +    bk->handle_command = ipmi_sim_handle_command;
> +}
> +
> +static const TypeInfo ipmi_sim_type = {
> +    .name          = TYPE_IPMI_BMC_SIMULATOR,
> +    .parent        = TYPE_IPMI_BMC,
> +    .instance_size = sizeof(IPMIBmcSim),
> +    .instance_init = ipmi_sim_init,
> +    .class_init    = ipmi_sim_class_init,
> +};
> +
> +static void ipmi_sim_register_types(void)
> +{
> +    type_register_static(&ipmi_sim_type);
> +}
> +
> +type_init(ipmi_sim_register_types)
>