Revert "smbioshandler:move bmc region complete,Write& Lock"
This reverts commit 946723b32e7c800e3a11c9c117654b277f91585b.
This patch is causing ipmid to segfault on boot.
Change-Id: I36322105370142d1224117732a65315038d7167e
Tested: No longer segfaults
Signed-off-by: James Feist <james.feist@linux.intel.com>
diff --git a/include/smbioshandler.hpp b/include/smbioshandler.hpp
index 40e081f..4e403ca 100644
--- a/include/smbioshandler.hpp
+++ b/include/smbioshandler.hpp
@@ -60,6 +60,12 @@
MDRState State;
} __attribute__((packed));
+struct RegionCompleteRequest
+{
+ uint8_t sessionId;
+ uint8_t regionId;
+} __attribute__((packed));
+
struct RegionReadRequest
{
uint8_t regionId;
@@ -74,4 +80,21 @@
uint8_t data[msgPayloadSize];
} __attribute__((packed));
+struct RegionWriteRequest
+{
+ uint8_t sessionId;
+ uint8_t regionId;
+ uint8_t length;
+ uint16_t offset;
+ uint8_t data[msgPayloadSize];
+} __attribute__((packed));
+
+struct RegionLockRequest
+{
+ uint8_t sessionId;
+ uint8_t regionId;
+ uint8_t lockPolicy;
+ uint16_t msTimeout;
+} __attribute__((packed));
+
constexpr size_t maxMDRId = 5;
diff --git a/src/smbioshandler.cpp b/src/smbioshandler.cpp
index 0c288b1..9eebf7f 100644
--- a/src/smbioshandler.cpp
+++ b/src/smbioshandler.cpp
@@ -34,7 +34,6 @@
constexpr const char* DBUS_PROPERTIES = "org.freedesktop.DBus.Properties";
constexpr const char* MDRV1_PATH = "/xyz/openbmc_project/Smbios/MDR_V1";
constexpr const char* MDRV1_INTERFACE = "xyz.openbmc_project.Smbios.MDR_V1";
-static constexpr const uint8_t ccOemSetInProcess = 0x81;
static void register_netfn_smbios_functions() __attribute__((constructor));
@@ -126,62 +125,63 @@
return 0;
}
-/** @brief implements bmc region update Complete command
- * @param sessionId - session Lock Handle
- * @param regionId - data Region
- *
- * @returns IPMI completion code
- */
-ipmi::RspType<> bmcRegionUpdateComplete(uint8_t sessionId, uint8_t regionId)
+ipmi_ret_t cmd_region_complete(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)
{
- std::variant<uint8_t, uint16_t> value;
+ auto requestData = reinterpret_cast<const RegionCompleteRequest*>(request);
+ uint8_t status;
- if ((regionId >= maxMDRId) || (regionId == 0))
+ sdbusplus::message::variant<uint8_t, uint16_t> value;
+
+ if (*data_len != sizeof(RegionCompleteRequest))
+ {
+ *data_len = 0;
+ return IPMI_CC_REQ_DATA_LEN_INVALID;
+ }
+
+ uint8_t regionId = requestData->regionId - 1;
+ *data_len = 0;
+
+ if (regionId >= maxMDRId)
{
phosphor::logging::log<level::ERR>("Invalid region");
- return ipmi::responseParmOutOfRange();
+ return IPMI_CC_PARM_OUT_OF_RANGE;
}
std::shared_ptr<sdbusplus::asio::connection> bus = getSdBus();
std::string service = ipmi::getService(*bus, MDRV1_INTERFACE, MDRV1_PATH);
- uint8_t regionIdLocal = regionId - 1;
-
- if (regionIdLocal == 0)
- {
- phosphor::logging::log<level::ERR>("Invalid region");
- return ipmi::responseParmOutOfRange();
- }
- if (set_regionId(regionIdLocal, service) < 0)
+ if (set_regionId(regionId, service) < 0)
{
phosphor::logging::log<level::ERR>("Error setting regionId");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
if (0 > sdplus_mdrv1_get_property("LockPolicy", value, service))
{
phosphor::logging::log<level::ERR>("Error getting lockPolicy");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
if (regionLockUnlocked == std::get<uint8_t>(value))
{
- return ipmi::responseCommandNotAvailable();
+ return IPMI_CC_PARAMETER_NOT_SUPPORT_IN_PRESENT_STATE;
}
if (0 > sdplus_mdrv1_get_property("SessionId", value, service))
{
phosphor::logging::log<level::ERR>("Error getting sessionId");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
- if (sessionId != std::get<uint8_t>(value))
+ if (requestData->sessionId != std::get<uint8_t>(value))
{
- return ipmi::response(ccOemSetInProcess);
+ return IPMI_CC_OEM_SET_IN_PROCESS;
}
auto method = bus->new_method_call(service.c_str(), MDRV1_PATH,
MDRV1_INTERFACE, "RegionComplete");
- method.append(regionIdLocal);
+ method.append(regionId);
auto reply = bus->call(method);
if (reply.is_method_error())
@@ -190,19 +190,16 @@
"Error set region complete",
phosphor::logging::entry("SERVICE=%s", service.c_str()),
phosphor::logging::entry("PATH=%s", MDRV1_PATH));
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
- uint8_t status = 0;
reply.read(status);
if (status != 0)
- {
phosphor::logging::log<level::ERR>(
"Error set region complete, unexpected error");
- return ipmi::responseUnspecifiedError();
- }
+ return IPMI_CC_UNSPECIFIED_ERROR;
- return ipmi::responseSuccess();
+ return IPMI_CC_OK;
}
ipmi_ret_t cmd_region_read(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
@@ -293,64 +290,71 @@
return IPMI_CC_OK;
}
-/** @brief implements bmc region write command
- * @parameter
- * - sessionId - session Lock Handle
- * - regionId - data Region
- * - length - data Length
- * - offset - data Offset
- * - data - data to be written
- *
- * @returns IPMI completion code plus response data
- * - writeData - contains data Region, valid Data,
- * lock Status, max Region Length, region Used (in bytes)
- */
-ipmi::RspType<std::vector<uint8_t>>
- bmcRegionWrite(uint8_t sessionId, uint8_t regionId, uint8_t length,
- uint16_t offset, std::vector<uint8_t> data)
+ipmi_ret_t cmd_region_write(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)
{
- std::variant<uint8_t, uint16_t> value;
+ auto requestData = reinterpret_cast<const RegionWriteRequest*>(request);
+ uint8_t regionId = requestData->regionId - 1;
+ std::string res;
+ std::vector<uint8_t> writeData;
+ uint16_t index;
+ uint8_t tmp[255];
- if ((regionId >= maxMDRId) || (regionId == 0))
+ size_t minInputLen = &requestData->data[0] - &requestData->sessionId + 1;
+ if (*data_len < minInputLen)
+ { // this command need at least 6 bytes input
+ *data_len = 0;
+ return IPMI_CC_REQ_DATA_LEN_INVALID;
+ }
+
+ sdbusplus::message::variant<uint8_t, uint16_t> value;
+
+ *data_len = 0;
+
+ if (regionId >= maxMDRId)
{
phosphor::logging::log<level::ERR>("Invalid region");
- return ipmi::responseParmOutOfRange();
+ return IPMI_CC_PARM_OUT_OF_RANGE;
}
std::shared_ptr<sdbusplus::asio::connection> bus = getSdBus();
std::string service = ipmi::getService(*bus, MDRV1_INTERFACE, MDRV1_PATH);
- uint8_t regionIdLocal = regionId - 1;
-
- if (set_regionId(regionIdLocal, service) < 0)
+ if (set_regionId(regionId, service) < 0)
{
phosphor::logging::log<level::ERR>("Error setting regionId");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
if (0 > sdplus_mdrv1_get_property("LockPolicy", value, service))
{
phosphor::logging::log<level::ERR>("Error getting lockPolicy");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
if (regionLockUnlocked == std::get<uint8_t>(value))
{
- return ipmi::responseCommandNotAvailable();
+ return IPMI_CC_PARAMETER_NOT_SUPPORT_IN_PRESENT_STATE;
}
if (0 > sdplus_mdrv1_get_property("SessionId", value, service))
{
phosphor::logging::log<level::ERR>("Error getting sessionId");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
- if (sessionId != std::get<uint8_t>(value))
+ if (requestData->sessionId != std::get<uint8_t>(value))
{
- return ipmi::response(ccOemSetInProcess);
+ return IPMI_CC_OEM_SET_IN_PROCESS;
}
- std::vector<uint8_t> writeData;
- std::copy(data.begin(), data.begin() + length,
- writeData.data() + sizeof(regionIdLocal) + sizeof(length));
+ std::copy(&(requestData->length), &(requestData->data[requestData->length]),
+ tmp);
+ writeData.push_back(regionId);
+ for (index = 0; index < minInputLen + requestData->length - 2; index++)
+ {
+ writeData.push_back(tmp[index]);
+ }
+
auto method = bus->new_method_call(service.c_str(), MDRV1_PATH,
MDRV1_INTERFACE, "RegionWrite");
@@ -363,68 +367,67 @@
"Error write region data",
phosphor::logging::entry("SERVICE=%s", service.c_str()),
phosphor::logging::entry("PATH=%s", MDRV1_PATH));
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
- std::string res;
reply.read(res);
if (res == "NoData")
{
- return ipmi::responseParmOutOfRange();
+ return IPMI_CC_PARM_OUT_OF_RANGE;
}
else if (res != "Success")
{
phosphor::logging::log<level::ERR>(
"Error write region data, unexpected error");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
- return ipmi::responseSuccess(writeData);
+ return IPMI_CC_OK;
}
-/** @brief implements bmc region lock command
- * @parameter
- * - sessionId - session Lock Handle
- * - regionId - data Region
- * - lockPolicy - Lock Type
- * - msTimeout - timeout. Number of milliseconds allowed before lock is
- * released
- *
- * @returns IPMI completion code plus response data
- * - lockResponse - session Lock Handle
- */
-ipmi::RspType<uint8_t> // lockResponse
- bmcRegionLock(uint8_t sessionId, uint8_t regionId, uint8_t lockPolicy,
- uint16_t msTimeout)
+ipmi_ret_t cmd_region_lock(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)
{
- std::variant<uint8_t, uint16_t> value;
+ auto requestData = reinterpret_cast<const RegionLockRequest*>(request);
+ uint8_t regionId = requestData->regionId - 1;
+ sdbusplus::message::variant<uint8_t, uint16_t> value;
+ auto res = reinterpret_cast<uint8_t*>(response);
+ uint8_t lockResponse;
- if ((regionId >= maxMDRId) || (regionId == 0))
+ if (*data_len != sizeof(RegionLockRequest))
+ {
+ *data_len = 0;
+ return IPMI_CC_REQ_DATA_LEN_INVALID;
+ }
+
+ *data_len = 0;
+
+ if (regionId >= maxMDRId)
{
phosphor::logging::log<level::ERR>("Invalid region");
- return ipmi::responseParmOutOfRange();
+ return IPMI_CC_PARM_OUT_OF_RANGE;
}
std::shared_ptr<sdbusplus::asio::connection> bus = getSdBus();
std::string service = ipmi::getService(*bus, MDRV1_INTERFACE, MDRV1_PATH);
- uint8_t regionIdLocal = regionId - 1;
- if (set_regionId(regionIdLocal, service) < 0)
+ if (set_regionId(regionId, service) < 0)
{
phosphor::logging::log<level::ERR>("Error setting regionId");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
if (0 > sdplus_mdrv1_get_property("LockPolicy", value, service))
{
phosphor::logging::log<level::ERR>("Error getting lockPolicy");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
- if (lockPolicy == regionLockUnlocked)
+ if (requestData->lockPolicy == regionLockUnlocked)
{
if (regionLockUnlocked == std::get<uint8_t>(value))
{
- return ipmi::responseCommandNotAvailable();
+ return IPMI_CC_PARAMETER_NOT_SUPPORT_IN_PRESENT_STATE;
}
}
if (regionLockUnlocked != std::get<uint8_t>(value))
@@ -432,20 +435,21 @@
if (0 > sdplus_mdrv1_get_property("SessionId", value, service))
{
phosphor::logging::log<level::ERR>("Error getting sessionId");
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
- if (sessionId != std::get<uint8_t>(value))
+ if (requestData->sessionId != std::get<uint8_t>(value))
{
- if (lockPolicy != regionLockStrict)
+ if (requestData->lockPolicy != regionLockStrict)
{
- return ipmi::response(ccOemSetInProcess);
+ return IPMI_CC_OEM_SET_IN_PROCESS;
}
}
}
auto method = bus->new_method_call(service.c_str(), MDRV1_PATH,
MDRV1_INTERFACE, "RegionLock");
- method.append(sessionId, regionIdLocal, lockPolicy, msTimeout);
+ method.append(requestData->sessionId, regionId, requestData->lockPolicy,
+ requestData->msTimeout);
auto reply = bus->call(method);
if (reply.is_method_error())
@@ -454,12 +458,13 @@
"Error lock region ",
phosphor::logging::entry("SERVICE=%s", service.c_str()),
phosphor::logging::entry("PATH=%s", MDRV1_PATH));
- return ipmi::responseUnspecifiedError();
+ return IPMI_CC_UNSPECIFIED_ERROR;
}
- uint8_t lockResponse = 0;
reply.read(lockResponse);
- return ipmi::responseSuccess(lockResponse);
+ *data_len = sizeof(lockResponse);
+ *res = lockResponse;
+ return IPMI_CC_OK;
}
static void register_netfn_smbios_functions(void)
@@ -471,9 +476,9 @@
cmd_region_status, PRIVILEGE_OPERATOR);
// <Update Complete Status Command>
- ipmi::registerHandler(ipmi::prioOemBase, NETFUN_INTEL_APP_OEM,
- IPMI_NETFN_INTEL_OEM_APP_CMD::MDR_COMPLETE,
- ipmi::Privilege::Operator, bmcRegionUpdateComplete);
+ ipmi_register_callback(NETFUN_INTEL_APP_OEM,
+ IPMI_NETFN_INTEL_OEM_APP_CMD::MDR_COMPLETE, NULL,
+ cmd_region_complete, PRIVILEGE_OPERATOR);
// <Read MDR Command>
ipmi_register_callback(NETFUN_INTEL_APP_OEM,
@@ -481,12 +486,12 @@
cmd_region_read, PRIVILEGE_OPERATOR);
// <Write MDR Command>
- ipmi::registerHandler(ipmi::prioOemBase, NETFUN_INTEL_APP_OEM,
- IPMI_NETFN_INTEL_OEM_APP_CMD::MDR_WRITE,
- ipmi::Privilege::Operator, bmcRegionWrite);
+ ipmi_register_callback(NETFUN_INTEL_APP_OEM,
+ IPMI_NETFN_INTEL_OEM_APP_CMD::MDR_WRITE, NULL,
+ cmd_region_write, PRIVILEGE_OPERATOR);
// <Lock MDR Command>
- ipmi::registerHandler(ipmi::prioOemBase, NETFUN_INTEL_APP_OEM,
- IPMI_NETFN_INTEL_OEM_APP_CMD::MDR_LOCK,
- ipmi::Privilege::Operator, bmcRegionLock);
+ ipmi_register_callback(NETFUN_INTEL_APP_OEM,
+ IPMI_NETFN_INTEL_OEM_APP_CMD::MDR_LOCK, NULL,
+ cmd_region_lock, PRIVILEGE_OPERATOR);
}