[Skiboot] [PATCH V2 05/15] core/pldm: Decode the SetStateEffecterStates request
Christophe Lombard
clombard at linux.ibm.com
Wed May 24 19:13:41 AEST 2023
Le 26/04/2023 à 15:39, Frederic Barrat a écrit :
>
>
> On 29/04/2022 11:47, Christophe Lombard wrote:
>> The SetStateEffecterStates command is used to set the state of one or
>> more
>> effecters within a PLDM State Effecter.
>> The field comp_effecter_count indicates the number of individual sets of
>> state effecter information that are accessed by this 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-responder.c | 130 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 130 insertions(+)
>>
>> diff --git a/core/pldm/pldm-responder.c b/core/pldm/pldm-responder.c
>> index e44bb509..8c543b6c 100644
>> --- a/core/pldm/pldm-responder.c
>> +++ b/core/pldm/pldm-responder.c
>> @@ -8,8 +8,11 @@
>> #include <opal.h>
>> #include <stdio.h>
>> #include <string.h>
>> +#include <opal-msg.h>
>> +#include <debug_descriptor.h>
>> #include <pldm/ibm/libpldm/platform_oem_ibm.h>
>> #include <pldm/libpldm/platform.h>
>> +#include <pldm/libpldm/state_set.h>
>> #include <pldm/libpldm/utils.h>
>> #include "pldm.h"
>> @@ -647,6 +650,132 @@ static struct pldm_cmd
>> pldm_platform_get_state_sensor_readings = {
>> .handler = platform_get_state_sensor_readings,
>> };
>> +#define SOFT_OFF 0x00
>> +#define SOFT_REBOOT 0x01
>> +#define CHASSIS_PWR_DOWN 0x00
>> +#define DEFAULT_CHIP_ID 0
>> +
>> +struct effecter_states_req {
>> + uint16_t effecter_id;
>> + uint8_t comp_effecter_count;
>> + set_effecter_state_field field[8];
>> +};
>> +
>> +/*
>> + * SetStateEffecterStates (0x39)
>> + * The SetStateEffecterStates command is used to set the state of one
>> + * or more effecters within a PLDM State Effecter.
>> + */
>> +static int platform_set_state_effecter_states_handler(const struct
>> pldm_rx_data *req)
>> +{
>> + struct effecter_states_req states_req;
>> + set_effecter_state_field *field;
>> + int rc, i;
>> +
>> + /* decode SetStateEffecterStates request data */
>> + rc = decode_set_state_effecter_states_req(
>> + req->msg,
>> + PLDM_SET_STATE_EFFECTER_STATES_REQ_BYTES,
>> + &states_req.effecter_id,
>> + &states_req.comp_effecter_count,
>> + states_req.field);
>> + if (rc) {
>> + prlog(PR_ERR, "Failed to decode SetStateEffecterStates
>> 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 - effecter_id: %d, comp_effecter_count: %d\n",
>> + __func__,
>> + states_req.effecter_id,
>> + states_req.comp_effecter_count);
>> +
>
>
> Should we check that we match the value of states_req.effecter_id? How
> do we find out our own effecter ID?
>
We don't care about this effecter_id, which is not linked to a specific
PLDM state effecter.
A specific effecter_id will be retrieved when we will execute an action
(poweroff, restart, ...)
>
>> + for (i = 0; i < states_req.comp_effecter_count; i++) {
>> + field = states_req.field;
>
>
> This will force to always look at the first effecter state (see below).
>
Thanks
>
>> +
>> + /* other set_request not supported */
>> + if (field->set_request != PLDM_REQUEST_SET) {
>> + prlog(PR_ERR, "Got invalid set request 0x%x in "
>> + "SetStateEffecterStates request\n",
>> + field->set_request);
>> + pldm_cc_resp(req, req->hdrinf.pldm_type,
>> + req->hdrinf.command,
>> + PLDM_PLATFORM_INVALID_STATE_VALUE);
>> + return OPAL_PARAMETER;
>> + }
>> +
>> + switch (field->effecter_state) {
>> + case PLDM_SW_TERM_GRACEFUL_SHUTDOWN_REQUESTED:
>> + case PLDM_STATE_SET_SYS_POWER_STATE_OFF_SOFT_GRACEFUL:
>> + prlog(PR_NOTICE, "Soft shutdown requested\n");
>> + pldm_cc_resp(req, PLDM_PLATFORM,
>> + PLDM_SET_STATE_EFFECTER_STATES,
>> + PLDM_SUCCESS);
>> +
>> + if (opal_booting() && platform.cec_power_down) {
>> + prlog(PR_NOTICE, "Host not up, shutting down now\n");
>> + platform.cec_power_down(CHASSIS_PWR_DOWN);
>> + } else {
>> + opal_queue_msg(OPAL_MSG_SHUTDOWN,
>> + NULL, NULL,
>> + cpu_to_be64(SOFT_OFF));
>> + }
>> +
>> + break;
>> +
>> + case PLDM_SW_TERM_GRACEFUL_RESTART_REQUESTED:
>> + prlog(PR_NOTICE, "Soft reboot requested\n");
>> + pldm_cc_resp(req, PLDM_PLATFORM,
>> + PLDM_SET_STATE_EFFECTER_STATES,
>> + PLDM_SUCCESS);
>> +
>> + if (opal_booting() && platform.cec_reboot) {
>> + prlog(PR_NOTICE, "Host not up, rebooting now\n");
>> + platform.cec_reboot();
>> + } else {
>> + opal_queue_msg(OPAL_MSG_SHUTDOWN,
>> + NULL, NULL,
>> + cpu_to_be64(SOFT_REBOOT));
>> + }
>> +
>> + break;
>> +
>> + case PLDM_STATE_SET_BOOT_RESTART_CAUSE_WARM_RESET:
>> + case PLDM_STATE_SET_BOOT_RESTART_CAUSE_HARD_RESET:
>> + prlog(PR_NOTICE, "OCC reset requested\n");
>
>
> I didn't see those in my logs. Did you manage to test those? From the
> FIXME below, it seems we still have to worry about multiple chips.
>
Did you manage to test those? No :-)
>
>> + pldm_cc_resp(req, PLDM_PLATFORM,
>> + PLDM_SET_STATE_EFFECTER_STATES,
>> + PLDM_SUCCESS);
>> +
>> + /* invoke the appropriate callback handler */
>> + prd_occ_reset(DEFAULT_CHIP_ID); /* FIXME, others chip ? */
>> + break;
>> +
>> + default:
>> + prlog(PR_ERR, "Got invalid effecter state 0x%x in "
>> + "SetStateEffecterStates request\n",
>> + field->effecter_state);
>> + pldm_cc_resp(req, req->hdrinf.pldm_type,
>> + req->hdrinf.command,
>> + PLDM_PLATFORM_INVALID_STATE_VALUE);
>> + return OPAL_PARAMETER;
>> + }
>> +
>> + /* next */
>> + field++;
>
>
> This is overwritten at the top of the loop so it's useless (and we
> process the same element over and over).
>
> Fred
>
>
>> + }
>> +
>> + return OPAL_SUCCESS;
>> +}
>> +
>> +static struct pldm_cmd pldm_platform_set_state_effecter_states = {
>> + .name = "PLDM_SET_STATE_EFFECTER_STATES",
>> + .pldm_cmd_id = PLDM_SET_STATE_EFFECTER_STATES,
>> + .handler = platform_set_state_effecter_states_handler,
>> +};
>> +
>> int pldm_rx_handle_request(struct pldm_rx_data *rx)
>> {
>> const struct pldm_type *t;
>> @@ -692,6 +821,7 @@ int pldm_mctp_responder_init(void)
>> pldm_add_cmd(&pldm_platform_type,
>> &pldm_platform_set_event_receiver);
>> pldm_add_cmd(&pldm_platform_type, &pldm_platform_event_message);
>> pldm_add_cmd(&pldm_platform_type,
>> &pldm_platform_get_state_sensor_readings);
>> + pldm_add_cmd(&pldm_platform_type,
>> &pldm_platform_set_state_effecter_states);
>> return OPAL_SUCCESS;
>> }
More information about the Skiboot
mailing list