Skip to content

1120 rebase #695

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

Merged
merged 107 commits into from
Jun 18, 2025
Merged

1120 rebase #695

merged 107 commits into from
Jun 18, 2025

Conversation

Pavithrab7
Copy link
Contributor

REBASE set 1

@Pavithrab7 Pavithrab7 requested review from manojkiraneda and ArchanaKakani and removed request for manojkiraneda and ArchanaKakani May 6, 2025 07:13
@Pavithrab7 Pavithrab7 force-pushed the 1120_rebase branch 14 times, most recently from bb1f794 to ea386a5 Compare May 13, 2025 06:54
williamspatrick and others added 9 commits June 12, 2025 18:17
The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines.  Possible replacements are for:
  * bus_t
  * exception_t
  * manager_t
  * match_t
  * message_t
  * object_t
  * slot_t

Change-Id: I0e907f161bc6aec5e2253893f4ad7189cb8d5705
Signed-off-by: Patrick Williams <[email protected]>
Added eventManager to handle sensor event class(00h) which is defined in
table 11 of DSP0248 v1.3.0. In this commit, the eventManager supports to
receive event asynchronously. The commit will also log the Ipmitool SEL
log and Redfish log for PLDM sensor event messages.

Change-Id: I1b337ccae454067841ffbbd8754631216a995542
Signed-off-by: Thu Nguyen <[email protected]>
Signed-off-by: Gilbert Chen <[email protected]>
The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines.  Possible replacements are for:
  * bus_t
  * exception_t
  * manager_t
  * match_t
  * message_t
  * object_t
  * slot_t

Change-Id: I10cbc1d55e259c972eac765b28b8c73281b1b42c
Signed-off-by: Patrick Williams <[email protected]>
As we discussed in gerrit 51489 before, we reached consensus for the
pldm sensor state as following states:

PLDM_SENSOR_ENABLED      : Functional True,  Available True
PLDM_SENSOR_DISABLED     : Functional True,  Available False
PLDM_SENSOR_UNAVAILABLE  : Functional False, Available False
PLDM_SENSOR_FAILED       : Functional False, Available True
PLDM_SENSOR_INITIALIZING : Functional False, Available False

Correct the states handling for PLDM_SENSOR_DISABLED and
PLDM_SENSOR_FAILED.

Change-Id: I0cab9ab59a1e9c0f5c6d841c529a64cc4bb8caee
Signed-off-by: Ricky CX Wu <[email protected]>
Handle CPER event(0x07) which is defined in `Table 11 - PLDM Event
Type` and section `16.17 eventData format for CPEREvent` in DSP0248
v1.3.0.

The code supports:
1. Handle the PLDM event which has eventClass as CPEREvent (0x07).
2. Store the CPER data in PLDM CPER event to file at `/var/cper/`.
3. Call `CreateDump` method of `xyz.openbmc_project.Dump.Manager` D-Bus
service to create dump fault log.
4. The user can find the dump fault logs in Redfish FaultLog entries
thru URL `/redfish/v1/Managers/bmc/LogServices/FaultLog/Entries`. Each
CPER entry includes the URL to download the created CPER data file
`/redfish/v1/Managers/bmc/LogServices/FaultLog/Entries/<id>/attachment`.
5. The user can use `cper-parser` in `libcper` to parse the CPER data in
the attached file.

Signed-off-by: Thu Nguyen <[email protected]>
Change-Id: I85c53933183178c6b5acdfc12c805e8a4cf1ca2a
BMC uses PLDM `EventMessageSupported` command to get
`synchronyConfigurationSupported` by terminus as `16.8
EventMessageSupported Command` in DSP0248 V1.3.0. When the PLDM terminus
does not support this command, BMC will not know the supported
`synchronyConfiguration` type.

Change default `synchronyConfigurationSupported` of termini from
`disabled` to `enableAsyncKeepAlive`. This allows BMC still logs the
events from the terminus whenever it is willing to send the event to
BMC.

With this changing, BMC will call PLDM `SetEventReceiver` command to set
`synchronyConfiguration` mode of terminus to `enableAsyncKeepAlive`
(Asynchronous messaging with heartbeat). The default heartbeat is 120
and can be configured thru `heartbeat-timeout-seconds` option. This
allows BMC still logs the events from the terminus whenever it is
willing to send the event to BMC.

