Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to libpldm APIs for Allocating Instance IDs and change Transport APIs #213

Open
wants to merge 2 commits into
base: 1110
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 69 additions & 7 deletions include/pldm_fw.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#include "types.hpp"

#include <libpldm/instance-id.h>
#include <libpldm/transport.h>
#include <libpldm/transport/af-mctp.h>
#include <libpldm/transport/mctp-demux.h>
#include <stdint.h>

#include <vector>
Expand All @@ -21,12 +25,26 @@ class PldmFramework
/**
* @brief Construtor
*/
PldmFramework() = default;
PldmFramework()
{
int rc = pldm_instance_db_init_default(&pldmInstanceIdDb);
if (rc)
{
throw std::system_category().default_error_condition(rc);
}
}

/**
* @brief Destructor
*/
~PldmFramework() = default;
~PldmFramework()
{
int rc = pldm_instance_db_destroy(pldmInstanceIdDb);
if (rc)
{
std::cout << "pldm_instance_db_destroy failed, rc =" << rc << "\n";
}
}

/**
* @brief Send Panel Function to PHYP.
Expand All @@ -43,6 +61,25 @@ class PldmFramework
/** Host mctp eid */
static constexpr auto mctpEid = (types::Byte)9;

/** @brief Hardcoded TID */
using TerminusID = uint8_t;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this to types.hpp

static constexpr TerminusID tid = mctpEid;

/** @brief PLDM instance ID database object used to get instance IDs
*/
pldm_instance_db* pldmInstanceIdDb = nullptr;

/** pldm transport instance */
pldm_transport* pldmTransport = nullptr;

union TransportImpl
{
pldm_transport_mctp_demux* mctpDemux;
pldm_transport_af_mctp* afMctp;
};

TransportImpl impl;

// Constants required for PLDM packet.
static constexpr auto phypTerminusID = (types::Byte)208;
static constexpr auto frontPanelBoardEntityId = (uint16_t)32837;
Expand Down Expand Up @@ -81,12 +118,37 @@ class PldmFramework

/**
* @brief Get instance ID
* This api returns the instance id by making a dbus call to GetInstanceId
* api of Pldm. Instance id is to uniquely identify a message packet. This
* id is generated by pldm.
* This api returns the instance id by making a dbus call to libpldm
* api. Instance id is to uniquely identify a message packet.
* @return pldm_instance_id_t instance id.
*/
pldm_instance_id_t getInstanceID();

/**
* @brief Free PLDM requester instance id
* @param[in] pdrs - Panel PDR data.
*/
void freeInstanceID(pldm_instance_id_t instanceID);

/**
* @brief setup PLDM transport for sending and receiving messages
*
* @return file descriptor
*/
int openPldm();

/** @brief Opens the MCTP socket for sending and receiving messages.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Format: Move this to next line please.

*
* @return one byte instance id.
*/
types::Byte getInstanceID();
int openMctpDemuxTransport();

/** @brief Opens the MCTP AF_MCTP for sending and receiving messages.
*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is AF_MCTP? what it does? and why it is required here?
can you please add those details in doxygen.

*/
int openAfMctpTransport();

/** @brief Close the PLDM file */
void pldmClose();

};
} // namespace panel
10 changes: 10 additions & 0 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ systemd_system_unit_dir = systemd.get_variable('systemdsystemunitdir')

service_file = 'service_files/com.ibm.panel.service'

if(get_option('transport-implementation')) == 'mctp-demux'
add_project_arguments('-DPLDM_TRANSPORT_WITH_MCTP_DEMUX', language : 'cpp')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please add a comment here on what does the options DPLDM_TRANSPORT_WITH_MCTP_DEMUX and DPLDM_TRANSPORT_WITH_AF_MCTP do?

else
add_project_arguments('-DPLDM_TRANSPORT_WITH_AF_MCTP', language: 'cpp')
endif

if cxx.has_header('poll.h')
add_project_arguments('-DPLDM_HAS_POLL=1', language: 'cpp')
endif

