[Skiboot] [PATCH 3/4] ipmi: flush the ipmi message queue before booting a kernel

[Stewart Smith]

> On May 13, 2023, at 05:12, Nicholas Piggin <npiggin at gmail.com> wrote:
> 
> Bring ipmi to a consistent state before booting a kernel by flushing
> all outstanding messages. The OS may not start kicking the IPMI state
> machine for some time.
> 
> For example, without this change, when booting in QEMU, the IPMI command
> issued by ipmi_wdt_final_reset() to disable the watchdog is not sent to
> the BMC before the OS boots, effectively leaving the watchdog enabled
> until the OS begins to drive OPAL pollers.

Hah, that could be awkward and unpredictable for a payload to have to do a thing within a time.

Nice catch :)

Reviewed-by: Stewart Smith <stewart at flamingspork.com <mailto:stewart at flamingspork.com>>

> 
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
> core/ipmi.c                | 11 ++++++++++-
> hw/bt.c                    |  7 ++++++-
> hw/fsp/fsp-ipmi.c          | 12 ++++++++++--
> include/ipmi.h             |  7 ++++++-
> platforms/astbmc/common.c  |  6 ++++++
> platforms/ibm-fsp/common.c |  6 ++++++
> 6 files changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/core/ipmi.c b/core/ipmi.c
> index 59aa95fc..673aa0c9 100644
> --- a/core/ipmi.c
> +++ b/core/ipmi.c
> @@ -159,7 +159,7 @@ void ipmi_cmd_done(uint8_t cmd, uint8_t netfn, uint8_t cc, struct ipmi_msg *msg)
> 
> void ipmi_queue_msg_sync(struct ipmi_msg *msg)
> {
> -	void (*poll)(void) = msg->backend->poll;
> +	bool (*poll)(void) = msg->backend->poll;
> 
> 	if (!ipmi_present())
> 		return;
> @@ -192,6 +192,15 @@ void ipmi_queue_msg_sync(struct ipmi_msg *msg)
> 	}
> }
> 
> +void ipmi_flush(void)
> +{
> +	if (!ipmi_present())
> +		return;
> +
> +	while (ipmi_backend->poll())
> +		time_wait_ms(10);
> +}
> +
> static void ipmi_read_event_complete(struct ipmi_msg *msg)
> {
> 	prlog(PR_DEBUG, "IPMI read event %02x complete: %d bytes. cc: %02x\n",
> diff --git a/hw/bt.c b/hw/bt.c
> index 5016feab..1912cd3a 100644
> --- a/hw/bt.c
> +++ b/hw/bt.c
> @@ -519,9 +519,14 @@ static void bt_poll(struct timer *t __unused, void *data __unused,
> 		       bt.irq_ok ? TIMER_POLL : msecs_to_tb(BT_DEFAULT_POLL_MS));
> }
> 
> -static void bt_ipmi_poll(void)
> +static bool bt_ipmi_poll(void)
> {
> +	if (!lpc_ok())
> +		return false;
> +
> 	bt_poll(NULL, NULL, mftb());
> +
> +	return bt.queue_len > 0;
> }
> 
> static void bt_add_msg(struct bt_msg *bt_msg)
> diff --git a/hw/fsp/fsp-ipmi.c b/hw/fsp/fsp-ipmi.c
> index e368c282..9e600f3f 100644
> --- a/hw/fsp/fsp-ipmi.c
> +++ b/hw/fsp/fsp-ipmi.c
> @@ -243,14 +243,22 @@ static int fsp_ipmi_dequeue_msg(struct ipmi_msg *ipmi_msg)
> 	return 0;
> }
> 
> +
> +static bool fsp_ipmi_poll(void)
> +{
> +	/* fsp_opal_poll poller checks command responses */
> +	opal_run_pollers();
> +
> +	return !list_empty(&fsp_ipmi.msg_queue);
> +}
> +
> static struct ipmi_backend fsp_ipmi_backend = {
> 	.alloc_msg	= fsp_ipmi_alloc_msg,
> 	.free_msg	= fsp_ipmi_free_msg,
> 	.queue_msg	= fsp_ipmi_queue_msg,
> 	.queue_msg_head	= fsp_ipmi_queue_msg_head,
> 	.dequeue_msg	= fsp_ipmi_dequeue_msg,
> -	/* FIXME if ever use ipmi_queue_msg_sync on FSP */
> -	.poll           = NULL,
> +	.poll           = fsp_ipmi_poll,
> };
> 
> static bool fsp_ipmi_rr_notify(uint32_t cmd_sub_mod,
> diff --git a/include/ipmi.h b/include/ipmi.h
> index 3e629ba4..5b7efd1e 100644
> --- a/include/ipmi.h
> +++ b/include/ipmi.h
> @@ -178,8 +178,10 @@ struct ipmi_backend {
> 	 *
> 	 * So, ensure we have a way to drive any state machines that an IPMI
> 	 * backend may neeed to crank to ensure forward progress.
> +	 *
> +	 * This returns true while there are any messages queued.
> 	 */
> -	void (*poll)(void);
> +	bool (*poll)(void);
> };
> 
> extern struct ipmi_backend *ipmi_backend;
> @@ -220,6 +222,9 @@ void ipmi_queue_msg_sync(struct ipmi_msg *msg);
> /* Removes the message from the list, queued previously */
> int ipmi_dequeue_msg(struct ipmi_msg *msg);
> 
> +/* Polls the backend until all queued messages are completed */
> +void ipmi_flush(void);
> +
> /* Process a completed message */
> void ipmi_cmd_done(uint8_t cmd, uint8_t netfn, uint8_t cc, struct ipmi_msg *msg);
> 
> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
> index 9ce22b38..bfbba2d5 100644
> --- a/platforms/astbmc/common.c
> +++ b/platforms/astbmc/common.c
> @@ -504,6 +504,12 @@ void astbmc_exit(void)
> 	ipmi_wdt_final_reset();
> 
> 	ipmi_set_boot_count();
> +
> +	/*
> +	 * Booting into an OS that may not call back into skiboot for
> +	 * some time. Ensure all IPMI messages are processed first.
> +	 */
> +	ipmi_flush();
> }
> 
> static const struct bmc_sw_config bmc_sw_ami = {
> diff --git a/platforms/ibm-fsp/common.c b/platforms/ibm-fsp/common.c
> index 4a723b25..f289d1f2 100644
> --- a/platforms/ibm-fsp/common.c
> +++ b/platforms/ibm-fsp/common.c
> @@ -186,6 +186,12 @@ void ibm_fsp_exit(void)
> 
> 	/* Clear SRCs on the op-panel when Linux starts */
> 	op_panel_clear_src();
> +
> +	/*
> +	 * Booting into an OS that may not call back into skiboot for
> +	 * some time. Ensure all IPMI messages are processed first.
> +	 */
> +	ipmi_flush();
> }
> 
> int64_t ibm_fsp_cec_reboot(void)
> -- 
> 2.40.1
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/skiboot/attachments/20230514/ed094adc/attachment-0001.htm>