<html><head><meta http-equiv="content-type" content="text/html; charset=us-ascii"></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><br><div><br><blockquote type="cite"><div>On May 13, 2023, at 05:12, Nicholas Piggin <npiggin@gmail.com> wrote:</div><br class="Apple-interchange-newline"><div><div>Bring ipmi to a consistent state before booting a kernel by flushing<br>all outstanding messages. The OS may not start kicking the IPMI state<br>machine for some time.<br><br>For example, without this change, when booting in QEMU, the IPMI command<br>issued by ipmi_wdt_final_reset() to disable the watchdog is not sent to<br>the BMC before the OS boots, effectively leaving the watchdog enabled<br>until the OS begins to drive OPAL pollers.<br></div></div></blockquote><div><br></div>Hah, that could be awkward and unpredictable for a payload to have to do a thing within a time.</div><div><br></div><div>Nice catch :)</div><div><br></div><div>Reviewed-by: Stewart Smith <<a href="mailto:stewart@flamingspork.com">stewart@flamingspork.com</a>></div><div><br><blockquote type="cite"><div><div><br>Signed-off-by: Nicholas Piggin <npiggin@gmail.com><br>---<br> core/ipmi.c | 11 ++++++++++-<br> hw/bt.c | 7 ++++++-<br> hw/fsp/fsp-ipmi.c | 12 ++++++++++--<br> include/ipmi.h | 7 ++++++-<br> platforms/astbmc/common.c | 6 ++++++<br> platforms/ibm-fsp/common.c | 6 ++++++<br> 6 files changed, 44 insertions(+), 5 deletions(-)<br><br>diff --git a/core/ipmi.c b/core/ipmi.c<br>index 59aa95fc..673aa0c9 100644<br>--- a/core/ipmi.c<br>+++ b/core/ipmi.c<br>@@ -159,7 +159,7 @@ void ipmi_cmd_done(uint8_t cmd, uint8_t netfn, uint8_t cc, struct ipmi_msg *msg)<br><br> void ipmi_queue_msg_sync(struct ipmi_msg *msg)<br> {<br>-<span class="Apple-tab-span" style="white-space:pre"> </span>void (*poll)(void) = msg->backend->poll;<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>bool (*poll)(void) = msg->backend->poll;<br><br> <span class="Apple-tab-span" style="white-space:pre"> </span>if (!ipmi_present())<br> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return;<br>@@ -192,6 +192,15 @@ void ipmi_queue_msg_sync(struct ipmi_msg *msg)<br> <span class="Apple-tab-span" style="white-space:pre"> </span>}<br> }<br><br>+void ipmi_flush(void)<br>+{<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>if (!ipmi_present())<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return;<br>+<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>while (ipmi_backend->poll())<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>time_wait_ms(10);<br>+}<br>+<br> static void ipmi_read_event_complete(struct ipmi_msg *msg)<br> {<br> <span class="Apple-tab-span" style="white-space:pre"> </span>prlog(PR_DEBUG, "IPMI read event %02x complete: %d bytes. cc: %02x\n",<br>diff --git a/hw/bt.c b/hw/bt.c<br>index 5016feab..1912cd3a 100644<br>--- a/hw/bt.c<br>+++ b/hw/bt.c<br>@@ -519,9 +519,14 @@ static void bt_poll(struct timer *t __unused, void *data __unused,<br> <span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span> bt.irq_ok ? TIMER_POLL : msecs_to_tb(BT_DEFAULT_POLL_MS));<br> }<br><br>-static void bt_ipmi_poll(void)<br>+static bool bt_ipmi_poll(void)<br> {<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>if (!lpc_ok())<br>+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return false;<br>+<br> <span class="Apple-tab-span" style="white-space:pre"> </span>bt_poll(NULL, NULL, mftb());<br>+<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>return bt.queue_len > 0;<br> }<br><br> static void bt_add_msg(struct bt_msg *bt_msg)<br>diff --git a/hw/fsp/fsp-ipmi.c b/hw/fsp/fsp-ipmi.c<br>index e368c282..9e600f3f 100644<br>--- a/hw/fsp/fsp-ipmi.c<br>+++ b/hw/fsp/fsp-ipmi.c<br>@@ -243,14 +243,22 @@ static int fsp_ipmi_dequeue_msg(struct ipmi_msg *ipmi_msg)<br> <span class="Apple-tab-span" style="white-space:pre"> </span>return 0;<br> }<br><br>+<br>+static bool fsp_ipmi_poll(void)<br>+{<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>/* fsp_opal_poll poller checks command responses */<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>opal_run_pollers();<br>+<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>return !list_empty(&fsp_ipmi.msg_queue);<br>+}<br>+<br> static struct ipmi_backend fsp_ipmi_backend = {<br> <span class="Apple-tab-span" style="white-space:pre"> </span>.alloc_msg<span class="Apple-tab-span" style="white-space:pre"> </span>= fsp_ipmi_alloc_msg,<br> <span class="Apple-tab-span" style="white-space:pre"> </span>.free_msg<span class="Apple-tab-span" style="white-space:pre"> </span>= fsp_ipmi_free_msg,<br> <span class="Apple-tab-span" style="white-space:pre"> </span>.queue_msg<span class="Apple-tab-span" style="white-space:pre"> </span>= fsp_ipmi_queue_msg,<br> <span class="Apple-tab-span" style="white-space:pre"> </span>.queue_msg_head<span class="Apple-tab-span" style="white-space:pre"> </span>= fsp_ipmi_queue_msg_head,<br> <span class="Apple-tab-span" style="white-space:pre"> </span>.dequeue_msg<span class="Apple-tab-span" style="white-space:pre"> </span>= fsp_ipmi_dequeue_msg,<br>-<span class="Apple-tab-span" style="white-space:pre"> </span>/* FIXME if ever use ipmi_queue_msg_sync on FSP */<br>-<span class="Apple-tab-span" style="white-space:pre"> </span>.poll = NULL,<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>.poll = fsp_ipmi_poll,<br> };<br><br> static bool fsp_ipmi_rr_notify(uint32_t cmd_sub_mod,<br>diff --git a/include/ipmi.h b/include/ipmi.h<br>index 3e629ba4..5b7efd1e 100644<br>--- a/include/ipmi.h<br>+++ b/include/ipmi.h<br>@@ -178,8 +178,10 @@ struct ipmi_backend {<br> <span class="Apple-tab-span" style="white-space:pre"> </span> *<br> <span class="Apple-tab-span" style="white-space:pre"> </span> * So, ensure we have a way to drive any state machines that an IPMI<br> <span class="Apple-tab-span" style="white-space:pre"> </span> * backend may neeed to crank to ensure forward progress.<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> *<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * This returns true while there are any messages queued.<br> <span class="Apple-tab-span" style="white-space:pre"> </span> */<br>-<span class="Apple-tab-span" style="white-space:pre"> </span>void (*poll)(void);<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>bool (*poll)(void);<br> };<br><br> extern struct ipmi_backend *ipmi_backend;<br>@@ -220,6 +222,9 @@ void ipmi_queue_msg_sync(struct ipmi_msg *msg);<br> /* Removes the message from the list, queued previously */<br> int ipmi_dequeue_msg(struct ipmi_msg *msg);<br><br>+/* Polls the backend until all queued messages are completed */<br>+void ipmi_flush(void);<br>+<br> /* Process a completed message */<br> void ipmi_cmd_done(uint8_t cmd, uint8_t netfn, uint8_t cc, struct ipmi_msg *msg);<br><br>diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c<br>index 9ce22b38..bfbba2d5 100644<br>--- a/platforms/astbmc/common.c<br>+++ b/platforms/astbmc/common.c<br>@@ -504,6 +504,12 @@ void astbmc_exit(void)<br> <span class="Apple-tab-span" style="white-space:pre"> </span>ipmi_wdt_final_reset();<br><br> <span class="Apple-tab-span" style="white-space:pre"> </span>ipmi_set_boot_count();<br>+<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>/*<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * Booting into an OS that may not call back into skiboot for<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * some time. Ensure all IPMI messages are processed first.<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> */<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>ipmi_flush();<br> }<br><br> static const struct bmc_sw_config bmc_sw_ami = {<br>diff --git a/platforms/ibm-fsp/common.c b/platforms/ibm-fsp/common.c<br>index 4a723b25..f289d1f2 100644<br>--- a/platforms/ibm-fsp/common.c<br>+++ b/platforms/ibm-fsp/common.c<br>@@ -186,6 +186,12 @@ void ibm_fsp_exit(void)<br><br> <span class="Apple-tab-span" style="white-space:pre"> </span>/* Clear SRCs on the op-panel when Linux starts */<br> <span class="Apple-tab-span" style="white-space:pre"> </span>op_panel_clear_src();<br>+<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>/*<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * Booting into an OS that may not call back into skiboot for<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> * some time. Ensure all IPMI messages are processed first.<br>+<span class="Apple-tab-span" style="white-space:pre"> </span> */<br>+<span class="Apple-tab-span" style="white-space:pre"> </span>ipmi_flush();<br> }<br><br> int64_t ibm_fsp_cec_reboot(void)<br>-- <br>2.40.1<br><br>_______________________________________________<br>Skiboot mailing list<br>Skiboot@lists.ozlabs.org<br>https://lists.ozlabs.org/listinfo/skiboot<br></div></div></blockquote></div><br></body></html>