[PATCH openpower-host-ipmi-oem] Add IPMI interface to allow FW updates from Host

[Vishwanatha Subbanna]

"openbmc" <openbmc-bounces+vishwanath=in.ibm.com at lists.ozlabs.org> wrote on
09/03/2016 11:04:35 am:

> From: Cyril Bur <cyrilbur at gmail.com>
> To: anoo at us.ibm.com
> Cc: openbmc at lists.ozlabs.org, OpenBMC Patches <openbmc-patches at stwcx.xyz>
> Date: 09/03/2016 11:05 am
> Subject: Re: [PATCH openpower-host-ipmi-oem] Add IPMI interface to
> allow FW updates from Host
> Sent by: "openbmc" <openbmc-bounces
+vishwanath=in.ibm.com at lists.ozlabs.org>
>
> On Fri,  4 Mar 2016 12:20:26 -0600
> OpenBMC Patches <openbmc-patches at stwcx.xyz> wrote:
>
> > From: Adriana Kobylak <anoo at us.ibm.com>
> >
> > New OEM IPMI command to execute commands that would allow the Host
> to have access to the BMC to perform a FW update.
> > ---
> >  Makefile     |  2 +-
> >  oemhandler.C | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  oemhandler.h |  1 +
> >  3 files changed, 54 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index eaa7324..bacdef9 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -10,7 +10,7 @@ CXXFLAGS += -fPIC -Wall
> >  all:  $(LIB_OEM)
> >
> >  %.o: %.C
> > -   $(CXX) -c $< $(CXXFLAGS) -o $@
> > +   $(CXX) -std=c++14 -c $< $(CXXFLAGS) -o $@
> >
>
> This isn't a CXXFLAGS thing?
>
> >  $(LIB_OEM): $(LIB_OEM_OBJ)
> >     $(CXX) $^ -shared $(LDFLAGS) -o $@
> > diff --git a/oemhandler.C b/oemhandler.C
> > index f577d67..22452d6 100644
> > --- a/oemhandler.C
> > +++ b/oemhandler.C
> > @@ -1,7 +1,9 @@
> >  #include "oemhandler.h"
> >  #include <host-ipmid/ipmid-api.h>
> > +#include <fstream>
> >  #include <stdio.h>
> >  #include <string.h>
> > +#include <systemd/sd-bus.h>
> >
> >  void register_netfn_oem_partial_esel() __attribute__((constructor));
> >
> > @@ -70,10 +72,60 @@ ipmi_ret_t
> ipmi_ibm_oem_partial_esel(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
> >     return rc;
> >  }
> >
> > +// Prepare for FW Update.
> > +// Execute needed commands to prepare the system for a fw update
> from the host.
> > +ipmi_ret_t ipmi_ibm_oem_prep_fw_update(ipmi_netfn_t netfn, ipmi_cmd_t
cmd,
> > +                                     ipmi_request_t request,
> ipmi_response_t response,
> > +                                     ipmi_data_len_t data_len,
> ipmi_context_t context)
> > +{
> > +    ipmi_ret_t ipmi_rc = IPMI_CC_OK;
> > +    *data_len = 0;
> > +
> > +    int rc = 0;
> > +    std::ofstream rwfs_file;
> > +    const char  *busname = "org.openbmc.control.Bmc";
> > +    const char  *objname = "/org/openbmc/control/bmc0";
> > +    const char  *iface = "org.openbmc.control.Bmc";
> > +    sd_bus *bus = ipmid_get_sd_bus_connection();
> > +    sd_bus_message *reply = NULL;
> > +    sd_bus_error error = SD_BUS_ERROR_NULL;
> > +    int r = 0;
> > +
> > +    // Set one time flag
> > +    rc = system("fw_setenv openbmconce copy-files-to-ram
copy-base-filesystem-to-ram");
> > +    rc = WEXITSTATUS(rc);
> > +    if (rc != 0) {
> > +        fprintf(stderr, "fw_setenv openbmconce failed with rc=%d\n",
rc);
> > +        return IPMI_CC_UNSPECIFIED_ERROR;
> > +    }
> > +
> > +    // Touch the image-rwfs file to perform an empty update to
> force the save
> > +    // in case we're already in ram and the flash is the same
> causing the ram files
> > +    // to not be copied back to flash
> > +    rwfs_file.open("/run/initramfs/image-rwfs", std::ofstream::out |
std::ofstream::app);

Need to handle the file open and access errors. Also, any reason why
std::ofstream used instead of fopen(3).
fopen may have been lighter IMO.

> > +    rwfs_file.close();

Handle errors on closing. ?

> > +
> > +    // Reboot the BMC for settings to take effect
> > +    r = sd_bus_call_method(bus, busname, objname, iface,
> > +                           "warmReset", &error, &reply, NULL);
> > +    if (r < 0) {
> > +        fprintf(stderr, "Failed to reset BMC: %s\n", strerror(-r));
> > +        return -1;
> > +    }

Return needs to be one of the IPMI specified rc than a "-1". May be
IPMI_CC_INVALID is better suited here. ?
Also this has memory leak on error. Need to free the 'error' and 'reply'.

Also, if this call results in BMC being rebooted, does this call ever
return and the reboot is done Async ?.

> > +    printf("Warning: BMC is going down for reboot!\n");
> > +    sd_bus_error_free(&error);
> > +    reply = sd_bus_message_unref(reply);
> > +
> > +    return ipmi_rc;
> > +}
> >
> >  void register_netfn_oem_partial_esel()
> >  {
> >     printf("Registering NetFn:[0x%X], Cmd:[0x%X]\n",NETFUN_OEM,
> IPMI_CMD_PESEL);
> >     ipmi_register_callback(NETFUN_OEM, IPMI_CMD_PESEL, NULL,
> ipmi_ibm_oem_partial_esel);
> > +
> > +    printf("Registering NetFn:[0x%X], Cmd:[0x%X]\n", NETFUN_OEM,
> IPMI_CMD_PREP_FW_UPDATE);
> > +    ipmi_register_callback(NETFUN_OEM, IPMI_CMD_PREP_FW_UPDATE,
> NULL, ipmi_ibm_oem_prep_fw_update);
> > +
> >     return;
> >  }
> > diff --git a/oemhandler.h b/oemhandler.h
> > index 9cc397f..71ae01c 100644
> > --- a/oemhandler.h
> > +++ b/oemhandler.h
> > @@ -8,6 +8,7 @@
> >  // IPMI commands for net functions.
> >  enum ipmi_netfn_oem_cmds
> >  {
> > +    IPMI_CMD_PREP_FW_UPDATE = 0x10,
> >      IPMI_CMD_PESEL = 0xF0,
> >  };
> >
>
> _______________________________________________
> openbmc mailing list
> openbmc at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160309/7caf470f/attachment-0001.html>