dsp: firmware_update: Avoid integer overflow

A large fw_device_pkg_data_length could cause uint16_t
calc_min_record_length to wrap around. Instead use a size_t.

Change-Id: I1e0ee5a350d82cb477fd0955

dsp: firmware_update: Avoid integer overflow

A large fw_device_pkg_data_length could cause uint16_t
calc_min_record_length to wrap around. Instead use a size_t.

Change-Id: I1e0ee5a350d82cb477fd0955a11ded659a5c5933
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>

show more ...

dsp: Add FD side firmware_update encode/decode

This implements FD counterparts for firmware update (type 5)
encoding/decoding.

In tests after encoding a message, a subsequent decode is performed an

dsp: Add FD side firmware_update encode/decode

This implements FD counterparts for firmware update (type 5)
encoding/decoding.

In tests after encoding a message, a subsequent decode is performed and
the outputs are compared. This tests the FD portion of the message
decoding.

Change-Id: I5454acee19588b0679a9b0218588fc4c0a66b01d
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>

show more ...

clang-format: re-format for clang-19

clang-format-19 isn't compatible with the clang-format-18 output, so we
need to reformat the code with the latest version. A few parameters
in clang-tidy have b

clang-format: re-format for clang-19

clang-format-19 isn't compatible with the clang-format-18 output, so we
need to reformat the code with the latest version. A few parameters
in clang-tidy have been deprecated, so adjust the style file
accordingly.

See Ie2f6eb3b043f2d655c9df806815afd7971fd0947 for updated style.
See I88192b41ab7a95599a90915013579608af7bc56f for clang-19 enablement.

Change-Id: Ie38f1c94398df0d663d0e27d2e2e1d0d77a47fae
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

show more ...

dsp: firmware_update: Add iterator for downstream device parameters

The previous attempt where we invented a struct that made it possible to
hold full-sized version strings was awkward on several fr

dsp: firmware_update: Add iterator for downstream device parameters

The previous attempt where we invented a struct that made it possible to
hold full-sized version strings was awkward on several fronts. Replace
it with an iterator in the style of the downstream device descriptors.

Change-Id: If9b83f4704b3068de9113af7451051c086f39969
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: firmware_update: Expand "params" in symbol names

Try to keep the names aligned with the spec so that they're more easily
searched for. We can abbreviate other words such as request, response,
l

dsp: firmware_update: Expand "params" in symbol names

Try to keep the names aligned with the spec so that they're more easily
searched for. We can abbreviate other words such as request, response,
length etc as necessary.

Change-Id: Ia5a2c93c153c70107be0fcddb1043b2e08cdd026
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: firmware_update: pack decomposed parameters to struct

There're two APIs that have decomposed parameters:
`encode_query_downstream_identifiers_req()` and
`encode_get_downstream_firmware_params_r

dsp: firmware_update: pack decomposed parameters to struct

There're two APIs that have decomposed parameters:
`encode_query_downstream_identifiers_req()` and
`encode_get_downstream_firmware_params_req(),
which against the checklist of API/ABI stabilization,
squashed those parameters to a struct to meet the request.

Change-Id: Ia952251cf8dcaeba060985e759e1d7aadf7b5b4d
Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com>

show more ...

dsp: firmware_update: Change return type of downstream device ABI/APIs

So far all of the downstream device related ABI/APIs were marked as
`TESTING`, before stabilize them, any deprecated code shoul

dsp: firmware_update: Change return type of downstream device ABI/APIs

So far all of the downstream device related ABI/APIs were marked as
`TESTING`, before stabilize them, any deprecated code should be
removed, including PLDM Completion Code, therefore, change all of the
return type of these to `ERRNO`.

Change-Id: Ie6b390fcc1c91a425f9181ec4ce4495729baab51
Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com>

show more ...

meson: Specify OEM extensions in an array

Prevent a proliferation of options by exploiting choice-constrained
arrays.

Change-Id: I5bb5c9bfd41d2854fafee189734abcf58b058d82
Signed-off-by: Andrew Jeff

meson: Specify OEM extensions in an array

Prevent a proliferation of options by exploiting choice-constrained
arrays.

Change-Id: I5bb5c9bfd41d2854fafee189734abcf58b058d82
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: firmware_update: Fix missing #include <span>

