[Skiboot] [PATCH V2 03/15] core/pldm: Decode the PlatformEventMessage request

Frederic Barrat fbarrat at linux.ibm.com
Wed Apr 26 23:19:57 AEST 2023



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

   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