[Skiboot] [PATCH V2 03/15] core/pldm: Decode the PlatformEventMessage request
Christophe Lombard
clombard at linux.ibm.com
Tue May 16 22:42:21 AEST 2023
Le 26/04/2023 à 15:19, Frederic Barrat a écrit :
>
>
> On 29/04/2022 11:47, Christophe Lombard wrote:
>> PLDM Event Messages are sent as PLDM request messages to the Event
>> Receiver
>> using the PlatformEventMessage command.
>>
>> The Event Receiver acknowledges receiving the PLDM Event Message in the
>> response to this command.
>>
>> Signed-off-by: Christophe Lombard <clombard at linux.vnet.ibm.com>
>> ---
>> core/pldm/pldm-platform-requests.c | 10 +++
>> core/pldm/pldm-responder.c | 108 +++++++++++++++++++++++++++++
>> core/pldm/pldm.h | 1 +
>> 3 files changed, 119 insertions(+)
>>
>> diff --git a/core/pldm/pldm-platform-requests.c
>> b/core/pldm/pldm-platform-requests.c
>> index 5b0bdac4..c3baad54 100644
>> --- a/core/pldm/pldm-platform-requests.c
>> +++ b/core/pldm/pldm-platform-requests.c
>> @@ -419,6 +419,16 @@ int pldm_platform_init(void)
>> return OPAL_SUCCESS;
>> }
>> +int plmd_platform_reload_pdrs(void)
>
>
> The version of this function I'm reviewing has been updated. The new
> version has a memory leak:
> struct pldm_pdrs *pdrs = zalloc(sizeof(struct pldm_pdrs));
> pdrs is never freed as it is re-allocated each time we are notified of
> an PLDM_PDR_REPOSITORY_CHG_EVENT event
>
>
>> +{
>> + if (pdr_ready) {
>> + pldm_pdr_destroy(repo);
>> +
>> + return pdrs_init();
>> + } else
>> + return OPAL_HARDWARE;
>> +}
>> +
>> void pldm_platform_exit(void)
>> {
>> if (pdr_ready)
>> diff --git a/core/pldm/pldm-responder.c b/core/pldm/pldm-responder.c
>> index 232447e2..5eb0c3f3 100644
>> --- a/core/pldm/pldm-responder.c
>> +++ b/core/pldm/pldm-responder.c
>> @@ -8,6 +8,7 @@
>> #include <opal.h>
>> #include <stdio.h>
>> #include <string.h>
>> +#include <pldm/ibm/libpldm/platform_oem_ibm.h>
>> #include <pldm/libpldm/platform.h>
>> #include <pldm/libpldm/utils.h>
>> #include "pldm.h"
>> @@ -461,6 +462,112 @@ static struct pldm_cmd
>> pldm_platform_set_event_receiver = {
>> .handler = platform_set_event_receiver_handler,
>> };
>> +struct event_message_req {
>> + uint8_t format_version;
>> + uint8_t tid;
>> + uint8_t event_class;
>> + size_t event_data_offset;
>> +};
>
>
> As mentioned in previous patch, that is another example of a
> duplicated structure with libpldm (struct
> pldm_platform_event_message_req).
>
>
>> +
>> +/*
>> + * PlatformEventMessage (0x10)
>> + * PLDM Event Messages are sent as PLDM request messages to the Event
>> + * Receiver using the PlatformEventMessage command.
>> + */
>> +static int platform_event_message(const struct pldm_rx_data *req)
>> +{
>> + char response_msg[PKT_SIZE(struct
>> pldm_platform_event_message_resp)];
>> + struct pldm_bios_attribute_update_event_req *request;
>> + struct event_message_req message_req;
>> + uint8_t *bios_attribute_handles;
>> + uint8_t cc = PLDM_SUCCESS;
>> + int rc, i;
>> +
>> + /* decode PlatformEventMessage request data */
>> + rc = decode_platform_event_message_req(
>> + req->msg,
>> + sizeof(struct event_message_req),
>> + &message_req.format_version,
>> + &message_req.tid,
>> + &message_req.event_class,
>> + &message_req.event_data_offset);
>> + if (rc) {
>> + prlog(PR_ERR, "Failed to decode PlatformEventMessage
>> request, rc = %d\n", rc);
>> + pldm_cc_resp(req, req->hdrinf.pldm_type,
>> + req->hdrinf.command, PLDM_ERROR);
>> + return OPAL_INTERNAL_ERROR;
>> + }
>> +
>> + prlog(PR_DEBUG, "%s - format_version: %d, "
>> + "tid: %d "
>> + "event_class: %d "
>> + "event_data_offset: 0x%lx\n",
>> + __func__,
>> + message_req.format_version,
>> + message_req.tid,
>> + message_req.event_class,
>> + message_req.event_data_offset);
>> +
>> + /* we don't support any other event than the PDR Repo Changed
>> event */
>> + if ((message_req.event_class != PLDM_PDR_REPOSITORY_CHG_EVENT) &&
>> + (message_req.event_class !=
>> PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE)) {
>> + prlog(PR_ERR, "%s - Invalid event class %d in platform event
>> handler\n",
>> + __func__, message_req.event_class);
>> + cc = PLDM_ERROR;
>> + }
>> +
>> + rc = encode_platform_event_message_resp(
>> + req->hdrinf.instance,
>> + cc,
>> + PLDM_EVENT_NO_LOGGING,
>> + (struct pldm_msg *)response_msg);
>> + if (rc != PLDM_SUCCESS) {
>> + prlog(PR_ERR, "Encode PlatformEventMessage Error, rc: %d\n",
>> rc);
>> + pldm_cc_resp(req, req->hdrinf.pldm_type,
>> + req->hdrinf.command, PLDM_ERROR);
>> + return OPAL_PARAMETER;
>> + }
>> +
>> + /* send PLDM message over MCTP */
>> + rc = pldm_send(req->source_eid, response_msg,
>> sizeof(response_msg));
>> + if (rc) {
>> + prlog(PR_ERR, "Failed to send PlatformEventMessage response,
>> rc = %d\n", rc);
>> + return OPAL_HARDWARE;
>> + }
>> +
>> + /* invoke the appropriate callback handler */
>> + if (message_req.event_class == PLDM_PDR_REPOSITORY_CHG_EVENT)
>> + return plmd_platform_reload_pdrs();
>> +
>> + /* When the attribute value changes for any BIOS attribute, then
>> + * PlatformEventMessage command with OEM event type
>> + * PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE is send to
>> + * host with the list of BIOS attribute handles.
>> + */
>> + if (message_req.event_class ==
>> PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE) {
>> + request = (struct pldm_bios_attribute_update_event_req
>> *)req->msg->payload;
>> + bios_attribute_handles = (uint8_t
>> *)request->bios_attribute_handles;
>> +
>> + prlog(PR_DEBUG, "%s - OEM_EVENT_BIOS_ATTRIBUTE_UPDATE,
>> handles: %d\n",
>> + __func__, request->num_handles);
>> +
>> + /* list of BIOS attribute handles */
>> + for (i = 0; i < request->num_handles; i++) {
>> + prlog(PR_DEBUG, "%s - OEM_EVENT_BIOS_ATTRIBUTE_UPDATE:
>> handle(%d): %d\n",
>> + __func__, i, *bios_attribute_handles);
>> + bios_attribute_handles += sizeof(uint16_t);
>> + }
>> + }
>
>
> So anything about PLDM_EVENT_TYPE_OEM_EVENT_BIOS_ATTRIBUTE_UPDATE is
> just printed here for debug? We don't seem to be doing anything with it
>
Correct, for the time being, nothing is done, just a debug message.
> Fred
>
>
>> +
>> + return OPAL_SUCCESS;
>> +}
>> +
>> +static struct pldm_cmd pldm_platform_event_message = {
>> + .name = "PLDM_PLATFORM_EVENT_MESSAGE",
>> + .pldm_cmd_id = PLDM_PLATFORM_EVENT_MESSAGE,
>> + .handler = platform_event_message,
>> +};
>> +
>> int pldm_rx_handle_request(struct pldm_rx_data *rx)
>> {
>> const struct pldm_type *t;
>> @@ -504,6 +611,7 @@ int pldm_mctp_responder_init(void)
>> /* Register platform commands we'll respond to - DSP0248 */
>> pldm_add_type(&pldm_platform_type);
>> pldm_add_cmd(&pldm_platform_type,
>> &pldm_platform_set_event_receiver);
>> + pldm_add_cmd(&pldm_platform_type, &pldm_platform_event_message);
>> return OPAL_SUCCESS;
>> }
>> diff --git a/core/pldm/pldm.h b/core/pldm/pldm.h
>> index 5470f64a..938a554c 100644
>> --- a/core/pldm/pldm.h
>> +++ b/core/pldm/pldm.h
>> @@ -80,6 +80,7 @@ int pldm_bios_init(void);
>> int pldm_base_get_tid_req(void);
>> +int plmd_platform_reload_pdrs(void);
>> int pldm_platform_init(void);
>> void pldm_platform_exit(void);
More information about the Skiboot
mailing list