Open
Conversation
When use component comparison stamp, the stamp shall not be invalid.
See also [1]. [1]: https://amboar.github.io/notes/2023/06/19/representations-and-design-boundaries-with-endianness.html Change-Id: I1ca3accfd8a4940ff753f70a104b0615e49ec605 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
After removing the contained entity from the entity association PDR, if there is no other contained entity present, we need to remove the entity association PDR. Also modified the testcase to delete all the entities from the entity association PDR. Fixes: b31e4c6 ("pdr: Remove contained entity from PDR repo") Change-Id: If426314a571e91d178e8ae802a49bb7a3746080e Signed-off-by: Archana Kakani <archana.kakani@ibm.com>
These can be used to populate a msgbuf by passing a buffer/length to an external function that doesn't use msgbuf. pldm_msgbuf_peek_remaining() provides a buffer and length to pass, and pldm_msgbuf_skip() is called after the data has been written into that buffer, to update the used length. Change-Id: I55012e2e724842665f5317e252c9e3ae81803936 Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
This provides a FD responder implementation, with an ops struct to allow for platform customization. Applications provide incoming messages to pldm_fd_handle_msg(), and periodically call pldm_fd_progress() for asynchronous events. Change-Id: I034262e8b2c45b5bea61369d5f966dd7e530ba9e Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
This will respond to the mandatory PLDM control command types. Change-Id: I483bfdb6513cc6ec77a04480397993e42160c0ae Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Supported PLDM firmware (type 5) commands are registered with the PLDM control responder. Change-Id: I6a4840a820bf2a1a7967ab9fe927f69681f375d8 Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
This includes a fuzz target fd-fuzz, and infrastructure to run with either honggfuzz or AFL. fd-fuzz-input1.dat was crafted from parts of a pldm firmware update packet capture, as a seed to guide fuzzers. Signed-off-by: Matt Johnston <matt@codeconstruct.com.au> Change-Id: I424761a29a22bc964201fd7bd94ddc09a6ac89df
Explicitly run git diff --exit-code. When the --quiet argument was removed, the implicit --exit-code was lost. gitlint-ignore: UC1 Fixes: 566f2fc ("scripts: pre-submit: Print meson format diff on failure") Change-Id: Ia890107b0a8accdf6e1602105487426d6e1ec1b4 Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
The install path for a generated `sizes.h` is missing includedir. This results in the header being installed into `/usr/libpldm/sizes.h` rather than `/usr/include/libpldm/sizes.h`. This was detected by a recipe failure in the Yocto bump for this repository. Tested: ``` $ meson install -C builddir --destdir /tmp/libpldm $ find /tmp/libpldm -name sizes.h /tmp/libpldm/usr/local/include/libpldm/sizes.h ``` Fixes: 8c2bfb1 ("libpldm: Add firmware update FD responder") Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I7dd8cccc7c51302f87005ec540c523ab4561b8f4
Break the section on adding new APIs into sub-sections. Hopefully setting some boundaries on the content makes it easier to maintain. Change-Id: I43bb20ae1761f0509b480ec7d681afc24317c4f4 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Make it stand out a little more while improving consistency of presentation with respect to other similar notes. Change-Id: Iac10f0c8c1c813fa19b1bc1dda59fadb3408c990 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Almost all of the APIs punt allocation to the caller or use iterators to avoid forcing it altogether. Note this in the changes checklist so contributors have a shot at designing desirable APIs from the outset. Change-Id: I3d815d4c5bd1e67fa6248933feab365fff0ba2ca Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Prefer the use of `goto` in error-path handling, and always clean up in stack-order. Change-Id: If2324755c7bbb3c755b83d48f1f3c470fce10c2a Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Added changes to meet the meson style guide https://mesonbuild.com/Style-guide.html https://mesonbuild.com/Commands.html#format Change-Id: Ifad2a81a7535a60f7a3a2a3ec475a3f339a8e42e Signed-off-by: Archana Kakani <archana.kakani@ibm.com>
Apply the `meson format` results. Change-Id: I6b322381863e65b8b05471f334566cb68b74ed14 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
These ABIs are now required by PLDM to query information from downstream devices[1]. Mark them as stable to make the symbols visible. [1]: https://gerrit.openbmc.org/c/openbmc/pldm/+/75390 Change-Id: I6892fb6ffa878b8530e57ce338fe02048d9b08f7 Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com>
Specify `native: false` as we're testing a library against which the tests must link. Resolves the following: ``` meson.build:17: WARNING: add_languages is missing native:, assuming languages are wanted for both host and build. ``` Change-Id: Ie3173489c8738a76077e5f278bb0eef0d3697de1 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
We hit some hazards previously with include-ordering when libpldm
was built as a meson subproject. Specifying the full path avoids the
problem.
Fix the current implementation, which causes the following output:
```
WARNING: Broken features used:
* 1.3.0: {'str.format: Value other than strings, integers, bools, options, dictionaries and lists thereof.'}
```
Using file.full_path() requires we bump the minimum meson version to
1.4.0.
Change-Id: I4326ef40a811c0d9df229683e7d79833424e5c6d
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Also note how to install recent versions of meson, for those whose distros tend to be behind the times. Change-Id: I43c3ded89e7d6d66c0968565908545f423f05a5c Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
FD_T1 timeout should also take effect when waiting for UA responses to RequestFirmwareData, VerifyComplete, ApplyCompete, and ApplyComplete. Previously the timeout was only used when waiting for UA-initiated requests. Fixes: 8c2bfb1 ("libpldm: Add firmware update FD responder") Change-Id: Id43fb0b991a30b613f1e4e92e42481dea3f8adce Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
The changelog regularly gets entries in the wrong places. Document a process that might help avoid the problem in the future. Change-Id: I85f30ded23557e77c54d22c3bd8f1eb7e5bf1307 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Entries that should have been under `Unreleased` were spread throughout prior release notes. Change-Id: I78f03b88474109c9856cb6bfedf5d485023dee20 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Eliminate the rebase-conflict pain of re-numbering. Change-Id: Id77cf380454a92567fde6e7d5870f54661c1a1b3 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
For some historical reason it was previously mapped to PLDM_ERROR_INVALID_LENGTH. -EBADMSG typically means the provided information is inconsistent in some way rather than improperly bounded, so fix that now. Change-Id: Ie0fe2c6f9742387f951d5abe81f12f9f7b7aff24 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Begin the process of migrating package parsing APIs away from using PLDM protocol completion codes for signaling errors. For now keep the new symbols internal. They will eventually be exposed and the current stable APIs deprecated. Change-Id: Ieb67c43ebb0782b9da530c52de99b59edca4a648 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
openbmc-build-scripts' unit-test.py currently[1] modifies its own environment to exploit clang. It seems like a pretty dicey approach (modify the environment passed to the subprocess instead?), for now work around it by forcing use of GCC for the pre-submit script. [1]: https://github.com/openbmc/openbmc-build-scripts/blob/966d67da385aef095d19b20fd778fcb88fa7be7b/scripts/unit-test.py#L1064-L1065 Change-Id: Ia6259eb949c2bab7fd30b49fc5c2d69af4f6470d Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
PLDM Base specification defines in DSP0240 the optional command SetTID Add the minimum required size for sending this command Change-Id: Ibea4e72a090088166f1584145bfca5a7489b1920 Signed-off-by: Roger G. Coscojuela <roger.gili-coscojuela@sipearl.com>
While normalizing the Entity association PDR, record handle is passed to the library function. Considering PDR contains logical and physical contained entities, two pdrs with same record handle are created. To fix the issue, we are propagating the recently used record handle from the pldm_pdr_add to entity_association_pdr_add. Then incrementing the record handle before creating next PDR. gitlint-ignore: B1, UC1 Fixes: 25ddbcc ("pdr: Add pldm_entity_association_pdr_add_from_node_with_record_handle()") Change-Id: Ifaa5694cabe7ad38d8881f0b7be0a79d9d3e06c0 Signed-off-by: Archana Kakani <archana.kakani@ibm.com>
pldm_instance_id_alloc() function does not check for a valid ctx object before dereferencing it. Hence it could cause crash when its clients pass a null pointer by accident. Change-Id: I340aa8171cd397f5af772a9cc6d4f80c8263a089 Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
In order to achieve [1], this commit updates some encoding APIs to accept the length of the message as an in/out parameter (pointer to size_t). The APIs are then updated to return the encoded payload length through this parameter. The unit tests for these APIs are updated accordingly. The change list includes: - encode_pldm_base_multipart_receive_req() - encode_pldm_base_negotiate_transfer_params_req() [1]: https://github.com/openbmc/libpldm/blob/main/CONTRIBUTING.md?plain=1#L270-L272 Change-Id: I17cb3641b2c4a7a30fe757a3275b1713499484cc Signed-off-by: Chau Ly <chaul@amperecomputing.com>
In order to achieve [1], this commit updates some encoding APIs to accept the length of the message as an in/out parameter (pointer to size_t). The APIs are then updated to return the encoded payload length through this parameter. The unit tests for these APIs are updated accordingly. The change list includes: - encode_pldm_file_df_open_req() - encode_pldm_file_df_close_req() - encode_pldm_file_df_heartbeat_req() [1]: https://github.com/openbmc/libpldm/blob/main/CONTRIBUTING.md?plain=1#L270-L272 Change-Id: Ic81327438190bfa0541333f35e0b52a51010db91 Signed-off-by: Chau Ly <chaul@amperecomputing.com>
Thit commit stabilizes PLDM File Transfer API. The APIs include: - decode_pldm_platform_file_descriptor_pdr() - encode_pldm_base_multipart_receive_req() - decode_pldm_base_multipart_receive_resp() - encode_pldm_base_negotiate_transfer_params_req() - decode_pldm_base_negotiate_transfer_params_resp() - encode_pldm_file_df_open_req() - decode_pldm_file_df_open_resp() - encode_pldm_file_df_close_req() - decode_pldm_file_df_close_resp() - encode_pldm_file_df_heartbeat_req() - decode_pldm_file_df_heartbeat_resp() Use of decode_pldm_platform_file_descriptor_pdr() is demonstrated in [1], and the remainder in [2]. Therefore, the gcc.dump file is regenerated accordingly. [1]: https://gerrit.openbmc.org/c/openbmc/pldm/+/82342 [2]: https://gerrit.openbmc.org/c/openbmc/pldm/+/82471 Change-Id: I6f60845b459c6fd0371925468927f8ff15ec4033 Signed-off-by: Chau Ly <chaul@amperecomputing.com>
Fix:
```
In file included from ../tests/msgbuf.cpp:6:
In function ‘testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = short int; T2 = int]’,
inlined from ‘static testing::AssertionResult testing::internal::EqHelper::Compare(const char*, const char*, const T1&, const T2&) [with T1 = short int; T2 = int; typename std::enable_if<((! std::is_integral<_Tp>::value) || (! std::is_pointer<_Dp>::value))>::type* <anonymous> = 0]’ at ../subprojects/googletest/googletest/include/gtest/gtest.h:1413:64,
inlined from ‘virtual void msgbuf_extract_one_int16_Test::TestBody()’ at ../tests/msgbuf.cpp:229:5:
../subprojects/googletest/googletest/include/gtest/gtest.h:1394:3: error: ‘val’ may be used uninitialized [-Werror=maybe-uninitialized]
1394 | if (lhs == rhs) {
| ^~
../tests/msgbuf.cpp: In member function ‘virtual void msgbuf_extract_one_int16_Test::TestBody()’:
../tests/msgbuf.cpp:225:13: note: ‘val’ was declared here
225 | int16_t val;
| ^~~
In function ‘testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = unsigned int; T2 = int]’,
inlined from ‘static testing::AssertionResult testing::internal::EqHelper::Compare(const char*, const char*, const T1&, const T2&) [with T1 = unsigned int; T2 = int; typename std::enable_if<((! std::is_integral<_Tp>::value) || (! std::is_pointer<_Dp>::value))>::type* <anonymous> = 0]’ at ../subprojects/googletest/googletest/include/gtest/gtest.h:1413:64,
inlined from ‘virtual void msgbuf_extract_one_uint32_Test::TestBody()’ at ../tests/msgbuf.cpp:268:5:
../subprojects/googletest/googletest/include/gtest/gtest.h:1394:3: error: ‘val’ may be used uninitialized [-Werror=maybe-uninitialized]
1394 | if (lhs == rhs) {
| ^~
../tests/msgbuf.cpp: In member function ‘virtual void msgbuf_extract_one_uint32_Test::TestBody()’:
../tests/msgbuf.cpp:264:14: note: ‘val’ was declared here
264 | uint32_t val;
| ^~~
```
Change-Id: I0aac7ba5958f3278e2079183a245c2258c0e92f4
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Encoding the necessary sequence of calls with an approximation of linear
types is hampered by DSP0267's introduction of entirely new sections
into the package format across revisions. The existing design enforced a
sequence that precluded _not_ calling the decoder for downstream device
records in the case of pinning to format revision 1. The choice had the
further effect of stunting the API to future expansion of the spec.
Switch from linear types to tracking parse state at runtime based on the
provided pin and the extracted package header.
The state machine implementation is modeled on the TLA+ specification
below, with NR_FORMATS set to 4 in the model:
```tla
---- MODULE pldm_package_parser ----
EXTENDS Integers, Sequences
\* pin and format have non-deterministic init, but are then constant
VARIABLE state, pin, format
vars == << state, pin, format >>
States == {
"Init",
"Header",
"FirmwareDevices",
"DownstreamDevices",
"ComponentImageInfos",
"Complete",
"Unsupported",
"Error"
}
Formats == 1..4
DecodeHeader ==
/\ state = "Init"
/\ state' = IF format <= pin THEN "Header" ELSE "Unsupported"
/\ UNCHANGED << pin, format >>
DecodeFirmwareDevices ==
/\ state = "Header"
/\ state' = "FirmwareDevices"
/\ UNCHANGED << pin, format >>
DecodeDownstreamDevices ==
/\ state = "FirmwareDevices"
/\ state' = IF pin = 1 THEN "Error" ELSE "DownstreamDevices"
/\ UNCHANGED << pin, format >>
DecodeComponentImageInfos ==
/\ \/ /\ state = "FirmwareDevices"
/\ pin = 1
\/ /\ state = "DownstreamDevices"
/\ pin \in ( Formats \ { 1 } )
/\ state' = "Complete"
/\ UNCHANGED << pin, format >>
Done == state \in { "Complete", "Unsupported", "Error" } /\ UNCHANGED vars
Init ==
/\ state = "Init"
/\ pin \in Formats
/\ format \in Formats
Next ==
\/ DecodeHeader
\/ DecodeFirmwareDevices
\/ DecodeDownstreamDevices
\/ DecodeComponentImageInfos
\/ Done
Spec == Init /\ [][Next]_vars /\ WF_state(Next)
TypeInvariant ==
/\ state \in States
/\ pin \in Formats
/\ format \in Formats
Safety ==
/\ TypeInvariant
/\ state \in States \ { "Init", "Unsupported" } => format <= pin
Liveness ==
/\ [][(state \in { "Complete", "Unsupported", "Error" } => UNCHANGED state)]_vars
/\ [][UNCHANGED <<pin, format>>]_vars
====
```
For an introduction to TLA+ see https://www.learntla.com/
Note that the implemented state machine does not exactly replicate that
specified in the model. Specifically:
- No "Unsupported" state is defined. Instead, the APIs return -ENOTSUP
- No "Error" state is defined. Instead, the APIs return -EPROTO
It is expected that callers perform appropriate error handling.
Change-Id: Id8780c1f5f130b77e6eea2519b5d5734aa79040e
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Allow influencing the parser behavior down the track. Change-Id: If63284f5c67b7a8e40d5b3176f5b8367038eb98e Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
fix clang-tidy error in CI. Experienced this error on an unrelated code
change [1]
```
/data0/jenkins/workspace/ci-repository/openbmc/libpldm/include/libpldm/firmware_update.h:2298:8: error: declaration uses identifier 'pldm__package_header_information', which is a reserved identifier [bugprone-reserved-identifier,cert-dcl37-c,cert-dcl51-cpp,-warnings-as-errors]
2298 | struct pldm__package_header_information {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| pldm_package_header_information
2299 | pldm_uuid package_header_identifier;
2300 | uint8_t package_header_format_revision;
2301 | uint8_t package_release_date_time[PLDM_TIMESTAMP104_SIZE];
2302 | uint16_t component_bitmap_bit_length;
2303 | uint8_t package_version_string_type;
2304 |
2305 | /** A field pointing to the package version string in the provided package data */
2306 | struct variable_field package_version_string;
2307 | };
2308 | /* TODO: Deprecate the other struct pldm_package_header_information, remove, drop typedef */
2309 | typedef struct pldm__package_header_information
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| pldm_package_header_information
```
References:
[1] https://jenkins.openbmc.org/job/ci-repository/115058//console
Change-Id: I3e9dee5e266ee734f86039fc08dc28983d48c267
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
Change the bitfield structs in pldm_types.h to be named structs. This fixes an issue with the abi-dumper mistakenly seeing those as 1 byte wide when compiling with the C++ binding. Change-Id: Ifecc4d62355e9233e88b75baffb4fcf89efe1c06 Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
According to DSP0240 v1.2.0 - Table 17 MultipartReceive Request and Response Message Format, `DataIntegrityChecksum` field: - Shall be included with all part transfers. - Shall be omitted in response to a request message where TransferOperation was set to XFER_COMPLETE or XFER_ABORT. The only possible TransferFlag in a response for a request, where TransferOperation was set to `XFER_COMPLETE` or `XFER_ABORT`, is `ACKNOWLEDGE_COMPLETION`. So this is the only condition to ignore the `DataIntegrityChecksum` field. In the current implementation, `DataIntegrityChecksum` field is only extracted when `TransferOperation` is `END` or `START_AND_END` and this is incorrect. Therefore, this commit updates `decode_pldm_base_multipart_receive_resp` function to not extract `DataIntegrityChecksum` only when `TransferFlag` field is `ACKNOWLEDGE_COMPLETION`. This also adds 2 more unit tests for the API, which are respectively decoding an `ACKNOWLEDGE_COMPLETION` response with a redundant checksum field and decoding a non-`ACKNOWLEDGE_COMPLETION` response without a checksum field. Change-Id: I325a6393eaabfecc08f758dcae417470dde65efb Fixes: 8836784 ("dsp: base: Add encode req & decode resp for MultipartReceive command") Signed-off-by: Chau Ly <chaul@amperecomputing.com>
The test case name for the group of unit tests for `decode_multipart_receive_req()` API was incorrectly renamed to `DecodeMultipartReceiveResponse` in [1]. This commit updates the test case name to `DecodeMultipartReceiveRequest` to match the API name, and not duplicate with that of `decode_pldm_base_multipart_receive_resp()` [1]: https://gerrit.openbmc.org/c/openbmc/libpldm/+/82805 Change-Id: I037fed711b35dcdc7295e40dc365c4fb71ab0b0f Signed-off-by: Chau Ly <chaul@amperecomputing.com>
Change-Id: I5521a8356e0cc795bd0551aaddad354598d75c1e Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Below new APIs will be consumed by openbmc/pldm[1]: - `decode_pldm_firmware_update_package()` - `pldm_package_firmware_device_id_record_iter_init()` - `decode_pldm_package_firmware_device_id_record_from_iter()` - `pldm_package_downstream_device_id_record_iter_init()` - `decode_pldm_package_downstream_device_id_record_from_iter()` - `pldm_package_component_image_information_iter_init()` - `decode_pldm_package_component_image_information_from_iter()` - The newly added APIs uses iterators to parse the firmware update packages, which provide the efficiency and safety benefits than the old ones. - The new APIs support DSP0267 v1.1.0 to v1.3.0, which provides additional fields in the firmware update package. - Since the old APIs will be soon marked as deprecated, this is a good opportunity to stabilize the new APIs. - Build pass and passed google test on CI. - Build pass and tested successfully on Yosemite4. [1]: https://gerrit.openbmc.org/c/openbmc/pldm/+/80194 Change-Id: I44bd1be07b0cb19a7a1044d6b36e26ffbd771285 Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com> Signed-off-by: Carter Chen <carter.chen.wiwynn@gmail.com>
Avoid allocating struct pldm_msg without member initialization in PLDM_MSG_DEFINE_P. Otherwise unit tests may fail with -Werror=maybe-uninitialized. Change-Id: I3d0118a0067e373dc7bd2fd9b28eec2fdf5780e4 Signed-off-by: John Chung <john.chung@arm.com>
Prefixing the commit subject with the impacted subsystem is often helpful when glancing at the oneline commit logs. Change-Id: I5d57540526435ebc3b347c8510aaefd615f35b05 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Frequently they're so vague it's not even clear what testing occurred, making the whole statement moot. Please only include them if you have something substantial to say. Change-Id: I2d4e9b16cbdd0a29100a99542534b2efd97533b5 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
This patch updates the file descriptor PDR min length macro header to note the inclusion of common PDR header bytes. Change-Id: I479bea1f82680ca57f26c5b45b763669488f2eb6 Signed-off-by: Kasun Athukorala <kasunath@google.com>
Change-Id: Ic1627b0ba2a4c87f4a8352e7c63eede0a2513b80 Signed-off-by: Kasun Athukorala <kasunath@google.com>
ACKNOWLEDGE_COMPLETION flag is defined in DSP0240 v1.1.0 under MultipartReceive response data structure definition. Change-Id: I1bd340ebd2d52175991f2084c1586bf3fe75b6ae Signed-off-by: Kasun Athukorala <kasunath@google.com>
Valgrind complains and exits with an error if the limit is "too large":
--426:0:libcfile Valgrind: FATAL: Private file creation failed.
The current file descriptor limit is 1073741804.
If you are running in Docker please consider
lowering this limit with the shell built-in limit command.
--426:0:libcfile Exiting now.
Change-Id: I8b58f0c17ae3e22a11f1678e171c3ee54dc432c3
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
After switching to Debian Trixie these surfaced as issues when building locally. Change-Id: I92a4d19579257e796aa29be8d4fb79fe92466780 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Define separate msgbuf structures to avoid casting away const-qualifiers in the msgbuf constructor function: * pldm_msgbuf_rw: for encode functions with non const-qualified buffer * pldm_msgbuf_ro: for decode functions with const-qualified buffer Further, use _Generic() to keep the API ergonomic while still yielding a compile error when wrong msgbuf type is passed. Change-Id: I71dbcb7996e9fb402b49870fce539a939c1497e5 Signed-off-by: John Chung <john.chung@arm.com> Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
In context, this should now be PLDM_MSGBUF_RW_DEFINE_P() for
const-correctness.
Fixes:
```
../src/dsp/platform.c:3550:2: error: call to undeclared function 'PLDM_MSGBUF_DEFINE_P'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
3550 | PLDM_MSGBUF_DEFINE_P(buf);
| ^
```
Fixes: 7a8d932 ("msgbuf: Define a separate msgbuf structure for encode/decode function")
Change-Id: I448395b6bc1f4914ca01f3edfd8c3cf7ce60a7d9
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
These are now marked stable and shouldn't be conditionally compiled.
Fixes:
```
../tests/dsp/firmware_update.cpp:53:5: error: unused variable 'PLDM_FWUP_PACKAGE_HEADER_IDENTIFIER_V1_1' [-Werror,-Wunused-const-variable]
53 | PLDM_FWUP_PACKAGE_HEADER_IDENTIFIER_V1_1{
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../tests/dsp/firmware_update.cpp:59:5: error: unused variable 'PLDM_FWUP_PACKAGE_HEADER_IDENTIFIER_V1_2' [-Werror,-Wunused-const-variable]
59 | PLDM_FWUP_PACKAGE_HEADER_IDENTIFIER_V1_2{0x31, 0x19, 0xce, 0x2f, 0xe8, 0x0a,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../tests/dsp/firmware_update.cpp:64:5: error: unused variable 'PLDM_FWUP_PACKAGE_HEADER_IDENTIFIER_V1_3' [-Werror,-Wunused-const-variable]
64 | PLDM_FWUP_PACKAGE_HEADER_IDENTIFIER_V1_3{0x7b, 0x29, 0x1c, 0x99, 0x6d, 0xb6,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
Fixes: f6ef78f ("dsp: firmware_update: Stabilized new firmware package parsing ABI")
Change-Id: I3386d3827b80d31cb702cb9f3f67d616b09be478
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Change-Id: I6c6fce214b98bc8f7284bc3e58319a9ef659e1b0 Signed-off-by: Kasun Athukorala <kasunath@google.com>
This command is based on DSP0240 v1.2.0 section 9.6.2 NegotiateTransferParameters (0x07). gitlint-ignore: T1 Change-Id: I4fa193444df9ba5b109ca063163d7f7d109fcbda Signed-off-by: Kasun Athukorala <kasunath@google.com>
This command is based on DSP0240 v1.2.0 section 9.6.2 NegotiateTransferParameters (0x07). gitlint-ignore: T1 Signed-off-by: Kasun Athukorala <kasunath@google.com> Change-Id: I619ca0b356df065b662b16d4a469730fd145f9de
I have been committing code related to DSP0242 - PLDM File Transfer, so I volunteer to review future commits related to this feature in the repo. Change-Id: If2393e19e9461e54b2e00e0f47b5eafe53d2d847 Signed-off-by: Chau Ly <chaul@amperecomputing.com>
cb987c5 ("dsp: base: Added decode_pldm_base_negotiate_transfer_params_req()") accidentally set `decode_pldm_base_negotiate_transfer_params_resp()` back to `LIBPLDM_ABI_TESTING` when it was stabilized by 26c8eb2 ("Stabilize PLDM File Transfer related APIs"). Further, the existing integration of abi-compliance-checker was unable to find the fault as the generated dump contained testing symbols, and so decode_pldm_base_negotiate_transfer_params_resp() wasn't found to be absent. We rework scripts/pre-submit such that this kind of problem results in a CI failure. Change-Id: I27d48e275646bbd75f19a3e1fe309a584d893cf2 Fixes: cb987c5 ("dsp: base: Added decode_pldm_base_negotiate_transfer_params_req()") Signed-off-by: Chau Ly <chaul@amperecomputing.com> Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Remove the comp_comparison_stamp check as discussed in Issue 17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
19 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When use component comparison stamp, the stamp shall not be invalid.