std::span started being used in
dec237bcdafe ("tests: firmware_update: Clean up some use of
reinterpret_cast<>()").

It is unclear why CI was OK, a

dsp: firmware_update: Fix missing #include <span>

std::span started being used in
dec237bcdafe ("tests: firmware_update: Clean up some use of
reinterpret_cast<>()").

It is unclear why CI was OK, a failure is reported with gcc
13.2.0-23ubuntu4 (building externally, not OpenBMC tools).

gitlint-ignore: UC1, B1
Fixes: dec237bcdafe ("tests: firmware_update: Clean up some use of reinterpret_cast<>()")
Change-Id: If5bfbcc72ddcbd31764d84338cf9f915c9d467a0
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>

show more ...

dsp: firmware_update: Iterators for downstream device descriptors

Provide an ergonomic and safe means to iterate through downstream
devices and their descriptors while avoiding the need to allocate.

dsp: firmware_update: Iterators for downstream device descriptors

Provide an ergonomic and safe means to iterate through downstream
devices and their descriptors while avoiding the need to allocate. The
strategy leaves the library user to make their own choices about how the
data is handled.

The user-facing portion of the change takes the form of two new macros,
to be nested inside one another:

```
foreach_pldm_downstream_device(...) {
foreach_pldm_downstream_device_descriptor(...) {
// Do something with the device-specific descriptor
}
}
```

Examples uses are provided in the documentation and in changes to the
test suite.

Change-Id: I6e79454b94868da73f318635bcaae57cd51fbf97
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

tests: firmware_update: Clean up some use of reinterpret_cast<>()

I'm reworking the QueryDownstreamIdentifiers tests, so let's get them
into reasonable shape before further modifications.

The prima

tests: firmware_update: Clean up some use of reinterpret_cast<>()

I'm reworking the QueryDownstreamIdentifiers tests, so let's get them
into reasonable shape before further modifications.

The primary change is replacing open-coded buffer sizing, allocation,
and type-casting with use of PLDM_MSG_DEFINE_P(), which gives us a
pointer to an appropriately-aligned stack-allocated instance of `struct
pldm_msg` with appropriate space in the object for the payload flexible
array.

Change-Id: Ic01a84caa14df1300c48c0049ac3bf6318987ff3
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

tests: firmware_update: Fix 'complition' typo

Change-Id: I787f2d2a226761c78bac66b646d80380281f0be6
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

dsp: platform: Fix location of closing paren in overflow detection

I suspect this was the result of editor auto-parenthesis support and the
result got overlooked.

Add some tests while we're in the

dsp: platform: Fix location of closing paren in overflow detection

I suspect this was the result of editor auto-parenthesis support and the
result got overlooked.

Add some tests while we're in the area.

As seems to be the case when we expand the tests associated with
argument values, also update the ABI dump to reflect the change in
recorded register allocation.

gitlint-ignore: UC1
Fixes: #13
Fixes: ad33b99abcc4 ("dsp: platform: Bounds check encode_state_effecter_pdr()")
Reported-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Change-Id: Iab4c1c337400678ac424936151a38baf0e0d554d
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: platform: Fix decode_set_event_receiver_req()

Per DSP0248 V1.3.0 table13, the heartbeatTimer field shall be omitted
from the request data if eventMessageGlobalEnable is not set to
enableAsyncKe

dsp: platform: Fix decode_set_event_receiver_req()

Per DSP0248 V1.3.0 table13, the heartbeatTimer field shall be omitted
from the request data if eventMessageGlobalEnable is not set to
enableAsyncKeepAlive.

Rework the change in 8c43abb due to the issue found in
openbmc/pldm@35f25949fe4d ("Fix invalid read by adjusting request
size")

gitlint-ignore: B1, UC1
Fixes: 66c7723adbdc ("msgbuf: Enable pldm_msgbuf_extract() into packed members")
Fixes: 9667f5823930 ("platform: pldm_msgbuf for decode_set_event_receiver_req()")
Fixes: 6ef2aa90a793 ("platform: Test invalid heartbeat conditions after assignment")
Fixes: 9c76679224cf ("libpldm: Migrate to subproject")
Change-Id: I7ca50e487b9f1e6c6ea2b34f73c363def8b2d295
Signed-off-by: Gilbert Chen <gilbertc@nvidia.com>

show more ...

dsp: firmware_update: Bounds check decode_downstream_device_parameter_table_entry_versions()

```
../src/dsp/firmware_update.c: In function ‘decode_downstream_device_parameter_table_entry_versions’:

dsp: firmware_update: Bounds check decode_downstream_device_parameter_table_entry_versions()

```
../src/dsp/firmware_update.c: In function ‘decode_downstream_device_parameter_table_entry_versions’:
../src/dsp/firmware_update.c:1248:48: error: use of attacker-controlled value ‘*entry.active_comp_ver_str_len’ as offset without upper-bounds checking [CWE-823] [-Werror=analyzer-tainted-offset]
1248 | active[entry->active_comp_ver_str_len] = '\0';
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
```

gitlint-ignore: T1, B1, UC1
Fixes: b6ef35b48065 ("fw_update: Add encode req & decode resp for get_downstream_fw_params")
Change-Id: I15571804f391dc97de6d80c90325ded006aee500
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: pdr: Bound check pldm_entity_association_pdr_extract()

```
../src/dsp/pdr.c: In function ‘pldm_entity_association_pdr_extract’:
../src/dsp/pdr.c:1333:35: error: use of attacker-controlled value

dsp: pdr: Bound check pldm_entity_association_pdr_extract()

```
../src/dsp/pdr.c: In function ‘pldm_entity_association_pdr_extract’:
../src/dsp/pdr.c:1333:35: error: use of attacker-controlled value ‘(size_t)((int)*((char *)&*pdr + 10).num_children + 1) * 6’ as allocation size without upper-bounds checking [CWE-789] [-Werror=analyzer-tainted-allocation-size]
1333 | pldm_entity *l_entities = malloc(sizeof(pldm_entity) * l_num_entities);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

Fixes: 9c76679224cf ("libpldm: Migrate to subproject")
Change-Id: I96582ae2b19d22413919ae0a6a9b94e2d3d40f39
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Bounds checks that satisfy GCC's analyzer

The intent is that there is no change in behavior, but that the code
patterns better match the analyzer's expectations.

Change-Id: I58544aaf6b15209

msgbuf: Bounds checks that satisfy GCC's analyzer

The intent is that there is no change in behavior, but that the code
patterns better match the analyzer's expectations.

Change-Id: I58544aaf6b15209e754059bf72a55dc9d63c9d61
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Externalise error value conversion

We need to simplify the code to satisfy clang's analyzer, which seems
to struggle with assumptions if the code exceeds some unknown complexity
limit.

Spec

msgbuf: Externalise error value conversion

We need to simplify the code to satisfy clang's analyzer, which seems
to struggle with assumptions if the code exceeds some unknown complexity
limit.

Specifically, this does away with pldm_msgbuf_init_cc() and all the
associated pldm_msgbuf_status() error translation machinery. All the
call-sites are fixed up, with some additional safety checks put in place
along the way.

I believe this change is viable because unless we're converting legacy
API implementations to use msgbuf there's no additional trickery, and
if we're converting existing implementations then care is required
regardless. The change of approach has no impact on implementation of
new APIs with msgbuf, as the current philosophy is that they should
return negative errnos anyway.

As seems to be the case with this kind of work, the parameter register
allocation seems to have been affected for a number of library APIs.
These are listed in the changelog, and the ABI dump has been updated.

Finally, for msgbuf use in the test cases, all instances have
been converted to use errnos in place of PLDM completion codes in the
expectations. Hopefully there's no more malarky with PLDM completion
code misuse in the future.

Change-Id: Id4a7366ee9f60fb991dfe84aa0bb5aadc9855fcc
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

Revert "dsp: platform: Fix decode_set_event_receiver_req()"

This reverts commit 8c43abb70aeadde39d99af2c1b6b5d4a1416fc47.

As found in openbmc/pldm@35f25949fe4d ("Fix invalid read by adjusting
reque

Revert "dsp: platform: Fix decode_set_event_receiver_req()"

This reverts commit 8c43abb70aeadde39d99af2c1b6b5d4a1416fc47.

As found in openbmc/pldm@35f25949fe4d ("Fix invalid read by adjusting
request size") the change in 8c43abb70aea ("dsp: platform: Fix
decode_set_event_receiver_req()") reduces the value of
PLDM_SET_EVENT_RECEIVER_REQ_BYTES and causes an out-of-bounds access.

A new macro should be introduced rather than changing the value of
PLDM_SET_EVENT_RECEIVER_REQ_BYTES.

Change-Id: I922c7eff86919f513e302bec393c0a516046e923
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: platform: Fix decode_set_event_receiver_req()

Per DSP0248 V1.3.0 table13, the heartbeatTimer field shall be omitted
from the request data if eventMessageGlobalEnable is not set to
enableAsyncKe

dsp: platform: Fix decode_set_event_receiver_req()

Per DSP0248 V1.3.0 table13, the heartbeatTimer field shall be omitted
from the request data if eventMessageGlobalEnable is not set to
enableAsyncKeepAlive.

gitlint-ignore: B1, UC1
Fixes: 66c7723adbdc ("msgbuf: Enable pldm_msgbuf_extract() into packed members")
Fixes: 9667f5823930 ("platform: pldm_msgbuf for decode_set_event_receiver_req()")
Fixes: 6ef2aa90a793 ("platform: Test invalid heartbeat conditions after assignment")
Fixes: 9c76679224cf ("libpldm: Migrate to subproject")
Change-Id: Ia2a0c71a1f37a0fd32670b9b66b051e18fb73dbe
Signed-off-by: Gilbert Chen <gilbertc@nvidia.com>

show more ...

platform: Fix checking `eventIDToAcknowledge`

As DSP0248 V1.3.0, `Figure 22 - Switching from asynchronous eventing to
poll for an event with large data` when the `tranferFlag` of
`PollForPlatformEve

platform: Fix checking `eventIDToAcknowledge`

As DSP0248 V1.3.0, `Figure 22 - Switching from asynchronous eventing to
poll for an event with large data` when the `tranferFlag` of
`PollForPlatformEventMessage` is `AcknowledgementOnly` the
`eventIDToAcknowledge` will be 0xffff.

But the contents in lines #1678 to #1681 of the same specs details
```
1678 0x0000 shall be returned to indicate the terminus event queue is empty. The PLDM Event Receiver shall
1679 acknowledge reception of the event by issuing the command again with the eventIDToAcknowledge set
1680 to the previously retrieved eventID (from the PLDM terminus). The PLDM terminus shall remove the
1681 acknowledged event message from its internal FIFO upon reception of the acknowledgment
```

When the `tranferFlag` is `AcknowledgementOnly` the
`eventIDToAcknowledge` should be `the previously retrieved eventID (from
the PLDM terminus)` which is not 0x0000 and 0xffff. Because `The PLDM
terminus shall remove the acknowledged event message` so the contents in
lines #1678 to #1681 are correct.

Update the failure condition in
`pldm_platform_poll_for_platform_event_message_validate` helper function
to follow the contents in lines #1678 to #1681. The helper will return
error when `tranferFlag` is `AcknowledgementOnly` and
`eventIDToAcknowledge` is 0xffff or 0x0000 (which can't be a real
previous event ID from terminus).

gitlint-ignore: B1, UC1
Fixes: 387b10f6cd37 ("platform: fix encode/decode_poll_for_platform_event_message_req")
Change-Id: Icf84494dbe2c43fba8ae2ec72e7197e8dd4abf3e
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

platform: Support PLDM_CPER_EVENT in encode_platform_event_message_req()

DSP0248 v1.3.0 add CPEREvent for PLDM Event Types as Table 11 and add
it in `encode_platform_event_message_req` func.

Change

platform: Support PLDM_CPER_EVENT in encode_platform_event_message_req()

DSP0248 v1.3.0 add CPEREvent for PLDM Event Types as Table 11 and add
it in `encode_platform_event_message_req` func.

Change-Id: I94e73938694ce8367489244b75c7e8e0a6d1d8ee
Signed-off-by: John Chung <john.chung@arm.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Improve type-specific ergonomics to match generic macros

Don't require that a pointer be passed. Rather, take the pointer inside
the type-safe macro definition, and perform the void cast to

msgbuf: Improve type-specific ergonomics to match generic macros

Don't require that a pointer be passed. Rather, take the pointer inside
the type-safe macro definition, and perform the void cast to avoid the
alignment warning.

Change-Id: I5fbfc4a95591d2640595107e6f5fcae44a95950f
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

platform: fix encode/decode_poll_for_platform_event_message_req

The checking `TransferOperationFlag` and `eventIDToAcknowledge` in
`encode/decode_poll_for_platform_event_message_req` APIs is not
cor

platform: fix encode/decode_poll_for_platform_event_message_req

The checking `TransferOperationFlag` and `eventIDToAcknowledge` in
`encode/decode_poll_for_platform_event_message_req` APIs is not
corrected. Update the conditions to follow the section `16.7
PollForPlatformEventMessage command` in DSP0248 V1.3.0.

Change-Id: Ie799d38f4a492b7719c2ff7c14fe83a1f81dc0b1
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

clang-tidy: Enable cppcoreguidelines-pro-type-reinterpret-cast

There are ways to avoid reinterpret_cast<>() in many circumstances.
For libpldm, often its use can be replaced with placement-new.
Enab

clang-tidy: Enable cppcoreguidelines-pro-type-reinterpret-cast

There are ways to avoid reinterpret_cast<>() in many circumstances.
For libpldm, often its use can be replaced with placement-new.
Enable the lint for reinterpret_cast<>() to prevent its use in future
changes. This way we can stay inside the language rather than rely on
implementation-defined behavior.

Existing uses of reinterpret_cast<>() are marked NOLINT for now, with
the intent that we clean them up over time. The insertion of the NOLINTs
was automated with sed.

Change-Id: If6654f774e438de9262fe9f9012c6b574764c326
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...