Signed-off-by: Thu Nguyen <[email protected]>
Change-Id: I0c76e2867800da50ca2b795b022bde223db29f2d
The recent changes in `libpldm`[1] introduced a modification in the
size definitions for PLDM requests. Specifically, the size for
`PLDM_SET_EVENT_RECEIVER_REQ_BYTES` was changed from 5 bytes to 3
bytes, with the addition of a new definition
`PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES` (2 bytes).

Previously, the event manager code used the
`PLDM_SET_EVENT_RECEIVER_REQ_BYTES` definition directly, which
accounted for 5 bytes. However, after the update in `libpldm`, the test
cases began failing when run under Valgrind due to invalid reads, as
the total request size was no longer correctly accounted for.

This commit resolves the issue by adjusting the size of the request in
the `setEventReceiver` method, using both the
`PLDM_SET_EVENT_RECEIVER_REQ_BYTES` and
`PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES` definitions to ensure the
proper size is used.

Tested by:
```bash
meson test -t 10 -C build --print-errorlogs --wrapper "valgrind --error-exitcode=1"
```

[1]: openbmc/libpldm@8c43abb

Change-Id: Ieb86d764b0c60a79d0a7f0f92f60ddf8812e6e84
Signed-off-by: Manojkiran Eda <[email protected]>
As [1], The sensor D-Bus object path `/xyz/openbmc_project/sensors/` and
`xyz.openbmc_project.Sensor.Value` interface are only used for the
telemetry sensor types which are listed. The non-telemetry PLDM sensor
with the `baseUnit` type `count`, `corrected_errors`,
`uncorrected_errors` and `oemunit` are none-telemetry types and are not
belong to that list.
Those unit types are metric types which are count of somethings so the
metric D-Bus object path `/xyz/openbmc_project/metric/` and D-Bus
`xyz.openbmc_project.Metric.Value` should be used as [2]. Support
creating D-Bus object path and polling sensor values for the numeric
sensor/compact numeric sensor PDRs with the BaseUnit type is
none-telemetry.

[1] https://github.com/openbmc/phosphor-dbus-interfaces/blob/90cfce16584253a5f524c718ce5a6ae7c33f7b8c/yaml/xyz/openbmc_project/Sensor/Value.interface.yaml#L1
[2] https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/yaml/xyz/openbmc_project/Sensor/Value.interface.yaml

Signed-off-by: Thu Nguyen <[email protected]>
Change-Id: Iaf8d842e6ec0cb082b139d15da1a1bd10a701acf
This reverts commit 35f2594.

The related libpldm commit [1] has also been reverted, as it poses the
risk of affecting other users of libpldm beyond just pldm. Addressing
the issue solely within pldm would only provide a limited fix.

Therefore, we have decided to revert the changes in order to explore a
more comprehensive solution within libpldm, ensuring compatibility for
all users.

[1] https://gerrit.openbmc.org/c/openbmc/libpldm/+/75123

Change-Id: If4b0f91195f9defe7f015f9e3fb686f33931fa01
Signed-off-by: Manojkiran Eda <[email protected]>
Pavithrab7 and others added 24 commits June 12, 2025 18:17
reinterpret_cast is prohibited by the C++ core guidelines because
it takes the behavior outside the language definition and gives
problems with type safety. Placement-new on the other-hand allows
to control the object storage while still properly instantiating
an object,keeping the behavior inside the C++ language
specification.

Change-Id: I75dd9e2372b32bf4258c497152f61cd0ca6ad9bd
Signed-off-by: Pavithra Barithaya <[email protected]>
`addNumericSensor` and `addCompactNumericSensor` functions currently try
to construct the sensor name it self. This causes code duplication.

This commit separates that work into `getSensorNames` function, which
returns a vector of sensor name strings that might enlist Sensor
Auxiliary Names PDRs. Depending on the existence of the mentioned PDR,
and the sensor count that the PDR supports, the size of the returned
vector might vary. At this time, callers only get the first element of
the vector, but this function can cover future implementations of
Compact Numeric Sensor.

Tested: There's no functional change applied, so the behavior is the
same after this commit.

Change-Id: I02844f5d0820b0dea2d16d1695350e92ef0383d2
Signed-off-by: Chau Ly <[email protected]>
After all PDRs retrieved from the other terminus are parsed into pdr
structs. They will be processed further to create sensor objects (e.g
via `addNumericSensor` function for Numeric Sensors).

