diff mbox

[v2,02/10] ipmi: replace IPMI_ADD_RSP_DATA() macro with inline helpers

Message ID 1456913698-11452-3-git-send-email-clg@fr.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Cédric Le Goater March 2, 2016, 10:14 a.m. UTC
The IPMI command handlers in the BMC simulator use a macro
IPMI_ADD_RSP_DATA() to push bytes in a response buffer. The macro
hides the fact that it implicitly uses variables local to the handler,
which is misleading.

This patch introduces a simple 'struct rsp_buffer' and inlined helper
routines to store byte(s) in a response buffer. rsp_buffer_push()
replaces the macro IPMI_ADD_RSP_DATA() and rsp_buffer_pushmore() is
new helper to push multiple bytes. The latest is used in the command
handlers get_msg() and get_sdr() which are manipulating the buffer
directly.

Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
 hw/ipmi/ipmi_bmc_sim.c | 482 +++++++++++++++++++++++--------------------------
 1 file changed, 229 insertions(+), 253 deletions(-)

Comments

Michael S. Tsirkin March 2, 2016, 10:25 a.m. UTC | #1
On Wed, Mar 02, 2016 at 11:14:50AM +0100, Cédric Le Goater wrote:
> The IPMI command handlers in the BMC simulator use a macro
> IPMI_ADD_RSP_DATA() to push bytes in a response buffer. The macro
> hides the fact that it implicitly uses variables local to the handler,
> which is misleading.
> 
> This patch introduces a simple 'struct rsp_buffer' and inlined helper
> routines to store byte(s) in a response buffer. rsp_buffer_push()
> replaces the macro IPMI_ADD_RSP_DATA() and rsp_buffer_pushmore() is
> new helper to push multiple bytes. The latest is used in the command
> handlers get_msg() and get_sdr() which are manipulating the buffer
> directly.
> 
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>


Looks good overall.
Some stylistic comments.

