<html><body><p><tt>"openbmc" <openbmc-bounces+vishwanath=in.ibm.com@lists.ozlabs.org> wrote on 09/03/2016 11:04:35 am:<br><br>> From: Cyril Bur <cyrilbur@gmail.com></tt><br><tt>> To: anoo@us.ibm.com</tt><br><tt>> Cc: openbmc@lists.ozlabs.org, OpenBMC Patches <openbmc-patches@stwcx.xyz></tt><br><tt>> Date: 09/03/2016 11:05 am</tt><br><tt>> Subject: Re: [PATCH openpower-host-ipmi-oem] Add IPMI interface to <br>> allow FW updates from Host</tt><br><tt>> Sent by: "openbmc" <openbmc-bounces+vishwanath=in.ibm.com@lists.ozlabs.org></tt><br><tt>> <br>> On Fri, 4 Mar 2016 12:20:26 -0600<br>> OpenBMC Patches <openbmc-patches@stwcx.xyz> wrote:<br>> <br>> > From: Adriana Kobylak <anoo@us.ibm.com><br>> > <br>> > New OEM IPMI command to execute commands that would allow the Host<br>> to have access to the BMC to perform a FW update.<br>> > ---<br>> > Makefile | 2 +-<br>> > oemhandler.C | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++<br>> > oemhandler.h | 1 +<br>> > 3 files changed, 54 insertions(+), 1 deletion(-)<br>> > <br>> > diff --git a/Makefile b/Makefile<br>> > index eaa7324..bacdef9 100644<br>> > --- a/Makefile<br>> > +++ b/Makefile<br>> > @@ -10,7 +10,7 @@ CXXFLAGS += -fPIC -Wall<br>> > all: $(LIB_OEM)<br>> > <br>> > %.o: %.C<br>> > - $(CXX) -c $< $(CXXFLAGS) -o $@<br>> > + $(CXX) -std=c++14 -c $< $(CXXFLAGS) -o $@<br>> > <br>> <br>> This isn't a CXXFLAGS thing?<br>> <br>> > $(LIB_OEM): $(LIB_OEM_OBJ)<br>> > $(CXX) $^ -shared $(LDFLAGS) -o $@<br>> > diff --git a/oemhandler.C b/oemhandler.C<br>> > index f577d67..22452d6 100644<br>> > --- a/oemhandler.C<br>> > +++ b/oemhandler.C<br>> > @@ -1,7 +1,9 @@<br>> > #include "oemhandler.h"<br>> > #include <host-ipmid/ipmid-api.h><br>> > +#include <fstream><br>> > #include <stdio.h><br>> > #include <string.h><br>> > +#include <systemd/sd-bus.h><br>> > <br>> > void register_netfn_oem_partial_esel() __attribute__((constructor));<br>> > <br>> > @@ -70,10 +72,60 @@ ipmi_ret_t <br>> ipmi_ibm_oem_partial_esel(ipmi_netfn_t netfn, ipmi_cmd_t cmd,<br>> > return rc;<br>> > }<br>> > <br>> > +// Prepare for FW Update.<br>> > +// Execute needed commands to prepare the system for a fw update <br>> from the host.<br>> > +ipmi_ret_t ipmi_ibm_oem_prep_fw_update(ipmi_netfn_t netfn, ipmi_cmd_t cmd,<br>> > + ipmi_request_t request, <br>> ipmi_response_t response,<br>> > + ipmi_data_len_t data_len, <br>> ipmi_context_t context)<br>> > +{<br>> > + ipmi_ret_t ipmi_rc = IPMI_CC_OK;<br>> > + *data_len = 0;<br>> > +<br>> > + int rc = 0;<br>> > + std::ofstream rwfs_file;<br>> > + const char *busname = "org.openbmc.control.Bmc";<br>> > + const char *objname = "/org/openbmc/control/bmc0";<br>> > + const char *iface = "org.openbmc.control.Bmc";<br>> > + sd_bus *bus = ipmid_get_sd_bus_connection();<br>> > + sd_bus_message *reply = NULL;<br>> > + sd_bus_error error = SD_BUS_ERROR_NULL;<br>> > + int r = 0;<br>> > +<br>> > + // Set one time flag<br>> > + rc = system("fw_setenv openbmconce copy-files-to-ram copy-base-filesystem-to-ram");<br>> > + rc = WEXITSTATUS(rc);<br>> > + if (rc != 0) {<br>> > + fprintf(stderr, "fw_setenv openbmconce failed with rc=%d\n", rc);<br>> > + return IPMI_CC_UNSPECIFIED_ERROR; <br>> > + }<br>> > +<br>> > + // Touch the image-rwfs file to perform an empty update to <br>> force the save<br>> > + // in case we're already in ram and the flash is the same <br>> causing the ram files<br>> > + // to not be copied back to flash<br>> > + rwfs_file.open("/run/initramfs/image-rwfs", std::ofstream::out | std::ofstream::app);</tt><br><br><tt>Need to handle the file open and access errors. Also, any reason why std::ofstream used instead of fopen(3).</tt><br><tt>fopen may have been lighter IMO.</tt><br><tt><br>> > + rwfs_file.close();</tt><br><br><tt>Handle errors on closing. ?</tt><br><tt><br>> > +<br>> > + // Reboot the BMC for settings to take effect<br>> > + r = sd_bus_call_method(bus, busname, objname, iface,<br>> > + "warmReset", &error, &reply, NULL);<br>> > + if (r < 0) {<br>> > + fprintf(stderr, "Failed to reset BMC: %s\n", strerror(-r));<br>> > + return -1;<br>> > + }</tt><br><br><tt>Return needs to be one of the IPMI specified rc than a "-1". May be IPMI_CC_INVALID is better suited here. ?</tt><br><tt>Also this has memory leak on error. Need to free the 'error' and 'reply'.</tt><br><br><tt>Also, if this call results in BMC being rebooted, does this call ever return and the reboot is done Async ?.</tt><br><tt><br>> > + printf("Warning: BMC is going down for reboot!\n");<br>> > + sd_bus_error_free(&error);<br>> > + reply = sd_bus_message_unref(reply);<br>> > +<br>> > + return ipmi_rc;<br>> > +}<br>> > <br>> > void register_netfn_oem_partial_esel()<br>> > {<br>> > printf("Registering NetFn:[0x%X], Cmd:[0x%X]\n",NETFUN_OEM, <br>> IPMI_CMD_PESEL);<br>> > ipmi_register_callback(NETFUN_OEM, IPMI_CMD_PESEL, NULL, <br>> ipmi_ibm_oem_partial_esel);<br>> > +<br>> > + printf("Registering NetFn:[0x%X], Cmd:[0x%X]\n", NETFUN_OEM, <br>> IPMI_CMD_PREP_FW_UPDATE);<br>> > + ipmi_register_callback(NETFUN_OEM, IPMI_CMD_PREP_FW_UPDATE, <br>> NULL, ipmi_ibm_oem_prep_fw_update);<br>> > +<br>> > return;<br>> > }<br>> > diff --git a/oemhandler.h b/oemhandler.h<br>> > index 9cc397f..71ae01c 100644<br>> > --- a/oemhandler.h<br>> > +++ b/oemhandler.h<br>> > @@ -8,6 +8,7 @@<br>> > // IPMI commands for net functions.<br>> > enum ipmi_netfn_oem_cmds<br>> > {<br>> > + IPMI_CMD_PREP_FW_UPDATE = 0x10,<br>> > IPMI_CMD_PESEL = 0xF0,<br>> > };<br>> > <br>> <br>> _______________________________________________<br>> openbmc mailing list<br>> openbmc@lists.ozlabs.org<br>> <a href="https://lists.ozlabs.org/listinfo/openbmc">https://lists.ozlabs.org/listinfo/openbmc</a><br></tt><BR>
</body></html>