During this phase for one sensor, sensor name is achieved (may enlist
Sensor Aux Name PDRs), and NumericSensor object is constructed.
Sensor object construction involves parsing pdr struct elements into
sensor class variables, D-Bus interface initialization and many calls
to set D-Bus object properties.

They are actually not blocking actions, but as it continuously loops
through sensor PDRs in `parseTerminusPDRs` to add sensors, they take
too much time and prevent BMC from processing events coming from other
termini. Not to mention the adding of new sensor types in the future
and the increase in sensor numbers, the total time will be large, while
this is not a primary task in the initialization phase of a terminus.

This commit defers `addNumericSensor` and `addCompactNumericSensor`
calls using sdeventplus::source::Defer event source, instead of
continuously calling them while looping through the sensor PDRs, to let
events coming from other termini break in and be processed by BMC.

Tested:

While BMC is getting, parsing sensor PDRs and creating sensor objects
from those PDRs, events can still be received from the other termini.

Change-Id: I4a3bacd4139b51c302e36614757fa1b5e5105f21
Signed-off-by: Chau Ly <[email protected]>
In the current state , pldm build breaks when attempting to perform
a debug-optimized build (`-O2` optimization), leading to the following
linker error:

```
undefined reference to `pldm::platform_mc::TerminusManager::getActiveEidByName
(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'
```

This issue is not encountered in the CI environment, as CI uses the
`-Og` optimization flag, which does not aggressively inline functions.
Consequently, the reference to `getActiveEidByName()` is resolved
without issue. However, when building the project with default
optimizations (debugoptimized [-O2]), the build fails because the
linker cannot resolve the reference to `getActiveEidByName()`, which is
inlined by the compiler.

To address this problem, there are three potential solutions:

1. Prevent Inlining of the Function:
   We could use `__attribute__((noinline))` to prevent the compiler
   from inlining `getActiveEidByName()`.

2. Move Source Files into `libpldmresponder`:
   We could move the `platform-mc/manager.cpp` and
   `platform-mc/terminus_manager.cpp` files into the `libpldmresponder`
   so the compiler can resolve the reference directly within the
   library.

3. Migrate `dbus_to_terminus_effecter.cpp` to the `platform-mc` folder:

The most appropriate solution appears to be migrating the
`dbus_to_terminus_effecter.cpp` file into the `platform-mc` directory.
This file is not inherently tied to `libpldmresponder` but functions as
a requester. Additionally, there are existing community patches that
allow the system to scale from a single host terminus to multiple
terminii, further justifying this move. So, solution ibm-openbmc#3 is the most
fitting at this stage. By relocating the `dbus_to_terminus_effecter`
code to the `platform-mc` folder, we can ensure proper modularity,
while also resolving the build issue in a clean and scalable manner.

Tested By:
1. meson build -Doptimization=2 works fine with the patchset.

Change-Id: I0ac8be58253bfb0394500f1d34e8431c6103c924
Signed-off-by: Manojkiran Eda <[email protected]>
libpldm is now an external dependency rather than an in-tree library.
Adjust the includes accordingly.

Change-Id: Ib2590b823039d3127d65f66976b294a2fb88e9c1
Signed-off-by: Manojkiran Eda <[email protected]>
reinterpret_cast is prohibited by the C++ core guidelines because
it takes the behavior outside the language definition and gives
problems with type safety. Placement-new on the other-hand allows
to control the object storage while still properly instantiating
an object,keeping the behavior inside the C++ language
specification.

Change-Id: I58dd021d2947e72919d33778e9e7832a50e79bae
Signed-off-by: Pavithra Barithaya <[email protected]>
reinterpret_cast is prohibited by the C++ core guidelines because
it takes the behavior outside the language definition and gives
problems with type safety. Placement-new on the other-hand allows
to control the object storage while still properly instantiating
an object,keeping the behavior inside the C++ language
specification.

Change-Id: Iaebfaa78c4d3c0c3658dbf9f2f6eaa932b061e83
Signed-off-by: Pavithra Barithaya <[email protected]>
The "NotifyDump" method was newly introduced in the PDI with
additional parameters [1]. This commit changes the Notify
method to the new "NotifyDump" method.

[1]: https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/yaml/com/ibm/Dump/Notify.interface.yaml

Change-Id: Id37cda0eaf3ecb7bc0ad823f659a993376a4edb0
Signed-off-by: Pavithra Barithaya <[email protected]>
The commits adds handler support for the oem-ibm file I/O
newFileAvailableWithMetaData command.
This also adds a support for NotifyDump method call as part
of DumpHandler for the newFileAvailableWithMetaData command.

Tested: The newFileAvailableWithMetaData command was honored and
the Notify Dump was triggered when the file type was DUMP.

Change-Id: I654c4586341019850b3010e975a9948ed22b50f9
Signed-off-by: Pavithra Barithaya <[email protected]>
The flight data recorder has a performance cost and should be enabled
only when necessary. In the case of PLDM T5, where large amount of data
is transferred to device there is a performance cost and disabling
improved performance time.

By disabling this feature, we observed a
performance improvement of approximately 25 seconds for packages
exceeding 140MB in size.

Change-Id: I08d175e4e19d511befd4894068e53737b2c7e6b0
Signed-off-by: P Arun Kumar Reddy <[email protected]>
reinterpret_cast is prohibited by the C++ core guidelines because
it takes the behavior outside the language definition and gives
problems with type safety. Placement-new on the other-hand allows
to control the object storage while still properly instantiating
an object,keeping the behavior inside the C++ language
specification.

Change-Id: If96afcb2c9dbea8fb6815d74a2181ca9ed04903a
Signed-off-by: Pavithra Barithaya <[email protected]>
reinterpret_cast is prohibited by the C++ core guidelines because
it takes the behavior outside the language definition and gives
problems with type safety. Placement-new on the other-hand allows
to control the object storage while still properly instantiating
an object,keeping the behavior inside the C++ language
specification.

Change-Id: I9147096ac333b2ec02f137b19d13566cc0ede105
Signed-off-by: Pavithra Barithaya <[email protected]>
reinterpret_cast is prohibited by the C++ core guidelines because
it takes the behavior outside the language definition and gives
problems with type safety. Placement-new on the other-hand allows
to control the object storage while still properly instantiating
an object,keeping the behavior inside the C++ language
specification.

Change-Id: Id5d75d41a83fbbae109506a8059e756596ba1cbc
Signed-off-by: Pavithra Barithaya <[email protected]>
reinterpret_cast is prohibited by the C++ core guidelines because
it takes the behavior outside the language definition and gives
problems with type safety. Placement-new on the other-hand allows
to control the object storage while still properly instantiating
an object,keeping the behavior inside the C++ language
specification.

Change-Id: Ifab9ea58b932db11d7af0b9def119bed1bfdc44d
Signed-off-by: Pavithra Barithaya <[email protected]>
Added SetTID command to set the TID of a terminus.

Tested results:
```
root@bmc:~# pldmtool base SetTID -h
set the Terminus ID (TID) for a PLDM Terminus.
Usage: pldmtool base SetTID [OPTIONS]

Options:
  -h,--help                   Print this help message and exit
  -m,--mctp_eid UINT          MCTP endpoint ID
  -v,--verbose
  -t,--tid UINT REQUIRED      The TID to be set.
                              Special value: 0x00, 0xFF = reserved.
root@bmc:~# pldmtool base SetTID -m 42 -t 36
{
    "completionCode": 0
}

```

Change-Id: I7c6d371ea8462fe7ea35420f7bf885743e41f1a4
Signed-off-by: Roger G. Coscojuela <[email protected]>
Add error handling for pollEndpointQueue in registerRequest to
prevent incorrect PLDM_SUCCESS returns.
This ensures SendRecvMsgSender can handle send failures, preventing
discoverMctpTerminusTask from stalling and remaining incomplete.

Change-Id: I153934490ee848166e7dce5892136bbbdeeacdd9
Signed-off-by: Eric Yang <[email protected]>
crc32 is deprecated in the libpldm API due to a lack of a common symbol
prefix. Migrate to pldm_edac_crc32().

Change-Id: I7336267c2325d72b9d0666e0222a591d1468ded4
Signed-off-by: Andrew Jeffery <[email protected]>
The original design of the Requester D-Bus API was ill-considered. While
it offered allocation functionality, it did not provide a corresponding
mechanism to release instance IDs. Consequently, pldmd was forced to
monitor all PLDM traffic and implement workarounds to manage instance ID
lifecycles on behalf of applications, effectively assuming the role of
global instance ID manager.

Additionally, many applications duplicated PLDM logic - implementing
their own retry mechanisms - and, in some cases, violated the PLDM
specification by setting timeouts as high as 40 seconds.

Now that all applications rely directly on the libpldm allocator, it
makes sense to remove this legacy interface entirely to prevent any
future usage.

Change-Id: Iadc3a036a65877de164447fbd334ca9bf085ee1c
Signed-off-by: Andrew Jeffery <[email protected]>
Add helper utility to map numeric PLDM completion codes
to human-readable strings. This enhances readability
and debugging when interpreting PLDM responses.

Change-Id: I6dfe98741fe51203c26562ae988d74d8fd093f09
Signed-off-by: Rajeev Ranjan <[email protected]>
Add support for the PLDM RequestUpdate command defined
in the PLDM Firmware Update specification. This enables
pldmtool to initiate firmware update requests with
parameters such as maximum transfer size, number of
components, and component image version info.

Test Results:
```

pldmtool fw_update RequestUpdate --max_transfer_size 32
--num_comps 2 --max_transfer_reqs 10 --package_data_length
 10 --comp_img_ver_str_type ASCII --comp_img_ver_str_len 4
--comp_img_set_ver_str ver1 -m 13

{
  "CompletionCode": "SUCCESS",
  "FirmwareDeviceMetaDataLength": 0,
  "FDWillSendGetPackageDataCommand": "0x00"
}

```
Change-Id: I02ac06070dbb12a5756cae440dff28788751dceb
Signed-off-by: Rajeev Ranjan <[email protected]>
This commit contains changes to Dynamic I/O drawer attachment
value for some specific system configurations.

Tested By:
Verified that changes got reflected in BMC GUI. System power on
operation worked good.

Change-Id: I31c62053414a170de99e148f4d3b5a86285bc177
Signed-off-by: Jayashankar Padath <[email protected]>
To support UNR in pldm sensors, add fatal_high/fatal_low bit
of PDR, and then the values will update to ThresholdHardShutdown
interface for pldm sensors.

Tested:
- Check the UNR of the pldm sensors

root@bmc:~# mfg-tool sensor-display 2>/dev/null | table-sensor-display
sensor                                  ...  units    ... UNR
----------------------------------------...  -------- ... -------
SENTINEL_DOME_SLOT_1_MB_SSD_BOOT_TEMP_C      DegreesC     85
SENTINEL_DOME_SLOT_1_MB_SSD_DATA_TEMP_C      DegreesC     85
SENTINEL_DOME_SLOT_1_MB_VR_CPU0_TEMP_C       DegreesC     125
SENTINEL_DOME_SLOT_1_MB_VR_CPU1_TEMP_C       DegreesC     125
SENTINEL_DOME_SLOT_1_MB_VR_PVDD11_TEMP_C     DegreesC     125
SENTINEL_DOME_SLOT_1_MB_VR_PVDDIO_TEMP_C     DegreesC     125

Change-Id: If146a6191fc864988218c39f36038abff1f5d772
Signed-off-by: Zoey YJ Chung <[email protected]>
Added support to fetch pdr record handles from the repository change
request(pldmPDRRepositoryChgEvent).
The change data record in repository change event supports 3
event data operations ADDED, REMOVED, MODIFIED. For the event data
format "FormatPDRHandles" fetching the pdr handles from the request
for all the event data operations.

Change-Id: I82bcf7ae8455eb5974646eecb4cf4a5e5e503669
Signed-off-by: Archana Kakani <[email protected]>
Currently, pldmd listens for new MCTP endpoint exposed by mctpd, but
they only shows their EID, Network Id, and SupportedMessageTypes, which
cannot fulfill some requirements, e.g., get the device's name or which
board it belongs to.

In openbmc, the additional information are exposed by Entity
Manager[1][2], so add this ability to `MctpDiscovery, it queries the
association between MCTP endpoints and Entity Manager configurations
from MCTP Reactor, when a new MCTP endpoint has been register by mctpd.

Added unit test for this commit to validate the association does work,
passed.

[1]: https://github.com/openbmc/entity-manager/blob/master/schemas/mctp.json
[2]: https://gerrit.openbmc.org/c/openbmc/dbus-sensors/+/69111

Change-Id: Ibf1717f1840e527f21bd8397e747ae121e2dcd25
Signed-off-by: Unive Tien <[email protected]>
The commit adds some CI fixes and clang-format errors.

Signed-off-by: Pavithra Barithaya <[email protected]>
@rfrandse rfrandse merged commit 06797bc into ibm-openbmc:1120 Jun 18, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.