> ---
>  hw/ipmi/ipmi_bmc_sim.c | 482 +++++++++++++++++++++++--------------------------
>  1 file changed, 229 insertions(+), 253 deletions(-)
> 
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 30b9fb48ea2d..32efb87c8232 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -153,14 +153,14 @@ typedef struct IPMISensor {
>  #define IPMI_WATCHDOG_SENSOR 0
>  
>  typedef struct IPMIBmcSim IPMIBmcSim;
> +struct rsp_buffer;
>

typedef struct RspBuffer RspBuffer;

  
>  #define MAX_NETFNS 64
>  
>  typedef struct IPMICmdHandler {
>      void (*cmd_handler)(IPMIBmcSim *s,
>                          uint8_t *cmd, unsigned int cmd_len,
> -                        uint8_t *rsp, unsigned int *rsp_len,
> -                        unsigned int max_rsp_len);
> +                        struct rsp_buffer *rsp);
>      unsigned int cmd_len_min;
>  } IPMICmdHandler;
>  
> @@ -263,22 +263,41 @@ struct IPMIBmcSim {
>  #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN      2
>  #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE     3
>  
> +struct rsp_buffer {
> +    uint8_t buffer[MAX_IPMI_MSG_SIZE];
> +    unsigned int len;
> +    unsigned int max_len;
> +};
> +
> +#define RSP_BUFFER_INITIALIZER { { 0 }, 0, MAX_IPMI_MSG_SIZE }

max_len seems to always be a constant. So just use ARRAY_SIZE(buffer)
and RSP_BUFFER_INITIALIZER can then be replaced with plain { };

>  
>  /* 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;       \
> -            return;                                        \
> -        }                                                  \
> -        rsp[(*rsp_len)++] = (b);                           \
> -    } while (0)
> +static inline void rsp_buffer_push(struct rsp_buffer *rsp, uint8_t byte)
> +{
> +    if (rsp->len >= rsp->max_len) {

Why not simply
	rsp->len >= ARRAY_SIZE(rsp->buffer)
?



> +        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +        return;
> +    }
> +    rsp->buffer[rsp->len++] = byte;
> +}
> +
> +static inline void rsp_buffer_pushmore(struct rsp_buffer *rsp, uint8_t *bytes,
> +                                       unsigned int n)
> +{
> +    if (rsp->len + n >= rsp->max_len) {
> +        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +        return;
> +    }
> +
> +    memcpy(&rsp->buffer[rsp->len], bytes, n);
> +    rsp->len += n;
> +}
>  
>  /* 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;          \
> +            rsp->buffer[2] = IPMI_CC_INVALID_RESERVATION;     \
>              return;                                        \
>          }                                                  \
>      } while (0)
> @@ -585,35 +604,32 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>      IPMIInterface *s = ibs->parent.intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>      unsigned int netfn;
> -    uint8_t rsp[MAX_IPMI_MSG_SIZE];
> -    unsigned int rsp_len_holder = 0;
> -    unsigned int *rsp_len = &rsp_len_holder;
> -    unsigned int max_rsp_len = sizeof(rsp);
> +    struct rsp_buffer rsp = RSP_BUFFER_INITIALIZER;
>  
>      /* Set up the response, set the low bit of NETFN. */
>      /* Note that max_rsp_len must be at least 3 */
> -    if (max_rsp_len < 3) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +    if (rsp.max_len < 3) {
> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>          goto out;
>      }
>  
> -    IPMI_ADD_RSP_DATA(cmd[0] | 0x04);
> -    IPMI_ADD_RSP_DATA(cmd[1]);
> -    IPMI_ADD_RSP_DATA(0); /* Assume success */
> +    rsp_buffer_push(&rsp, cmd[0] | 0x04);
> +    rsp_buffer_push(&rsp, cmd[1]);
> +    rsp_buffer_push(&rsp, 0); /* Assume success */
>  
>      /* If it's too short or it was truncated, return an error. */
>      if (cmd_len < 2) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>          goto out;
>      }
>      if (cmd_len > max_cmd_len) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>          goto out;
>      }
>  
>      if ((cmd[0] & 0x03) != 0) {
>          /* Only have stuff on LUN 0 */
> -        rsp[2] = IPMI_CC_COMMAND_INVALID_FOR_LUN;
> +        rsp.buffer[2] = IPMI_CC_COMMAND_INVALID_FOR_LUN;
>          goto out;
>      }
>  
> @@ -623,20 +639,20 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>      if ((netfn & 1) || !ibs->netfns[netfn / 2] ||
>          (cmd[1] >= ibs->netfns[netfn / 2]->cmd_nums) ||
>          (!ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler)) {
> -        rsp[2] = IPMI_CC_INVALID_CMD;
> +        rsp.buffer[2] = IPMI_CC_INVALID_CMD;
>          goto out;
>      }
>  
>      if (cmd_len < ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_len_min) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>          goto out;
>      }
>  
>      ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler(ibs, cmd, cmd_len,
> -                                                rsp, rsp_len, max_rsp_len);
> +                                                &rsp);
>  
>   out:
> -    k->handle_rsp(s, msg_id, rsp, *rsp_len);
> +    k->handle_rsp(s, msg_id, rsp.buffer, rsp.len);
>  
>      next_timeout(ibs);
>  }
> @@ -711,86 +727,82 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
>  
>  static void chassis_capabilities(IPMIBmcSim *ibs,
>                                   uint8_t *cmd, unsigned int cmd_len,
> -                                 uint8_t *rsp, unsigned int *rsp_len,
> -                                 unsigned int max_rsp_len)
> +                                 struct rsp_buffer *rsp)
>  {
> -    IPMI_ADD_RSP_DATA(0);
> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
> +    rsp_buffer_push(rsp, 0);
> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
>  }
>  
>  static void chassis_status(IPMIBmcSim *ibs,
>                             uint8_t *cmd, unsigned int cmd_len,
> -                           uint8_t *rsp, unsigned int *rsp_len,
> -                           unsigned int max_rsp_len)
> +                           struct rsp_buffer *rsp)
>  {
> -    IPMI_ADD_RSP_DATA(0x61); /* Unknown power restore, power is on */
> -    IPMI_ADD_RSP_DATA(0);
> -    IPMI_ADD_RSP_DATA(0);
> -    IPMI_ADD_RSP_DATA(0);
> +    rsp_buffer_push(rsp, 0x61); /* Unknown power restore, power is on */
> +    rsp_buffer_push(rsp, 0);
> +    rsp_buffer_push(rsp, 0);
> +    rsp_buffer_push(rsp, 0);
>  }
>  
>  static void chassis_control(IPMIBmcSim *ibs,
>                              uint8_t *cmd, unsigned int cmd_len,
> -                            uint8_t *rsp, unsigned int *rsp_len,
> -                            unsigned int max_rsp_len)
> +                            struct rsp_buffer *rsp)
>  {
>      IPMIInterface *s = ibs->parent.intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>  
>      switch (cmd[2] & 0xf) {
>      case 0: /* power down */
> -        rsp[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
>          break;
>      case 1: /* power up */
> -        rsp[2] = k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0);
>          break;
>      case 2: /* power cycle */
> -        rsp[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
>          break;
>      case 3: /* hard reset */
> -        rsp[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
>          break;
>      case 4: /* pulse diagnostic interrupt */
> -        rsp[2] = k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0);
>          break;
>      case 5: /* soft shutdown via ACPI by overtemp emulation */
> -        rsp[2] = k->do_hw_op(s,
> +        rsp->buffer[2] = k->do_hw_op(s,
>                               IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 0);
>          break;
>      default:
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>  }
>  
>  static void chassis_get_sys_restart_cause(IPMIBmcSim *ibs,
>                             uint8_t *cmd, unsigned int cmd_len,
> -                           uint8_t *rsp, unsigned int *rsp_len,
> -                           unsigned int max_rsp_len)
> +                           struct rsp_buffer *rsp)
> +
>  {
> -    IPMI_ADD_RSP_DATA(ibs->restart_cause & 0xf); /* Restart Cause */
> -    IPMI_ADD_RSP_DATA(0);  /* Channel 0 */
> +    rsp_buffer_push(rsp, ibs->restart_cause & 0xf); /* Restart Cause */
> +    rsp_buffer_push(rsp, 0);  /* Channel 0 */
>  }
>  
>  static void get_device_id(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> -{
> -    IPMI_ADD_RSP_DATA(ibs->device_id);
> -    IPMI_ADD_RSP_DATA(ibs->device_rev & 0xf);
> -    IPMI_ADD_RSP_DATA(ibs->fwrev1 & 0x7f);
> -    IPMI_ADD_RSP_DATA(ibs->fwrev2);
> -    IPMI_ADD_RSP_DATA(ibs->ipmi_version);
> -    IPMI_ADD_RSP_DATA(0x07); /* sensor, SDR, and SEL. */
> -    IPMI_ADD_RSP_DATA(ibs->mfg_id[0]);
> -    IPMI_ADD_RSP_DATA(ibs->mfg_id[1]);
> -    IPMI_ADD_RSP_DATA(ibs->mfg_id[2]);
> -    IPMI_ADD_RSP_DATA(ibs->product_id[0]);
> -    IPMI_ADD_RSP_DATA(ibs->product_id[1]);
> +                          struct rsp_buffer *rsp)
> +{
> +    rsp_buffer_push(rsp, ibs->device_id);
> +    rsp_buffer_push(rsp, ibs->device_rev & 0xf);
> +    rsp_buffer_push(rsp, ibs->fwrev1 & 0x7f);
> +    rsp_buffer_push(rsp, ibs->fwrev2);
> +    rsp_buffer_push(rsp, ibs->ipmi_version);
> +    rsp_buffer_push(rsp, 0x07); /* sensor, SDR, and SEL. */
> +    rsp_buffer_push(rsp, ibs->mfg_id[0]);
> +    rsp_buffer_push(rsp, ibs->mfg_id[1]);
> +    rsp_buffer_push(rsp, ibs->mfg_id[2]);
> +    rsp_buffer_push(rsp, ibs->product_id[0]);
> +    rsp_buffer_push(rsp, ibs->product_id[1]);
>  }
>  
>  static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
> @@ -809,8 +821,7 @@ static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
>  
>  static void cold_reset(IPMIBmcSim *ibs,
>                         uint8_t *cmd, unsigned int cmd_len,
> -                       uint8_t *rsp, unsigned int *rsp_len,
> -                       unsigned int max_rsp_len)
> +                       struct rsp_buffer *rsp)
>  {
>      IPMIInterface *s = ibs->parent.intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -825,8 +836,7 @@ static void cold_reset(IPMIBmcSim *ibs,
>  
>  static void warm_reset(IPMIBmcSim *ibs,
>                         uint8_t *cmd, unsigned int cmd_len,
> -                       uint8_t *rsp, unsigned int *rsp_len,
> -                       unsigned int max_rsp_len)
> +                       struct rsp_buffer *rsp)
>  {
>      IPMIInterface *s = ibs->parent.intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -836,55 +846,49 @@ static void warm_reset(IPMIBmcSim *ibs,
>      }
>  }
>  static void set_acpi_power_state(IPMIBmcSim *ibs,
> -                          uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                                 uint8_t *cmd, unsigned int cmd_len,
> +                                 struct rsp_buffer *rsp)
>  {
>      ibs->acpi_power_state[0] = cmd[2];
>      ibs->acpi_power_state[1] = cmd[3];
>  }
>  
>  static void get_acpi_power_state(IPMIBmcSim *ibs,
> -                          uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                                 uint8_t *cmd, unsigned int cmd_len,
> +                                 struct rsp_buffer *rsp)
>  {
> -    IPMI_ADD_RSP_DATA(ibs->acpi_power_state[0]);
> -    IPMI_ADD_RSP_DATA(ibs->acpi_power_state[1]);
> +    rsp_buffer_push(rsp, ibs->acpi_power_state[0]);
> +    rsp_buffer_push(rsp, ibs->acpi_power_state[1]);
>  }
>  
>  static void get_device_guid(IPMIBmcSim *ibs,
> -                          uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                            uint8_t *cmd, unsigned int cmd_len,
> +                            struct rsp_buffer *rsp)
>  {
>      unsigned int i;
>  
>      for (i = 0; i < 16; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->uuid[i]);
> +        rsp_buffer_push(rsp, ibs->uuid[i]);
>      }
>  }
>  
>  static void set_bmc_global_enables(IPMIBmcSim *ibs,
>                                     uint8_t *cmd, unsigned int cmd_len,
> -                                   uint8_t *rsp, unsigned int *rsp_len,
> -                                   unsigned int max_rsp_len)
> +                                   struct rsp_buffer *rsp)
>  {
>      set_global_enables(ibs, cmd[2]);
>  }
>  
>  static void get_bmc_global_enables(IPMIBmcSim *ibs,
>                                     uint8_t *cmd, unsigned int cmd_len,
> -                                   uint8_t *rsp, unsigned int *rsp_len,
> -                                   unsigned int max_rsp_len)
> +                                   struct rsp_buffer *rsp)
>  {
> -    IPMI_ADD_RSP_DATA(ibs->bmc_global_enables);
> +    rsp_buffer_push(rsp, ibs->bmc_global_enables);
>  }
>  
>  static void clr_msg_flags(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>  {
>      IPMIInterface *s = ibs->parent.intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -895,27 +899,25 @@ static void clr_msg_flags(IPMIBmcSim *ibs,
>  
>  static void get_msg_flags(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>  {
> -    IPMI_ADD_RSP_DATA(ibs->msg_flags);
> +    rsp_buffer_push(rsp, ibs->msg_flags);
>  }
>  
>  static void read_evt_msg_buf(IPMIBmcSim *ibs,
>                               uint8_t *cmd, unsigned int cmd_len,
> -                             uint8_t *rsp, unsigned int *rsp_len,
> -                            unsigned int max_rsp_len)
> +                             struct rsp_buffer *rsp)
>  {
>      IPMIInterface *s = ibs->parent.intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>      unsigned int i;
>  
>      if (!(ibs->msg_flags & IPMI_BMC_MSG_FLAG_EVT_BUF_FULL)) {
> -        rsp[2] = 0x80;
> +        rsp->buffer[2] = 0x80;
>          return;
>      }
>      for (i = 0; i < 16; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->evtbuf[i]);
> +        rsp_buffer_push(rsp, ibs->evtbuf[i]);
>      }
>      ibs->msg_flags &= ~IPMI_BMC_MSG_FLAG_EVT_BUF_FULL;
>      k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs));
> @@ -923,21 +925,18 @@ static void read_evt_msg_buf(IPMIBmcSim *ibs,
>  
>  static void get_msg(IPMIBmcSim *ibs,
>                      uint8_t *cmd, unsigned int cmd_len,
> -                    uint8_t *rsp, unsigned int *rsp_len,
> -                    unsigned int max_rsp_len)
> +                    struct rsp_buffer *rsp)
>  {
>      IPMIRcvBufEntry *msg;
>  
>      qemu_mutex_lock(&ibs->lock);
>      if (QTAILQ_EMPTY(&ibs->rcvbufs)) {
> -        rsp[2] = 0x80; /* Queue empty */
> +        rsp->buffer[2] = 0x80; /* Queue empty */
>          goto out;
>      }
> -    rsp[3] = 0; /* Channel 0 */
> -    *rsp_len += 1;
> +    rsp_buffer_push(rsp, 0); /* Channel 0 */
>      msg = QTAILQ_FIRST(&ibs->rcvbufs);
> -    memcpy(rsp + 4, msg->buf, msg->len);
> -    *rsp_len += msg->len;
> +    rsp_buffer_pushmore(rsp, msg->buf, msg->len);
>      QTAILQ_REMOVE(&ibs->rcvbufs, msg, entry);
>      g_free(msg);
>  
> @@ -966,8 +965,7 @@ ipmb_checksum(unsigned char *data, int size, unsigned char csum)
>  
>  static void send_msg(IPMIBmcSim *ibs,
>                       uint8_t *cmd, unsigned int cmd_len,
> -                     uint8_t *rsp, unsigned int *rsp_len,
> -                     unsigned int max_rsp_len)
> +                     struct rsp_buffer *rsp)
>  {
>      IPMIInterface *s = ibs->parent.intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -977,18 +975,18 @@ static void send_msg(IPMIBmcSim *ibs,
>  
>      if (cmd[2] != 0) {
>          /* We only handle channel 0 with no options */
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>  
>      if (cmd_len < 10) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>          return;
>      }
>  
>      if (cmd[3] != 0x40) {
>          /* We only emulate a MC at address 0x40. */
> -        rsp[2] = 0x83; /* NAK on write */
> +        rsp->buffer[2] = 0x83; /* NAK on write */
>          return;
>      }
>  
> @@ -1074,11 +1072,10 @@ static void do_watchdog_reset(IPMIBmcSim *ibs)
>  
>  static void reset_watchdog_timer(IPMIBmcSim *ibs,
>                                   uint8_t *cmd, unsigned int cmd_len,
> -                                 uint8_t *rsp, unsigned int *rsp_len,
> -                                 unsigned int max_rsp_len)
> +                                 struct rsp_buffer *rsp)
>  {
>      if (!ibs->watchdog_initialized) {
> -        rsp[2] = 0x80;
> +        rsp->buffer[2] = 0x80;
>          return;
>      }
>      do_watchdog_reset(ibs);
> @@ -1086,8 +1083,7 @@ static void reset_watchdog_timer(IPMIBmcSim *ibs,
>  
>  static void set_watchdog_timer(IPMIBmcSim *ibs,
>                                 uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                               struct rsp_buffer *rsp)
>  {
>      IPMIInterface *s = ibs->parent.intf;
>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -1095,7 +1091,7 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>  
>      val = cmd[2] & 0x7; /* Validate use */
>      if (val == 0 || val > 5) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>      val = cmd[3] & 0x7; /* Validate action */
> @@ -1104,22 +1100,22 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>          break;
>  
>      case IPMI_BMC_WATCHDOG_ACTION_RESET:
> -        rsp[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 1);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 1);
>          break;
>  
>      case IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN:
> -        rsp[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1);
>          break;
>  
>      case IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE:
> -        rsp[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1);
>          break;
>  
>      default:
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>      }
> -    if (rsp[2]) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +    if (rsp->buffer[2]) {
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>  
> @@ -1132,14 +1128,14 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>      case IPMI_BMC_WATCHDOG_PRE_NMI:
>          if (!k->do_hw_op(s, IPMI_SEND_NMI, 1)) {
>              /* NMI not supported. */
> -            rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +            rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>              return;
>          }
>          break;
>  
>      default:
>          /* We don't support PRE_SMI */
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>  
> @@ -1158,60 +1154,56 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>  
>  static void get_watchdog_timer(IPMIBmcSim *ibs,
>                                 uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                               struct rsp_buffer *rsp)
>  {
> -    IPMI_ADD_RSP_DATA(ibs->watchdog_use);
> -    IPMI_ADD_RSP_DATA(ibs->watchdog_action);
> -    IPMI_ADD_RSP_DATA(ibs->watchdog_pretimeout);
> -    IPMI_ADD_RSP_DATA(ibs->watchdog_expired);
> +    rsp_buffer_push(rsp, ibs->watchdog_use);
> +    rsp_buffer_push(rsp, ibs->watchdog_action);
> +    rsp_buffer_push(rsp, ibs->watchdog_pretimeout);
> +    rsp_buffer_push(rsp, ibs->watchdog_expired);
>      if (ibs->watchdog_running) {
>          long timeout;
>          timeout = ((ibs->watchdog_expiry - ipmi_getmonotime() + 50000000)
>                     / 100000000);
> -        IPMI_ADD_RSP_DATA(timeout & 0xff);
> -        IPMI_ADD_RSP_DATA((timeout >> 8) & 0xff);
> +        rsp_buffer_push(rsp, timeout & 0xff);
> +        rsp_buffer_push(rsp, (timeout >> 8) & 0xff);
>      } else {
> -        IPMI_ADD_RSP_DATA(0);
> -        IPMI_ADD_RSP_DATA(0);
> +        rsp_buffer_push(rsp, 0);
> +        rsp_buffer_push(rsp, 0);
>      }
>  }
>  
>  static void get_sdr_rep_info(IPMIBmcSim *ibs,
>                               uint8_t *cmd, unsigned int cmd_len,
> -                             uint8_t *rsp, unsigned int *rsp_len,
> -                             unsigned int max_rsp_len)
> +                             struct rsp_buffer *rsp)
>  {
>      unsigned int i;
>  
> -    IPMI_ADD_RSP_DATA(0x51); /* Conform to IPMI 1.5 spec */
> -    IPMI_ADD_RSP_DATA(ibs->sdr.next_rec_id & 0xff);
> -    IPMI_ADD_RSP_DATA((ibs->sdr.next_rec_id >> 8) & 0xff);
> -    IPMI_ADD_RSP_DATA((MAX_SDR_SIZE - ibs->sdr.next_free) & 0xff);
> -    IPMI_ADD_RSP_DATA(((MAX_SDR_SIZE - ibs->sdr.next_free) >> 8) & 0xff);
> +    rsp_buffer_push(rsp, 0x51); /* Conform to IPMI 1.5 spec */
> +    rsp_buffer_push(rsp, ibs->sdr.next_rec_id & 0xff);
> +    rsp_buffer_push(rsp, (ibs->sdr.next_rec_id >> 8) & 0xff);
> +    rsp_buffer_push(rsp, (MAX_SDR_SIZE - ibs->sdr.next_free) & 0xff);
> +    rsp_buffer_push(rsp, ((MAX_SDR_SIZE - ibs->sdr.next_free) >> 8) & 0xff);
>      for (i = 0; i < 4; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->sdr.last_addition[i]);
> +        rsp_buffer_push(rsp, ibs->sdr.last_addition[i]);
>      }
>      for (i = 0; i < 4; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->sdr.last_clear[i]);
> +        rsp_buffer_push(rsp, ibs->sdr.last_clear[i]);
>      }
>      /* Only modal support, reserve supported */
> -    IPMI_ADD_RSP_DATA((ibs->sdr.overflow << 7) | 0x22);
> +    rsp_buffer_push(rsp, (ibs->sdr.overflow << 7) | 0x22);
>  }
>  
>  static void reserve_sdr_rep(IPMIBmcSim *ibs,
>                              uint8_t *cmd, unsigned int cmd_len,
> -                            uint8_t *rsp, unsigned int *rsp_len,
> -                            unsigned int max_rsp_len)
> +                            struct rsp_buffer *rsp)
>  {
> -    IPMI_ADD_RSP_DATA(ibs->sdr.reservation & 0xff);
> -    IPMI_ADD_RSP_DATA((ibs->sdr.reservation >> 8) & 0xff);
> +    rsp_buffer_push(rsp, ibs->sdr.reservation & 0xff);
> +    rsp_buffer_push(rsp, (ibs->sdr.reservation >> 8) & 0xff);
>  }
>  
>  static void get_sdr(IPMIBmcSim *ibs,
>                      uint8_t *cmd, unsigned int cmd_len,
> -                    uint8_t *rsp, unsigned int *rsp_len,
> -                    unsigned int max_rsp_len)
> +                    struct rsp_buffer *rsp)
>  {
>      unsigned int pos;
>      uint16_t nextrec;
> @@ -1223,108 +1215,103 @@ static void get_sdr(IPMIBmcSim *ibs,
>      pos = 0;
>      if (sdr_find_entry(&ibs->sdr, cmd[4] | (cmd[5] << 8),
>                         &pos, &nextrec)) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>  
>      sdrh = (struct ipmi_sdr_header *) &ibs->sdr.sdr[pos];
>  
>      if (cmd[6] > ipmi_sdr_length(sdrh)) {
> -        rsp[2] = IPMI_CC_PARM_OUT_OF_RANGE;
> +        rsp->buffer[2] = IPMI_CC_PARM_OUT_OF_RANGE;
>          return;
>      }
>  
> -    IPMI_ADD_RSP_DATA(nextrec & 0xff);
> -    IPMI_ADD_RSP_DATA((nextrec >> 8) & 0xff);
> +    rsp_buffer_push(rsp, nextrec & 0xff);
> +    rsp_buffer_push(rsp, (nextrec >> 8) & 0xff);
>  
>      if (cmd[7] == 0xff) {
>          cmd[7] = ipmi_sdr_length(sdrh) - cmd[6];
>      }
>  
> -    if ((cmd[7] + *rsp_len) > max_rsp_len) {
> -        rsp[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
> +    if ((cmd[7] + rsp->len) > rsp->max_len) {
> +        rsp->buffer[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
>          return;
>      }
> -    memcpy(rsp + *rsp_len, ibs->sdr.sdr + pos + cmd[6], cmd[7]);
> -    *rsp_len += cmd[7];
> +
> +    rsp_buffer_pushmore(rsp, ibs->sdr.sdr + pos + cmd[6], cmd[7]);
>  }
>  
>  static void add_sdr(IPMIBmcSim *ibs,
>                      uint8_t *cmd, unsigned int cmd_len,
> -                    uint8_t *rsp, unsigned int *rsp_len,
> -                    unsigned int max_rsp_len)
> +                    struct rsp_buffer *rsp)
>  {
>      uint16_t recid;
>      struct ipmi_sdr_header *sdrh = (struct ipmi_sdr_header *) cmd + 2;
>  
>      if (sdr_add_entry(ibs, sdrh, cmd_len - 2, &recid)) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
> -    IPMI_ADD_RSP_DATA(recid & 0xff);
> -    IPMI_ADD_RSP_DATA((recid >> 8) & 0xff);
> +    rsp_buffer_push(rsp, recid & 0xff);
> +    rsp_buffer_push(rsp, (recid >> 8) & 0xff);
>  }
>  
>  static void clear_sdr_rep(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>  {
>      IPMI_CHECK_RESERVATION(2, ibs->sdr.reservation);
>      if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>      if (cmd[7] == 0xaa) {
>          ibs->sdr.next_free = 0;
>          ibs->sdr.overflow = 0;
>          set_timestamp(ibs, ibs->sdr.last_clear);
> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>          sdr_inc_reservation(&ibs->sdr);
>      } else if (cmd[7] == 0) {
> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>      } else {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>  }
>  
>  static void get_sel_info(IPMIBmcSim *ibs,
>                           uint8_t *cmd, unsigned int cmd_len,
> -                         uint8_t *rsp, unsigned int *rsp_len,
> -                         unsigned int max_rsp_len)
> +                         struct rsp_buffer *rsp)
>  {
>      unsigned int i, val;
>  
> -    IPMI_ADD_RSP_DATA(0x51); /* Conform to IPMI 1.5 */
> -    IPMI_ADD_RSP_DATA(ibs->sel.next_free & 0xff);
> -    IPMI_ADD_RSP_DATA((ibs->sel.next_free >> 8) & 0xff);
> +    rsp_buffer_push(rsp, 0x51); /* Conform to IPMI 1.5 */
> +    rsp_buffer_push(rsp, ibs->sel.next_free & 0xff);
> +    rsp_buffer_push(rsp, (ibs->sel.next_free >> 8) & 0xff);
>      val = (MAX_SEL_SIZE - ibs->sel.next_free) * 16;
> -    IPMI_ADD_RSP_DATA(val & 0xff);
> -    IPMI_ADD_RSP_DATA((val >> 8) & 0xff);
> +    rsp_buffer_push(rsp, val & 0xff);
> +    rsp_buffer_push(rsp, (val >> 8) & 0xff);
>      for (i = 0; i < 4; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->sel.last_addition[i]);
> +        rsp_buffer_push(rsp, ibs->sel.last_addition[i]);
>      }
>      for (i = 0; i < 4; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->sel.last_clear[i]);
> +        rsp_buffer_push(rsp, ibs->sel.last_clear[i]);
>      }
>      /* Only support Reserve SEL */
> -    IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02);
> +    rsp_buffer_push(rsp, (ibs->sel.overflow << 7) | 0x02);
>  }
>  
>  static void reserve_sel(IPMIBmcSim *ibs,
>                          uint8_t *cmd, unsigned int cmd_len,
> -                        uint8_t *rsp, unsigned int *rsp_len,
> -                        unsigned int max_rsp_len)
> +                        struct rsp_buffer *rsp)
>  {
> -    IPMI_ADD_RSP_DATA(ibs->sel.reservation & 0xff);
> -    IPMI_ADD_RSP_DATA((ibs->sel.reservation >> 8) & 0xff);
> +    rsp_buffer_push(rsp, ibs->sel.reservation & 0xff);
> +    rsp_buffer_push(rsp, (ibs->sel.reservation >> 8) & 0xff);
>  }
>  
>  static void get_sel_entry(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>  {
>      unsigned int val;
>  
> @@ -1332,17 +1319,17 @@ static void get_sel_entry(IPMIBmcSim *ibs,
>          IPMI_CHECK_RESERVATION(2, ibs->sel.reservation);
>      }
>      if (ibs->sel.next_free == 0) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>      if (cmd[6] > 15) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>      if (cmd[7] == 0xff) {
>          cmd[7] = 16;
>      } else if ((cmd[7] + cmd[6]) > 16) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      } else {
>          cmd[7] += cmd[6];
> @@ -1352,79 +1339,75 @@ static void get_sel_entry(IPMIBmcSim *ibs,
>      if (val == 0xffff) {
>          val = ibs->sel.next_free - 1;
>      } else if (val >= ibs->sel.next_free) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>      if ((val + 1) == ibs->sel.next_free) {
> -        IPMI_ADD_RSP_DATA(0xff);
> -        IPMI_ADD_RSP_DATA(0xff);
> +        rsp_buffer_push(rsp, 0xff);
> +        rsp_buffer_push(rsp, 0xff);
>      } else {
> -        IPMI_ADD_RSP_DATA((val + 1) & 0xff);
> -        IPMI_ADD_RSP_DATA(((val + 1) >> 8) & 0xff);
> +        rsp_buffer_push(rsp, (val + 1) & 0xff);
> +        rsp_buffer_push(rsp, ((val + 1) >> 8) & 0xff);
>      }
>      for (; cmd[6] < cmd[7]; cmd[6]++) {
> -        IPMI_ADD_RSP_DATA(ibs->sel.sel[val][cmd[6]]);
> +        rsp_buffer_push(rsp, ibs->sel.sel[val][cmd[6]]);
>      }
>  }
>  
>  static void add_sel_entry(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>  {
>      if (sel_add_event(ibs, cmd + 2)) {
> -        rsp[2] = IPMI_CC_OUT_OF_SPACE;
> +        rsp->buffer[2] = IPMI_CC_OUT_OF_SPACE;
>          return;
>      }
>      /* sel_add_event fills in the record number. */
> -    IPMI_ADD_RSP_DATA(cmd[2]);
> -    IPMI_ADD_RSP_DATA(cmd[3]);
> +    rsp_buffer_push(rsp, cmd[2]);
> +    rsp_buffer_push(rsp, cmd[3]);
>  }
>  
>  static void clear_sel(IPMIBmcSim *ibs,
>                        uint8_t *cmd, unsigned int cmd_len,
> -                      uint8_t *rsp, unsigned int *rsp_len,
> -                      unsigned int max_rsp_len)
> +                      struct rsp_buffer *rsp)
>  {
>      IPMI_CHECK_RESERVATION(2, ibs->sel.reservation);
>      if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>      if (cmd[7] == 0xaa) {
>          ibs->sel.next_free = 0;
>          ibs->sel.overflow = 0;
>          set_timestamp(ibs, ibs->sdr.last_clear);
> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>          sel_inc_reservation(&ibs->sel);
>      } else if (cmd[7] == 0) {
> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>      } else {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>  }
>  
>  static void get_sel_time(IPMIBmcSim *ibs,
>                           uint8_t *cmd, unsigned int cmd_len,
> -                         uint8_t *rsp, unsigned int *rsp_len,
> -                         unsigned int max_rsp_len)
> +                         struct rsp_buffer *rsp)
>  {
>      uint32_t val;
>      struct ipmi_time now;
>  
>      ipmi_gettime(&now);
>      val = now.tv_sec + ibs->sel.time_offset;
> -    IPMI_ADD_RSP_DATA(val & 0xff);
> -    IPMI_ADD_RSP_DATA((val >> 8) & 0xff);
> -    IPMI_ADD_RSP_DATA((val >> 16) & 0xff);
> -    IPMI_ADD_RSP_DATA((val >> 24) & 0xff);
> +    rsp_buffer_push(rsp, val & 0xff);
> +    rsp_buffer_push(rsp, (val >> 8) & 0xff);
> +    rsp_buffer_push(rsp, (val >> 16) & 0xff);
> +    rsp_buffer_push(rsp, (val >> 24) & 0xff);
>  }
>  
>  static void set_sel_time(IPMIBmcSim *ibs,
>                           uint8_t *cmd, unsigned int cmd_len,
> -                         uint8_t *rsp, unsigned int *rsp_len,
> -                         unsigned int max_rsp_len)
> +                         struct rsp_buffer *rsp)
>  {
>      uint32_t val;
>      struct ipmi_time now;
> @@ -1436,14 +1419,13 @@ static void set_sel_time(IPMIBmcSim *ibs,
>  
>  static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>                                    uint8_t *cmd, unsigned int cmd_len,
> -                                  uint8_t *rsp, unsigned int *rsp_len,
> -                                  unsigned int max_rsp_len)
> +                                  struct rsp_buffer *rsp)
>  {
>      IPMISensor *sens;
>  
>      if ((cmd[2] >= MAX_SENSORS) ||
>              !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>      sens = ibs->sensors + cmd[2];
> @@ -1479,7 +1461,7 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>          }
>          break;
>      case 3:
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>          return;
>      }
>      IPMI_SENSOR_SET_RET_STATUS(sens, cmd[3]);
> @@ -1487,34 +1469,32 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>  
>  static void get_sensor_evt_enable(IPMIBmcSim *ibs,
>                                    uint8_t *cmd, unsigned int cmd_len,
> -                                  uint8_t *rsp, unsigned int *rsp_len,
> -                                  unsigned int max_rsp_len)
> +                                  struct rsp_buffer *rsp)
>  {
>      IPMISensor *sens;
>  
>      if ((cmd[2] >= MAX_SENSORS) ||
>          !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>      sens = ibs->sensors + cmd[2];
> -    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
> -    IPMI_ADD_RSP_DATA(sens->assert_enable & 0xff);
> -    IPMI_ADD_RSP_DATA((sens->assert_enable >> 8) & 0xff);
> -    IPMI_ADD_RSP_DATA(sens->deassert_enable & 0xff);
> -    IPMI_ADD_RSP_DATA((sens->deassert_enable >> 8) & 0xff);
> +    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
> +    rsp_buffer_push(rsp, sens->assert_enable & 0xff);
> +    rsp_buffer_push(rsp, (sens->assert_enable >> 8) & 0xff);
> +    rsp_buffer_push(rsp, sens->deassert_enable & 0xff);
> +    rsp_buffer_push(rsp, (sens->deassert_enable >> 8) & 0xff);
>  }
>  
>  static void rearm_sensor_evts(IPMIBmcSim *ibs,
>                                uint8_t *cmd, unsigned int cmd_len,
> -                              uint8_t *rsp, unsigned int *rsp_len,
> -                              unsigned int max_rsp_len)
> +                              struct rsp_buffer *rsp)
>  {
>      IPMISensor *sens;
>  
>      if ((cmd[2] >= MAX_SENSORS) ||
>          !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>      sens = ibs->sensors + cmd[2];
> @@ -1528,57 +1508,54 @@ static void rearm_sensor_evts(IPMIBmcSim *ibs,
>  
>  static void get_sensor_evt_status(IPMIBmcSim *ibs,
>                                    uint8_t *cmd, unsigned int cmd_len,
> -                                  uint8_t *rsp, unsigned int *rsp_len,
> -                                  unsigned int max_rsp_len)
> +                                  struct rsp_buffer *rsp)
>  {
>      IPMISensor *sens;
>  
>      if ((cmd[2] >= MAX_SENSORS) ||
>          !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>      sens = ibs->sensors + cmd[2];
> -    IPMI_ADD_RSP_DATA(sens->reading);
> -    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
> -    IPMI_ADD_RSP_DATA(sens->assert_states & 0xff);
> -    IPMI_ADD_RSP_DATA((sens->assert_states >> 8) & 0xff);
> -    IPMI_ADD_RSP_DATA(sens->deassert_states & 0xff);
> -    IPMI_ADD_RSP_DATA((sens->deassert_states >> 8) & 0xff);
> +    rsp_buffer_push(rsp, sens->reading);
> +    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
> +    rsp_buffer_push(rsp, sens->assert_states & 0xff);
> +    rsp_buffer_push(rsp, (sens->assert_states >> 8) & 0xff);
> +    rsp_buffer_push(rsp, sens->deassert_states & 0xff);
> +    rsp_buffer_push(rsp, (sens->deassert_states >> 8) & 0xff);
>  }
>  
>  static void get_sensor_reading(IPMIBmcSim *ibs,
>                                 uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                               struct rsp_buffer *rsp)
>  {
>      IPMISensor *sens;
>  
>      if ((cmd[2] >= MAX_SENSORS) ||
>              !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>      sens = ibs->sensors + cmd[2];
> -    IPMI_ADD_RSP_DATA(sens->reading);
> -    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
> -    IPMI_ADD_RSP_DATA(sens->states & 0xff);
> +    rsp_buffer_push(rsp, sens->reading);
> +    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
> +    rsp_buffer_push(rsp, sens->states & 0xff);
>      if (IPMI_SENSOR_IS_DISCRETE(sens)) {
> -        IPMI_ADD_RSP_DATA((sens->states >> 8) & 0xff);
> +        rsp_buffer_push(rsp, (sens->states >> 8) & 0xff);
>      }
>  }
>  
>  static void set_sensor_type(IPMIBmcSim *ibs,
> -                               uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                            uint8_t *cmd, unsigned int cmd_len,
> +                            struct rsp_buffer *rsp)
>  {
>      IPMISensor *sens;
>  
>  
>      if ((cmd[2] >= MAX_SENSORS) ||
>              !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>      sens = ibs->sensors + cmd[2];
> @@ -1587,21 +1564,20 @@ static void set_sensor_type(IPMIBmcSim *ibs,
>  }
>  
>  static void get_sensor_type(IPMIBmcSim *ibs,
> -                               uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                            uint8_t *cmd, unsigned int cmd_len,
> +                            struct rsp_buffer *rsp)
>  {
>      IPMISensor *sens;
>  
>  
>      if ((cmd[2] >= MAX_SENSORS) ||
>              !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>          return;
>      }
>      sens = ibs->sensors + cmd[2];
> -    IPMI_ADD_RSP_DATA(sens->sensor_type);
> -    IPMI_ADD_RSP_DATA(sens->evt_reading_type_code);
> +    rsp_buffer_push(rsp, sens->sensor_type);
> +    rsp_buffer_push(rsp, sens->evt_reading_type_code);
>  }
>  
>  
> -- 
> 2.1.4
Cédric Le Goater March 2, 2016, 5:53 p.m. UTC | #2
On 03/02/2016 11:25 AM, Michael S. Tsirkin wrote:
> On Wed, Mar 02, 2016 at 11:14:50AM +0100, Cédric Le Goater wrote:
>> The IPMI command handlers in the BMC simulator use a macro
>> IPMI_ADD_RSP_DATA() to push bytes in a response buffer. The macro
>> hides the fact that it implicitly uses variables local to the handler,
>> which is misleading.
>>
>> This patch introduces a simple 'struct rsp_buffer' and inlined helper
>> routines to store byte(s) in a response buffer. rsp_buffer_push()
>> replaces the macro IPMI_ADD_RSP_DATA() and rsp_buffer_pushmore() is
>> new helper to push multiple bytes. The latest is used in the command
>> handlers get_msg() and get_sdr() which are manipulating the buffer
>> directly.
>>
>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> 
> 
> Looks good overall.
> Some stylistic comments.
> 
>> ---
>>  hw/ipmi/ipmi_bmc_sim.c | 482 +++++++++++++++++++++++--------------------------
>>  1 file changed, 229 insertions(+), 253 deletions(-)
>>
>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
>> index 30b9fb48ea2d..32efb87c8232 100644
>> --- a/hw/ipmi/ipmi_bmc_sim.c
>> +++ b/hw/ipmi/ipmi_bmc_sim.c
>> @@ -153,14 +153,14 @@ typedef struct IPMISensor {
>>  #define IPMI_WATCHDOG_SENSOR 0
>>  
>>  typedef struct IPMIBmcSim IPMIBmcSim;
>> +struct rsp_buffer;
>>
> 
> typedef struct RspBuffer RspBuffer;

OK. So that's the rule for structs in qemu. It is not that clear
when you look at the code around. I will change. np.

>>  #define MAX_NETFNS 64
>>  
>>  typedef struct IPMICmdHandler {
>>      void (*cmd_handler)(IPMIBmcSim *s,
>>                          uint8_t *cmd, unsigned int cmd_len,
>> -                        uint8_t *rsp, unsigned int *rsp_len,
>> -                        unsigned int max_rsp_len);
>> +                        struct rsp_buffer *rsp);
>>      unsigned int cmd_len_min;
>>  } IPMICmdHandler;
>>  
>> @@ -263,22 +263,41 @@ struct IPMIBmcSim {
>>  #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN      2
>>  #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE     3
>>  
>> +struct rsp_buffer {
>> +    uint8_t buffer[MAX_IPMI_MSG_SIZE];
>> +    unsigned int len;
>> +    unsigned int max_len;
>> +};
>> +
>> +#define RSP_BUFFER_INITIALIZER { { 0 }, 0, MAX_IPMI_MSG_SIZE }
> 
> max_len seems to always be a constant. So just use ARRAY_SIZE(buffer)
> and RSP_BUFFER_INITIALIZER can then be replaced with plain { };

Yes. or : 

#define RSP_BUFFER_INITIALIZER \
	{ { 0 }, 0, ARRAY_SIZE(((struct rsp_buffer *)NULL)->buffer) }

This is purely stylistic. I agree that specifying MAX_IPMI_MSG_SIZE
twice is ugly. It needs a fix.

>>  
>>  /* 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;       \
>> -            return;                                        \
>> -        }                                                  \
>> -        rsp[(*rsp_len)++] = (b);                           \
>> -    } while (0)
>> +static inline void rsp_buffer_push(struct rsp_buffer *rsp, uint8_t byte)
>> +{
>> +    if (rsp->len >= rsp->max_len) {
> 
> Why not simply
> 	rsp->len >= ARRAY_SIZE(rsp->buffer)
> ?
> 
> 
> 
>> +        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>> +        return;
>> +    }
>> +    rsp->buffer[rsp->len++] = byte;
>> +}
>> +
>> +static inline void rsp_buffer_pushmore(struct rsp_buffer *rsp, uint8_t *bytes,
>> +                                       unsigned int n)
>> +{
>> +    if (rsp->len + n >= rsp->max_len) {
>> +        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>> +        return;
>> +    }
>> +
>> +    memcpy(&rsp->buffer[rsp->len], bytes, n);
>> +    rsp->len += n;
>> +}
>>  
>>  /* 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;          \
>> +            rsp->buffer[2] = IPMI_CC_INVALID_RESERVATION;     \
>>              return;                                        \
>>          }                                                  \
>>      } while (0)
>> @@ -585,35 +604,32 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>>      IPMIInterface *s = ibs->parent.intf;
>>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>>      unsigned int netfn;
>> -    uint8_t rsp[MAX_IPMI_MSG_SIZE];
>> -    unsigned int rsp_len_holder = 0;
>> -    unsigned int *rsp_len = &rsp_len_holder;
>> -    unsigned int max_rsp_len = sizeof(rsp);
>> +    struct rsp_buffer rsp = RSP_BUFFER_INITIALIZER;
>>  
>>      /* Set up the response, set the low bit of NETFN. */
>>      /* Note that max_rsp_len must be at least 3 */
>> -    if (max_rsp_len < 3) {
>> -        rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>> +    if (rsp.max_len < 3) {
>> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>>          goto out;
>>      }
>>  
>> -    IPMI_ADD_RSP_DATA(cmd[0] | 0x04);
>> -    IPMI_ADD_RSP_DATA(cmd[1]);
>> -    IPMI_ADD_RSP_DATA(0); /* Assume success */
>> +    rsp_buffer_push(&rsp, cmd[0] | 0x04);
>> +    rsp_buffer_push(&rsp, cmd[1]);
>> +    rsp_buffer_push(&rsp, 0); /* Assume success */
>>  
>>      /* If it's too short or it was truncated, return an error. */
>>      if (cmd_len < 2) {
>> -        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>>          goto out;
>>      }
>>      if (cmd_len > max_cmd_len) {
>> -        rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>>          goto out;
>>      }
>>  
>>      if ((cmd[0] & 0x03) != 0) {
>>          /* Only have stuff on LUN 0 */
>> -        rsp[2] = IPMI_CC_COMMAND_INVALID_FOR_LUN;
>> +        rsp.buffer[2] = IPMI_CC_COMMAND_INVALID_FOR_LUN;
>>          goto out;
>>      }
>>  
>> @@ -623,20 +639,20 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>>      if ((netfn & 1) || !ibs->netfns[netfn / 2] ||
>>          (cmd[1] >= ibs->netfns[netfn / 2]->cmd_nums) ||
>>          (!ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler)) {
>> -        rsp[2] = IPMI_CC_INVALID_CMD;
>> +        rsp.buffer[2] = IPMI_CC_INVALID_CMD;
>>          goto out;
>>      }
>>  
>>      if (cmd_len < ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_len_min) {
>> -        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>>          goto out;
>>      }
>>  
>>      ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler(ibs, cmd, cmd_len,
>> -                                                rsp, rsp_len, max_rsp_len);
>> +                                                &rsp);
>>  
>>   out:
>> -    k->handle_rsp(s, msg_id, rsp, *rsp_len);
>> +    k->handle_rsp(s, msg_id, rsp.buffer, rsp.len);
>>  
>>      next_timeout(ibs);
>>  }
>> @@ -711,86 +727,82 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
>>  
>>  static void chassis_capabilities(IPMIBmcSim *ibs,
>>                                   uint8_t *cmd, unsigned int cmd_len,
>> -                                 uint8_t *rsp, unsigned int *rsp_len,
>> -                                 unsigned int max_rsp_len)
>> +                                 struct rsp_buffer *rsp)
>>  {
>> -    IPMI_ADD_RSP_DATA(0);
>> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
>> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
>> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
>> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
>> +    rsp_buffer_push(rsp, 0);
>> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
>> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
>> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
>> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
>>  }
>>  
>>  static void chassis_status(IPMIBmcSim *ibs,
>>                             uint8_t *cmd, unsigned int cmd_len,
>> -                           uint8_t *rsp, unsigned int *rsp_len,
>> -                           unsigned int max_rsp_len)
>> +                           struct rsp_buffer *rsp)
>>  {
>> -    IPMI_ADD_RSP_DATA(0x61); /* Unknown power restore, power is on */
>> -    IPMI_ADD_RSP_DATA(0);
>> -    IPMI_ADD_RSP_DATA(0);
>> -    IPMI_ADD_RSP_DATA(0);
>> +    rsp_buffer_push(rsp, 0x61); /* Unknown power restore, power is on */
>> +    rsp_buffer_push(rsp, 0);
>> +    rsp_buffer_push(rsp, 0);
>> +    rsp_buffer_push(rsp, 0);
>>  }
>>  
>>  static void chassis_control(IPMIBmcSim *ibs,
>>                              uint8_t *cmd, unsigned int cmd_len,
>> -                            uint8_t *rsp, unsigned int *rsp_len,
>> -                            unsigned int max_rsp_len)
>> +                            struct rsp_buffer *rsp)
>>  {
>>      IPMIInterface *s = ibs->parent.intf;
>>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>>  
>>      switch (cmd[2] & 0xf) {
>>      case 0: /* power down */
>> -        rsp[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
>> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
>>          break;
>>      case 1: /* power up */
>> -        rsp[2] = k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0);
>> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0);
>>          break;
>>      case 2: /* power cycle */
>> -        rsp[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
>> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
>>          break;
>>      case 3: /* hard reset */
>> -        rsp[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
>> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
>>          break;
>>      case 4: /* pulse diagnostic interrupt */
>> -        rsp[2] = k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0);
>> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0);
>>          break;
>>      case 5: /* soft shutdown via ACPI by overtemp emulation */
>> -        rsp[2] = k->do_hw_op(s,
>> +        rsp->buffer[2] = k->do_hw_op(s,
>>                               IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 0);
>>          break;
>>      default:
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>  }
>>  
>>  static void chassis_get_sys_restart_cause(IPMIBmcSim *ibs,
>>                             uint8_t *cmd, unsigned int cmd_len,
>> -                           uint8_t *rsp, unsigned int *rsp_len,
>> -                           unsigned int max_rsp_len)
>> +                           struct rsp_buffer *rsp)
>> +
>>  {
>> -    IPMI_ADD_RSP_DATA(ibs->restart_cause & 0xf); /* Restart Cause */
>> -    IPMI_ADD_RSP_DATA(0);  /* Channel 0 */
>> +    rsp_buffer_push(rsp, ibs->restart_cause & 0xf); /* Restart Cause */
>> +    rsp_buffer_push(rsp, 0);  /* Channel 0 */
>>  }
>>  
>>  static void get_device_id(IPMIBmcSim *ibs,
>>                            uint8_t *cmd, unsigned int cmd_len,
>> -                          uint8_t *rsp, unsigned int *rsp_len,
>> -                          unsigned int max_rsp_len)
>> -{
>> -    IPMI_ADD_RSP_DATA(ibs->device_id);
>> -    IPMI_ADD_RSP_DATA(ibs->device_rev & 0xf);
>> -    IPMI_ADD_RSP_DATA(ibs->fwrev1 & 0x7f);
>> -    IPMI_ADD_RSP_DATA(ibs->fwrev2);
>> -    IPMI_ADD_RSP_DATA(ibs->ipmi_version);
>> -    IPMI_ADD_RSP_DATA(0x07); /* sensor, SDR, and SEL. */
>> -    IPMI_ADD_RSP_DATA(ibs->mfg_id[0]);
>> -    IPMI_ADD_RSP_DATA(ibs->mfg_id[1]);
>> -    IPMI_ADD_RSP_DATA(ibs->mfg_id[2]);
>> -    IPMI_ADD_RSP_DATA(ibs->product_id[0]);
>> -    IPMI_ADD_RSP_DATA(ibs->product_id[1]);
>> +                          struct rsp_buffer *rsp)
>> +{
>> +    rsp_buffer_push(rsp, ibs->device_id);
>> +    rsp_buffer_push(rsp, ibs->device_rev & 0xf);
>> +    rsp_buffer_push(rsp, ibs->fwrev1 & 0x7f);
>> +    rsp_buffer_push(rsp, ibs->fwrev2);
>> +    rsp_buffer_push(rsp, ibs->ipmi_version);
>> +    rsp_buffer_push(rsp, 0x07); /* sensor, SDR, and SEL. */
>> +    rsp_buffer_push(rsp, ibs->mfg_id[0]);
>> +    rsp_buffer_push(rsp, ibs->mfg_id[1]);
>> +    rsp_buffer_push(rsp, ibs->mfg_id[2]);
>> +    rsp_buffer_push(rsp, ibs->product_id[0]);
>> +    rsp_buffer_push(rsp, ibs->product_id[1]);
>>  }
>>  
>>  static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
>> @@ -809,8 +821,7 @@ static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
>>  
>>  static void cold_reset(IPMIBmcSim *ibs,
>>                         uint8_t *cmd, unsigned int cmd_len,
>> -                       uint8_t *rsp, unsigned int *rsp_len,
>> -                       unsigned int max_rsp_len)
>> +                       struct rsp_buffer *rsp)
>>  {
>>      IPMIInterface *s = ibs->parent.intf;
>>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>> @@ -825,8 +836,7 @@ static void cold_reset(IPMIBmcSim *ibs,
>>  
>>  static void warm_reset(IPMIBmcSim *ibs,
>>                         uint8_t *cmd, unsigned int cmd_len,
>> -                       uint8_t *rsp, unsigned int *rsp_len,
>> -                       unsigned int max_rsp_len)
>> +                       struct rsp_buffer *rsp)
>>  {
>>      IPMIInterface *s = ibs->parent.intf;
>>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>> @@ -836,55 +846,49 @@ static void warm_reset(IPMIBmcSim *ibs,
>>      }
>>  }
>>  static void set_acpi_power_state(IPMIBmcSim *ibs,
>> -                          uint8_t *cmd, unsigned int cmd_len,
>> -                          uint8_t *rsp, unsigned int *rsp_len,
>> -                          unsigned int max_rsp_len)
>> +                                 uint8_t *cmd, unsigned int cmd_len,
>> +                                 struct rsp_buffer *rsp)
>>  {
>>      ibs->acpi_power_state[0] = cmd[2];
>>      ibs->acpi_power_state[1] = cmd[3];
>>  }
>>  
>>  static void get_acpi_power_state(IPMIBmcSim *ibs,
>> -                          uint8_t *cmd, unsigned int cmd_len,
>> -                          uint8_t *rsp, unsigned int *rsp_len,
>> -                          unsigned int max_rsp_len)
>> +                                 uint8_t *cmd, unsigned int cmd_len,
>> +                                 struct rsp_buffer *rsp)
>>  {
>> -    IPMI_ADD_RSP_DATA(ibs->acpi_power_state[0]);
>> -    IPMI_ADD_RSP_DATA(ibs->acpi_power_state[1]);
>> +    rsp_buffer_push(rsp, ibs->acpi_power_state[0]);
>> +    rsp_buffer_push(rsp, ibs->acpi_power_state[1]);
>>  }
>>  
>>  static void get_device_guid(IPMIBmcSim *ibs,
>> -                          uint8_t *cmd, unsigned int cmd_len,
>> -                          uint8_t *rsp, unsigned int *rsp_len,
>> -                          unsigned int max_rsp_len)
>> +                            uint8_t *cmd, unsigned int cmd_len,
>> +                            struct rsp_buffer *rsp)
>>  {
>>      unsigned int i;
>>  
>>      for (i = 0; i < 16; i++) {
>> -        IPMI_ADD_RSP_DATA(ibs->uuid[i]);
>> +        rsp_buffer_push(rsp, ibs->uuid[i]);
>>      }
>>  }
>>  
>>  static void set_bmc_global_enables(IPMIBmcSim *ibs,
>>                                     uint8_t *cmd, unsigned int cmd_len,
>> -                                   uint8_t *rsp, unsigned int *rsp_len,
>> -                                   unsigned int max_rsp_len)
>> +                                   struct rsp_buffer *rsp)
>>  {
>>      set_global_enables(ibs, cmd[2]);
>>  }
>>  
>>  static void get_bmc_global_enables(IPMIBmcSim *ibs,
>>                                     uint8_t *cmd, unsigned int cmd_len,
>> -                                   uint8_t *rsp, unsigned int *rsp_len,
>> -                                   unsigned int max_rsp_len)
>> +                                   struct rsp_buffer *rsp)
>>  {
>> -    IPMI_ADD_RSP_DATA(ibs->bmc_global_enables);
>> +    rsp_buffer_push(rsp, ibs->bmc_global_enables);
>>  }
>>  
>>  static void clr_msg_flags(IPMIBmcSim *ibs,
>>                            uint8_t *cmd, unsigned int cmd_len,
>> -                          uint8_t *rsp, unsigned int *rsp_len,
>> -                          unsigned int max_rsp_len)
>> +                          struct rsp_buffer *rsp)
>>  {
>>      IPMIInterface *s = ibs->parent.intf;
>>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>> @@ -895,27 +899,25 @@ static void clr_msg_flags(IPMIBmcSim *ibs,
>>  
>>  static void get_msg_flags(IPMIBmcSim *ibs,
>>                            uint8_t *cmd, unsigned int cmd_len,
>> -                          uint8_t *rsp, unsigned int *rsp_len,
>> -                          unsigned int max_rsp_len)
>> +                          struct rsp_buffer *rsp)
>>  {
>> -    IPMI_ADD_RSP_DATA(ibs->msg_flags);
>> +    rsp_buffer_push(rsp, ibs->msg_flags);
>>  }
>>  
>>  static void read_evt_msg_buf(IPMIBmcSim *ibs,
>>                               uint8_t *cmd, unsigned int cmd_len,
>> -                             uint8_t *rsp, unsigned int *rsp_len,
>> -                            unsigned int max_rsp_len)
>> +                             struct rsp_buffer *rsp)
>>  {
>>      IPMIInterface *s = ibs->parent.intf;
>>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>>      unsigned int i;
>>  
>>      if (!(ibs->msg_flags & IPMI_BMC_MSG_FLAG_EVT_BUF_FULL)) {
>> -        rsp[2] = 0x80;
>> +        rsp->buffer[2] = 0x80;
>>          return;
>>      }
>>      for (i = 0; i < 16; i++) {
>> -        IPMI_ADD_RSP_DATA(ibs->evtbuf[i]);
>> +        rsp_buffer_push(rsp, ibs->evtbuf[i]);
>>      }
>>      ibs->msg_flags &= ~IPMI_BMC_MSG_FLAG_EVT_BUF_FULL;
>>      k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs));
>> @@ -923,21 +925,18 @@ static void read_evt_msg_buf(IPMIBmcSim *ibs,
>>  
>>  static void get_msg(IPMIBmcSim *ibs,
>>                      uint8_t *cmd, unsigned int cmd_len,
>> -                    uint8_t *rsp, unsigned int *rsp_len,
>> -                    unsigned int max_rsp_len)
>> +                    struct rsp_buffer *rsp)
>>  {
>>      IPMIRcvBufEntry *msg;
>>  
>>      qemu_mutex_lock(&ibs->lock);
>>      if (QTAILQ_EMPTY(&ibs->rcvbufs)) {
>> -        rsp[2] = 0x80; /* Queue empty */
>> +        rsp->buffer[2] = 0x80; /* Queue empty */
>>          goto out;
>>      }
>> -    rsp[3] = 0; /* Channel 0 */
>> -    *rsp_len += 1;
>> +    rsp_buffer_push(rsp, 0); /* Channel 0 */
>>      msg = QTAILQ_FIRST(&ibs->rcvbufs);
>> -    memcpy(rsp + 4, msg->buf, msg->len);
>> -    *rsp_len += msg->len;
>> +    rsp_buffer_pushmore(rsp, msg->buf, msg->len);
>>      QTAILQ_REMOVE(&ibs->rcvbufs, msg, entry);
>>      g_free(msg);
>>  
>> @@ -966,8 +965,7 @@ ipmb_checksum(unsigned char *data, int size, unsigned char csum)
>>  
>>  static void send_msg(IPMIBmcSim *ibs,
>>                       uint8_t *cmd, unsigned int cmd_len,
>> -                     uint8_t *rsp, unsigned int *rsp_len,
>> -                     unsigned int max_rsp_len)
>> +                     struct rsp_buffer *rsp)
>>  {
>>      IPMIInterface *s = ibs->parent.intf;
>>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>> @@ -977,18 +975,18 @@ static void send_msg(IPMIBmcSim *ibs,
>>  
>>      if (cmd[2] != 0) {
>>          /* We only handle channel 0 with no options */
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>  
>>      if (cmd_len < 10) {
>> -        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>> +        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>>          return;
>>      }
>>  
>>      if (cmd[3] != 0x40) {
>>          /* We only emulate a MC at address 0x40. */
>> -        rsp[2] = 0x83; /* NAK on write */
>> +        rsp->buffer[2] = 0x83; /* NAK on write */
>>          return;
>>      }
>>  
>> @@ -1074,11 +1072,10 @@ static void do_watchdog_reset(IPMIBmcSim *ibs)
>>  
>>  static void reset_watchdog_timer(IPMIBmcSim *ibs,
>>                                   uint8_t *cmd, unsigned int cmd_len,
>> -                                 uint8_t *rsp, unsigned int *rsp_len,
>> -                                 unsigned int max_rsp_len)
>> +                                 struct rsp_buffer *rsp)
>>  {
>>      if (!ibs->watchdog_initialized) {
>> -        rsp[2] = 0x80;
>> +        rsp->buffer[2] = 0x80;
>>          return;
>>      }
>>      do_watchdog_reset(ibs);
>> @@ -1086,8 +1083,7 @@ static void reset_watchdog_timer(IPMIBmcSim *ibs,
>>  
>>  static void set_watchdog_timer(IPMIBmcSim *ibs,
>>                                 uint8_t *cmd, unsigned int cmd_len,
>> -                               uint8_t *rsp, unsigned int *rsp_len,
>> -                               unsigned int max_rsp_len)
>> +                               struct rsp_buffer *rsp)
>>  {
>>      IPMIInterface *s = ibs->parent.intf;
>>      IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>> @@ -1095,7 +1091,7 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>>  
>>      val = cmd[2] & 0x7; /* Validate use */
>>      if (val == 0 || val > 5) {
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>      val = cmd[3] & 0x7; /* Validate action */
>> @@ -1104,22 +1100,22 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>>          break;
>>  
>>      case IPMI_BMC_WATCHDOG_ACTION_RESET:
>> -        rsp[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 1);
>> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 1);
>>          break;
>>  
>>      case IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN:
>> -        rsp[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1);
>> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1);
>>          break;
>>  
>>      case IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE:
>> -        rsp[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1);
>> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1);
>>          break;
>>  
>>      default:
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>      }
>> -    if (rsp[2]) {
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +    if (rsp->buffer[2]) {
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>  
>> @@ -1132,14 +1128,14 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>>      case IPMI_BMC_WATCHDOG_PRE_NMI:
>>          if (!k->do_hw_op(s, IPMI_SEND_NMI, 1)) {
>>              /* NMI not supported. */
>> -            rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +            rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>              return;
>>          }
>>          break;
>>  
>>      default:
>>          /* We don't support PRE_SMI */
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>  
>> @@ -1158,60 +1154,56 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>>  
>>  static void get_watchdog_timer(IPMIBmcSim *ibs,
>>                                 uint8_t *cmd, unsigned int cmd_len,
>> -                               uint8_t *rsp, unsigned int *rsp_len,
>> -                               unsigned int max_rsp_len)
>> +                               struct rsp_buffer *rsp)
>>  {
>> -    IPMI_ADD_RSP_DATA(ibs->watchdog_use);
>> -    IPMI_ADD_RSP_DATA(ibs->watchdog_action);
>> -    IPMI_ADD_RSP_DATA(ibs->watchdog_pretimeout);
>> -    IPMI_ADD_RSP_DATA(ibs->watchdog_expired);
>> +    rsp_buffer_push(rsp, ibs->watchdog_use);
>> +    rsp_buffer_push(rsp, ibs->watchdog_action);
>> +    rsp_buffer_push(rsp, ibs->watchdog_pretimeout);
>> +    rsp_buffer_push(rsp, ibs->watchdog_expired);
>>      if (ibs->watchdog_running) {
>>          long timeout;
>>          timeout = ((ibs->watchdog_expiry - ipmi_getmonotime() + 50000000)
>>                     / 100000000);
>> -        IPMI_ADD_RSP_DATA(timeout & 0xff);
>> -        IPMI_ADD_RSP_DATA((timeout >> 8) & 0xff);
>> +        rsp_buffer_push(rsp, timeout & 0xff);
>> +        rsp_buffer_push(rsp, (timeout >> 8) & 0xff);
>>      } else {
>> -        IPMI_ADD_RSP_DATA(0);
>> -        IPMI_ADD_RSP_DATA(0);
>> +        rsp_buffer_push(rsp, 0);
>> +        rsp_buffer_push(rsp, 0);
>>      }
>>  }
>>  
>>  static void get_sdr_rep_info(IPMIBmcSim *ibs,
>>                               uint8_t *cmd, unsigned int cmd_len,
>> -                             uint8_t *rsp, unsigned int *rsp_len,
>> -                             unsigned int max_rsp_len)
>> +                             struct rsp_buffer *rsp)
>>  {
>>      unsigned int i;
>>  
>> -    IPMI_ADD_RSP_DATA(0x51); /* Conform to IPMI 1.5 spec */
>> -    IPMI_ADD_RSP_DATA(ibs->sdr.next_rec_id & 0xff);
>> -    IPMI_ADD_RSP_DATA((ibs->sdr.next_rec_id >> 8) & 0xff);
>> -    IPMI_ADD_RSP_DATA((MAX_SDR_SIZE - ibs->sdr.next_free) & 0xff);
>> -    IPMI_ADD_RSP_DATA(((MAX_SDR_SIZE - ibs->sdr.next_free) >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, 0x51); /* Conform to IPMI 1.5 spec */
>> +    rsp_buffer_push(rsp, ibs->sdr.next_rec_id & 0xff);
>> +    rsp_buffer_push(rsp, (ibs->sdr.next_rec_id >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, (MAX_SDR_SIZE - ibs->sdr.next_free) & 0xff);
>> +    rsp_buffer_push(rsp, ((MAX_SDR_SIZE - ibs->sdr.next_free) >> 8) & 0xff);
>>      for (i = 0; i < 4; i++) {
>> -        IPMI_ADD_RSP_DATA(ibs->sdr.last_addition[i]);
>> +        rsp_buffer_push(rsp, ibs->sdr.last_addition[i]);
>>      }
>>      for (i = 0; i < 4; i++) {
>> -        IPMI_ADD_RSP_DATA(ibs->sdr.last_clear[i]);
>> +        rsp_buffer_push(rsp, ibs->sdr.last_clear[i]);
>>      }
>>      /* Only modal support, reserve supported */
>> -    IPMI_ADD_RSP_DATA((ibs->sdr.overflow << 7) | 0x22);
>> +    rsp_buffer_push(rsp, (ibs->sdr.overflow << 7) | 0x22);
>>  }
>>  
>>  static void reserve_sdr_rep(IPMIBmcSim *ibs,
>>                              uint8_t *cmd, unsigned int cmd_len,
>> -                            uint8_t *rsp, unsigned int *rsp_len,
>> -                            unsigned int max_rsp_len)
>> +                            struct rsp_buffer *rsp)
>>  {
>> -    IPMI_ADD_RSP_DATA(ibs->sdr.reservation & 0xff);
>> -    IPMI_ADD_RSP_DATA((ibs->sdr.reservation >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, ibs->sdr.reservation & 0xff);
>> +    rsp_buffer_push(rsp, (ibs->sdr.reservation >> 8) & 0xff);
>>  }
>>  
>>  static void get_sdr(IPMIBmcSim *ibs,
>>                      uint8_t *cmd, unsigned int cmd_len,
>> -                    uint8_t *rsp, unsigned int *rsp_len,
>> -                    unsigned int max_rsp_len)
>> +                    struct rsp_buffer *rsp)
>>  {
>>      unsigned int pos;
>>      uint16_t nextrec;
>> @@ -1223,108 +1215,103 @@ static void get_sdr(IPMIBmcSim *ibs,
>>      pos = 0;
>>      if (sdr_find_entry(&ibs->sdr, cmd[4] | (cmd[5] << 8),
>>                         &pos, &nextrec)) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>  
>>      sdrh = (struct ipmi_sdr_header *) &ibs->sdr.sdr[pos];
>>  
>>      if (cmd[6] > ipmi_sdr_length(sdrh)) {
>> -        rsp[2] = IPMI_CC_PARM_OUT_OF_RANGE;
>> +        rsp->buffer[2] = IPMI_CC_PARM_OUT_OF_RANGE;
>>          return;
>>      }
>>  
>> -    IPMI_ADD_RSP_DATA(nextrec & 0xff);
>> -    IPMI_ADD_RSP_DATA((nextrec >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, nextrec & 0xff);
>> +    rsp_buffer_push(rsp, (nextrec >> 8) & 0xff);
>>  
>>      if (cmd[7] == 0xff) {
>>          cmd[7] = ipmi_sdr_length(sdrh) - cmd[6];
>>      }
>>  
>> -    if ((cmd[7] + *rsp_len) > max_rsp_len) {
>> -        rsp[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
>> +    if ((cmd[7] + rsp->len) > rsp->max_len) {
>> +        rsp->buffer[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
>>          return;
>>      }
>> -    memcpy(rsp + *rsp_len, ibs->sdr.sdr + pos + cmd[6], cmd[7]);
>> -    *rsp_len += cmd[7];
>> +
>> +    rsp_buffer_pushmore(rsp, ibs->sdr.sdr + pos + cmd[6], cmd[7]);
>>  }
>>  
>>  static void add_sdr(IPMIBmcSim *ibs,
>>                      uint8_t *cmd, unsigned int cmd_len,
>> -                    uint8_t *rsp, unsigned int *rsp_len,
>> -                    unsigned int max_rsp_len)
>> +                    struct rsp_buffer *rsp)
>>  {
>>      uint16_t recid;
>>      struct ipmi_sdr_header *sdrh = (struct ipmi_sdr_header *) cmd + 2;
>>  
>>      if (sdr_add_entry(ibs, sdrh, cmd_len - 2, &recid)) {
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>> -    IPMI_ADD_RSP_DATA(recid & 0xff);
>> -    IPMI_ADD_RSP_DATA((recid >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, recid & 0xff);
>> +    rsp_buffer_push(rsp, (recid >> 8) & 0xff);
>>  }
>>  
>>  static void clear_sdr_rep(IPMIBmcSim *ibs,
>>                            uint8_t *cmd, unsigned int cmd_len,
>> -                          uint8_t *rsp, unsigned int *rsp_len,
>> -                          unsigned int max_rsp_len)
>> +                          struct rsp_buffer *rsp)
>>  {
>>      IPMI_CHECK_RESERVATION(2, ibs->sdr.reservation);
>>      if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>      if (cmd[7] == 0xaa) {
>>          ibs->sdr.next_free = 0;
>>          ibs->sdr.overflow = 0;
>>          set_timestamp(ibs, ibs->sdr.last_clear);
>> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
>> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>>          sdr_inc_reservation(&ibs->sdr);
>>      } else if (cmd[7] == 0) {
>> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
>> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>>      } else {
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>  }
>>  
>>  static void get_sel_info(IPMIBmcSim *ibs,
>>                           uint8_t *cmd, unsigned int cmd_len,
>> -                         uint8_t *rsp, unsigned int *rsp_len,
>> -                         unsigned int max_rsp_len)
>> +                         struct rsp_buffer *rsp)
>>  {
>>      unsigned int i, val;
>>  
>> -    IPMI_ADD_RSP_DATA(0x51); /* Conform to IPMI 1.5 */
>> -    IPMI_ADD_RSP_DATA(ibs->sel.next_free & 0xff);
>> -    IPMI_ADD_RSP_DATA((ibs->sel.next_free >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, 0x51); /* Conform to IPMI 1.5 */
>> +    rsp_buffer_push(rsp, ibs->sel.next_free & 0xff);
>> +    rsp_buffer_push(rsp, (ibs->sel.next_free >> 8) & 0xff);
>>      val = (MAX_SEL_SIZE - ibs->sel.next_free) * 16;
>> -    IPMI_ADD_RSP_DATA(val & 0xff);
>> -    IPMI_ADD_RSP_DATA((val >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, val & 0xff);
>> +    rsp_buffer_push(rsp, (val >> 8) & 0xff);
>>      for (i = 0; i < 4; i++) {
>> -        IPMI_ADD_RSP_DATA(ibs->sel.last_addition[i]);
>> +        rsp_buffer_push(rsp, ibs->sel.last_addition[i]);
>>      }
>>      for (i = 0; i < 4; i++) {
>> -        IPMI_ADD_RSP_DATA(ibs->sel.last_clear[i]);
>> +        rsp_buffer_push(rsp, ibs->sel.last_clear[i]);
>>      }
>>      /* Only support Reserve SEL */
>> -    IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02);
>> +    rsp_buffer_push(rsp, (ibs->sel.overflow << 7) | 0x02);
>>  }
>>  
>>  static void reserve_sel(IPMIBmcSim *ibs,
>>                          uint8_t *cmd, unsigned int cmd_len,
>> -                        uint8_t *rsp, unsigned int *rsp_len,
>> -                        unsigned int max_rsp_len)
>> +                        struct rsp_buffer *rsp)
>>  {
>> -    IPMI_ADD_RSP_DATA(ibs->sel.reservation & 0xff);
>> -    IPMI_ADD_RSP_DATA((ibs->sel.reservation >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, ibs->sel.reservation & 0xff);
>> +    rsp_buffer_push(rsp, (ibs->sel.reservation >> 8) & 0xff);
>>  }
>>  
>>  static void get_sel_entry(IPMIBmcSim *ibs,
>>                            uint8_t *cmd, unsigned int cmd_len,
>> -                          uint8_t *rsp, unsigned int *rsp_len,
>> -                          unsigned int max_rsp_len)
>> +                          struct rsp_buffer *rsp)
>>  {
>>      unsigned int val;
>>  
>> @@ -1332,17 +1319,17 @@ static void get_sel_entry(IPMIBmcSim *ibs,
>>          IPMI_CHECK_RESERVATION(2, ibs->sel.reservation);
>>      }
>>      if (ibs->sel.next_free == 0) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>      if (cmd[6] > 15) {
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>      if (cmd[7] == 0xff) {
>>          cmd[7] = 16;
>>      } else if ((cmd[7] + cmd[6]) > 16) {
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      } else {
>>          cmd[7] += cmd[6];
>> @@ -1352,79 +1339,75 @@ static void get_sel_entry(IPMIBmcSim *ibs,
>>      if (val == 0xffff) {
>>          val = ibs->sel.next_free - 1;
>>      } else if (val >= ibs->sel.next_free) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>      if ((val + 1) == ibs->sel.next_free) {
>> -        IPMI_ADD_RSP_DATA(0xff);
>> -        IPMI_ADD_RSP_DATA(0xff);
>> +        rsp_buffer_push(rsp, 0xff);
>> +        rsp_buffer_push(rsp, 0xff);
>>      } else {
>> -        IPMI_ADD_RSP_DATA((val + 1) & 0xff);
>> -        IPMI_ADD_RSP_DATA(((val + 1) >> 8) & 0xff);
>> +        rsp_buffer_push(rsp, (val + 1) & 0xff);
>> +        rsp_buffer_push(rsp, ((val + 1) >> 8) & 0xff);
>>      }
>>      for (; cmd[6] < cmd[7]; cmd[6]++) {
>> -        IPMI_ADD_RSP_DATA(ibs->sel.sel[val][cmd[6]]);
>> +        rsp_buffer_push(rsp, ibs->sel.sel[val][cmd[6]]);
>>      }
>>  }
>>  
>>  static void add_sel_entry(IPMIBmcSim *ibs,
>>                            uint8_t *cmd, unsigned int cmd_len,
>> -                          uint8_t *rsp, unsigned int *rsp_len,
>> -                          unsigned int max_rsp_len)
>> +                          struct rsp_buffer *rsp)
>>  {
>>      if (sel_add_event(ibs, cmd + 2)) {
>> -        rsp[2] = IPMI_CC_OUT_OF_SPACE;
>> +        rsp->buffer[2] = IPMI_CC_OUT_OF_SPACE;
>>          return;
>>      }
>>      /* sel_add_event fills in the record number. */
>> -    IPMI_ADD_RSP_DATA(cmd[2]);
>> -    IPMI_ADD_RSP_DATA(cmd[3]);
>> +    rsp_buffer_push(rsp, cmd[2]);
>> +    rsp_buffer_push(rsp, cmd[3]);
>>  }
>>  
>>  static void clear_sel(IPMIBmcSim *ibs,
>>                        uint8_t *cmd, unsigned int cmd_len,
>> -                      uint8_t *rsp, unsigned int *rsp_len,
>> -                      unsigned int max_rsp_len)
>> +                      struct rsp_buffer *rsp)
>>  {
>>      IPMI_CHECK_RESERVATION(2, ibs->sel.reservation);
>>      if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>      if (cmd[7] == 0xaa) {
>>          ibs->sel.next_free = 0;
>>          ibs->sel.overflow = 0;
>>          set_timestamp(ibs, ibs->sdr.last_clear);
>> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
>> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>>          sel_inc_reservation(&ibs->sel);
>>      } else if (cmd[7] == 0) {
>> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
>> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>>      } else {
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>  }
>>  
>>  static void get_sel_time(IPMIBmcSim *ibs,
>>                           uint8_t *cmd, unsigned int cmd_len,
>> -                         uint8_t *rsp, unsigned int *rsp_len,
>> -                         unsigned int max_rsp_len)
>> +                         struct rsp_buffer *rsp)
>>  {
>>      uint32_t val;
>>      struct ipmi_time now;
>>  
>>      ipmi_gettime(&now);
>>      val = now.tv_sec + ibs->sel.time_offset;
>> -    IPMI_ADD_RSP_DATA(val & 0xff);
>> -    IPMI_ADD_RSP_DATA((val >> 8) & 0xff);
>> -    IPMI_ADD_RSP_DATA((val >> 16) & 0xff);
>> -    IPMI_ADD_RSP_DATA((val >> 24) & 0xff);
>> +    rsp_buffer_push(rsp, val & 0xff);
>> +    rsp_buffer_push(rsp, (val >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, (val >> 16) & 0xff);
>> +    rsp_buffer_push(rsp, (val >> 24) & 0xff);
>>  }
>>  
>>  static void set_sel_time(IPMIBmcSim *ibs,
>>                           uint8_t *cmd, unsigned int cmd_len,
>> -                         uint8_t *rsp, unsigned int *rsp_len,
>> -                         unsigned int max_rsp_len)
>> +                         struct rsp_buffer *rsp)
>>  {
>>      uint32_t val;
>>      struct ipmi_time now;
>> @@ -1436,14 +1419,13 @@ static void set_sel_time(IPMIBmcSim *ibs,
>>  
>>  static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>>                                    uint8_t *cmd, unsigned int cmd_len,
>> -                                  uint8_t *rsp, unsigned int *rsp_len,
>> -                                  unsigned int max_rsp_len)
>> +                                  struct rsp_buffer *rsp)
>>  {
>>      IPMISensor *sens;
>>  
>>      if ((cmd[2] >= MAX_SENSORS) ||
>>              !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>      sens = ibs->sensors + cmd[2];
>> @@ -1479,7 +1461,7 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>>          }
>>          break;
>>      case 3:
>> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>>          return;
>>      }
>>      IPMI_SENSOR_SET_RET_STATUS(sens, cmd[3]);
>> @@ -1487,34 +1469,32 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>>  
>>  static void get_sensor_evt_enable(IPMIBmcSim *ibs,
>>                                    uint8_t *cmd, unsigned int cmd_len,
>> -                                  uint8_t *rsp, unsigned int *rsp_len,
>> -                                  unsigned int max_rsp_len)
>> +                                  struct rsp_buffer *rsp)
>>  {
>>      IPMISensor *sens;
>>  
>>      if ((cmd[2] >= MAX_SENSORS) ||
>>          !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>      sens = ibs->sensors + cmd[2];
>> -    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
>> -    IPMI_ADD_RSP_DATA(sens->assert_enable & 0xff);
>> -    IPMI_ADD_RSP_DATA((sens->assert_enable >> 8) & 0xff);
>> -    IPMI_ADD_RSP_DATA(sens->deassert_enable & 0xff);
>> -    IPMI_ADD_RSP_DATA((sens->deassert_enable >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
>> +    rsp_buffer_push(rsp, sens->assert_enable & 0xff);
>> +    rsp_buffer_push(rsp, (sens->assert_enable >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, sens->deassert_enable & 0xff);
>> +    rsp_buffer_push(rsp, (sens->deassert_enable >> 8) & 0xff);
>>  }
>>  
>>  static void rearm_sensor_evts(IPMIBmcSim *ibs,
>>                                uint8_t *cmd, unsigned int cmd_len,
>> -                              uint8_t *rsp, unsigned int *rsp_len,
>> -                              unsigned int max_rsp_len)
>> +                              struct rsp_buffer *rsp)
>>  {
>>      IPMISensor *sens;
>>  
>>      if ((cmd[2] >= MAX_SENSORS) ||
>>          !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>      sens = ibs->sensors + cmd[2];
>> @@ -1528,57 +1508,54 @@ static void rearm_sensor_evts(IPMIBmcSim *ibs,
>>  
>>  static void get_sensor_evt_status(IPMIBmcSim *ibs,
>>                                    uint8_t *cmd, unsigned int cmd_len,
>> -                                  uint8_t *rsp, unsigned int *rsp_len,
>> -                                  unsigned int max_rsp_len)
>> +                                  struct rsp_buffer *rsp)
>>  {
>>      IPMISensor *sens;
>>  
>>      if ((cmd[2] >= MAX_SENSORS) ||
>>          !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>      sens = ibs->sensors + cmd[2];
>> -    IPMI_ADD_RSP_DATA(sens->reading);
>> -    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
>> -    IPMI_ADD_RSP_DATA(sens->assert_states & 0xff);
>> -    IPMI_ADD_RSP_DATA((sens->assert_states >> 8) & 0xff);
>> -    IPMI_ADD_RSP_DATA(sens->deassert_states & 0xff);
>> -    IPMI_ADD_RSP_DATA((sens->deassert_states >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, sens->reading);
>> +    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
>> +    rsp_buffer_push(rsp, sens->assert_states & 0xff);
>> +    rsp_buffer_push(rsp, (sens->assert_states >> 8) & 0xff);
>> +    rsp_buffer_push(rsp, sens->deassert_states & 0xff);
>> +    rsp_buffer_push(rsp, (sens->deassert_states >> 8) & 0xff);
>>  }
>>  
>>  static void get_sensor_reading(IPMIBmcSim *ibs,
>>                                 uint8_t *cmd, unsigned int cmd_len,
>> -                               uint8_t *rsp, unsigned int *rsp_len,
>> -                               unsigned int max_rsp_len)
>> +                               struct rsp_buffer *rsp)
>>  {
>>      IPMISensor *sens;
>>  
>>      if ((cmd[2] >= MAX_SENSORS) ||
>>              !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>      sens = ibs->sensors + cmd[2];
>> -    IPMI_ADD_RSP_DATA(sens->reading);
>> -    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
>> -    IPMI_ADD_RSP_DATA(sens->states & 0xff);
>> +    rsp_buffer_push(rsp, sens->reading);
>> +    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
>> +    rsp_buffer_push(rsp, sens->states & 0xff);
>>      if (IPMI_SENSOR_IS_DISCRETE(sens)) {
>> -        IPMI_ADD_RSP_DATA((sens->states >> 8) & 0xff);
>> +        rsp_buffer_push(rsp, (sens->states >> 8) & 0xff);
>>      }
>>  }
>>  
>>  static void set_sensor_type(IPMIBmcSim *ibs,
>> -                               uint8_t *cmd, unsigned int cmd_len,
>> -                               uint8_t *rsp, unsigned int *rsp_len,
>> -                               unsigned int max_rsp_len)
>> +                            uint8_t *cmd, unsigned int cmd_len,
>> +                            struct rsp_buffer *rsp)
>>  {
>>      IPMISensor *sens;
>>  
>>  
>>      if ((cmd[2] >= MAX_SENSORS) ||
>>              !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>      sens = ibs->sensors + cmd[2];
>> @@ -1587,21 +1564,20 @@ static void set_sensor_type(IPMIBmcSim *ibs,
>>  }
>>  
>>  static void get_sensor_type(IPMIBmcSim *ibs,
>> -                               uint8_t *cmd, unsigned int cmd_len,
>> -                               uint8_t *rsp, unsigned int *rsp_len,
>> -                               unsigned int max_rsp_len)
>> +                            uint8_t *cmd, unsigned int cmd_len,
>> +                            struct rsp_buffer *rsp)
>>  {
>>      IPMISensor *sens;
>>  
>>  
>>      if ((cmd[2] >= MAX_SENSORS) ||
>>              !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
>> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>>          return;
>>      }
>>      sens = ibs->sensors + cmd[2];
>> -    IPMI_ADD_RSP_DATA(sens->sensor_type);
>> -    IPMI_ADD_RSP_DATA(sens->evt_reading_type_code);
>> +    rsp_buffer_push(rsp, sens->sensor_type);
>> +    rsp_buffer_push(rsp, sens->evt_reading_type_code);
>>  }
>>  
>>  
>> -- 
>> 2.1.4
> 
>
Michael S. Tsirkin March 2, 2016, 6:02 p.m. UTC | #3
On Wed, Mar 02, 2016 at 06:53:08PM +0100, Cédric Le Goater wrote:
> > typedef struct RspBuffer RspBuffer;
> 
> OK. So that's the rule for structs in qemu. It is not that clear
> when you look at the code around. I will change. np.

Did you look at CODING_STYLE? Pls do.
Cédric Le Goater March 2, 2016, 6:08 p.m. UTC | #4
On 03/02/2016 07:02 PM, Michael S. Tsirkin wrote:
> On Wed, Mar 02, 2016 at 06:53:08PM +0100, Cédric Le Goater wrote:
>>> typedef struct RspBuffer RspBuffer;
>>
>> OK. So that's the rule for structs in qemu. It is not that clear
>> when you look at the code around. I will change. np.
> 
> Did you look at CODING_STYLE? Pls do.

This is clear. Thanks.

C.
Corey Minyard March 5, 2016, 11:41 a.m. UTC | #5
On 03/02/2016 04:14 AM, Cédric Le Goater wrote:
> The IPMI command handlers in the BMC simulator use a macro
> IPMI_ADD_RSP_DATA() to push bytes in a response buffer. The macro
> hides the fact that it implicitly uses variables local to the handler,
> which is misleading.
>
> This patch introduces a simple 'struct rsp_buffer' and inlined helper
> routines to store byte(s) in a response buffer. rsp_buffer_push()
> replaces the macro IPMI_ADD_RSP_DATA() and rsp_buffer_pushmore() is
> new helper to push multiple bytes. The latest is used in the command
> handlers get_msg() and get_sdr() which are manipulating the buffer
> directly.
>
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>

Acked-by: Corey Minyard <cminyard@mvista.com>

> ---
>   hw/ipmi/ipmi_bmc_sim.c | 482 +++++++++++++++++++++++--------------------------
>   1 file changed, 229 insertions(+), 253 deletions(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 30b9fb48ea2d..32efb87c8232 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -153,14 +153,14 @@ typedef struct IPMISensor {
>   #define IPMI_WATCHDOG_SENSOR 0
>
>   typedef struct IPMIBmcSim IPMIBmcSim;
> +struct rsp_buffer;
>
>   #define MAX_NETFNS 64
>
>   typedef struct IPMICmdHandler {
>       void (*cmd_handler)(IPMIBmcSim *s,
>                           uint8_t *cmd, unsigned int cmd_len,
> -                        uint8_t *rsp, unsigned int *rsp_len,
> -                        unsigned int max_rsp_len);
> +                        struct rsp_buffer *rsp);
>       unsigned int cmd_len_min;
>   } IPMICmdHandler;
>
> @@ -263,22 +263,41 @@ struct IPMIBmcSim {
>   #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN      2
>   #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE     3
>
> +struct rsp_buffer {
> +    uint8_t buffer[MAX_IPMI_MSG_SIZE];
> +    unsigned int len;
> +    unsigned int max_len;
> +};
> +
> +#define RSP_BUFFER_INITIALIZER { { 0 }, 0, MAX_IPMI_MSG_SIZE }
>
>   /* 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;       \
> -            return;                                        \
> -        }                                                  \
> -        rsp[(*rsp_len)++] = (b);                           \
> -    } while (0)
> +static inline void rsp_buffer_push(struct rsp_buffer *rsp, uint8_t byte)
> +{
> +    if (rsp->len >= rsp->max_len) {
> +        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +        return;
> +    }
> +    rsp->buffer[rsp->len++] = byte;
> +}
> +
> +static inline void rsp_buffer_pushmore(struct rsp_buffer *rsp, uint8_t *bytes,
> +                                       unsigned int n)
> +{
> +    if (rsp->len + n >= rsp->max_len) {
> +        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +        return;
> +    }
> +
> +    memcpy(&rsp->buffer[rsp->len], bytes, n);
> +    rsp->len += n;
> +}
>
>   /* 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;          \
> +            rsp->buffer[2] = IPMI_CC_INVALID_RESERVATION;     \
>               return;                                        \
>           }                                                  \
>       } while (0)
> @@ -585,35 +604,32 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>       IPMIInterface *s = ibs->parent.intf;
>       IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>       unsigned int netfn;
> -    uint8_t rsp[MAX_IPMI_MSG_SIZE];
> -    unsigned int rsp_len_holder = 0;
> -    unsigned int *rsp_len = &rsp_len_holder;
> -    unsigned int max_rsp_len = sizeof(rsp);
> +    struct rsp_buffer rsp = RSP_BUFFER_INITIALIZER;
>
>       /* Set up the response, set the low bit of NETFN. */
>       /* Note that max_rsp_len must be at least 3 */
> -    if (max_rsp_len < 3) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +    if (rsp.max_len < 3) {
> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>           goto out;
>       }
>
> -    IPMI_ADD_RSP_DATA(cmd[0] | 0x04);
> -    IPMI_ADD_RSP_DATA(cmd[1]);
> -    IPMI_ADD_RSP_DATA(0); /* Assume success */
> +    rsp_buffer_push(&rsp, cmd[0] | 0x04);
> +    rsp_buffer_push(&rsp, cmd[1]);
> +    rsp_buffer_push(&rsp, 0); /* Assume success */
>
>       /* If it's too short or it was truncated, return an error. */
>       if (cmd_len < 2) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>           goto out;
>       }
>       if (cmd_len > max_cmd_len) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
>           goto out;
>       }
>
>       if ((cmd[0] & 0x03) != 0) {
>           /* Only have stuff on LUN 0 */
> -        rsp[2] = IPMI_CC_COMMAND_INVALID_FOR_LUN;
> +        rsp.buffer[2] = IPMI_CC_COMMAND_INVALID_FOR_LUN;
>           goto out;
>       }
>
> @@ -623,20 +639,20 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>       if ((netfn & 1) || !ibs->netfns[netfn / 2] ||
>           (cmd[1] >= ibs->netfns[netfn / 2]->cmd_nums) ||
>           (!ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler)) {
> -        rsp[2] = IPMI_CC_INVALID_CMD;
> +        rsp.buffer[2] = IPMI_CC_INVALID_CMD;
>           goto out;
>       }
>
>       if (cmd_len < ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_len_min) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>           goto out;
>       }
>
>       ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler(ibs, cmd, cmd_len,
> -                                                rsp, rsp_len, max_rsp_len);
> +                                                &rsp);
>
>    out:
> -    k->handle_rsp(s, msg_id, rsp, *rsp_len);
> +    k->handle_rsp(s, msg_id, rsp.buffer, rsp.len);
>
>       next_timeout(ibs);
>   }
> @@ -711,86 +727,82 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
>
>   static void chassis_capabilities(IPMIBmcSim *ibs,
>                                    uint8_t *cmd, unsigned int cmd_len,
> -                                 uint8_t *rsp, unsigned int *rsp_len,
> -                                 unsigned int max_rsp_len)
> +                                 struct rsp_buffer *rsp)
>   {
> -    IPMI_ADD_RSP_DATA(0);
> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
> -    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
> +    rsp_buffer_push(rsp, 0);
> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
> +    rsp_buffer_push(rsp, ibs->parent.slave_addr);
>   }
>
>   static void chassis_status(IPMIBmcSim *ibs,
>                              uint8_t *cmd, unsigned int cmd_len,
> -                           uint8_t *rsp, unsigned int *rsp_len,
> -                           unsigned int max_rsp_len)
> +                           struct rsp_buffer *rsp)
>   {
> -    IPMI_ADD_RSP_DATA(0x61); /* Unknown power restore, power is on */
> -    IPMI_ADD_RSP_DATA(0);
> -    IPMI_ADD_RSP_DATA(0);
> -    IPMI_ADD_RSP_DATA(0);
> +    rsp_buffer_push(rsp, 0x61); /* Unknown power restore, power is on */
> +    rsp_buffer_push(rsp, 0);
> +    rsp_buffer_push(rsp, 0);
> +    rsp_buffer_push(rsp, 0);
>   }
>
>   static void chassis_control(IPMIBmcSim *ibs,
>                               uint8_t *cmd, unsigned int cmd_len,
> -                            uint8_t *rsp, unsigned int *rsp_len,
> -                            unsigned int max_rsp_len)
> +                            struct rsp_buffer *rsp)
>   {
>       IPMIInterface *s = ibs->parent.intf;
>       IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>
>       switch (cmd[2] & 0xf) {
>       case 0: /* power down */
> -        rsp[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
>           break;
>       case 1: /* power up */
> -        rsp[2] = k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0);
>           break;
>       case 2: /* power cycle */
> -        rsp[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
>           break;
>       case 3: /* hard reset */
> -        rsp[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
>           break;
>       case 4: /* pulse diagnostic interrupt */
> -        rsp[2] = k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0);
>           break;
>       case 5: /* soft shutdown via ACPI by overtemp emulation */
> -        rsp[2] = k->do_hw_op(s,
> +        rsp->buffer[2] = k->do_hw_op(s,
>                                IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 0);
>           break;
>       default:
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>   }
>
>   static void chassis_get_sys_restart_cause(IPMIBmcSim *ibs,
>                              uint8_t *cmd, unsigned int cmd_len,
> -                           uint8_t *rsp, unsigned int *rsp_len,
> -                           unsigned int max_rsp_len)
> +                           struct rsp_buffer *rsp)
> +
>   {
> -    IPMI_ADD_RSP_DATA(ibs->restart_cause & 0xf); /* Restart Cause */
> -    IPMI_ADD_RSP_DATA(0);  /* Channel 0 */
> +    rsp_buffer_push(rsp, ibs->restart_cause & 0xf); /* Restart Cause */
> +    rsp_buffer_push(rsp, 0);  /* Channel 0 */
>   }
>
>   static void get_device_id(IPMIBmcSim *ibs,
>                             uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> -{
> -    IPMI_ADD_RSP_DATA(ibs->device_id);
> -    IPMI_ADD_RSP_DATA(ibs->device_rev & 0xf);
> -    IPMI_ADD_RSP_DATA(ibs->fwrev1 & 0x7f);
> -    IPMI_ADD_RSP_DATA(ibs->fwrev2);
> -    IPMI_ADD_RSP_DATA(ibs->ipmi_version);
> -    IPMI_ADD_RSP_DATA(0x07); /* sensor, SDR, and SEL. */
> -    IPMI_ADD_RSP_DATA(ibs->mfg_id[0]);
> -    IPMI_ADD_RSP_DATA(ibs->mfg_id[1]);
> -    IPMI_ADD_RSP_DATA(ibs->mfg_id[2]);
> -    IPMI_ADD_RSP_DATA(ibs->product_id[0]);
> -    IPMI_ADD_RSP_DATA(ibs->product_id[1]);
> +                          struct rsp_buffer *rsp)
> +{
> +    rsp_buffer_push(rsp, ibs->device_id);
> +    rsp_buffer_push(rsp, ibs->device_rev & 0xf);
> +    rsp_buffer_push(rsp, ibs->fwrev1 & 0x7f);
> +    rsp_buffer_push(rsp, ibs->fwrev2);
> +    rsp_buffer_push(rsp, ibs->ipmi_version);
> +    rsp_buffer_push(rsp, 0x07); /* sensor, SDR, and SEL. */
> +    rsp_buffer_push(rsp, ibs->mfg_id[0]);
> +    rsp_buffer_push(rsp, ibs->mfg_id[1]);
> +    rsp_buffer_push(rsp, ibs->mfg_id[2]);
> +    rsp_buffer_push(rsp, ibs->product_id[0]);
> +    rsp_buffer_push(rsp, ibs->product_id[1]);
>   }
>
>   static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
> @@ -809,8 +821,7 @@ static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
>
>   static void cold_reset(IPMIBmcSim *ibs,
>                          uint8_t *cmd, unsigned int cmd_len,
> -                       uint8_t *rsp, unsigned int *rsp_len,
> -                       unsigned int max_rsp_len)
> +                       struct rsp_buffer *rsp)
>   {
>       IPMIInterface *s = ibs->parent.intf;
>       IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -825,8 +836,7 @@ static void cold_reset(IPMIBmcSim *ibs,
>
>   static void warm_reset(IPMIBmcSim *ibs,
>                          uint8_t *cmd, unsigned int cmd_len,
> -                       uint8_t *rsp, unsigned int *rsp_len,
> -                       unsigned int max_rsp_len)
> +                       struct rsp_buffer *rsp)
>   {
>       IPMIInterface *s = ibs->parent.intf;
>       IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -836,55 +846,49 @@ static void warm_reset(IPMIBmcSim *ibs,
>       }
>   }
>   static void set_acpi_power_state(IPMIBmcSim *ibs,
> -                          uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                                 uint8_t *cmd, unsigned int cmd_len,
> +                                 struct rsp_buffer *rsp)
>   {
>       ibs->acpi_power_state[0] = cmd[2];
>       ibs->acpi_power_state[1] = cmd[3];
>   }
>
>   static void get_acpi_power_state(IPMIBmcSim *ibs,
> -                          uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                                 uint8_t *cmd, unsigned int cmd_len,
> +                                 struct rsp_buffer *rsp)
>   {
> -    IPMI_ADD_RSP_DATA(ibs->acpi_power_state[0]);
> -    IPMI_ADD_RSP_DATA(ibs->acpi_power_state[1]);
> +    rsp_buffer_push(rsp, ibs->acpi_power_state[0]);
> +    rsp_buffer_push(rsp, ibs->acpi_power_state[1]);
>   }
>
>   static void get_device_guid(IPMIBmcSim *ibs,
> -                          uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                            uint8_t *cmd, unsigned int cmd_len,
> +                            struct rsp_buffer *rsp)
>   {
>       unsigned int i;
>
>       for (i = 0; i < 16; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->uuid[i]);
> +        rsp_buffer_push(rsp, ibs->uuid[i]);
>       }
>   }
>
>   static void set_bmc_global_enables(IPMIBmcSim *ibs,
>                                      uint8_t *cmd, unsigned int cmd_len,
> -                                   uint8_t *rsp, unsigned int *rsp_len,
> -                                   unsigned int max_rsp_len)
> +                                   struct rsp_buffer *rsp)
>   {
>       set_global_enables(ibs, cmd[2]);
>   }
>
>   static void get_bmc_global_enables(IPMIBmcSim *ibs,
>                                      uint8_t *cmd, unsigned int cmd_len,
> -                                   uint8_t *rsp, unsigned int *rsp_len,
> -                                   unsigned int max_rsp_len)
> +                                   struct rsp_buffer *rsp)
>   {
> -    IPMI_ADD_RSP_DATA(ibs->bmc_global_enables);
> +    rsp_buffer_push(rsp, ibs->bmc_global_enables);
>   }
>
>   static void clr_msg_flags(IPMIBmcSim *ibs,
>                             uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>   {
>       IPMIInterface *s = ibs->parent.intf;
>       IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -895,27 +899,25 @@ static void clr_msg_flags(IPMIBmcSim *ibs,
>
>   static void get_msg_flags(IPMIBmcSim *ibs,
>                             uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>   {
> -    IPMI_ADD_RSP_DATA(ibs->msg_flags);
> +    rsp_buffer_push(rsp, ibs->msg_flags);
>   }
>
>   static void read_evt_msg_buf(IPMIBmcSim *ibs,
>                                uint8_t *cmd, unsigned int cmd_len,
> -                             uint8_t *rsp, unsigned int *rsp_len,
> -                            unsigned int max_rsp_len)
> +                             struct rsp_buffer *rsp)
>   {
>       IPMIInterface *s = ibs->parent.intf;
>       IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>       unsigned int i;
>
>       if (!(ibs->msg_flags & IPMI_BMC_MSG_FLAG_EVT_BUF_FULL)) {
> -        rsp[2] = 0x80;
> +        rsp->buffer[2] = 0x80;
>           return;
>       }
>       for (i = 0; i < 16; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->evtbuf[i]);
> +        rsp_buffer_push(rsp, ibs->evtbuf[i]);
>       }
>       ibs->msg_flags &= ~IPMI_BMC_MSG_FLAG_EVT_BUF_FULL;
>       k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs));
> @@ -923,21 +925,18 @@ static void read_evt_msg_buf(IPMIBmcSim *ibs,
>
>   static void get_msg(IPMIBmcSim *ibs,
>                       uint8_t *cmd, unsigned int cmd_len,
> -                    uint8_t *rsp, unsigned int *rsp_len,
> -                    unsigned int max_rsp_len)
> +                    struct rsp_buffer *rsp)
>   {
>       IPMIRcvBufEntry *msg;
>
>       qemu_mutex_lock(&ibs->lock);
>       if (QTAILQ_EMPTY(&ibs->rcvbufs)) {
> -        rsp[2] = 0x80; /* Queue empty */
> +        rsp->buffer[2] = 0x80; /* Queue empty */
>           goto out;
>       }
> -    rsp[3] = 0; /* Channel 0 */
> -    *rsp_len += 1;
> +    rsp_buffer_push(rsp, 0); /* Channel 0 */
>       msg = QTAILQ_FIRST(&ibs->rcvbufs);
> -    memcpy(rsp + 4, msg->buf, msg->len);
> -    *rsp_len += msg->len;
> +    rsp_buffer_pushmore(rsp, msg->buf, msg->len);
>       QTAILQ_REMOVE(&ibs->rcvbufs, msg, entry);
>       g_free(msg);
>
> @@ -966,8 +965,7 @@ ipmb_checksum(unsigned char *data, int size, unsigned char csum)
>
>   static void send_msg(IPMIBmcSim *ibs,
>                        uint8_t *cmd, unsigned int cmd_len,
> -                     uint8_t *rsp, unsigned int *rsp_len,
> -                     unsigned int max_rsp_len)
> +                     struct rsp_buffer *rsp)
>   {
>       IPMIInterface *s = ibs->parent.intf;
>       IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -977,18 +975,18 @@ static void send_msg(IPMIBmcSim *ibs,
>
>       if (cmd[2] != 0) {
>           /* We only handle channel 0 with no options */
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>
>       if (cmd_len < 10) {
> -        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
>           return;
>       }
>
>       if (cmd[3] != 0x40) {
>           /* We only emulate a MC at address 0x40. */
> -        rsp[2] = 0x83; /* NAK on write */
> +        rsp->buffer[2] = 0x83; /* NAK on write */
>           return;
>       }
>
> @@ -1074,11 +1072,10 @@ static void do_watchdog_reset(IPMIBmcSim *ibs)
>
>   static void reset_watchdog_timer(IPMIBmcSim *ibs,
>                                    uint8_t *cmd, unsigned int cmd_len,
> -                                 uint8_t *rsp, unsigned int *rsp_len,
> -                                 unsigned int max_rsp_len)
> +                                 struct rsp_buffer *rsp)
>   {
>       if (!ibs->watchdog_initialized) {
> -        rsp[2] = 0x80;
> +        rsp->buffer[2] = 0x80;
>           return;
>       }
>       do_watchdog_reset(ibs);
> @@ -1086,8 +1083,7 @@ static void reset_watchdog_timer(IPMIBmcSim *ibs,
>
>   static void set_watchdog_timer(IPMIBmcSim *ibs,
>                                  uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                               struct rsp_buffer *rsp)
>   {
>       IPMIInterface *s = ibs->parent.intf;
>       IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> @@ -1095,7 +1091,7 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>
>       val = cmd[2] & 0x7; /* Validate use */
>       if (val == 0 || val > 5) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>       val = cmd[3] & 0x7; /* Validate action */
> @@ -1104,22 +1100,22 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>           break;
>
>       case IPMI_BMC_WATCHDOG_ACTION_RESET:
> -        rsp[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 1);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 1);
>           break;
>
>       case IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN:
> -        rsp[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1);
>           break;
>
>       case IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE:
> -        rsp[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1);
> +        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1);
>           break;
>
>       default:
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>       }
> -    if (rsp[2]) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +    if (rsp->buffer[2]) {
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>
> @@ -1132,14 +1128,14 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>       case IPMI_BMC_WATCHDOG_PRE_NMI:
>           if (!k->do_hw_op(s, IPMI_SEND_NMI, 1)) {
>               /* NMI not supported. */
> -            rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +            rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>               return;
>           }
>           break;
>
>       default:
>           /* We don't support PRE_SMI */
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>
> @@ -1158,60 +1154,56 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>
>   static void get_watchdog_timer(IPMIBmcSim *ibs,
>                                  uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                               struct rsp_buffer *rsp)
>   {
> -    IPMI_ADD_RSP_DATA(ibs->watchdog_use);
> -    IPMI_ADD_RSP_DATA(ibs->watchdog_action);
> -    IPMI_ADD_RSP_DATA(ibs->watchdog_pretimeout);
> -    IPMI_ADD_RSP_DATA(ibs->watchdog_expired);
> +    rsp_buffer_push(rsp, ibs->watchdog_use);
> +    rsp_buffer_push(rsp, ibs->watchdog_action);
> +    rsp_buffer_push(rsp, ibs->watchdog_pretimeout);
> +    rsp_buffer_push(rsp, ibs->watchdog_expired);
>       if (ibs->watchdog_running) {
>           long timeout;
>           timeout = ((ibs->watchdog_expiry - ipmi_getmonotime() + 50000000)
>                      / 100000000);
> -        IPMI_ADD_RSP_DATA(timeout & 0xff);
> -        IPMI_ADD_RSP_DATA((timeout >> 8) & 0xff);
> +        rsp_buffer_push(rsp, timeout & 0xff);
> +        rsp_buffer_push(rsp, (timeout >> 8) & 0xff);
>       } else {
> -        IPMI_ADD_RSP_DATA(0);
> -        IPMI_ADD_RSP_DATA(0);
> +        rsp_buffer_push(rsp, 0);
> +        rsp_buffer_push(rsp, 0);
>       }
>   }
>
>   static void get_sdr_rep_info(IPMIBmcSim *ibs,
>                                uint8_t *cmd, unsigned int cmd_len,
> -                             uint8_t *rsp, unsigned int *rsp_len,
> -                             unsigned int max_rsp_len)
> +                             struct rsp_buffer *rsp)
>   {
>       unsigned int i;
>
> -    IPMI_ADD_RSP_DATA(0x51); /* Conform to IPMI 1.5 spec */
> -    IPMI_ADD_RSP_DATA(ibs->sdr.next_rec_id & 0xff);
> -    IPMI_ADD_RSP_DATA((ibs->sdr.next_rec_id >> 8) & 0xff);
> -    IPMI_ADD_RSP_DATA((MAX_SDR_SIZE - ibs->sdr.next_free) & 0xff);
> -    IPMI_ADD_RSP_DATA(((MAX_SDR_SIZE - ibs->sdr.next_free) >> 8) & 0xff);
> +    rsp_buffer_push(rsp, 0x51); /* Conform to IPMI 1.5 spec */
> +    rsp_buffer_push(rsp, ibs->sdr.next_rec_id & 0xff);
> +    rsp_buffer_push(rsp, (ibs->sdr.next_rec_id >> 8) & 0xff);
> +    rsp_buffer_push(rsp, (MAX_SDR_SIZE - ibs->sdr.next_free) & 0xff);
> +    rsp_buffer_push(rsp, ((MAX_SDR_SIZE - ibs->sdr.next_free) >> 8) & 0xff);
>       for (i = 0; i < 4; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->sdr.last_addition[i]);
> +        rsp_buffer_push(rsp, ibs->sdr.last_addition[i]);
>       }
>       for (i = 0; i < 4; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->sdr.last_clear[i]);
> +        rsp_buffer_push(rsp, ibs->sdr.last_clear[i]);
>       }
>       /* Only modal support, reserve supported */
> -    IPMI_ADD_RSP_DATA((ibs->sdr.overflow << 7) | 0x22);
> +    rsp_buffer_push(rsp, (ibs->sdr.overflow << 7) | 0x22);
>   }
>
>   static void reserve_sdr_rep(IPMIBmcSim *ibs,
>                               uint8_t *cmd, unsigned int cmd_len,
> -                            uint8_t *rsp, unsigned int *rsp_len,
> -                            unsigned int max_rsp_len)
> +                            struct rsp_buffer *rsp)
>   {
> -    IPMI_ADD_RSP_DATA(ibs->sdr.reservation & 0xff);
> -    IPMI_ADD_RSP_DATA((ibs->sdr.reservation >> 8) & 0xff);
> +    rsp_buffer_push(rsp, ibs->sdr.reservation & 0xff);
> +    rsp_buffer_push(rsp, (ibs->sdr.reservation >> 8) & 0xff);
>   }
>
>   static void get_sdr(IPMIBmcSim *ibs,
>                       uint8_t *cmd, unsigned int cmd_len,
> -                    uint8_t *rsp, unsigned int *rsp_len,
> -                    unsigned int max_rsp_len)
> +                    struct rsp_buffer *rsp)
>   {
>       unsigned int pos;
>       uint16_t nextrec;
> @@ -1223,108 +1215,103 @@ static void get_sdr(IPMIBmcSim *ibs,
>       pos = 0;
>       if (sdr_find_entry(&ibs->sdr, cmd[4] | (cmd[5] << 8),
>                          &pos, &nextrec)) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>
>       sdrh = (struct ipmi_sdr_header *) &ibs->sdr.sdr[pos];
>
>       if (cmd[6] > ipmi_sdr_length(sdrh)) {
> -        rsp[2] = IPMI_CC_PARM_OUT_OF_RANGE;
> +        rsp->buffer[2] = IPMI_CC_PARM_OUT_OF_RANGE;
>           return;
>       }
>
> -    IPMI_ADD_RSP_DATA(nextrec & 0xff);
> -    IPMI_ADD_RSP_DATA((nextrec >> 8) & 0xff);
> +    rsp_buffer_push(rsp, nextrec & 0xff);
> +    rsp_buffer_push(rsp, (nextrec >> 8) & 0xff);
>
>       if (cmd[7] == 0xff) {
>           cmd[7] = ipmi_sdr_length(sdrh) - cmd[6];
>       }
>
> -    if ((cmd[7] + *rsp_len) > max_rsp_len) {
> -        rsp[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
> +    if ((cmd[7] + rsp->len) > rsp->max_len) {
> +        rsp->buffer[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
>           return;
>       }
> -    memcpy(rsp + *rsp_len, ibs->sdr.sdr + pos + cmd[6], cmd[7]);
> -    *rsp_len += cmd[7];
> +
> +    rsp_buffer_pushmore(rsp, ibs->sdr.sdr + pos + cmd[6], cmd[7]);
>   }
>
>   static void add_sdr(IPMIBmcSim *ibs,
>                       uint8_t *cmd, unsigned int cmd_len,
> -                    uint8_t *rsp, unsigned int *rsp_len,
> -                    unsigned int max_rsp_len)
> +                    struct rsp_buffer *rsp)
>   {
>       uint16_t recid;
>       struct ipmi_sdr_header *sdrh = (struct ipmi_sdr_header *) cmd + 2;
>
>       if (sdr_add_entry(ibs, sdrh, cmd_len - 2, &recid)) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
> -    IPMI_ADD_RSP_DATA(recid & 0xff);
> -    IPMI_ADD_RSP_DATA((recid >> 8) & 0xff);
> +    rsp_buffer_push(rsp, recid & 0xff);
> +    rsp_buffer_push(rsp, (recid >> 8) & 0xff);
>   }
>
>   static void clear_sdr_rep(IPMIBmcSim *ibs,
>                             uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>   {
>       IPMI_CHECK_RESERVATION(2, ibs->sdr.reservation);
>       if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>       if (cmd[7] == 0xaa) {
>           ibs->sdr.next_free = 0;
>           ibs->sdr.overflow = 0;
>           set_timestamp(ibs, ibs->sdr.last_clear);
> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>           sdr_inc_reservation(&ibs->sdr);
>       } else if (cmd[7] == 0) {
> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>       } else {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>   }
>
>   static void get_sel_info(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
> -                         uint8_t *rsp, unsigned int *rsp_len,
> -                         unsigned int max_rsp_len)
> +                         struct rsp_buffer *rsp)
>   {
>       unsigned int i, val;
>
> -    IPMI_ADD_RSP_DATA(0x51); /* Conform to IPMI 1.5 */
> -    IPMI_ADD_RSP_DATA(ibs->sel.next_free & 0xff);
> -    IPMI_ADD_RSP_DATA((ibs->sel.next_free >> 8) & 0xff);
> +    rsp_buffer_push(rsp, 0x51); /* Conform to IPMI 1.5 */
> +    rsp_buffer_push(rsp, ibs->sel.next_free & 0xff);
> +    rsp_buffer_push(rsp, (ibs->sel.next_free >> 8) & 0xff);
>       val = (MAX_SEL_SIZE - ibs->sel.next_free) * 16;
> -    IPMI_ADD_RSP_DATA(val & 0xff);
> -    IPMI_ADD_RSP_DATA((val >> 8) & 0xff);
> +    rsp_buffer_push(rsp, val & 0xff);
> +    rsp_buffer_push(rsp, (val >> 8) & 0xff);
>       for (i = 0; i < 4; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->sel.last_addition[i]);
> +        rsp_buffer_push(rsp, ibs->sel.last_addition[i]);
>       }
>       for (i = 0; i < 4; i++) {
> -        IPMI_ADD_RSP_DATA(ibs->sel.last_clear[i]);
> +        rsp_buffer_push(rsp, ibs->sel.last_clear[i]);
>       }
>       /* Only support Reserve SEL */
> -    IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02);
> +    rsp_buffer_push(rsp, (ibs->sel.overflow << 7) | 0x02);
>   }
>
>   static void reserve_sel(IPMIBmcSim *ibs,
>                           uint8_t *cmd, unsigned int cmd_len,
> -                        uint8_t *rsp, unsigned int *rsp_len,
> -                        unsigned int max_rsp_len)
> +                        struct rsp_buffer *rsp)
>   {
> -    IPMI_ADD_RSP_DATA(ibs->sel.reservation & 0xff);
> -    IPMI_ADD_RSP_DATA((ibs->sel.reservation >> 8) & 0xff);
> +    rsp_buffer_push(rsp, ibs->sel.reservation & 0xff);
> +    rsp_buffer_push(rsp, (ibs->sel.reservation >> 8) & 0xff);
>   }
>
>   static void get_sel_entry(IPMIBmcSim *ibs,
>                             uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>   {
>       unsigned int val;
>
> @@ -1332,17 +1319,17 @@ static void get_sel_entry(IPMIBmcSim *ibs,
>           IPMI_CHECK_RESERVATION(2, ibs->sel.reservation);
>       }
>       if (ibs->sel.next_free == 0) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>       if (cmd[6] > 15) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>       if (cmd[7] == 0xff) {
>           cmd[7] = 16;
>       } else if ((cmd[7] + cmd[6]) > 16) {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       } else {
>           cmd[7] += cmd[6];
> @@ -1352,79 +1339,75 @@ static void get_sel_entry(IPMIBmcSim *ibs,
>       if (val == 0xffff) {
>           val = ibs->sel.next_free - 1;
>       } else if (val >= ibs->sel.next_free) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>       if ((val + 1) == ibs->sel.next_free) {
> -        IPMI_ADD_RSP_DATA(0xff);
> -        IPMI_ADD_RSP_DATA(0xff);
> +        rsp_buffer_push(rsp, 0xff);
> +        rsp_buffer_push(rsp, 0xff);
>       } else {
> -        IPMI_ADD_RSP_DATA((val + 1) & 0xff);
> -        IPMI_ADD_RSP_DATA(((val + 1) >> 8) & 0xff);
> +        rsp_buffer_push(rsp, (val + 1) & 0xff);
> +        rsp_buffer_push(rsp, ((val + 1) >> 8) & 0xff);
>       }
>       for (; cmd[6] < cmd[7]; cmd[6]++) {
> -        IPMI_ADD_RSP_DATA(ibs->sel.sel[val][cmd[6]]);
> +        rsp_buffer_push(rsp, ibs->sel.sel[val][cmd[6]]);
>       }
>   }
>
>   static void add_sel_entry(IPMIBmcSim *ibs,
>                             uint8_t *cmd, unsigned int cmd_len,
> -                          uint8_t *rsp, unsigned int *rsp_len,
> -                          unsigned int max_rsp_len)
> +                          struct rsp_buffer *rsp)
>   {
>       if (sel_add_event(ibs, cmd + 2)) {
> -        rsp[2] = IPMI_CC_OUT_OF_SPACE;
> +        rsp->buffer[2] = IPMI_CC_OUT_OF_SPACE;
>           return;
>       }
>       /* sel_add_event fills in the record number. */
> -    IPMI_ADD_RSP_DATA(cmd[2]);
> -    IPMI_ADD_RSP_DATA(cmd[3]);
> +    rsp_buffer_push(rsp, cmd[2]);
> +    rsp_buffer_push(rsp, cmd[3]);
>   }
>
>   static void clear_sel(IPMIBmcSim *ibs,
>                         uint8_t *cmd, unsigned int cmd_len,
> -                      uint8_t *rsp, unsigned int *rsp_len,
> -                      unsigned int max_rsp_len)
> +                      struct rsp_buffer *rsp)
>   {
>       IPMI_CHECK_RESERVATION(2, ibs->sel.reservation);
>       if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>       if (cmd[7] == 0xaa) {
>           ibs->sel.next_free = 0;
>           ibs->sel.overflow = 0;
>           set_timestamp(ibs, ibs->sdr.last_clear);
> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>           sel_inc_reservation(&ibs->sel);
>       } else if (cmd[7] == 0) {
> -        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
> +        rsp_buffer_push(rsp, 1); /* Erasure complete */
>       } else {
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>   }
>
>   static void get_sel_time(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
> -                         uint8_t *rsp, unsigned int *rsp_len,
> -                         unsigned int max_rsp_len)
> +                         struct rsp_buffer *rsp)
>   {
>       uint32_t val;
>       struct ipmi_time now;
>
>       ipmi_gettime(&now);
>       val = now.tv_sec + ibs->sel.time_offset;
> -    IPMI_ADD_RSP_DATA(val & 0xff);
> -    IPMI_ADD_RSP_DATA((val >> 8) & 0xff);
> -    IPMI_ADD_RSP_DATA((val >> 16) & 0xff);
> -    IPMI_ADD_RSP_DATA((val >> 24) & 0xff);
> +    rsp_buffer_push(rsp, val & 0xff);
> +    rsp_buffer_push(rsp, (val >> 8) & 0xff);
> +    rsp_buffer_push(rsp, (val >> 16) & 0xff);
> +    rsp_buffer_push(rsp, (val >> 24) & 0xff);
>   }
>
>   static void set_sel_time(IPMIBmcSim *ibs,
>                            uint8_t *cmd, unsigned int cmd_len,
> -                         uint8_t *rsp, unsigned int *rsp_len,
> -                         unsigned int max_rsp_len)
> +                         struct rsp_buffer *rsp)
>   {
>       uint32_t val;
>       struct ipmi_time now;
> @@ -1436,14 +1419,13 @@ static void set_sel_time(IPMIBmcSim *ibs,
>
>   static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>                                     uint8_t *cmd, unsigned int cmd_len,
> -                                  uint8_t *rsp, unsigned int *rsp_len,
> -                                  unsigned int max_rsp_len)
> +                                  struct rsp_buffer *rsp)
>   {
>       IPMISensor *sens;
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>               !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> @@ -1479,7 +1461,7 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>           }
>           break;
>       case 3:
> -        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
>           return;
>       }
>       IPMI_SENSOR_SET_RET_STATUS(sens, cmd[3]);
> @@ -1487,34 +1469,32 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>
>   static void get_sensor_evt_enable(IPMIBmcSim *ibs,
>                                     uint8_t *cmd, unsigned int cmd_len,
> -                                  uint8_t *rsp, unsigned int *rsp_len,
> -                                  unsigned int max_rsp_len)
> +                                  struct rsp_buffer *rsp)
>   {
>       IPMISensor *sens;
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>           !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> -    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
> -    IPMI_ADD_RSP_DATA(sens->assert_enable & 0xff);
> -    IPMI_ADD_RSP_DATA((sens->assert_enable >> 8) & 0xff);
> -    IPMI_ADD_RSP_DATA(sens->deassert_enable & 0xff);
> -    IPMI_ADD_RSP_DATA((sens->deassert_enable >> 8) & 0xff);
> +    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
> +    rsp_buffer_push(rsp, sens->assert_enable & 0xff);
> +    rsp_buffer_push(rsp, (sens->assert_enable >> 8) & 0xff);
> +    rsp_buffer_push(rsp, sens->deassert_enable & 0xff);
> +    rsp_buffer_push(rsp, (sens->deassert_enable >> 8) & 0xff);
>   }
>
>   static void rearm_sensor_evts(IPMIBmcSim *ibs,
>                                 uint8_t *cmd, unsigned int cmd_len,
> -                              uint8_t *rsp, unsigned int *rsp_len,
> -                              unsigned int max_rsp_len)
> +                              struct rsp_buffer *rsp)
>   {
>       IPMISensor *sens;
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>           !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> @@ -1528,57 +1508,54 @@ static void rearm_sensor_evts(IPMIBmcSim *ibs,
>
>   static void get_sensor_evt_status(IPMIBmcSim *ibs,
>                                     uint8_t *cmd, unsigned int cmd_len,
> -                                  uint8_t *rsp, unsigned int *rsp_len,
> -                                  unsigned int max_rsp_len)
> +                                  struct rsp_buffer *rsp)
>   {
>       IPMISensor *sens;
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>           !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> -    IPMI_ADD_RSP_DATA(sens->reading);
> -    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
> -    IPMI_ADD_RSP_DATA(sens->assert_states & 0xff);
> -    IPMI_ADD_RSP_DATA((sens->assert_states >> 8) & 0xff);
> -    IPMI_ADD_RSP_DATA(sens->deassert_states & 0xff);
> -    IPMI_ADD_RSP_DATA((sens->deassert_states >> 8) & 0xff);
> +    rsp_buffer_push(rsp, sens->reading);
> +    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
> +    rsp_buffer_push(rsp, sens->assert_states & 0xff);
> +    rsp_buffer_push(rsp, (sens->assert_states >> 8) & 0xff);
> +    rsp_buffer_push(rsp, sens->deassert_states & 0xff);
> +    rsp_buffer_push(rsp, (sens->deassert_states >> 8) & 0xff);
>   }
>
>   static void get_sensor_reading(IPMIBmcSim *ibs,
>                                  uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                               struct rsp_buffer *rsp)
>   {
>       IPMISensor *sens;
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>               !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> -    IPMI_ADD_RSP_DATA(sens->reading);
> -    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
> -    IPMI_ADD_RSP_DATA(sens->states & 0xff);
> +    rsp_buffer_push(rsp, sens->reading);
> +    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
> +    rsp_buffer_push(rsp, sens->states & 0xff);
>       if (IPMI_SENSOR_IS_DISCRETE(sens)) {
> -        IPMI_ADD_RSP_DATA((sens->states >> 8) & 0xff);
> +        rsp_buffer_push(rsp, (sens->states >> 8) & 0xff);
>       }
>   }
>
>   static void set_sensor_type(IPMIBmcSim *ibs,
> -                               uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                            uint8_t *cmd, unsigned int cmd_len,
> +                            struct rsp_buffer *rsp)
>   {
>       IPMISensor *sens;
>
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>               !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> @@ -1587,21 +1564,20 @@ static void set_sensor_type(IPMIBmcSim *ibs,
>   }
>
>   static void get_sensor_type(IPMIBmcSim *ibs,
> -                               uint8_t *cmd, unsigned int cmd_len,
> -                               uint8_t *rsp, unsigned int *rsp_len,
> -                               unsigned int max_rsp_len)
> +                            uint8_t *cmd, unsigned int cmd_len,
> +                            struct rsp_buffer *rsp)
>   {
>       IPMISensor *sens;
>
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>               !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> -    IPMI_ADD_RSP_DATA(sens->sensor_type);
> -    IPMI_ADD_RSP_DATA(sens->evt_reading_type_code);
> +    rsp_buffer_push(rsp, sens->sensor_type);
> +    rsp_buffer_push(rsp, sens->evt_reading_type_code);
>   }
>
>
diff mbox

Patch

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 30b9fb48ea2d..32efb87c8232 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -153,14 +153,14 @@  typedef struct IPMISensor {
 #define IPMI_WATCHDOG_SENSOR 0
 
 typedef struct IPMIBmcSim IPMIBmcSim;
+struct rsp_buffer;
 
 #define MAX_NETFNS 64
 
 typedef struct IPMICmdHandler {
     void (*cmd_handler)(IPMIBmcSim *s,
                         uint8_t *cmd, unsigned int cmd_len,
-                        uint8_t *rsp, unsigned int *rsp_len,
-                        unsigned int max_rsp_len);
+                        struct rsp_buffer *rsp);
     unsigned int cmd_len_min;
 } IPMICmdHandler;
 
@@ -263,22 +263,41 @@  struct IPMIBmcSim {
 #define IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN      2
 #define IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE     3
 
+struct rsp_buffer {
+    uint8_t buffer[MAX_IPMI_MSG_SIZE];
+    unsigned int len;
+    unsigned int max_len;
+};
+
+#define RSP_BUFFER_INITIALIZER { { 0 }, 0, MAX_IPMI_MSG_SIZE }
 
 /* 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;       \
-            return;                                        \
-        }                                                  \
-        rsp[(*rsp_len)++] = (b);                           \
-    } while (0)
+static inline void rsp_buffer_push(struct rsp_buffer *rsp, uint8_t byte)
+{
+    if (rsp->len >= rsp->max_len) {
+        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
+        return;
+    }
+    rsp->buffer[rsp->len++] = byte;
+}
+
+static inline void rsp_buffer_pushmore(struct rsp_buffer *rsp, uint8_t *bytes,
+                                       unsigned int n)
+{
+    if (rsp->len + n >= rsp->max_len) {
+        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
+        return;
+    }
+
+    memcpy(&rsp->buffer[rsp->len], bytes, n);
+    rsp->len += n;
+}
 
 /* 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;          \
+            rsp->buffer[2] = IPMI_CC_INVALID_RESERVATION;     \
             return;                                        \
         }                                                  \
     } while (0)
@@ -585,35 +604,32 @@  static void ipmi_sim_handle_command(IPMIBmc *b,
     IPMIInterface *s = ibs->parent.intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     unsigned int netfn;
-    uint8_t rsp[MAX_IPMI_MSG_SIZE];
-    unsigned int rsp_len_holder = 0;
-    unsigned int *rsp_len = &rsp_len_holder;
-    unsigned int max_rsp_len = sizeof(rsp);
+    struct rsp_buffer rsp = RSP_BUFFER_INITIALIZER;
 
     /* Set up the response, set the low bit of NETFN. */
     /* Note that max_rsp_len must be at least 3 */
-    if (max_rsp_len < 3) {
-        rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
+    if (rsp.max_len < 3) {
+        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
         goto out;
     }
 
-    IPMI_ADD_RSP_DATA(cmd[0] | 0x04);
-    IPMI_ADD_RSP_DATA(cmd[1]);
-    IPMI_ADD_RSP_DATA(0); /* Assume success */
+    rsp_buffer_push(&rsp, cmd[0] | 0x04);
+    rsp_buffer_push(&rsp, cmd[1]);
+    rsp_buffer_push(&rsp, 0); /* Assume success */
 
     /* If it's too short or it was truncated, return an error. */
     if (cmd_len < 2) {
-        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
+        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
         goto out;
     }
     if (cmd_len > max_cmd_len) {
-        rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
+        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
         goto out;
     }
 
     if ((cmd[0] & 0x03) != 0) {
         /* Only have stuff on LUN 0 */
-        rsp[2] = IPMI_CC_COMMAND_INVALID_FOR_LUN;
+        rsp.buffer[2] = IPMI_CC_COMMAND_INVALID_FOR_LUN;
         goto out;
     }
 
@@ -623,20 +639,20 @@  static void ipmi_sim_handle_command(IPMIBmc *b,
     if ((netfn & 1) || !ibs->netfns[netfn / 2] ||
         (cmd[1] >= ibs->netfns[netfn / 2]->cmd_nums) ||
         (!ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler)) {
-        rsp[2] = IPMI_CC_INVALID_CMD;
+        rsp.buffer[2] = IPMI_CC_INVALID_CMD;
         goto out;
     }
 
     if (cmd_len < ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_len_min) {
-        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
+        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
         goto out;
     }
 
     ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_handler(ibs, cmd, cmd_len,
-                                                rsp, rsp_len, max_rsp_len);
+                                                &rsp);
 
  out:
-    k->handle_rsp(s, msg_id, rsp, *rsp_len);
+    k->handle_rsp(s, msg_id, rsp.buffer, rsp.len);
 
     next_timeout(ibs);
 }
@@ -711,86 +727,82 @@  static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
 
 static void chassis_capabilities(IPMIBmcSim *ibs,
                                  uint8_t *cmd, unsigned int cmd_len,
-                                 uint8_t *rsp, unsigned int *rsp_len,
-                                 unsigned int max_rsp_len)
+                                 struct rsp_buffer *rsp)
 {
-    IPMI_ADD_RSP_DATA(0);
-    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
-    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
-    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
-    IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
+    rsp_buffer_push(rsp, 0);
+    rsp_buffer_push(rsp, ibs->parent.slave_addr);
+    rsp_buffer_push(rsp, ibs->parent.slave_addr);
+    rsp_buffer_push(rsp, ibs->parent.slave_addr);
+    rsp_buffer_push(rsp, ibs->parent.slave_addr);
 }
 
 static void chassis_status(IPMIBmcSim *ibs,
                            uint8_t *cmd, unsigned int cmd_len,
-                           uint8_t *rsp, unsigned int *rsp_len,
-                           unsigned int max_rsp_len)
+                           struct rsp_buffer *rsp)
 {
-    IPMI_ADD_RSP_DATA(0x61); /* Unknown power restore, power is on */
-    IPMI_ADD_RSP_DATA(0);
-    IPMI_ADD_RSP_DATA(0);
-    IPMI_ADD_RSP_DATA(0);
+    rsp_buffer_push(rsp, 0x61); /* Unknown power restore, power is on */
+    rsp_buffer_push(rsp, 0);
+    rsp_buffer_push(rsp, 0);
+    rsp_buffer_push(rsp, 0);
 }
 
 static void chassis_control(IPMIBmcSim *ibs,
                             uint8_t *cmd, unsigned int cmd_len,
-                            uint8_t *rsp, unsigned int *rsp_len,
-                            unsigned int max_rsp_len)
+                            struct rsp_buffer *rsp)
 {
     IPMIInterface *s = ibs->parent.intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
 
     switch (cmd[2] & 0xf) {
     case 0: /* power down */
-        rsp[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
+        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
         break;
     case 1: /* power up */
-        rsp[2] = k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0);
+        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0);
         break;
     case 2: /* power cycle */
-        rsp[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
+        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
         break;
     case 3: /* hard reset */
-        rsp[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
+        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
         break;
     case 4: /* pulse diagnostic interrupt */
-        rsp[2] = k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0);
+        rsp->buffer[2] = k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0);
         break;
     case 5: /* soft shutdown via ACPI by overtemp emulation */
-        rsp[2] = k->do_hw_op(s,
+        rsp->buffer[2] = k->do_hw_op(s,
                              IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 0);
         break;
     default:
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
 }
 
 static void chassis_get_sys_restart_cause(IPMIBmcSim *ibs,
                            uint8_t *cmd, unsigned int cmd_len,
-                           uint8_t *rsp, unsigned int *rsp_len,
-                           unsigned int max_rsp_len)
+                           struct rsp_buffer *rsp)
+
 {
-    IPMI_ADD_RSP_DATA(ibs->restart_cause & 0xf); /* Restart Cause */
-    IPMI_ADD_RSP_DATA(0);  /* Channel 0 */
+    rsp_buffer_push(rsp, ibs->restart_cause & 0xf); /* Restart Cause */
+    rsp_buffer_push(rsp, 0);  /* Channel 0 */
 }
 
 static void get_device_id(IPMIBmcSim *ibs,
                           uint8_t *cmd, unsigned int cmd_len,
-                          uint8_t *rsp, unsigned int *rsp_len,
-                          unsigned int max_rsp_len)
-{
-    IPMI_ADD_RSP_DATA(ibs->device_id);
-    IPMI_ADD_RSP_DATA(ibs->device_rev & 0xf);
-    IPMI_ADD_RSP_DATA(ibs->fwrev1 & 0x7f);
-    IPMI_ADD_RSP_DATA(ibs->fwrev2);
-    IPMI_ADD_RSP_DATA(ibs->ipmi_version);
-    IPMI_ADD_RSP_DATA(0x07); /* sensor, SDR, and SEL. */
-    IPMI_ADD_RSP_DATA(ibs->mfg_id[0]);
-    IPMI_ADD_RSP_DATA(ibs->mfg_id[1]);
-    IPMI_ADD_RSP_DATA(ibs->mfg_id[2]);
-    IPMI_ADD_RSP_DATA(ibs->product_id[0]);
-    IPMI_ADD_RSP_DATA(ibs->product_id[1]);
+                          struct rsp_buffer *rsp)
+{
+    rsp_buffer_push(rsp, ibs->device_id);
+    rsp_buffer_push(rsp, ibs->device_rev & 0xf);
+    rsp_buffer_push(rsp, ibs->fwrev1 & 0x7f);
+    rsp_buffer_push(rsp, ibs->fwrev2);
+    rsp_buffer_push(rsp, ibs->ipmi_version);
+    rsp_buffer_push(rsp, 0x07); /* sensor, SDR, and SEL. */
+    rsp_buffer_push(rsp, ibs->mfg_id[0]);
+    rsp_buffer_push(rsp, ibs->mfg_id[1]);
+    rsp_buffer_push(rsp, ibs->mfg_id[2]);
+    rsp_buffer_push(rsp, ibs->product_id[0]);
+    rsp_buffer_push(rsp, ibs->product_id[1]);
 }
 
 static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
@@ -809,8 +821,7 @@  static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
 
 static void cold_reset(IPMIBmcSim *ibs,
                        uint8_t *cmd, unsigned int cmd_len,
-                       uint8_t *rsp, unsigned int *rsp_len,
-                       unsigned int max_rsp_len)
+                       struct rsp_buffer *rsp)
 {
     IPMIInterface *s = ibs->parent.intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
@@ -825,8 +836,7 @@  static void cold_reset(IPMIBmcSim *ibs,
 
 static void warm_reset(IPMIBmcSim *ibs,
                        uint8_t *cmd, unsigned int cmd_len,
-                       uint8_t *rsp, unsigned int *rsp_len,
-                       unsigned int max_rsp_len)
+                       struct rsp_buffer *rsp)
 {
     IPMIInterface *s = ibs->parent.intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
@@ -836,55 +846,49 @@  static void warm_reset(IPMIBmcSim *ibs,
     }
 }
 static void set_acpi_power_state(IPMIBmcSim *ibs,
-                          uint8_t *cmd, unsigned int cmd_len,
-                          uint8_t *rsp, unsigned int *rsp_len,
-                          unsigned int max_rsp_len)
+                                 uint8_t *cmd, unsigned int cmd_len,
+                                 struct rsp_buffer *rsp)
 {
     ibs->acpi_power_state[0] = cmd[2];
     ibs->acpi_power_state[1] = cmd[3];
 }
 
 static void get_acpi_power_state(IPMIBmcSim *ibs,
-                          uint8_t *cmd, unsigned int cmd_len,
-                          uint8_t *rsp, unsigned int *rsp_len,
-                          unsigned int max_rsp_len)
+                                 uint8_t *cmd, unsigned int cmd_len,
+                                 struct rsp_buffer *rsp)
 {
-    IPMI_ADD_RSP_DATA(ibs->acpi_power_state[0]);
-    IPMI_ADD_RSP_DATA(ibs->acpi_power_state[1]);
+    rsp_buffer_push(rsp, ibs->acpi_power_state[0]);
+    rsp_buffer_push(rsp, ibs->acpi_power_state[1]);
 }
 
 static void get_device_guid(IPMIBmcSim *ibs,
-                          uint8_t *cmd, unsigned int cmd_len,
-                          uint8_t *rsp, unsigned int *rsp_len,
-                          unsigned int max_rsp_len)
+                            uint8_t *cmd, unsigned int cmd_len,
+                            struct rsp_buffer *rsp)
 {
     unsigned int i;
 
     for (i = 0; i < 16; i++) {
-        IPMI_ADD_RSP_DATA(ibs->uuid[i]);
+        rsp_buffer_push(rsp, ibs->uuid[i]);
     }
 }
 
 static void set_bmc_global_enables(IPMIBmcSim *ibs,
                                    uint8_t *cmd, unsigned int cmd_len,
-                                   uint8_t *rsp, unsigned int *rsp_len,
-                                   unsigned int max_rsp_len)
+                                   struct rsp_buffer *rsp)
 {
     set_global_enables(ibs, cmd[2]);
 }
 
 static void get_bmc_global_enables(IPMIBmcSim *ibs,
                                    uint8_t *cmd, unsigned int cmd_len,
-                                   uint8_t *rsp, unsigned int *rsp_len,
-                                   unsigned int max_rsp_len)
+                                   struct rsp_buffer *rsp)
 {
-    IPMI_ADD_RSP_DATA(ibs->bmc_global_enables);
+    rsp_buffer_push(rsp, ibs->bmc_global_enables);
 }
 
 static void clr_msg_flags(IPMIBmcSim *ibs,
                           uint8_t *cmd, unsigned int cmd_len,
-                          uint8_t *rsp, unsigned int *rsp_len,
-                          unsigned int max_rsp_len)
+                          struct rsp_buffer *rsp)
 {
     IPMIInterface *s = ibs->parent.intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
@@ -895,27 +899,25 @@  static void clr_msg_flags(IPMIBmcSim *ibs,
 
 static void get_msg_flags(IPMIBmcSim *ibs,
                           uint8_t *cmd, unsigned int cmd_len,
-                          uint8_t *rsp, unsigned int *rsp_len,
-                          unsigned int max_rsp_len)
+                          struct rsp_buffer *rsp)
 {
-    IPMI_ADD_RSP_DATA(ibs->msg_flags);
+    rsp_buffer_push(rsp, ibs->msg_flags);
 }
 
 static void read_evt_msg_buf(IPMIBmcSim *ibs,
                              uint8_t *cmd, unsigned int cmd_len,
-                             uint8_t *rsp, unsigned int *rsp_len,
-                            unsigned int max_rsp_len)
+                             struct rsp_buffer *rsp)
 {
     IPMIInterface *s = ibs->parent.intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     unsigned int i;
 
     if (!(ibs->msg_flags & IPMI_BMC_MSG_FLAG_EVT_BUF_FULL)) {
-        rsp[2] = 0x80;
+        rsp->buffer[2] = 0x80;
         return;
     }
     for (i = 0; i < 16; i++) {
-        IPMI_ADD_RSP_DATA(ibs->evtbuf[i]);
+        rsp_buffer_push(rsp, ibs->evtbuf[i]);
     }
     ibs->msg_flags &= ~IPMI_BMC_MSG_FLAG_EVT_BUF_FULL;
     k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs));
