diff mbox series

[V2,05/15] core/pldm: Decode the SetStateEffecterStates request

Message ID 20220429094744.72855-6-clombard@linux.vnet.ibm.com
State Superseded
Headers show
Series Complete PLDM responder and enable PLDM support | expand

Commit Message

Christophe Lombard April 29, 2022, 9:47 a.m. UTC
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@linux.vnet.ibm.com>
---
 core/pldm/pldm-responder.c | 130 +++++++++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)

Comments

Frederic Barrat April 26, 2023, 1:39 p.m. UTC | #1
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@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?


> +	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).


> +
> +		/* 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.


> +			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;
>   }
Christophe Lombard May 24, 2023, 9:13 a.m. UTC | #2
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@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;
>>   }
diff mbox series

Patch

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);
+
+	for (i = 0; i < states_req.comp_effecter_count; i++) {
+		field = states_req.field;
+
+		/* 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");
+			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++;
+	}
+
+	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;
 }