[v2,04/10] ipmi: add rsp_buffer_set_error() helper

The third byte in the response buffer of an IPMI command holds the
error code. In many IPMI command handlers, this byte is updated
directly. This patch adds a helper routine to clarify why this byte is
being used.

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

On 03/02/2016 04:14 AM, Cédric Le Goater wrote:
> The third byte in the response buffer of an IPMI command holds the
> error code. In many IPMI command handlers, this byte is updated
> directly. This patch adds a helper routine to clarify why this byte is
> being used.
>
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>

Yes, more clear.

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

> ---
>   hw/ipmi/ipmi_bmc_sim.c | 115 ++++++++++++++++++++++++++-----------------------
>   1 file changed, 60 insertions(+), 55 deletions(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 72166a7da291..2626aff06c60 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -271,11 +271,16 @@ struct rsp_buffer {
>
>   #define RSP_BUFFER_INITIALIZER { { 0 }, 0, MAX_IPMI_MSG_SIZE }
>
> +static inline void rsp_buffer_set_error(struct rsp_buffer *rsp, uint8_t byte)
> +{
> +    rsp->buffer[2] = byte;
> +}
> +
>   /* Add a byte to the response. */
>   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;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED);
>           return;
>       }
>       rsp->buffer[rsp->len++] = byte;
> @@ -285,7 +290,7 @@ 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;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_TRUNCATED);
>           return;
>       }
>
> @@ -599,7 +604,7 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>       /* Set up the response, set the low bit of NETFN. */
>       /* Note that max_rsp_len must be at least 3 */
>       if (rsp.max_len < 3) {
> -        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +        rsp_buffer_set_error(&rsp, IPMI_CC_REQUEST_DATA_TRUNCATED);
>           goto out;
>       }
>
> @@ -609,17 +614,17 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
>
>       /* If it's too short or it was truncated, return an error. */
>       if (cmd_len < 2) {
> -        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +        rsp_buffer_set_error(&rsp, IPMI_CC_REQUEST_DATA_LENGTH_INVALID);
>           goto out;
>       }
>       if (cmd_len > max_cmd_len) {
> -        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
> +        rsp_buffer_set_error(&rsp, IPMI_CC_REQUEST_DATA_TRUNCATED);
>           goto out;
>       }
>
>       if ((cmd[0] & 0x03) != 0) {
>           /* Only have stuff on LUN 0 */
> -        rsp.buffer[2] = IPMI_CC_COMMAND_INVALID_FOR_LUN;
> +        rsp_buffer_set_error(&rsp, IPMI_CC_COMMAND_INVALID_FOR_LUN);
>           goto out;
>       }
>
> @@ -629,12 +634,12 @@ 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.buffer[2] = IPMI_CC_INVALID_CMD;
> +        rsp_buffer_set_error(&rsp, IPMI_CC_INVALID_CMD);
>           goto out;
>       }
>
>       if (cmd_len < ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]].cmd_len_min) {
> -        rsp.buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +        rsp_buffer_set_error(&rsp, IPMI_CC_REQUEST_DATA_LENGTH_INVALID);
>           goto out;
>       }
>
> @@ -745,26 +750,26 @@ static void chassis_control(IPMIBmcSim *ibs,
>
>       switch (cmd[2] & 0xf) {
>       case 0: /* power down */
> -        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
> +        rsp_buffer_set_error(rsp, k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0));
>           break;
>       case 1: /* power up */
> -        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0);
> +        rsp_buffer_set_error(rsp, k->do_hw_op(s, IPMI_POWERON_CHASSIS, 0));
>           break;
>       case 2: /* power cycle */
> -        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
> +        rsp_buffer_set_error(rsp, k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0));
>           break;
>       case 3: /* hard reset */
> -        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
> +        rsp_buffer_set_error(rsp, k->do_hw_op(s, IPMI_RESET_CHASSIS, 0));
>           break;
>       case 4: /* pulse diagnostic interrupt */
> -        rsp->buffer[2] = k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0);
> +        rsp_buffer_set_error(rsp, k->do_hw_op(s, IPMI_PULSE_DIAG_IRQ, 0));
>           break;
>       case 5: /* soft shutdown via ACPI by overtemp emulation */
> -        rsp->buffer[2] = k->do_hw_op(s,
> -                             IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 0);
> +        rsp_buffer_set_error(rsp, k->do_hw_op(s,
> +                                          IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 0));
>           break;
>       default:
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>   }
> @@ -903,7 +908,7 @@ static void read_evt_msg_buf(IPMIBmcSim *ibs,
>       unsigned int i;
>
>       if (!(ibs->msg_flags & IPMI_BMC_MSG_FLAG_EVT_BUF_FULL)) {
> -        rsp->buffer[2] = 0x80;
> +        rsp_buffer_set_error(rsp, 0x80);
>           return;
>       }
>       for (i = 0; i < 16; i++) {
> @@ -921,7 +926,7 @@ static void get_msg(IPMIBmcSim *ibs,
>
>       qemu_mutex_lock(&ibs->lock);
>       if (QTAILQ_EMPTY(&ibs->rcvbufs)) {
> -        rsp->buffer[2] = 0x80; /* Queue empty */
> +        rsp_buffer_set_error(rsp, 0x80); /* Queue empty */
>           goto out;
>       }
>       rsp_buffer_push(rsp, 0); /* Channel 0 */
> @@ -965,18 +970,18 @@ static void send_msg(IPMIBmcSim *ibs,
>
>       if (cmd[2] != 0) {
>           /* We only handle channel 0 with no options */
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>
>       if (cmd_len < 10) {
> -        rsp->buffer[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_LENGTH_INVALID);
>           return;
>       }
>
>       if (cmd[3] != 0x40) {
>           /* We only emulate a MC at address 0x40. */
> -        rsp->buffer[2] = 0x83; /* NAK on write */
> +        rsp_buffer_set_error(rsp, 0x83); /* NAK on write */
>           return;
>       }
>
> @@ -1065,7 +1070,7 @@ static void reset_watchdog_timer(IPMIBmcSim *ibs,
>                                    struct rsp_buffer *rsp)
>   {
>       if (!ibs->watchdog_initialized) {
> -        rsp->buffer[2] = 0x80;
> +        rsp_buffer_set_error(rsp, 0x80);
>           return;
>       }
>       do_watchdog_reset(ibs);
> @@ -1081,7 +1086,7 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>
>       val = cmd[2] & 0x7; /* Validate use */
>       if (val == 0 || val > 5) {
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>       val = cmd[3] & 0x7; /* Validate action */
> @@ -1090,22 +1095,22 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>           break;
>
>       case IPMI_BMC_WATCHDOG_ACTION_RESET:
> -        rsp->buffer[2] = k->do_hw_op(s, IPMI_RESET_CHASSIS, 1);
> +        rsp_buffer_set_error(rsp, k->do_hw_op(s, IPMI_RESET_CHASSIS, 1));
>           break;
>
>       case IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN:
> -        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1);
> +        rsp_buffer_set_error(rsp, k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 1));
>           break;
>
>       case IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE:
> -        rsp->buffer[2] = k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1);
> +        rsp_buffer_set_error(rsp, k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 1));
>           break;
>
>       default:
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>       }
>       if (rsp->buffer[2]) {
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>
> @@ -1118,14 +1123,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->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +            rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>               return;
>           }
>           break;
>
>       default:
>           /* We don't support PRE_SMI */
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>
> @@ -1201,7 +1206,7 @@ static void get_sdr(IPMIBmcSim *ibs,
>
>       if (cmd[6]) {
>           if ((cmd[2] | (cmd[3] << 8)) != ibs->sdr.reservation) {
> -            rsp->buffer[2] = IPMI_CC_INVALID_RESERVATION;
> +            rsp_buffer_set_error(rsp, IPMI_CC_INVALID_RESERVATION);
>               return;
>           }
>       }
> @@ -1209,14 +1214,14 @@ static void get_sdr(IPMIBmcSim *ibs,
>       pos = 0;
>       if (sdr_find_entry(&ibs->sdr, cmd[4] | (cmd[5] << 8),
>                          &pos, &nextrec)) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>
>       sdrh = (struct ipmi_sdr_header *) &ibs->sdr.sdr[pos];
>
>       if (cmd[6] > ipmi_sdr_length(sdrh)) {
> -        rsp->buffer[2] = IPMI_CC_PARM_OUT_OF_RANGE;
> +        rsp_buffer_set_error(rsp, IPMI_CC_PARM_OUT_OF_RANGE);
>           return;
>       }
>
> @@ -1228,7 +1233,7 @@ static void get_sdr(IPMIBmcSim *ibs,
>       }
>
>       if ((cmd[7] + rsp->len) > rsp->max_len) {
> -        rsp->buffer[2] = IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES;
> +        rsp_buffer_set_error(rsp, IPMI_CC_CANNOT_RETURN_REQ_NUM_BYTES);
>           return;
>       }
>
> @@ -1243,7 +1248,7 @@ static void add_sdr(IPMIBmcSim *ibs,
>       struct ipmi_sdr_header *sdrh = (struct ipmi_sdr_header *) cmd + 2;
>
>       if (sdr_add_entry(ibs, sdrh, cmd_len - 2, &recid)) {
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>       rsp_buffer_push(rsp, recid & 0xff);
> @@ -1255,12 +1260,12 @@ static void clear_sdr_rep(IPMIBmcSim *ibs,
>                             struct rsp_buffer *rsp)
>   {
>       if ((cmd[2] | (cmd[3] << 8)) != ibs->sdr.reservation) {
> -        rsp->buffer[2] = IPMI_CC_INVALID_RESERVATION;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_RESERVATION);
>           return;
>       }
>
>       if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>       if (cmd[7] == 0xaa) {
> @@ -1272,7 +1277,7 @@ static void clear_sdr_rep(IPMIBmcSim *ibs,
>       } else if (cmd[7] == 0) {
>           rsp_buffer_push(rsp, 1); /* Erasure complete */
>       } else {
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>   }
> @@ -1315,22 +1320,22 @@ static void get_sel_entry(IPMIBmcSim *ibs,
>
>       if (cmd[6]) {
>           if ((cmd[2] | (cmd[3] << 8)) != ibs->sel.reservation) {
> -            rsp->buffer[2] = IPMI_CC_INVALID_RESERVATION;
> +            rsp_buffer_set_error(rsp, IPMI_CC_INVALID_RESERVATION);
>               return;
>           }
>       }
>       if (ibs->sel.next_free == 0) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>       if (cmd[6] > 15) {
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>       if (cmd[7] == 0xff) {
>           cmd[7] = 16;
>       } else if ((cmd[7] + cmd[6]) > 16) {
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       } else {
>           cmd[7] += cmd[6];
> @@ -1340,7 +1345,7 @@ static void get_sel_entry(IPMIBmcSim *ibs,
>       if (val == 0xffff) {
>           val = ibs->sel.next_free - 1;
>       } else if (val >= ibs->sel.next_free) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>       if ((val + 1) == ibs->sel.next_free) {
> @@ -1360,7 +1365,7 @@ static void add_sel_entry(IPMIBmcSim *ibs,
>                             struct rsp_buffer *rsp)
>   {
>       if (sel_add_event(ibs, cmd + 2)) {
> -        rsp->buffer[2] = IPMI_CC_OUT_OF_SPACE;
> +        rsp_buffer_set_error(rsp, IPMI_CC_OUT_OF_SPACE);
>           return;
>       }
>       /* sel_add_event fills in the record number. */
> @@ -1373,12 +1378,12 @@ static void clear_sel(IPMIBmcSim *ibs,
>                         struct rsp_buffer *rsp)
>   {
>       if ((cmd[2] | (cmd[3] << 8)) != ibs->sel.reservation) {
> -        rsp->buffer[2] = IPMI_CC_INVALID_RESERVATION;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_RESERVATION);
>           return;
>       }
>
>       if (cmd[4] != 'C' || cmd[5] != 'L' || cmd[6] != 'R') {
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>       if (cmd[7] == 0xaa) {
> @@ -1390,7 +1395,7 @@ static void clear_sel(IPMIBmcSim *ibs,
>       } else if (cmd[7] == 0) {
>           rsp_buffer_push(rsp, 1); /* Erasure complete */
>       } else {
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>   }
> @@ -1430,7 +1435,7 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>               !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> @@ -1466,7 +1471,7 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs,
>           }
>           break;
>       case 3:
> -        rsp->buffer[2] = IPMI_CC_INVALID_DATA_FIELD;
> +        rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
>           return;
>       }
>       IPMI_SENSOR_SET_RET_STATUS(sens, cmd[3]);
> @@ -1480,7 +1485,7 @@ static void get_sensor_evt_enable(IPMIBmcSim *ibs,
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>           !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> @@ -1499,7 +1504,7 @@ static void rearm_sensor_evts(IPMIBmcSim *ibs,
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>           !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> @@ -1519,7 +1524,7 @@ static void get_sensor_evt_status(IPMIBmcSim *ibs,
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>           !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> @@ -1539,7 +1544,7 @@ static void get_sensor_reading(IPMIBmcSim *ibs,
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>               !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> @@ -1560,7 +1565,7 @@ static void set_sensor_type(IPMIBmcSim *ibs,
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>               !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>       sens = ibs->sensors + cmd[2];
> @@ -1577,7 +1582,7 @@ static void get_sensor_type(IPMIBmcSim *ibs,
>
>       if ((cmd[2] >= MAX_SENSORS) ||
>               !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> -        rsp->buffer[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +        rsp_buffer_set_error(rsp, IPMI_CC_REQ_ENTRY_NOT_PRESENT);
>           return;
>       }
>       sens = ibs->sensors + cmd[2];