@@ -923,21 +925,18 @@  static void read_evt_msg_buf(IPMIBmcSim *ibs,
 
 static void get_msg(IPMIBmcSim *ibs,
                     uint8_t *cmd, unsigned int cmd_len,
-                    uint8_t *rsp, unsigned int *rsp_len,
-                    unsigned int max_rsp_len)
+                    struct rsp_buffer *rsp)
 {
     IPMIRcvBufEntry *msg;
 
     qemu_mutex_lock(&ibs->lock);
     if (QTAILQ_EMPTY(&ibs->rcvbufs)) {
-        rsp[2] = 0x80; /* Queue empty */
+        rsp->buffer[2] = 0x80; /* Queue empty */
         goto out;
     }
-    rsp[3] = 0; /* Channel 0 */
-    *rsp_len += 1;
+    rsp_buffer_push(rsp, 0); /* Channel 0 */
     msg = QTAILQ_FIRST(&ibs->rcvbufs);
-    memcpy(rsp + 4, msg->buf, msg->len);
-    *rsp_len += msg->len;
+    rsp_buffer_pushmore(rsp, msg->buf, msg->len);
     QTAILQ_REMOVE(&ibs->rcvbufs, msg, entry);
     g_free(msg);
 
@@ -966,8 +965,7 @@  ipmb_checksum(unsigned char *data, int size, unsigned char csum)
 
 static void send_msg(IPMIBmcSim *ibs,
                      uint8_t *cmd, unsigned int cmd_len,
-                     uint8_t *rsp, unsigned int *rsp_len,
-                     unsigned int max_rsp_len)
+                     struct rsp_buffer *rsp)
 {
     IPMIInterface *s = ibs->parent.intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
@@ -977,18 +975,18 @@  static void send_msg(IPMIBmcSim *ibs,
 
     if (cmd[2] != 0) {
         /* We only handle channel 0 with no options */
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
 
     if (cmd_len < 10) {
-        rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
+        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
         return;
     }
 
     if (cmd[3] != 0x40) {
         /* We only emulate a MC at address 0x40. */
-        rsp[2] = 0x83; /* NAK on write */
+        rsp->buffer[2] = 0x83; /* NAK on write */
         return;
     }
 
@@ -1074,11 +1072,10 @@  static void do_watchdog_reset(IPMIBmcSim *ibs)
 
 static void reset_watchdog_timer(IPMIBmcSim *ibs,
                                  uint8_t *cmd, unsigned int cmd_len,
-                                 uint8_t *rsp, unsigned int *rsp_len,
-                                 unsigned int max_rsp_len)
+                                 struct rsp_buffer *rsp)
 {
     if (!ibs->watchdog_initialized) {
-        rsp[2] = 0x80;
+        rsp->buffer[2] = 0x80;
         return;
     }
     do_watchdog_reset(ibs);
@@ -1086,8 +1083,7 @@  static void reset_watchdog_timer(IPMIBmcSim *ibs,
 
 static void set_watchdog_timer(IPMIBmcSim *ibs,
                                uint8_t *cmd, unsigned int cmd_len,
-                               uint8_t *rsp, unsigned int *rsp_len,
-                               unsigned int max_rsp_len)
+                               struct rsp_buffer *rsp)
 {
     IPMIInterface *s = ibs->parent.intf;
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
@@ -1095,7 +1091,7 @@  static void set_watchdog_timer(IPMIBmcSim *ibs,
 
     val = cmd[2] & 0x7; /* Validate use */
     if (val == 0 || val > 5) {
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
     val = cmd[3] & 0x7; /* Validate action */
@@ -1104,22 +1100,22 @@  static void set_watchdog_timer(IPMIBmcSim *ibs,
         break;
 
     case IPMI_BMC_WATCHDOG_ACTION_RESET:
-        rsp[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 1);
+        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 1);
         break;
 
     case IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN:
-        rsp[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1);
+        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1);
         break;
 
     case IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE:
-        rsp[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1);
+        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1);
         break;
 
     default:
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
     }
-    if (rsp[2]) {
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+    if (rsp->buffer[2]) {
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
 
@@ -1132,14 +1128,14 @@  static void set_watchdog_timer(IPMIBmcSim *ibs,
     case IPMI_BMC_WATCHDOG_PRE_NMI:
         if (!k->do_hw_op(s, IPMI_SEND_NMI, 1)) {
             /* NMI not supported. */
-            rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+            rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
             return;
         }
         break;
 
     default:
         /* We don't support PRE_SMI */
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
 
@@ -1158,60 +1154,56 @@  static void set_watchdog_timer(IPMIBmcSim *ibs,
 
 static void get_watchdog_timer(IPMIBmcSim *ibs,
                                uint8_t *cmd, unsigned int cmd_len,
-                               uint8_t *rsp, unsigned int *rsp_len,
-                               unsigned int max_rsp_len)
+                               struct rsp_buffer *rsp)
 {
-    IPMI_ADD_RSP_DATA(ibs->watchdog_use);
-    IPMI_ADD_RSP_DATA(ibs->watchdog_action);
-    IPMI_ADD_RSP_DATA(ibs->watchdog_pretimeout);
-    IPMI_ADD_RSP_DATA(ibs->watchdog_expired);
+    rsp_buffer_push(rsp, ibs->watchdog_use);
+    rsp_buffer_push(rsp, ibs->watchdog_action);
+    rsp_buffer_push(rsp, ibs->watchdog_pretimeout);
+    rsp_buffer_push(rsp, ibs->watchdog_expired);
     if (ibs->watchdog_running) {
         long timeout;
         timeout = ((ibs->watchdog_expiry - ipmi_getmonotime() + 50000000)
                    / 100000000);
-        IPMI_ADD_RSP_DATA(timeout & 0xff);
-        IPMI_ADD_RSP_DATA((timeout >> 8) & 0xff);
+        rsp_buffer_push(rsp, timeout & 0xff);
+        rsp_buffer_push(rsp, (timeout >> 8) & 0xff);
     } else {
-        IPMI_ADD_RSP_DATA(0);
-        IPMI_ADD_RSP_DATA(0);
+        rsp_buffer_push(rsp, 0);
+        rsp_buffer_push(rsp, 0);
     }
 }
 
 static void get_sdr_rep_info(IPMIBmcSim *ibs,
                              uint8_t *cmd, unsigned int cmd_len,
-                             uint8_t *rsp, unsigned int *rsp_len,
-                             unsigned int max_rsp_len)
+                             struct rsp_buffer *rsp)
 {
     unsigned int i;
 
-    IPMI_ADD_RSP_DATA(0x51); /* Conform to IPMI 1.5 spec */
-    IPMI_ADD_RSP_DATA(ibs->sdr.next_rec_id & 0xff);
-    IPMI_ADD_RSP_DATA((ibs->sdr.next_rec_id >> 8) & 0xff);
-    IPMI_ADD_RSP_DATA((MAX_SDR_SIZE - ibs->sdr.next_free) & 0xff);
-    IPMI_ADD_RSP_DATA(((MAX_SDR_SIZE - ibs->sdr.next_free) >> 8) & 0xff);
+    rsp_buffer_push(rsp, 0x51); /* Conform to IPMI 1.5 spec */
+    rsp_buffer_push(rsp, ibs->sdr.next_rec_id & 0xff);
+    rsp_buffer_push(rsp, (ibs->sdr.next_rec_id >> 8) & 0xff);
+    rsp_buffer_push(rsp, (MAX_SDR_SIZE - ibs->sdr.next_free) & 0xff);
+    rsp_buffer_push(rsp, ((MAX_SDR_SIZE - ibs->sdr.next_free) >> 8) & 0xff);
     for (i = 0; i < 4; i++) {
-        IPMI_ADD_RSP_DATA(ibs->sdr.last_addition[i]);
+        rsp_buffer_push(rsp, ibs->sdr.last_addition[i]);
     }
     for (i = 0; i < 4; i++) {
-        IPMI_ADD_RSP_DATA(ibs->sdr.last_clear[i]);
+        rsp_buffer_push(rsp, ibs->sdr.last_clear[i]);
     }
     /* Only modal support, reserve supported */
-    IPMI_ADD_RSP_DATA((ibs->sdr.overflow << 7) | 0x22);
+    rsp_buffer_push(rsp, (ibs->sdr.overflow << 7) | 0x22);
 }
 
 static void reserve_sdr_rep(IPMIBmcSim *ibs,
                             uint8_t *cmd, unsigned int cmd_len,
-                            uint8_t *rsp, unsigned int *rsp_len,
-                            unsigned int max_rsp_len)
+                            struct rsp_buffer *rsp)
 {
-    IPMI_ADD_RSP_DATA(ibs->sdr.reservation & 0xff);
-    IPMI_ADD_RSP_DATA((ibs->sdr.reservation >> 8) & 0xff);
+    rsp_buffer_push(rsp, ibs->sdr.reservation & 0xff);
+    rsp_buffer_push(rsp, (ibs->sdr.reservation >> 8) & 0xff);
 }
 
 static void get_sdr(IPMIBmcSim *ibs,
                     uint8_t *cmd, unsigned int cmd_len,
-                    uint8_t *rsp, unsigned int *rsp_len,
-                    unsigned int max_rsp_len)
+                    struct rsp_buffer *rsp)
 {
     unsigned int pos;
     uint16_t nextrec;
@@ -1223,108 +1215,103 @@  static void get_sdr(IPMIBmcSim *ibs,
     pos = 0;
     if (sdr_find_entry(&ibs->sdr, cmd[4] | (cmd[5] << 8),
                        &pos, &nextrec)) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
 
     sdrh = (struct ipmi_sdr_header *) &ibs->sdr.sdr[pos];
 
     if (cmd[6] > ipmi_sdr_length(sdrh)) {
-        rsp[2] = IPMI_CC_PARM_OUT_OF_RANGE;
+        rsp->buffer[2] = IPMI_CC_PARM_OUT_OF_RANGE;
         return;
     }
 
-    IPMI_ADD_RSP_DATA(nextrec & 0xff);
-    IPMI_ADD_RSP_DATA((nextrec >> 8) & 0xff);
+    rsp_buffer_push(rsp, nextrec & 0xff);
+    rsp_buffer_push(rsp, (nextrec >> 8) & 0xff);
 
     if (cmd[7] == 0xff) {
         cmd[7] = ipmi_sdr_length(sdrh) - cmd[6];
     }
 
-    if ((cmd[7] + *rsp_len) > max_rsp_len) {
-        rsp[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
+    if ((cmd[7] + rsp->len) > rsp->max_len) {
+        rsp->buffer[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
         return;
     }
-    memcpy(rsp + *rsp_len, ibs->sdr.sdr + pos + cmd[6], cmd[7]);
-    *rsp_len += cmd[7];
+
+    rsp_buffer_pushmore(rsp, ibs->sdr.sdr + pos + cmd[6], cmd[7]);
 }
 
 static void add_sdr(IPMIBmcSim *ibs,
                     uint8_t *cmd, unsigned int cmd_len,
-                    uint8_t *rsp, unsigned int *rsp_len,
-                    unsigned int max_rsp_len)
+                    struct rsp_buffer *rsp)
 {
     uint16_t recid;
     struct ipmi_sdr_header *sdrh = (struct ipmi_sdr_header *) cmd + 2;
 
     if (sdr_add_entry(ibs, sdrh, cmd_len - 2, &recid)) {
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
-    IPMI_ADD_RSP_DATA(recid & 0xff);
-    IPMI_ADD_RSP_DATA((recid >> 8) & 0xff);
+    rsp_buffer_push(rsp, recid & 0xff);
+    rsp_buffer_push(rsp, (recid >> 8) & 0xff);
 }
 
 static void clear_sdr_rep(IPMIBmcSim *ibs,
                           uint8_t *cmd, unsigned int cmd_len,
-                          uint8_t *rsp, unsigned int *rsp_len,
-                          unsigned int max_rsp_len)
+                          struct rsp_buffer *rsp)
 {
     IPMI_CHECK_RESERVATION(2, ibs->sdr.reservation);
     if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
     if (cmd[7] == 0xaa) {
         ibs->sdr.next_free = 0;
         ibs->sdr.overflow = 0;
         set_timestamp(ibs, ibs->sdr.last_clear);
-        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
+        rsp_buffer_push(rsp, 1); /* Erasure complete */
         sdr_inc_reservation(&ibs->sdr);
     } else if (cmd[7] == 0) {
-        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
+        rsp_buffer_push(rsp, 1); /* Erasure complete */
     } else {
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
 }
 
 static void get_sel_info(IPMIBmcSim *ibs,
                          uint8_t *cmd, unsigned int cmd_len,
-                         uint8_t *rsp, unsigned int *rsp_len,
-                         unsigned int max_rsp_len)
+                         struct rsp_buffer *rsp)
 {
     unsigned int i, val;
 
-    IPMI_ADD_RSP_DATA(0x51); /* Conform to IPMI 1.5 */
-    IPMI_ADD_RSP_DATA(ibs->sel.next_free & 0xff);
-    IPMI_ADD_RSP_DATA((ibs->sel.next_free >> 8) & 0xff);
+    rsp_buffer_push(rsp, 0x51); /* Conform to IPMI 1.5 */
+    rsp_buffer_push(rsp, ibs->sel.next_free & 0xff);
+    rsp_buffer_push(rsp, (ibs->sel.next_free >> 8) & 0xff);
     val = (MAX_SEL_SIZE - ibs->sel.next_free) * 16;
-    IPMI_ADD_RSP_DATA(val & 0xff);
-    IPMI_ADD_RSP_DATA((val >> 8) & 0xff);
+    rsp_buffer_push(rsp, val & 0xff);
+    rsp_buffer_push(rsp, (val >> 8) & 0xff);
     for (i = 0; i < 4; i++) {
-        IPMI_ADD_RSP_DATA(ibs->sel.last_addition[i]);
+        rsp_buffer_push(rsp, ibs->sel.last_addition[i]);
     }
     for (i = 0; i < 4; i++) {
-        IPMI_ADD_RSP_DATA(ibs->sel.last_clear[i]);
+        rsp_buffer_push(rsp, ibs->sel.last_clear[i]);
     }
     /* Only support Reserve SEL */
-    IPMI_ADD_RSP_DATA((ibs->sel.overflow << 7) | 0x02);
+    rsp_buffer_push(rsp, (ibs->sel.overflow << 7) | 0x02);
 }
 
 static void reserve_sel(IPMIBmcSim *ibs,
                         uint8_t *cmd, unsigned int cmd_len,
-                        uint8_t *rsp, unsigned int *rsp_len,
-                        unsigned int max_rsp_len)
+                        struct rsp_buffer *rsp)
 {
-    IPMI_ADD_RSP_DATA(ibs->sel.reservation & 0xff);
-    IPMI_ADD_RSP_DATA((ibs->sel.reservation >> 8) & 0xff);
+    rsp_buffer_push(rsp, ibs->sel.reservation & 0xff);
+    rsp_buffer_push(rsp, (ibs->sel.reservation >> 8) & 0xff);
 }
 
 static void get_sel_entry(IPMIBmcSim *ibs,
                           uint8_t *cmd, unsigned int cmd_len,
-                          uint8_t *rsp, unsigned int *rsp_len,
-                          unsigned int max_rsp_len)
+                          struct rsp_buffer *rsp)
 {
     unsigned int val;
 
@@ -1332,17 +1319,17 @@  static void get_sel_entry(IPMIBmcSim *ibs,
         IPMI_CHECK_RESERVATION(2, ibs->sel.reservation);
     }
     if (ibs->sel.next_free == 0) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
     if (cmd[6] > 15) {
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
     if (cmd[7] == 0xff) {
         cmd[7] = 16;
     } else if ((cmd[7] + cmd[6]) > 16) {
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     } else {
         cmd[7] += cmd[6];
@@ -1352,79 +1339,75 @@  static void get_sel_entry(IPMIBmcSim *ibs,
     if (val == 0xffff) {
         val = ibs->sel.next_free - 1;
     } else if (val >= ibs->sel.next_free) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
     if ((val + 1) == ibs->sel.next_free) {
-        IPMI_ADD_RSP_DATA(0xff);
-        IPMI_ADD_RSP_DATA(0xff);
+        rsp_buffer_push(rsp, 0xff);
+        rsp_buffer_push(rsp, 0xff);
     } else {
-        IPMI_ADD_RSP_DATA((val + 1) & 0xff);
-        IPMI_ADD_RSP_DATA(((val + 1) >> 8) & 0xff);
+        rsp_buffer_push(rsp, (val + 1) & 0xff);
+        rsp_buffer_push(rsp, ((val + 1) >> 8) & 0xff);
     }
     for (; cmd[6] < cmd[7]; cmd[6]++) {
-        IPMI_ADD_RSP_DATA(ibs->sel.sel[val][cmd[6]]);
+        rsp_buffer_push(rsp, ibs->sel.sel[val][cmd[6]]);
     }
 }
 
 static void add_sel_entry(IPMIBmcSim *ibs,
                           uint8_t *cmd, unsigned int cmd_len,
-                          uint8_t *rsp, unsigned int *rsp_len,
-                          unsigned int max_rsp_len)
+                          struct rsp_buffer *rsp)
 {
     if (sel_add_event(ibs, cmd + 2)) {
-        rsp[2] = IPMI_CC_OUT_OF_SPACE;
+        rsp->buffer[2] = IPMI_CC_OUT_OF_SPACE;
         return;
     }
     /* sel_add_event fills in the record number. */
-    IPMI_ADD_RSP_DATA(cmd[2]);
-    IPMI_ADD_RSP_DATA(cmd[3]);
+    rsp_buffer_push(rsp, cmd[2]);
+    rsp_buffer_push(rsp, cmd[3]);
 }
 
 static void clear_sel(IPMIBmcSim *ibs,
                       uint8_t *cmd, unsigned int cmd_len,
-                      uint8_t *rsp, unsigned int *rsp_len,
-                      unsigned int max_rsp_len)
+                      struct rsp_buffer *rsp)
 {
     IPMI_CHECK_RESERVATION(2, ibs->sel.reservation);
     if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
     if (cmd[7] == 0xaa) {
         ibs->sel.next_free = 0;
         ibs->sel.overflow = 0;
         set_timestamp(ibs, ibs->sdr.last_clear);
-        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
+        rsp_buffer_push(rsp, 1); /* Erasure complete */
         sel_inc_reservation(&ibs->sel);
     } else if (cmd[7] == 0) {
-        IPMI_ADD_RSP_DATA(1); /* Erasure complete */
+        rsp_buffer_push(rsp, 1); /* Erasure complete */
     } else {
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
 }
 
 static void get_sel_time(IPMIBmcSim *ibs,
                          uint8_t *cmd, unsigned int cmd_len,
-                         uint8_t *rsp, unsigned int *rsp_len,
-                         unsigned int max_rsp_len)
+                         struct rsp_buffer *rsp)
 {
     uint32_t val;
     struct ipmi_time now;
 
     ipmi_gettime(&now);
     val = now.tv_sec + ibs->sel.time_offset;
-    IPMI_ADD_RSP_DATA(val & 0xff);
-    IPMI_ADD_RSP_DATA((val >> 8) & 0xff);
-    IPMI_ADD_RSP_DATA((val >> 16) & 0xff);
-    IPMI_ADD_RSP_DATA((val >> 24) & 0xff);
+    rsp_buffer_push(rsp, val & 0xff);
+    rsp_buffer_push(rsp, (val >> 8) & 0xff);
+    rsp_buffer_push(rsp, (val >> 16) & 0xff);
+    rsp_buffer_push(rsp, (val >> 24) & 0xff);
 }
 
 static void set_sel_time(IPMIBmcSim *ibs,
                          uint8_t *cmd, unsigned int cmd_len,
-                         uint8_t *rsp, unsigned int *rsp_len,
-                         unsigned int max_rsp_len)
+                         struct rsp_buffer *rsp)
 {
     uint32_t val;
     struct ipmi_time now;
@@ -1436,14 +1419,13 @@  static void set_sel_time(IPMIBmcSim *ibs,
 
 static void set_sensor_evt_enable(IPMIBmcSim *ibs,
                                   uint8_t *cmd, unsigned int cmd_len,
-                                  uint8_t *rsp, unsigned int *rsp_len,
-                                  unsigned int max_rsp_len)
+                                  struct rsp_buffer *rsp)
 {
     IPMISensor *sens;
 
     if ((cmd[2] >= MAX_SENSORS) ||
             !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
     sens = ibs->sensors + cmd[2];
@@ -1479,7 +1461,7 @@  static void set_sensor_evt_enable(IPMIBmcSim *ibs,
         }
         break;
     case 3:
-        rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
+        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
         return;
     }
     IPMI_SENSOR_SET_RET_STATUS(sens, cmd[3]);
@@ -1487,34 +1469,32 @@  static void set_sensor_evt_enable(IPMIBmcSim *ibs,
 
 static void get_sensor_evt_enable(IPMIBmcSim *ibs,
                                   uint8_t *cmd, unsigned int cmd_len,
-                                  uint8_t *rsp, unsigned int *rsp_len,
-                                  unsigned int max_rsp_len)
+                                  struct rsp_buffer *rsp)
 {
     IPMISensor *sens;
 
     if ((cmd[2] >= MAX_SENSORS) ||
         !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
     sens = ibs->sensors + cmd[2];
-    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
-    IPMI_ADD_RSP_DATA(sens->assert_enable & 0xff);
-    IPMI_ADD_RSP_DATA((sens->assert_enable >> 8) & 0xff);
-    IPMI_ADD_RSP_DATA(sens->deassert_enable & 0xff);
-    IPMI_ADD_RSP_DATA((sens->deassert_enable >> 8) & 0xff);
+    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
+    rsp_buffer_push(rsp, sens->assert_enable & 0xff);
+    rsp_buffer_push(rsp, (sens->assert_enable >> 8) & 0xff);
+    rsp_buffer_push(rsp, sens->deassert_enable & 0xff);
+    rsp_buffer_push(rsp, (sens->deassert_enable >> 8) & 0xff);
 }
 
 static void rearm_sensor_evts(IPMIBmcSim *ibs,
                               uint8_t *cmd, unsigned int cmd_len,
-                              uint8_t *rsp, unsigned int *rsp_len,
-                              unsigned int max_rsp_len)
+                              struct rsp_buffer *rsp)
 {
     IPMISensor *sens;
 
     if ((cmd[2] >= MAX_SENSORS) ||
         !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
     sens = ibs->sensors + cmd[2];
@@ -1528,57 +1508,54 @@  static void rearm_sensor_evts(IPMIBmcSim *ibs,
 
 static void get_sensor_evt_status(IPMIBmcSim *ibs,
                                   uint8_t *cmd, unsigned int cmd_len,
-                                  uint8_t *rsp, unsigned int *rsp_len,
-                                  unsigned int max_rsp_len)
+                                  struct rsp_buffer *rsp)
 {
     IPMISensor *sens;
 
     if ((cmd[2] >= MAX_SENSORS) ||
         !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
     sens = ibs->sensors + cmd[2];
-    IPMI_ADD_RSP_DATA(sens->reading);
-    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
-    IPMI_ADD_RSP_DATA(sens->assert_states & 0xff);
-    IPMI_ADD_RSP_DATA((sens->assert_states >> 8) & 0xff);
-    IPMI_ADD_RSP_DATA(sens->deassert_states & 0xff);
-    IPMI_ADD_RSP_DATA((sens->deassert_states >> 8) & 0xff);
+    rsp_buffer_push(rsp, sens->reading);
+    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
+    rsp_buffer_push(rsp, sens->assert_states & 0xff);
+    rsp_buffer_push(rsp, (sens->assert_states >> 8) & 0xff);
+    rsp_buffer_push(rsp, sens->deassert_states & 0xff);
+    rsp_buffer_push(rsp, (sens->deassert_states >> 8) & 0xff);
 }
 
 static void get_sensor_reading(IPMIBmcSim *ibs,
                                uint8_t *cmd, unsigned int cmd_len,
-                               uint8_t *rsp, unsigned int *rsp_len,
-                               unsigned int max_rsp_len)
+                               struct rsp_buffer *rsp)
 {
     IPMISensor *sens;
 
     if ((cmd[2] >= MAX_SENSORS) ||
             !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
     sens = ibs->sensors + cmd[2];
-    IPMI_ADD_RSP_DATA(sens->reading);
-    IPMI_ADD_RSP_DATA(IPMI_SENSOR_GET_RET_STATUS(sens));
-    IPMI_ADD_RSP_DATA(sens->states & 0xff);
+    rsp_buffer_push(rsp, sens->reading);
+    rsp_buffer_push(rsp, IPMI_SENSOR_GET_RET_STATUS(sens));
+    rsp_buffer_push(rsp, sens->states & 0xff);
     if (IPMI_SENSOR_IS_DISCRETE(sens)) {
-        IPMI_ADD_RSP_DATA((sens->states >> 8) & 0xff);
+        rsp_buffer_push(rsp, (sens->states >> 8) & 0xff);
     }
 }
 
 static void set_sensor_type(IPMIBmcSim *ibs,
-                               uint8_t *cmd, unsigned int cmd_len,
-                               uint8_t *rsp, unsigned int *rsp_len,
-                               unsigned int max_rsp_len)
+                            uint8_t *cmd, unsigned int cmd_len,
+                            struct rsp_buffer *rsp)
 {
     IPMISensor *sens;
 
 
     if ((cmd[2] >= MAX_SENSORS) ||
             !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
     sens = ibs->sensors + cmd[2];
@@ -1587,21 +1564,20 @@  static void set_sensor_type(IPMIBmcSim *ibs,
 }
 
 static void get_sensor_type(IPMIBmcSim *ibs,
-                               uint8_t *cmd, unsigned int cmd_len,
-                               uint8_t *rsp, unsigned int *rsp_len,
-                               unsigned int max_rsp_len)
+                            uint8_t *cmd, unsigned int cmd_len,
+                            struct rsp_buffer *rsp)
 {
     IPMISensor *sens;
 
 
     if ((cmd[2] >= MAX_SENSORS) ||
             !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
-        rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
+        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
         return;
     }
     sens = ibs->sensors + cmd[2];
-    IPMI_ADD_RSP_DATA(sens->sensor_type);
-    IPMI_ADD_RSP_DATA(sens->evt_reading_type_code);
+    rsp_buffer_push(rsp, sens->sensor_type);
+    rsp_buffer_push(rsp, sens->evt_reading_type_code);
 }