if get_option('system-vpd-dependency').enabled()
service_file = 'service_files/ibm.panel.system.vpd.dependent.service'
endif
Expand Down
6 changes: 6 additions & 0 deletions meson_options.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
option('tests', type: 'feature', value: 'enabled', description: 'Build tests.',)
option('system-vpd-dependency', type: 'feature', description: 'Enable/disable system vpd dependency.', value: 'disabled')
option(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, which one needs to be used and based on what criteria is it decided?

'transport-implementation',
type: 'combo',
choices: ['mctp-demux', 'af-mctp'],
description: 'transport via af-mctp or mctp-demux'
)
172 changes: 149 additions & 23 deletions src/pldm_fw.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove extra line.

#include "pldm_fw.hpp"

#include "exception.hpp"
Expand All @@ -7,33 +8,159 @@
#include <libpldm/platform.h>
#include <libpldm/pldm.h>
#include <libpldm/state_set.h>
#include <poll.h>

#include <iostream>
#include <sdbusplus/asio/connection.hpp>
#include <sdbusplus/asio/object_server.hpp>

namespace panel
{
types::Byte PldmFramework::getInstanceID()
pldm_instance_id_t PldmFramework::getInstanceID()
{
types::Byte instanceId = 0;
auto bus = sdbusplus::bus::new_default();
pldm_instance_id_t instanceID = 0;

try
auto rc = pldm_instance_id_alloc(pldmInstanceIdDb, tid, &instanceID);
if (rc == -EAGAIN)
{
auto method = bus.new_method_call(
"xyz.openbmc_project.PLDM", "/xyz/openbmc_project/pldm",
"xyz.openbmc_project.PLDM.Requester", "GetInstanceId");
method.append(mctpEid);
auto reply = bus.call(method);
reply.read(instanceId);
std::this_thread::sleep_for(std::chrono::milliseconds(100));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a comment about why this 100ms sleep is required? And how did we reach to this number?

rc = pldm_instance_id_alloc(pldmInstanceIdDb, tid, &instanceID);
}
catch (const sdbusplus::exception::SdBusError& e)

if (rc)
{
std::cerr << e.what() << std::endl;
std::cerr << "Return code = " << rc << std::endl;
throw FunctionFailure("pldm: call to GetInstanceId failed.");
}
return instanceId;
std::cout << "Got instanceId: " << static_cast<int>(instanceID)
<< " from PLDM eid: " << static_cast<int>(tid) << std::endl;
return instanceID;
}

void PldmFramework::freeInstanceID(pldm_instance_id_t instanceID)
{
auto rc = pldm_instance_id_free(pldmInstanceIdDb, tid, instanceID);
if (rc)
{
std::cerr << "pldm_instance_id_free failed to free id = "
<< static_cast<int>(instanceID)
<< " of tid = " << static_cast<int>(tid) << " rc = " << rc
<< std::endl;
}
}

int PldmFramework::openPldm()
{
auto fd = -1;
if (pldmTransport)
{
std::cerr << "open: pldmTransport already setup!" << std::endl;
throw FunctionFailure("Required host dump action via PLDM is not "
"allowed due to openPldm failed");
return fd;
}

#if defined(PLDM_TRANSPORT_WITH_MCTP_DEMUX)
fd = openMctpDemuxTransport();
#elif defined(PLDM_TRANSPORT_WITH_AF_MCTP)
fd = openAfMctpTransport();
#else
std::cerr << "open: No valid transport defined!" << std::endl;
throw FunctionFailure("Required host dump action via PLDM is not allowed: "
"No valid transport defined!");
#endif

if (fd < 0)
{
auto e = errno;
std::cerr << "openPldm failed, errno: " << e << ", FD: " << fd
<< std::endl;
throw FunctionFailure("Required host dump action via PLDM is not "
"allowed due to openPldm failed");
}
return fd;
}

[[maybe_unused]] int PldmFramework::openMctpDemuxTransport()
{
impl.mctpDemux = nullptr;
int rc = pldm_transport_mctp_demux_init(&impl.mctpDemux);
if (rc)
{
std::cerr << "openMctpDemuxTransport: Failed to init MCTP demux "
"transport. rc = "
<< rc << std::endl;
throw FunctionFailure("Failed to init MCTP demux transport");
}

rc = pldm_transport_mctp_demux_map_tid(impl.mctpDemux, tid, tid);
if (rc)
{
std::cerr << "openMctpDemuxTransport: Failed to setup tid to eid "
"mapping. rc = "
<< rc << std::endl;
pldmClose();
throw FunctionFailure("Failed to setup tid to eid mapping");
}
pldmTransport = pldm_transport_mctp_demux_core(impl.mctpDemux);

struct pollfd pollfd;
rc = pldm_transport_mctp_demux_init_pollfd(pldmTransport, &pollfd);
if (rc)
{
std::cerr << "openMctpDemuxTransport: Failed to get pollfd. rc = " << rc
<< std::endl;
pldmClose();
throw FunctionFailure("Failed to get pollfd");
}
return pollfd.fd;
}

[[maybe_unused]] int PldmFramework::openAfMctpTransport()
{
impl.afMctp = nullptr;
int rc = pldm_transport_af_mctp_init(&impl.afMctp);
if (rc)
{
std::cerr
<< "openAfMctpTransport: Failed to init AF MCTP transport. rc = "
<< rc << std::endl;
throw FunctionFailure("Failed to init AF MCTP transport");
}

rc = pldm_transport_af_mctp_map_tid(impl.afMctp, tid, tid);
if (rc)
{
std::cerr
<< "openAfMctpTransport: Failed to setup tid to eid mapping. rc = "
<< rc << std::endl;
pldmClose();
throw FunctionFailure("Failed to setup tid to eid mapping");
}
pldmTransport = pldm_transport_af_mctp_core(impl.afMctp);

struct pollfd pollfd;
rc = pldm_transport_af_mctp_init_pollfd(pldmTransport, &pollfd);
if (rc)
{
std::cerr << "openAfMctpTransport: Failed to get pollfd. rc = " << rc
<< std::endl;
pldmClose();
throw FunctionFailure("Failed to get pollfd");
}
return pollfd.fd;
}

void PldmFramework::pldmClose()
{
#if defined(PLDM_TRANSPORT_WITH_MCTP_DEMUX)
pldm_transport_mctp_demux_destroy(impl.mctpDemux);
impl.mctpDemux = nullptr;
#elif defined(PLDM_TRANSPORT_WITH_AF_MCTP)
pldm_transport_af_mctp_destroy(impl.afMctp);
impl.afMctp = nullptr;
#endif
pldmTransport = nullptr;
}

void PldmFramework::fetchPanelEffecterStateSet(const types::PdrList& pdrs,
Expand Down Expand Up @@ -128,7 +255,7 @@ void PldmFramework::sendPanelFunctionToPhyp(
return;
}

types::Byte instance = getInstanceID();
pldm_instance_id_t instance = getInstanceID();

types::PldmPacket packet =
prepareSetEffecterReq(pdrs, instance, funcNumber);
Expand All @@ -144,10 +271,10 @@ void PldmFramework::sendPanelFunctionToPhyp(
return;
}

int fd = pldm_open();
int fd = openPldm();
if (fd == -1)
{
std::cerr << "pldm_open() failed with error = " << strerror(errno)
std::cerr << "openPldm() failed with error = " << strerror(errno)
<< std::endl;
std::map<std::string, std::string> additionalData{};
additionalData.emplace("DESCRIPTION",
Expand All @@ -156,6 +283,7 @@ void PldmFramework::sendPanelFunctionToPhyp(
utils::createPEL("com.ibm.Panel.Error.HostCommunicationError",
"xyz.openbmc_project.Logging.Entry.Level.Warning",
additionalData);
freeInstanceID(instance);
return;
}

Expand All @@ -167,14 +295,12 @@ void PldmFramework::sendPanelFunctionToPhyp(
}
std::cout << std::endl;

auto rc = pldm_send(mctpEid, fd, packet.data(), packet.size());

if (close(fd) == -1)
{
std::cerr << "Close on File descriptor failed with error = "
<< strerror(errno) << std::endl;
}
pldm_tid_t pldmTID = static_cast<pldm_tid_t>(mctpEid);
auto rc = pldm_transport_send_msg(pldmTransport, pldmTID, packet.data(),
packet.size());

freeInstanceID(instance);
pldmClose();
if (rc)
{
std::map<std::string, std::string> additionalData{};
Expand Down