PLDM Message Structure Change
With this change, PLDM code is optimised. The PLDM payload message
structure previously consisted of a pointer to the payload message and
the payload length, this structure is removed with this commit and the
PLDM message structure now has the PLDM header structure and an array of
size 1 whose address is the starting byte of message payload. Therefore,
pldm msg struct can represent a request/response message in contiguous
memory, thereby enabling simple casts and avoiding memcpys to get from a
uint8_t* to pldm msg or vice versa.
This commit also introduces a change to have the response handlers allocate
memory for responses. This is aligned with the message struct change, and
enables varying length responses.
Change-Id: Ia46d852b8b16bfc7cf04f38435bd4079ad33c66b
Signed-off-by: vkaverap <vkaverap@in.ibm.com>
diff --git a/libpldmresponder/base.cpp b/libpldmresponder/base.cpp
index 6ae8ba6..37b4a63 100644
--- a/libpldmresponder/base.cpp
+++ b/libpldmresponder/base.cpp
@@ -22,7 +22,7 @@
{PLDM_BASE, {0xF1, 0xF0, 0xF0, 0x00}},
};
-void getPLDMTypes(const pldm_msg_payload* request, pldm_msg* response)
+Response getPLDMTypes(const pldm_msg* request, size_t payloadLength)
{
// DSP0240 has this as a bitfield8[N], where N = 0 to 7
std::array<bitfield8_t, 8> types{};
@@ -34,30 +34,37 @@
types[index].byte |= 1 << bit;
}
- encode_get_types_resp(0, PLDM_SUCCESS, types.data(), response);
+ Response response(sizeof(pldm_msg_hdr) + PLDM_GET_TYPES_RESP_BYTES, 0);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+ encode_get_types_resp(0, PLDM_SUCCESS, types.data(), responsePtr);
+
+ return response;
}
-void getPLDMCommands(const pldm_msg_payload* request, pldm_msg* response)
+Response getPLDMCommands(const pldm_msg* request, size_t payloadLength)
{
ver32_t version{};
Type type;
- if (request->payload_length != (sizeof(version) + sizeof(type)))
- {
- encode_get_commands_resp(0, PLDM_ERROR_INVALID_LENGTH, nullptr,
- response);
- return;
- }
+ Response response(sizeof(pldm_msg_hdr) + PLDM_GET_COMMANDS_RESP_BYTES, 0);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
- decode_get_commands_req(request, &type, &version);
+ auto rc = decode_get_commands_req(request->payload, payloadLength, &type,
+ &version);
+
+ if (rc != PLDM_SUCCESS)
+ {
+ encode_get_commands_resp(0, rc, nullptr, responsePtr);
+ return response;
+ }
// DSP0240 has this as a bitfield8[N], where N = 0 to 31
std::array<bitfield8_t, 32> cmds{};
if (capabilities.find(type) == capabilities.end())
{
encode_get_commands_resp(0, PLDM_ERROR_INVALID_PLDM_TYPE, nullptr,
- response);
- return;
+ responsePtr);
+ return response;
}
for (const auto& cmd : capabilities.at(type))
@@ -68,24 +75,28 @@
cmds[index].byte |= 1 << bit;
}
- encode_get_commands_resp(0, PLDM_SUCCESS, cmds.data(), response);
+ encode_get_commands_resp(0, PLDM_SUCCESS, cmds.data(), responsePtr);
+
+ return response;
}
-void getPLDMVersion(const pldm_msg_payload* request, pldm_msg* response)
+Response getPLDMVersion(const pldm_msg* request, size_t payloadLength)
{
uint32_t transferHandle;
Type type;
uint8_t transferFlag;
- if (request->payload_length !=
- (sizeof(transferHandle) + sizeof(type) + sizeof(transferFlag)))
- {
- encode_get_version_resp(0, PLDM_ERROR_INVALID_LENGTH, 0, 0, nullptr, 0,
- response);
- return;
- }
+ Response response(sizeof(pldm_msg_hdr) + PLDM_GET_VERSION_RESP_BYTES, 0);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
- decode_get_version_req(request, &transferHandle, &transferFlag, &type);
+ uint8_t rc = decode_get_version_req(request->payload, payloadLength,
+ &transferHandle, &transferFlag, &type);
+
+ if (rc != PLDM_SUCCESS)
+ {
+ encode_get_version_resp(0, rc, 0, 0, nullptr, 4, responsePtr);
+ return response;
+ }
ver32_t version{};
auto search = versions.find(type);
@@ -93,13 +104,15 @@
if (search == versions.end())
{
encode_get_version_resp(0, PLDM_ERROR_INVALID_PLDM_TYPE, 0, 0, nullptr,
- 0, response);
- return;
+ 4, responsePtr);
+ return response;
}
memcpy(&version, &(search->second), sizeof(version));
encode_get_version_resp(0, PLDM_SUCCESS, 0, PLDM_START_AND_END, &version,
- sizeof(pldm_version), response);
+ sizeof(pldm_version), responsePtr);
+
+ return response;
}
} // namespace responder
diff --git a/libpldmresponder/base.hpp b/libpldmresponder/base.hpp
index 74bc4af..466559a 100644
--- a/libpldmresponder/base.hpp
+++ b/libpldmresponder/base.hpp
@@ -11,29 +11,33 @@
using Type = uint8_t;
+using Response = std::vector<uint8_t>;
+
namespace responder
{
/** @brief Handler for getPLDMTypes
*
* @param[in] request - Request message payload
- * @param[out] response - Response message written here
+ * @param[in] payload_length - Request message payload length
+ * @param[return] Response - PLDM Response message
*/
-void getPLDMTypes(const pldm_msg_payload* request, pldm_msg* response);
+Response getPLDMTypes(const pldm_msg* request, size_t payloadLength);
/** @brief Handler for getPLDMCommands
*
* @param[in] request - Request message payload
- * @param[out] response - Response message written here
+ * @param[in] payload_length - Request message payload length
+ * @param[return] Response - PLDM Response message
*/
-void getPLDMCommands(const pldm_msg_payload* request, pldm_msg* response);
+Response getPLDMCommands(const pldm_msg* request, size_t payloadLength);
/** @brief Handler for getPLDMCommands
*
* @param[in] request - Request message payload
- * @param[out] response - Response messsage written here
+ * @param[in] payload_length - Request message payload length
+ * @param[return] Response - PLDM Response message
*/
-void getPLDMVersion(const pldm_msg_payload* request, pldm_msg* response);
-
+Response getPLDMVersion(const pldm_msg* request, size_t payloadLength);
} // namespace responder
} // namespace pldm
diff --git a/libpldmresponder/bios.cpp b/libpldmresponder/bios.cpp
index 5c4a270..11c8251 100644
--- a/libpldmresponder/bios.cpp
+++ b/libpldmresponder/bios.cpp
@@ -47,7 +47,7 @@
} // namespace utils
-void getDateTime(const pldm_msg_payload* request, pldm_msg* response)
+Response getDateTime(const pldm_msg* request)
{
uint8_t seconds = 0;
uint8_t minutes = 0;
@@ -58,6 +58,8 @@
constexpr auto timeInterface = "xyz.openbmc_project.Time.EpochTime";
constexpr auto bmcTimePath = "/xyz/openbmc_project/time/bmc";
+ Response response(sizeof(pldm_msg_hdr) + PLDM_GET_DATE_TIME_RESP_BYTES, 0);
+ auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
std::variant<EpochTimeUS> value;
auto bus = sdbusplus::bus::new_default();
@@ -79,8 +81,8 @@
entry("TIME INTERACE=%s", timeInterface));
encode_get_date_time_resp(0, PLDM_ERROR, seconds, minutes, hours, day,
- month, year, response);
- return;
+ month, year, responsePtr);
+ return response;
}
uint64_t timeUsec = std::get<EpochTimeUS>(value);
@@ -92,7 +94,8 @@
utils::epochToBCDTime(timeSec, seconds, minutes, hours, day, month, year);
encode_get_date_time_resp(0, PLDM_SUCCESS, seconds, minutes, hours, day,
- month, year, response);
+ month, year, responsePtr);
+ return response;
}
} // namespace responder
diff --git a/libpldmresponder/bios.hpp b/libpldmresponder/bios.hpp
index 80a732b..915a367 100644
--- a/libpldmresponder/bios.hpp
+++ b/libpldmresponder/bios.hpp
@@ -2,20 +2,24 @@
#include <stdint.h>
+#include <vector>
+
#include "libpldm/bios.h"
namespace pldm
{
+using Response = std::vector<uint8_t>;
+
namespace responder
{
/** @brief Handler for GetDateTime
*
* @param[in] request - Request message payload
- * @param[out] response - Response message written here
+ * @param[return] Response - PLDM Response message
*/
-void getDateTime(const pldm_msg_payload* request, pldm_msg* response);
+Response getDateTime(const pldm_msg* request);
namespace utils
{