<font face="Default Sans Serif,Verdana,Arial,Helvetica,sans-serif" size="2"><br>On 3/9/2016, "Vishwanatha Subbanna" <<a target="_blank" href="mailto:vishwanath@in.ibm.com">vishwanath@in.ibm.com</a>> wrote:<br>> Cyril Bur <<a target="_blank" href="mailto:cyrilbur@gmail.com">cyrilbur@gmail.com</a>> wrote:<br>>> On Fri, 4 Mar 2016 12:20:26 -0600<br>>> OpenBMC Patches <<a target="_blank" href="mailto:openbmc-patches@stwcx.xyz">openbmc-patches@stwcx.xyz</a>> wrote:<br>>> <br>>> > From: Adriana Kobylak <<a target="_blank" href="mailto:anoo@us.ibm.com">anoo@us.ibm.com</a>><br>>> > <br>>> > New OEM IPMI command to execute commands that would allow the<br>>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>I've seen some projects say CFLAGS etc are for the makefile<br>invoker not for changing for the needs of the individual file being<br>compiled. Others have mechanisims to extend the flags.<br><br>Is the standard only needed to pick up the attributes on the stream?<br><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()<br>>__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>><br>>> from the host.<br>>> > +ipmi_ret_t ipmi_ibm_oem_prep_fw_update(ipmi_netfn_t netfn,<br>>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<br>>copy-base-filesystem-to-ram");<br>>> > + rc = WEXITSTATUS(rc);<br>>> > + if (rc != 0) {<br>>> > + fprintf(stderr, "fw_setenv openbmconce failed with<br>>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",<br>>std::ofstream::out | std::ofstream::app);<br>><br>>Need to handle the file open and access errors. Also, any reason why<br>>std::ofstream used instead of fopen(3).<br>>fopen may have been lighter IMO.<br>><br><br>Actually a simple open system call would be even cheaper as no file <br>IO buffers are needed. We only need to make sure the file exists <br>and create it if not, without changing it if it does exist.<br><br>>> > + rwfs_file.close();<br>><br>>Handle errors on closing. ?<br>><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",<br>>strerror(-r));<br>>> > + return -1;<br>>> > + }<br>><br>>Return needs to be one of the IPMI specified rc than a "-1". May be<br>>IPMI_CC_INVALID is better suited here. ?<br>>Also this has memory leak on error. Need to free the 'error' and<br>>'reply'.<br>><br>>Also, if this call results in BMC being rebooted, does this call ever<br>>return and the reboot is done Async ?.<br>><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>><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>milton<br></font><BR>