[Skiboot] [PATCH V2 02/15] core/pldm: Decode the SetEventReceiver request
Christophe Lombard
clombard at linux.ibm.com
Tue May 16 19:33:28 AEST 2023
Le 26/04/2023 à 15:10, Frederic Barrat a écrit :
>
>
> On 29/04/2022 11:47, Christophe Lombard wrote:
>> The SetEventReceiver command is used to set the address of the Event
>> Receiver into a terminus that generates event messages. It is also
>> used to
>> globally enable or disable whether event messages are generated from the
>> terminus.
>>
>> For the time being, only the following global event message is
>> supported:
>> PLDM_EVENT_MESSAGE_GLOBAL_ENABLE_ASYNC_KEEP_ALIVE.
>>
>> 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/Makefile.inc | 1 +
>> core/pldm/pldm-responder.c | 96 ++++++++++++++++++++++++++++++++++++++
>> core/pldm/pldm.h | 2 +
>> 3 files changed, 99 insertions(+)
>>
>> diff --git a/core/pldm/Makefile.inc b/core/pldm/Makefile.inc
>> index c6f78822..01117680 100644
>> --- a/core/pldm/Makefile.inc
>> +++ b/core/pldm/Makefile.inc
>> @@ -10,6 +10,7 @@ CPPFLAGS += -I$(SRC)/pldm/ibm/libpldm/
>> CFLAGS_$(PLDM_DIR)/pldm-platform-requests.o = -Wno-strict-prototypes
>> CFLAGS_$(PLDM_DIR)/pldm-bios-requests.o = -Wno-strict-prototypes
>> CFLAGS_$(PLDM_DIR)/pldm-watchdog.o = -Wno-strict-prototypes
>> +CFLAGS_$(PLDM_DIR)/pldm-responder.o = -Wno-strict-prototypes
>> PLDM_OBJS = pldm-common.o pldm-responder.o pldm-requester.o
>> PLDM_OBJS += pldm-base-requests.o pldm-platform-requests.o
>> diff --git a/core/pldm/pldm-responder.c b/core/pldm/pldm-responder.c
>> index 16ce6b1f..232447e2 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/libpldm/platform.h>
>> #include <pldm/libpldm/utils.h>
>> #include "pldm.h"
>> @@ -369,6 +370,97 @@ static struct pldm_cmd pldm_base_get_version = {
>> .handler = base_get_version_handler,
>> };
>> +/*
>> + * PLDM Platform commands support
>> + */
>> +static struct pldm_type pldm_platform_type = {
>> + .name = "platform",
>> + .pldm_type_id = PLDM_PLATFORM,
>> +};
>> +
>> +struct event_receiver_req {
>> + uint8_t event_message_global_enable;
>> + uint8_t transport_protocol_type;
>> + uint8_t event_receiver_address_info;
>> + uint16_t heartbeat_timer;
>> +};
>
>
> why not reuse struct event_receiver_req from libpldm, which is
> identical? That would also be true for the following patches where we
> introduce several structures replicated in libpldm.
>
We want to avoid taking the address of packed member of struct
pldm_set_event_receiver_req, which usually results in an unaligned
pointer value.
For this reason we define a new structure based on what libpldm proposes.
>
>> +
>> +#define MIN_WATCHDOG_TIMEOUT_SEC 15
>> +
>> +/*
>> + * SetEventReceiver (0x04)
>> + * The SetEventReceiver command is used to set the address of the Event
>> + * Receiver into a terminus that generates event messages. It is also
>> + * used to globally enable or disable whether event messages are
>> + * generated from the terminus.
>> + */
>> +static int platform_set_event_receiver_handler(const struct
>> pldm_rx_data *req)
>> +{
>> + struct event_receiver_req receiver_req;
>> + uint8_t cc = PLDM_SUCCESS;
>> + int rc = OPAL_SUCCESS;
>> +
>> + /* decode SetEventReceiver request data */
>> + rc = decode_set_event_receiver_req(
>> + req->msg,
>> + PLDM_SET_EVENT_RECEIVER_REQ_BYTES,
>> + &receiver_req.event_message_global_enable,
>> + &receiver_req.transport_protocol_type,
>> + &receiver_req.event_receiver_address_info,
>> + &receiver_req.heartbeat_timer);
>> + if (rc) {
>> + prlog(PR_ERR, "Failed to decode SetEventReceiver request, rc
>> = %d\n", rc);
>> + pldm_cc_resp(req, req->hdrinf.pldm_type,
>> + req->hdrinf.command, PLDM_ERROR);
>> + return OPAL_INTERNAL_ERROR;
>> + }
>> +
>> + /* invoke the appropriate callback handler */
>> + prlog(PR_DEBUG, "%s - event_message_global_enable: %d, "
>> + "transport_protocol_type: %d "
>> + "event_receiver_address_info: %d "
>> + "heartbeat_timer: 0x%x\n",
>
>
> nitpick: could we print heartbeat_timer is decimal, since it's a
> number of seconds so it's more easily interpreted?
>
>
>> + __func__,
>> + receiver_req.event_message_global_enable,
>> + receiver_req.transport_protocol_type,
>> + receiver_req.event_receiver_address_info,
>> + receiver_req.heartbeat_timer);
>> +
>> + if (receiver_req.event_message_global_enable !=
>> + PLDM_EVENT_MESSAGE_GLOBAL_ENABLE_ASYNC_KEEP_ALIVE) {
>> +
>> + prlog(PR_ERR, "%s - invalid value for message global enable
>> received: %d\n",
>> + __func__, receiver_req.event_message_global_enable);
>> + cc = PLDM_PLATFORM_ENABLE_METHOD_NOT_SUPPORTED;
>> + }
>> +
>> + if (receiver_req.heartbeat_timer < MIN_WATCHDOG_TIMEOUT_SEC) {
>> + prlog(PR_ERR, "%s - BMC requested watchdog timeout that's
>> too small: %d\n",
>> + __func__, receiver_req.heartbeat_timer);
>> + cc = PLDM_PLATFORM_HEARTBEAT_FREQUENCY_TOO_HIGH;
>> + } else {
>> + /* set the internal watchdog period to what BMC indicated */
>> + watchdog_period_sec = receiver_req.heartbeat_timer;
>> + }
>> +
>> + /* send the response to BMC */
>> + pldm_cc_resp(req, PLDM_PLATFORM, PLDM_SET_EVENT_RECEIVER, cc);
>> +
>> + /* no error happened above, so arm the watchdog and set the
>> default timeout */
>> + if (cc == PLDM_SUCCESS) {
>> + watchdog_armed = true;
>
>
> We should define a single call to trigger the heartbeat and define its
> rate instead of having 2 global variables.
>
> Fred
More information about the Skiboot
mailing list