[PATCH] Retrieve BMC version info via IPMI

[Joel Stanley]

On Tue, Feb 9, 2016 at 12:54 PM, Sam Mendoza-Jonas <sam at mendozajonas.com> wrote:
> On BMC machines the "Get Device ID" and "Get BMC Golden Side Version"

Say "AMI BMC machines", as I think the golden side implementation is
AMI specific.

> IPMI commands are available. If possible retrieve some interesting
> version numbers and display them in the System Information screen.

>  enum ipmi_sensor_ids {
> diff --git a/discover/platform-powerpc.c b/discover/platform-powerpc.c
> index 8434d6f..b942c99 100644
> --- a/discover/platform-powerpc.c
> +++ b/discover/platform-powerpc.c
> @@ -1018,6 +1018,94 @@ static void get_ipmi_bmc_mac(struct platform *p, uint8_t *buf)
>                 }
>                 pb_debug("\n");
>         }
> +
> +}
> +
> +/*
> + * Retrieve info from the "Get Device ID" IPMI commands.
> + * See Chapter 20.1 in the IPMIv2 specification.
> + */
> +static void get_ipmi_bmc_versions(struct platform *p, struct system_info *info)
> +{
> +       struct platform_powerpc *platform = p->platform_data;
> +       uint16_t resp_len = 16;
> +       uint8_t resp[16], bcd;
> +       uint32_t aux_version;
> +       int i, rc;
> +
> +       /* Retrieve info from current side */
> +       rc = ipmi_transaction(platform->ipmi, IPMI_NETFN_APP,
> +                       IPMI_CMD_APP_GET_DEVICE_ID,
> +                       NULL, 0,
> +                       resp, &resp_len,
> +                       ipmi_timeout);
> +
> +       pb_debug("BMC version resp [%d][%d]:\n", rc, resp_len);
> +       if (resp_len > 0) {
> +               for (i = 0; i < resp_len; i++) {
> +                       pb_debug(" %x", resp[i]);
> +               }
> +               pb_debug("\n");
> +       }
> +
> +       if (rc == 0 && resp_len == 16) {
> +               info->bmc_current = talloc_array(info, char *, 4);
> +               info->n_bmc_current = 4;
> +
> +               info->bmc_current[0] = talloc_asprintf(info, "Device ID: 0x%x",
> +                                               resp[1]);
> +               info->bmc_current[1] = talloc_asprintf(info, "Device Rev: 0x%x",
> +                                               resp[2]);
> +               bcd = resp[4] & 0x0f;
> +               bcd += 10 * (resp[4] >> 4);

My eyes!

Perhaps a macro for these two lines? You could reuse it below.

> +               aux_version = *(uint32_t *)(&resp[12]);

I know no one uses Petitboot on anything other than power, but for the
sake of clean code: are we aligned here?

> +               info->bmc_current[2] = talloc_asprintf(info,
> +                                               "Firmware version: %u.%u.%u",
> +                                               resp[3], bcd, aux_version);
> +               bcd = resp[5] & 0x0f;
> +               bcd += 10 * (resp[5] >> 4);
> +               info->bmc_current[3] = talloc_asprintf(info, "IPMI version: %u",
> +                                               bcd);
> +       } else
> +               pb_log("Failed to retrieve Device ID from IPMI");
> +
> +       /* Retrieve info from golden side */

What do we expect this to do on non-AMI BMCs, such as OpenBMC?

> +       memset(resp, 0, sizeof(resp));
> +       resp_len = 16;
> +       rc = ipmi_transaction(platform->ipmi, IPMI_NETFN_AMI,
> +                       IPMI_CMD_APP_GET_DEVICE_ID_GOLDEN,
> +                       NULL, 0,
> +                       resp, &resp_len,
> +                       ipmi_timeout);
> +
> +       pb_debug("BMC golden resp [%d][%d]:\n", rc, resp_len);
> +       if (resp_len > 0) {
> +               for (i = 0; i < resp_len; i++) {
> +                       pb_debug(" %x", resp[i]);
> +               }
> +               pb_debug("\n");
> +       }
> +